From 8f7c6e23a641ac632dde57ba13a217a3542a68a0 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Fri, 11 Oct 2013 10:48:00 -0600 Subject: [PATCH] Changed the meaning of the uip_*input functions. They now return success when a packet is dropped; This is needed for the ENCX24J600 driver that must make a decision to return the packet or not: It should not retain dropped packets. From Max Holtzberg --- ChangeLog | 7 +++++- net/recvfrom.c | 5 ----- net/sendto.c | 7 +----- net/uip/uip_input.c | 17 ++++++++++----- net/uip/uip_internal.h | 4 ++-- net/uip/uip_udpcallback.c | 14 ++++-------- net/uip/uip_udpconn.c | 46 ++++++++++----------------------------- net/uip/uip_udpinput.c | 24 ++++++++++++++++---- net/uip/uip_udppoll.c | 2 +- 9 files changed, 57 insertions(+), 69 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1189fd6d22..9f6025c315 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5745,4 +5745,9 @@ * net/net_monitor.c: Notify the socket layer if a connection is lost before the monitoring callback has been registered. From Max Holtzberg (2013-10-11). - + * net/recvfrom.c, sendto.c, uip/uip_input.c, uip/uip_udpcallback.c, + uip/uip_udpconn.c, uip/uip_udpinput.c: Changed the meaning of the + uip_*input functions. They now return success when a packet is + dropped; This is needed for the ENCX24J600 driver that must make + a decision to return the packet or not: It should not retai + dropped packets. From Max Holtzberg (2013-10-11). diff --git a/net/recvfrom.c b/net/recvfrom.c index 99802a80ad..d274e9594c 100644 --- a/net/recvfrom.c +++ b/net/recvfrom.c @@ -953,10 +953,6 @@ static ssize_t udp_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len, state.rf_cb->priv = (void*)&state; state.rf_cb->event = recvfrom_udpinterrupt; - /* Enable the UDP socket */ - - uip_udpenable(conn); - /* Notify the device driver of the receive call */ netdev_rxnotify(conn->ripaddr); @@ -971,7 +967,6 @@ static ssize_t udp_recvfrom(FAR struct socket *psock, FAR void *buf, size_t len, /* Make sure that no further interrupts are processed */ - uip_udpdisable(conn); uip_udpcallbackfree(conn, state.rf_cb); ret = recvfrom_result(ret, &state); } diff --git a/net/sendto.c b/net/sendto.c index da3e5c860e..992d9066ea 100644 --- a/net/sendto.c +++ b/net/sendto.c @@ -66,7 +66,7 @@ * traffic that a UDP sendto could get delayed, but I would not expect this * generate a timeout. */ - + #undef CONFIG_NET_SENDTO_TIMEOUT /* If supported, the sendto timeout function would depend on socket options @@ -396,10 +396,6 @@ ssize_t psock_sendto(FAR struct socket *psock, FAR const void *buf, state.st_cb->priv = (void*)&state; state.st_cb->event = sendto_interrupt; - /* Enable the UDP socket */ - - uip_udpenable(conn); - /* Notify the device driver of the availabilty of TX data */ netdev_txnotify(conn->ripaddr); @@ -414,7 +410,6 @@ ssize_t psock_sendto(FAR struct socket *psock, FAR const void *buf, /* Make sure that no further interrupts are processed */ - uip_udpdisable(conn); uip_udpcallbackfree(conn, state.st_cb); } uip_unlock(save); diff --git a/net/uip/uip_input.c b/net/uip/uip_input.c index 73773b0edc..3dffcf0622 100644 --- a/net/uip/uip_input.c +++ b/net/uip/uip_input.c @@ -292,7 +292,11 @@ nullreturn: * Description: * * Returned Value: - * OK if packet could be processed, otherwise ERROR. + * OK The packet was processed (or dropped) and can be discarded. + * ERROR There is a matching connection, but could not dispatch the packet + * yet. Currently useful for UDP when a packet arrives before a recv + * call is in place. + * * * Assumptions: * @@ -431,7 +435,7 @@ int uip_input(struct uip_driver_s *dev) { nlldbg("Possible ping config packet received\n"); uip_icmpinput(dev); - goto done; + goto drop; } else #endif @@ -537,13 +541,16 @@ int uip_input(struct uip_driver_s *dev) goto drop; } - /* Return and let the caller do any actual transmission. */ + /* Return and let the caller do any pending transmission. */ return OK; + /* Drop the packet. NOTE that OK is returned meaning that the + * packet has been processed (although processed unsuccessfully). + */ + drop: dev->d_len = 0; - - return ERROR; + return OK; } #endif /* CONFIG_NET */ diff --git a/net/uip/uip_internal.h b/net/uip/uip_internal.h index 15c52f039f..32a3c6f7fe 100644 --- a/net/uip/uip_internal.h +++ b/net/uip/uip_internal.h @@ -192,8 +192,8 @@ int uip_udpinput(struct uip_driver_s *dev); /* Defined in uip_udpcallback.c *********************************************/ -int uip_udpcallback(struct uip_driver_s *dev, - struct uip_udp_conn *conn, uint16_t flags); +uint16_t uip_udpcallback(struct uip_driver_s *dev, + struct uip_udp_conn *conn, uint16_t flags); #endif /* CONFIG_NET_UDP */ #ifdef CONFIG_NET_ICMP diff --git a/net/uip/uip_udpcallback.c b/net/uip/uip_udpcallback.c index e8b03ac811..27e1c6e914 100644 --- a/net/uip/uip_udpcallback.c +++ b/net/uip/uip_udpcallback.c @@ -75,27 +75,21 @@ * ****************************************************************************/ -int uip_udpcallback(struct uip_driver_s *dev, struct uip_udp_conn *conn, - uint16_t flags) +uint16_t uip_udpcallback(struct uip_driver_s *dev, struct uip_udp_conn *conn, + uint16_t flags) { - int ret = ERROR; - nllvdbg("flags: %04x\n", flags); /* Some sanity checking */ if (conn) { - /* HACK to check if the packet could be processed */ - - ret = (conn->list->event && (flags & conn->list->flags) != 0) ? OK : ERROR; - /* Perform the callback */ - uip_callbackexecute(dev, conn, flags, conn->list); + flags = uip_callbackexecute(dev, conn, flags, conn->list); } - return ret; + return flags; } #endif /* CONFIG_NET && CONFIG_NET_UDP */ diff --git a/net/uip/uip_udpconn.c b/net/uip/uip_udpconn.c index 3c4ee5addb..a9f5c71cdb 100644 --- a/net/uip/uip_udpconn.c +++ b/net/uip/uip_udpconn.c @@ -250,6 +250,10 @@ struct uip_udp_conn *uip_udpalloc(void) /* Make sure that the connection is marked as uninitialized */ conn->lport = 0; + + /* Enqueue the connection into the active list */ + + dq_addlast(&conn->node, &g_active_udp_connections); } _uip_semgive(&g_free_sem); return conn; @@ -275,6 +279,13 @@ void uip_udpfree(struct uip_udp_conn *conn) _uip_semtake(&g_free_sem); conn->lport = 0; + + /* Remove the connection from the active list */ + + dq_rem(&conn->node, &g_active_udp_connections); + + /* Free the connection */ + dq_addlast(&conn->node, &g_free_udp_connections); _uip_semgive(&g_free_sem); } @@ -454,39 +465,4 @@ int uip_udpconnect(struct uip_udp_conn *conn, const struct sockaddr_in *addr) return OK; } -/**************************************************************************** - * Name: uip_udpenable() uip_udpdisable. - * - * Description: - * Enable/disable callbacks for the specified connection - * - * Assumptions: - * This function is called user code. Interrupts may be enabled. - * - ****************************************************************************/ - -void uip_udpenable(struct uip_udp_conn *conn) -{ - /* Add the connection structure to the active connectionlist. This list - * is modifiable from interrupt level, we we must disable interrupts to - * access it safely. - */ - - uip_lock_t flags = uip_lock(); - dq_addlast(&conn->node, &g_active_udp_connections); - uip_unlock(flags); -} - -void uip_udpdisable(struct uip_udp_conn *conn) -{ - /* Remove the connection structure from the active connectionlist. This list - * is modifiable from interrupt level, we we must disable interrupts to - * access it safely. - */ - - uip_lock_t flags = uip_lock(); - dq_rem(&conn->node, &g_active_udp_connections); - uip_unlock(flags); -} - #endif /* CONFIG_NET && CONFIG_NET_UDP */ diff --git a/net/uip/uip_udpinput.c b/net/uip/uip_udpinput.c index b2aed9e0c2..6411ca21d3 100644 --- a/net/uip/uip_udpinput.c +++ b/net/uip/uip_udpinput.c @@ -85,7 +85,9 @@ * dev - The device driver structure containing the received UDP packet * * Return: - * OK if packet has been processed, otherwise ERROR. + * OK The packet has been processed and can be deleted + * ERROR Hold the packet and try again later. There is a listening socket + * but no recv in place to catch the packet yet. * * Assumptions: * Called from the interrupt level or with interrupts disabled. @@ -96,8 +98,7 @@ int uip_udpinput(struct uip_driver_s *dev) { struct uip_udp_conn *conn; struct uip_udpip_hdr *pbuf = UDPBUF; - int ret = ERROR; - + int ret = OK; #ifdef CONFIG_NET_STATISTICS uip_stat.udp.recv++; @@ -128,6 +129,8 @@ int uip_udpinput(struct uip_driver_s *dev) conn = uip_udpactive(pbuf); if (conn) { + uint16_t flags; + /* Setup for the application callback */ dev->d_appdata = &dev->d_buf[UIP_LLH_LEN + UIP_IPUDPH_LEN]; @@ -136,7 +139,20 @@ int uip_udpinput(struct uip_driver_s *dev) /* Perform the application callback */ - ret = uip_udpcallback(dev, conn, UIP_NEWDATA); + flags = uip_udpcallback(dev, conn, UIP_NEWDATA); + + /* If the operation was successful, the UIP_NEWDATA flag is removed + * and thus the packet can be deleted (OK will be returned). + */ + + if ((flags & UIP_NEWDATA) != 0) + { + /* No.. the packet was not processed now. Return ERROR so + * that the driver may retry again later. + */ + + ret = ERROR; + } /* If the application has data to send, setup the UDP/IP header */ diff --git a/net/uip/uip_udppoll.c b/net/uip/uip_udppoll.c index 05c2508bca..5cb7643514 100644 --- a/net/uip/uip_udppoll.c +++ b/net/uip/uip_udppoll.c @@ -107,7 +107,7 @@ void uip_udppoll(struct uip_driver_s *dev, struct uip_udp_conn *conn) /* Perform the application callback */ - uip_udpcallback(dev, conn, UIP_POLL); + (void)uip_udpcallback(dev, conn, UIP_POLL); /* If the application has data to send, setup the UDP/IP header */