Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Link layer] GNRC PPP #5470

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ ifneq (,$(filter netdev2_tap,$(USEMODULE)))
endif
endif

ifneq (,$(filter gnrc_ppp,$(USEMODULE)))
USEMODULE += gnrc_netdev
USEMODULE += gnrc_netdev_msg_handler
endif

ifneq (,$(filter gnrc_zep,$(USEMODULE)))
USEMODULE += hashes
USEMODULE += ieee802154
Expand Down
1 change: 1 addition & 0 deletions Makefile.pseudomodules
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ PSEUDOMODULES += gnrc_ipv6_default
PSEUDOMODULES += gnrc_ipv6_router
PSEUDOMODULES += gnrc_ipv6_router_default
PSEUDOMODULES += gnrc_netdev_default
PSEUDOMODULES += gnrc_netdev_msg_handler
PSEUDOMODULES += gnrc_neterr
PSEUDOMODULES += gnrc_netapi_callbacks
PSEUDOMODULES += gnrc_netapi_mbox
Expand Down
3 changes: 2 additions & 1 deletion drivers/include/net/netdev2.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ enum {
NETDEV2_TYPE_ETHERNET,
NETDEV2_TYPE_IEEE802154,
NETDEV2_TYPE_CC110X,
NETDEV2_TYPE_NRFMIN
NETDEV2_TYPE_NRFMIN,
NETDEV2_TYPE_PPP
};

/**
Expand Down
111 changes: 111 additions & 0 deletions drivers/include/net/netdev2/ppp.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* Copyright (C) 2016 Fundación Inria Chile
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup drivers_netdev_netdev2
* @brief
* @{
*
* @file
* @brief Definitions for netdev2 common Point to Point Protocol code
*
* @author José Ignacio Alamos
*/
#ifndef NETDEV2_PPP_H
#define NETDEV2_PPP_H

#include "net/gnrc/ppp/prot.h"
#include "net/gnrc/ppp/prot.h"
#include "net/gnrc/ppp/lcp.h"
#include "net/gnrc/ppp/pap.h"
#include "net/gnrc/ppp/ipcp.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With all these separate headers: may I propose a net/gnrc/ppp.h header that includes all these headers (there you can also @defgroup net_gnrc_ppp instead of duplicate define it multiple times [see below])

#include "net/gnrc/nettype.h"
#include "net/netopt.h"
#include "net/netdev2.h"

