net/devif/devif_callback.c: corrected the connection event list to work as FIFO instead of LIFO.

In case of enabled packet forwarding mode, packets were forwarded in a reverse order
because of LIFO behavior of the connection event list.
The issue exposed only during high network traffic. Thus the event list started to grow
that resulted in changing the order of packets inside of groups of several packets
like the following: 3, 2, 1, 6, 5, 4, 8, 7 etc.

Remarks concerning the connection event list implementation:
* Now the queue (list) is FIFO as it should be.
* The list is singly linked.
* The list has a head pointer (inside of outer net_driver_s structure),
  and a tail pointer is added into outer net_driver_s structure.
* The list item is devif_callback_s structure.
  It still has two pointers to two different list chains (*nxtconn and *nxtdev).
* As before the first argument (*dev) of the list functions can be NULL,
  while the other argument (*list) is effective (not NULL).
* An extra (*tail) argument is added to devif_callback_alloc()
  and devif_conn_callback_free() functions.
* devif_callback_alloc() time complexity is O(1) (i.e. O(n) to fill the whole list).
* devif_callback_free() time complexity is O(n) (i.e. O(n^2) to empty the whole list).
* devif_conn_event() time complexity is O(n).
This commit is contained in:
Alexander Lunev 2021-08-29 23:57:26 +03:00 committed by Xiang Xiao
parent 63a17f5cdd
commit 36fbedcbfc
23 changed files with 123 additions and 52 deletions

View file

@ -382,7 +382,8 @@ struct net_driver_s
* NETDEV_DOWN - The network is down
*/
FAR struct devif_callback_s *d_conncb;
FAR struct devif_callback_s *d_conncb; /* This is the list head */
FAR struct devif_callback_s *d_conncb_tail; /* This is the list tail */
FAR struct devif_callback_s *d_devcb;
/* Driver callbacks */

View file

@ -75,7 +75,9 @@
/* Allocate a new ARP data callback */
#define arp_callback_alloc(dev) devif_callback_alloc(dev, &(dev)->d_conncb)
#define arp_callback_alloc(dev) devif_callback_alloc(dev, \
&(dev)->d_conncb, \
&(dev)->d_conncb_tail)
#define arp_callback_free(dev,cb) devif_dev_callback_free(dev, cb)
/****************************************************************************

View file

@ -42,9 +42,9 @@
/* Allocate a new Bluetooth socket data callback */
#define bluetooth_callback_alloc(dev,conn) \
devif_callback_alloc(dev, &conn->bc_list)
devif_callback_alloc(dev, &conn->bc_list, &conn->bc_list_tail)
#define bluetooth_callback_free(dev,conn,cb) \
devif_conn_callback_free(dev, cb, &conn->bc_list)
devif_conn_callback_free(dev, cb, &conn->bc_list, &conn->bc_list_tail)
/* Memory Pools */
@ -85,6 +85,7 @@ struct bluetooth_conn_s
*/
FAR struct devif_callback_s *bc_list; /* Bluetooth callbacks */
FAR struct devif_callback_s *bc_list_tail; /* Bluetooth callbacks */
/* Bluetooth-specific content follows. */

View file

@ -51,9 +51,9 @@
/* Allocate a new packet socket data callback */
#define can_callback_alloc(dev,conn) \
devif_callback_alloc(dev, &conn->list)
devif_callback_alloc(dev, &conn->list, &conn->list_tail)
#define can_callback_free(dev,conn,cb) \
devif_conn_callback_free(dev, cb, &conn->list)
devif_conn_callback_free(dev, cb, &conn->list, &conn->list_tail)
/****************************************************************************
* Public Type Definitions
@ -82,7 +82,8 @@ struct can_conn_s
* event.
*/
FAR struct devif_callback_s *list; /* NetLink callbacks */
FAR struct devif_callback_s *list; /* NetLink callbacks */
FAR struct devif_callback_s *list_tail; /* NetLink callbacks */
FAR struct net_driver_s *dev; /* Reference to CAN device */

View file

@ -330,7 +330,8 @@ void devif_callback_init(void);
FAR struct devif_callback_s *
devif_callback_alloc(FAR struct net_driver_s *dev,
FAR struct devif_callback_s **list);
FAR struct devif_callback_s **list_head,
FAR struct devif_callback_s **list_tail);
/****************************************************************************
* Name: devif_conn_callback_free
@ -352,7 +353,8 @@ FAR struct devif_callback_s *
void devif_conn_callback_free(FAR struct net_driver_s *dev,
FAR struct devif_callback_s *cb,
FAR struct devif_callback_s **list);
FAR struct devif_callback_s **list_head,
FAR struct devif_callback_s **list_tail);
/****************************************************************************
* Name: devif_dev_callback_free

View file

@ -62,7 +62,8 @@ static FAR struct devif_callback_s *g_cbfreelist = NULL;
static void devif_callback_free(FAR struct net_driver_s *dev,
FAR struct devif_callback_s *cb,
FAR struct devif_callback_s **list)
FAR struct devif_callback_s **list_head,
FAR struct devif_callback_s **list_tail)
{
FAR struct devif_callback_s *prev;
FAR struct devif_callback_s *curr;
@ -116,11 +117,11 @@ static void devif_callback_free(FAR struct net_driver_s *dev,
* it is supposed to be in the data notification list.
*/
if (list)
if (list_head)
{
/* Find the callback structure in the connection event list */
for (prev = NULL, curr = *list;
for (prev = NULL, curr = *list_head;
curr && curr != cb;
prev = curr, curr = curr->nxtconn)
{
@ -133,11 +134,25 @@ static void devif_callback_free(FAR struct net_driver_s *dev,
{
if (prev)
{
/* The found item to be removed is not in the head. */
prev->nxtconn = cb->nxtconn;
}
else
{
*list = cb->nxtconn;
/* The found item to be removed is in the head. */
*list_head = cb->nxtconn;
}
if (!cb->nxtconn)
{
/* If the tail item is being removed,
* update the tail pointer.
*/
DEBUGASSERT(list_tail);
*list_tail = prev;
}
}
}
@ -235,11 +250,12 @@ void devif_callback_init(void)
FAR struct devif_callback_s *
devif_callback_alloc(FAR struct net_driver_s *dev,
FAR struct devif_callback_s **list)
FAR struct devif_callback_s **list_head,
FAR struct devif_callback_s **list_tail)
{
FAR struct devif_callback_s *ret;
/* Check the head of the free list */
/* Check the head of the free list */
net_lock();
ret = g_cbfreelist;
@ -267,7 +283,7 @@ FAR struct devif_callback_s *
{
/* No.. release the callback structure and fail */
devif_callback_free(NULL, NULL, list);
devif_callback_free(NULL, NULL, list_head, list_tail);
net_unlock();
return NULL;
}
@ -276,12 +292,28 @@ FAR struct devif_callback_s *
dev->d_devcb = ret;
}
/* Add the newly allocated instance to the head of the specified list */
/* Add the newly allocated instance to the tail of the specified list */
if (list)
if (list_head && list_tail)
{
ret->nxtconn = *list;
*list = ret;
ret->nxtconn = NULL;
if (*list_tail)
{
/* If the list is not empty, add the item to the tail. */
(*list_tail)->nxtconn = ret;
}
else
{
/* If the list is empty, add the first item to the list. */
*list_head = ret;
}
/* Update the tail pointer */
*list_tail = ret;
}
}
#ifdef CONFIG_DEBUG_FEATURES
@ -315,7 +347,8 @@ FAR struct devif_callback_s *
void devif_conn_callback_free(FAR struct net_driver_s *dev,
FAR struct devif_callback_s *cb,
FAR struct devif_callback_s **list)
FAR struct devif_callback_s **list_head,
FAR struct devif_callback_s **list_tail)
{
/* Check if the device pointer is still valid. It could be invalid if, for
* example, the device were unregistered between the time when the callback
@ -331,7 +364,7 @@ void devif_conn_callback_free(FAR struct net_driver_s *dev,
/* Then free the callback */
devif_callback_free(dev, cb, list);
devif_callback_free(dev, cb, list_head, list_tail);
}
/****************************************************************************
@ -357,7 +390,8 @@ void devif_conn_callback_free(FAR struct net_driver_s *dev,
void devif_dev_callback_free(FAR struct net_driver_s *dev,
FAR struct devif_callback_s *cb)
{
FAR struct devif_callback_s **list;
FAR struct devif_callback_s **list_head;
FAR struct devif_callback_s **list_tail;
/* Check if the device pointer is still valid. It could be invalid if, for
* example, the device were unregistered between the time when the callback
@ -366,23 +400,25 @@ void devif_dev_callback_free(FAR struct net_driver_s *dev,
if (dev != NULL && netdev_verify(dev))
{
/* The device reference is valid.. the use the list pointer in the
/* The device reference is valid. Then use the list pointer in the
* device structure as well.
*/
list = &dev->d_conncb;
list_head = &dev->d_conncb;
list_tail = &dev->d_conncb_tail;
}
else
{
/* The device reference is longer valid */
dev = NULL;
list = NULL;
list_head = NULL;
list_tail = NULL;
}
/* Then free the callback */
devif_callback_free(dev, cb, list);
devif_callback_free(dev, cb, list_head, list_tail);
}
/****************************************************************************

View file

@ -47,9 +47,9 @@
/* Allocate/free an ICMP data callback */
#define icmp_callback_alloc(dev, conn) \
devif_callback_alloc((dev), &(conn)->list)
devif_callback_alloc((dev), &(conn)->list, &(conn)->list_tail)
#define icmp_callback_free(dev, conn, cb) \
devif_conn_callback_free((dev), (cb), &(conn)->list)
devif_conn_callback_free((dev), (cb), &(conn)->list, &(conn)->list_tail)
/****************************************************************************
* Public types
@ -84,6 +84,7 @@ struct icmp_conn_s
*/
FAR struct devif_callback_s *list;
FAR struct devif_callback_s *list_tail;
/* ICMP-specific content follows */