#ifdef __cplusplus
extern "C" {
#endif

#if defined(MODULE_GNRC_PPP) || doxygen
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defined(DOXYGEN)

/**
* @brief class of custom driver control protocol
* @extends ppp_protocol_t
*
* @details the DCP is in charge of monitoring the link and exchanging messages with the ppp device
*/
typedef struct gnrc_ppp_dcp {
gnrc_ppp_protocol_t prot; /**< base ppp_protocol_t object */
msg_t timer_msg; /**< msg struct for handling timeouts messages */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please align doc

xtimer_t xtimer; /**< xtimer struct for sending timeout messages */
uint8_t dead_counter; /**< when reaches zero, the link is assumed to be dead */
} gnrc_ppp_dcp_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why define that here and not in sys/include/net/gnrc/ppp/dcp.h?

#endif


/**
* @brief Extended structure to hold PPP driver state
*
* @extends netdev2_t
*
* Supposed to be extended by driver implementations.
* The extended structure should contain all variable driver state.
*/
typedef struct {
netdev2_t netdev; /**< @ref netdev2_t base class */
#if defined(MODULE_GNRC_PPP) || doxygen
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defined(DOXYGEN)

/**
* @brief PPP specific fields
* @{
*/
gnrc_ppp_dcp_t dcp; /**< Control protocol for driver */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please align doc

gnrc_ppp_lcp_t lcp; /**< Link Control Protocol */
gnrc_ppp_pap_t pap; /**< Password Authentication Protocol */
gnrc_ppp_ipcp_t ipcp; /**< IPv4 Network Control Protocol */
gnrc_ppp_ipv4_t ipv4; /**< Handler for IPv4 packets */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cast these to gnrc_ppp_protocol_t quite often in your code (https://github.com/RIOT-OS/RIOT/pull/5470/files#diff-6d2b26dab0a8a164f332c85c0910c139R35), but e.g. sizeof(gnrc_ppp_dcp_t) < sizeof(gnrc_ppp_protocol), so you will access data after netdev2_ppp_t.dcp (something in netdev2_ppp_t.lcp in this case).

#endif
} netdev2_ppp_t;


/**
* @brief Fallback function for netdev2 PPP devices' _get function
*
* Supposed to be used by netdev2 drivers as default case.
*
* @param[in] dev network device descriptor
* @param[in] opt option type
* @param[out] value pointer to store the option's value in
* @param[in] max_len maximal amount of byte that fit into @p value
*
* @return number of bytes written to @p value
* @return <0 on error
*/
int netdev2_ppp_get(netdev2_ppp_t *dev, netopt_t opt, void *value,
size_t max_len);

/**
* @brief Fallback function for netdev2 PPP devices' _set function
*
*
* @param[in] dev network device descriptor
* @param[in] opt option type
* @param[in] value value to set
* @param[in] value_len the length of @p value
*
* @return number of bytes used from @p value
* @return <0 on error
*/
int netdev2_ppp_set(netdev2_ppp_t *dev, netopt_t opt, void *value,
size_t value_len);

#ifdef __cplusplus
}
#endif

#endif /* NETDEV2_PPP_H */
/** @} */
1 change: 1 addition & 0 deletions drivers/netdev2_ppp/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include $(RIOTBASE)/Makefile.base
105 changes: 105 additions & 0 deletions drivers/netdev2_ppp/netdev2_ppp.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright (C) Fundación Inria Chile 2017
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup driver_netdev2_ppp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This group does not exist. Also it doesn't make sense to do this in a c-file, so please remove this line

* @{
*
* @file
* @brief Common code for netdev2 ppp drivers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this line

*
* @author José Ignacio Alamos <jose.alamos@inria.cl>
*
* @}
*/

#include <assert.h>
#include <errno.h>

#include "net/netdev2.h"
#include "net/netdev2/ppp.h"

#define ENABLE_DEBUG (0)
#include "debug.h"

int netdev2_ppp_get(netdev2_ppp_t *dev, netopt_t opt, void *value, size_t max_len)
{
int res;
switch (opt) {
case NETOPT_PPP_LCP_STATE:
*((uint8_t *) value) = ((gnrc_ppp_protocol_t*) &dev->lcp)->state;
res = 0;
break;
case NETOPT_PPP_AUTH_STATE:
*((uint8_t *) value) = ((gnrc_ppp_protocol_t*) &dev->pap)->state;
res = 0;
break;
case NETOPT_PPP_IPCP_STATE:
*((uint8_t *) value) = ((gnrc_ppp_protocol_t*) &dev->ipcp)->state;
res = 0;
break;
case NETOPT_PPP_IS_IPV6_READY:
res = ((gnrc_ppp_protocol_t*) &dev->ipv4)->state == PROTOCOL_UP;
*((uint8_t *) value) = res;
break;
case NETOPT_DEVICE_TYPE:
*((uint16_t*) value) = NETDEV2_TYPE_PPP;
res = 2;
break;
case NETOPT_LINK_STATE: {
gnrc_ppp_protocol_t *dcp = (gnrc_ppp_protocol_t *) &dev->dcp;
*((netopt_enable_t *)value) = (dcp->state == PROTOCOL_DOWN) ?
NETOPT_DISABLE : NETOPT_ENABLE;
res = sizeof(netopt_enable_t);
};
break;
default:
return -ENOTSUP;
break;
}
return res;
}

int netdev2_ppp_set(netdev2_ppp_t *dev, netopt_t opt, void *value, size_t value_len)
{
int res;
switch (opt) {
case NETOPT_TUNNEL_IPV4_ADDRESS:
dev->ipv4.tunnel_addr = *((ipv4_addr_t *) value);
res = 0;
break;
case NETOPT_TUNNEL_UDP_PORT:
dev->ipv4.tunnel_port = *((uint16_t *) value);
res = 0;
break;
case NETOPT_APN_USER:
memcpy(dev->pap.username, value, value_len);
dev->pap.user_size = value_len;
res = 0;
break;
case NETOPT_APN_PASS:
memcpy(dev->pap.password, value, value_len);
dev->pap.pass_size = value_len;
res = 0;
break;
case NETOPT_LINK_STATE:
if (*((netopt_enable_t *)value) == NETOPT_DISABLE) {
/* set link down in device context
* should not require dispatch_ppp_msg */
}
else {
/* set link up in device context
* should not require dispatch_ppp_msg */
}
break;
default:
return -ENOTSUP;
break;
}
return res;
}
3 changes: 3 additions & 0 deletions sys/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ endif
ifneq (,$(filter gnrc_netdev2,$(USEMODULE)))
DIRS += net/gnrc/link_layer/netdev2
endif
ifneq (,$(filter gnrc_ppp,$(USEMODULE)))
DIRS += net/gnrc/link_layer/ppp
endif
ifneq (,$(filter fib,$(USEMODULE)))
DIRS += net/network_layer/fib
endif
Expand Down
11 changes: 11 additions & 0 deletions sys/include/net/gnrc/netdev2.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ typedef struct gnrc_netdev2 {
*/
int (*send)(struct gnrc_netdev2 *dev, gnrc_pktsnip_t *snip);

#if defined(MODULE_GNRC_NETDEV_MSG_HANDLER) || DOXYGEN
/**
* @brief Handle custom messages
*
* This function handles additional message types to the
* @ref net_gnrc_netapi messages.
*
* @note Only available with gnrc_netdev_msg_handler pseudo-module.
*/
int (*msg_handler)(struct gnrc_netdev2 *dev, msg_t *msg);
#endif
/**
* @brief Receive a pktsnip from this device
*
Expand Down
43 changes: 43 additions & 0 deletions sys/include/net/gnrc/netdev2/ppp.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright (C) Fundación Inria Chile 2017
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @addtogroup net_gnrc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use @ingroup net_gnrc_netdev2 here.

* @{
*
* @file
* @brief GNRC to PPP netdev2 glue code interface
*
* @author José Ignacio Alamos <jose.alamos@inria.cl>
*/

#ifndef GNRC_NETDEV2_PPP_H_
#define GNRC_NETDEV2_PPP_H_

#include "net/netdev2/ppp.h"
#include "net/gnrc/netdev2.h"


#ifdef __cplusplus
extern "C" {
#endif

/**
* @brief Initialize GNRC handler for netdev2 PPP devices
*
* @param[out] gnrc_netdev2 gnrc_netdev2 struct to initialize
* @param[in] dev PPP device to handle
*/
void gnrc_netdev2_ppp_init(gnrc_netdev2_t *gnrc_netdev2, netdev2_ppp_t *dev);

#ifdef __cplusplus
}
#endif

#endif /* GNRC_NETDEV2_PPP_H_ */
/** @} */
Loading