View file

@ -48,9 +48,9 @@
/* Allocate a new ICMPv6 data callback */
#define icmpv6_callback_alloc(dev, conn) \
devif_callback_alloc((dev), &(conn)->list)
devif_callback_alloc((dev), &(conn)->list, &(conn)->list_tail)
#define icmpv6_callback_free(dev, conn, cb) \
devif_conn_callback_free((dev), (cb), &(conn)->list)
devif_conn_callback_free((dev), (cb), &(conn)->list, &(conn)->list_tail)
/****************************************************************************
* Public Type Definitions
@ -86,6 +86,7 @@ struct icmpv6_conn_s
*/
FAR struct devif_callback_s *list;
FAR struct devif_callback_s *list_tail;
/* ICMPv6-specific content follows */

View file

@ -198,7 +198,9 @@ static int icmpv6_send_message(FAR struct net_driver_s *dev, bool advertise)
* want anything to happen until we are ready.
*/
state.snd_cb = devif_callback_alloc(dev, &dev->d_conncb);
state.snd_cb = devif_callback_alloc(dev,
&dev->d_conncb,
&dev->d_conncb_tail);
if (!state.snd_cb)
{
nerr("ERROR: Failed to allocate a cllback\n");

View file

@ -240,7 +240,9 @@ int icmpv6_neighbor(const net_ipv6addr_t ipaddr)
*/
net_lock();
state.snd_cb = devif_callback_alloc((dev), &(dev)->d_conncb);
state.snd_cb = devif_callback_alloc((dev),
&(dev)->d_conncb,
&(dev)->d_conncb_tail);
if (!state.snd_cb)
{
nerr("ERROR: Failed to allocate a callback\n");

View file

@ -40,9 +40,9 @@
/* Allocate a new IEEE 802.15.4 socket data callback */
#define ieee802154_callback_alloc(dev,conn) \
devif_callback_alloc(dev, &conn->list)
devif_callback_alloc(dev, &conn->list, &conn->list_tail)
#define ieee802154_callback_free(dev,conn,cb) \
devif_conn_callback_free(dev, cb, &conn->list)
devif_conn_callback_free(dev, cb, &conn->list, &conn->list_tail)
/* Memory Pools */
@ -104,6 +104,7 @@ struct ieee802154_conn_s
*/
FAR struct devif_callback_s *list;
FAR struct devif_callback_s *list_tail;
/* IEEE 802.15.4-specific content follows */

View file

@ -44,7 +44,9 @@
/* Allocate a new IP forwarding data callback */
#define ipfwd_callback_alloc(dev) devif_callback_alloc(dev, &(dev)->d_conncb)
#define ipfwd_callback_alloc(dev) devif_callback_alloc(dev, \
&(dev)->d_conncb, \
&(dev)->d_conncb_tail)
#define ipfwd_callback_free(dev,cb) devif_dev_callback_free(dev, cb)
/****************************************************************************

View file

@ -364,6 +364,7 @@ int netdev_register(FAR struct net_driver_s *dev, enum net_lltype_e lltype)
/* There are no clients of the device yet */
dev->d_conncb = NULL;
dev->d_conncb_tail = NULL;
dev->d_devcb = NULL;
/* We need exclusive access for the following operations */

View file

@ -39,9 +39,9 @@
/* Allocate a new packet socket data callback */
#define pkt_callback_alloc(dev,conn) \
devif_callback_alloc(dev, &conn->list)
devif_callback_alloc(dev, &conn->list, &conn->list_tail)
#define pkt_callback_free(dev,conn,cb) \
devif_conn_callback_free(dev, cb, &conn->list)
devif_conn_callback_free(dev, cb, &conn->list, &conn->list_tail)
/****************************************************************************
* Public Type Definitions
@ -62,6 +62,7 @@ struct pkt_conn_s
*/
struct devif_callback_s *list;
struct devif_callback_s *list_tail;
/* Pkt socket-specific content follows */

View file

@ -284,6 +284,7 @@ struct iob_s; /* Forward reference */
int sixlowpan_send(FAR struct net_driver_s *dev,
FAR struct devif_callback_s **list,
FAR struct devif_callback_s **list_tail,
FAR const struct ipv6_hdr_s *ipv6hdr, FAR const void *buf,
size_t len, FAR const struct netdev_varaddr_s *destmac,
unsigned int timeout);

View file

@ -203,6 +203,7 @@ end_wait:
int sixlowpan_send(FAR struct net_driver_s *dev,
FAR struct devif_callback_s **list,
FAR struct devif_callback_s **list_tail,
FAR const struct ipv6_hdr_s *ipv6hdr, FAR const void *buf,
size_t len, FAR const struct netdev_varaddr_s *destmac,
unsigned int timeout)
@ -231,7 +232,7 @@ int sixlowpan_send(FAR struct net_driver_s *dev,
* device related events, no connect-related events.
*/
sinfo.s_cb = devif_callback_alloc(dev, list);
sinfo.s_cb = devif_callback_alloc(dev, list, list_tail);
if (sinfo.s_cb != NULL)
{
int ret;
@ -265,7 +266,7 @@ int sixlowpan_send(FAR struct net_driver_s *dev,
/* Make sure that no further events are processed */
devif_conn_callback_free(dev, sinfo.s_cb, list);
devif_conn_callback_free(dev, sinfo.s_cb, list, list_tail);
}
}

View file

@ -292,7 +292,9 @@ ssize_t psock_6lowpan_udp_sendto(FAR struct socket *psock,
* packet.
*/
ret = sixlowpan_send(dev, &conn->list,
ret = sixlowpan_send(dev,
&conn->list,
&conn->list_tail,
(FAR const struct ipv6_hdr_s *)&ipv6udp,
buf, buflen, &destmac,
_SO_TIMEOUT(psock->s_sndtimeo));

View file

@ -54,9 +54,9 @@
*/
#define tcp_callback_alloc(conn) \
devif_callback_alloc((conn)->dev, &(conn)->list)
devif_callback_alloc((conn)->dev, &(conn)->list, &(conn)->list_tail)
#define tcp_callback_free(conn,cb) \
devif_conn_callback_free((conn)->dev, (cb), &(conn)->list)
devif_conn_callback_free((conn)->dev, (cb), &(conn)->list, &(conn)->list_tail)
#ifdef CONFIG_NET_TCP_WRITE_BUFFERS
/* TCP write buffer access macros */
@ -164,6 +164,7 @@ struct tcp_conn_s
*/
FAR struct devif_callback_s *list;
FAR struct devif_callback_s *list_tail;
/* TCP-specific content follows */
@ -289,6 +290,7 @@ struct tcp_conn_s
*/
FAR struct devif_callback_s *connevents;
FAR struct devif_callback_s *connevents_tail;
/* accept() is called when the TCP logic has created a connection
*

View file

@ -213,7 +213,8 @@ static void tcp_shutdown_monitor(FAR struct tcp_conn_s *conn, uint16_t flags)
while (conn->connevents != NULL)
{
devif_conn_callback_free(conn->dev, conn->connevents,
&conn->connevents);
&conn->connevents,
&conn->connevents_tail);
}
net_unlock();
@ -284,7 +285,9 @@ int tcp_start_monitor(FAR struct socket *psock)
* the network goes down.
*/
cb = devif_callback_alloc(conn->dev, &conn->connevents);
cb = devif_callback_alloc(conn->dev,
&conn->connevents,
&conn->connevents_tail);
if (cb != NULL)
{
cb->event = tcp_monitor_event;
@ -372,7 +375,10 @@ void tcp_close_monitor(FAR struct socket *psock)
if (cb != NULL)
{
devif_conn_callback_free(conn->dev, cb, &conn->connevents);
devif_conn_callback_free(conn->dev,
cb,
&conn->connevents,
&conn->connevents_tail);
}
/* Make sure that this socket is explicitly marked as closed */

View file

@ -61,9 +61,9 @@
/* Allocate a new UDP data callback */
#define udp_callback_alloc(dev,conn) \
devif_callback_alloc((dev), &(conn)->list)
devif_callback_alloc((dev), &(conn)->list, &(conn)->list_tail)
#define udp_callback_free(dev,conn,cb) \
devif_conn_callback_free((dev), (cb), &(conn)->list)
devif_conn_callback_free((dev), (cb), &(conn)->list, &(conn)->list_tail)
/* Definitions for the UDP connection struct flag field */
@ -106,6 +106,7 @@ struct udp_conn_s
*/
FAR struct devif_callback_s *list;
FAR struct devif_callback_s *list_tail;
/* UDP-specific content follows */

View file

@ -105,6 +105,7 @@ struct usrsock_conn_s
*/
FAR struct devif_callback_s *list; /* Usersock callbacks */
FAR struct devif_callback_s *list_tail;
/* usrsock-specific content follows */

View file

@ -254,7 +254,7 @@ int usrsock_setup_request_callback(FAR struct usrsock_conn_s *conn,
/* Set up the callback in the connection */
pstate->cb = devif_callback_alloc(NULL, &conn->list);
pstate->cb = devif_callback_alloc(NULL, &conn->list, &conn->list_tail);
if (pstate->cb)
{
/* Take a lock since only one outstanding request is allowed */
@ -308,7 +308,7 @@ void usrsock_teardown_request_callback(FAR struct usrsock_reqstate_s *pstate)
/* Make sure that no further events are processed */
devif_conn_callback_free(NULL, pstate->cb, &conn->list);
devif_conn_callback_free(NULL, pstate->cb, &conn->list, &conn->list_tail);
nxsem_destroy(&pstate->recvsem);
pstate->cb = NULL;

View file

@ -187,7 +187,7 @@ static int usrsock_pollsetup(FAR struct socket *psock,
/* Allocate a usrsock callback structure */
cb = devif_callback_alloc(NULL, &conn->list);
cb = devif_callback_alloc(NULL, &conn->list, &conn->list_tail);
if (cb == NULL)
{
ret = -EBUSY;
@ -331,7 +331,10 @@ static int usrsock_pollteardown(FAR struct socket *psock,
{
/* Release the callback */
devif_conn_callback_free(NULL, info->cb, &conn->list);
devif_conn_callback_free(NULL,
info->cb,
&conn->list,
&conn->list_tail);
/* Release the poll/select data slot */