diff --git a/ChangeLog b/ChangeLog index c00de77a9a5..5ec6b5d5cee 100755 --- a/ChangeLog +++ b/ChangeLog @@ -10889,4 +10889,13 @@ with the SAMV71 QSPI hardware (2015-08-25). * drivers/rwbuffer.c: Fix some logic errors. From Dmitry Nikolaev via Juha Niskanen (2015-08-26). + * net/socket and net/tcp: Fix a problem in whent there are multiple + network devices. Polls were being sent to all TCP sockets before. + This is not good because it means that packets may sometimes be + sent out on the wrong device. That is inefficient because it + will cause retransmissions and bad performance. But, worse, when + one of the devices is not Ethernet, it will have a different MSS + and, as a result, incorrect data transfers can cause crashes. + The fix is to lock into a single device once the MSS is locked + locked down (2015-08-27). diff --git a/arch b/arch index 5f3f8a53bbc..131d15e8191 160000 --- a/arch +++ b/arch @@ -1 +1 @@ -Subproject commit 5f3f8a53bbcc0fd201bed99de4f69a6fa97597eb +Subproject commit 131d15e81914f13308331ebe10a8737bade31601 diff --git a/configs b/configs index e1e1873700f..0198c46e0bd 160000 --- a/configs +++ b/configs @@ -1 +1 @@ -Subproject commit e1e1873700ffda7aeb632e2b1234333d88ce3457 +Subproject commit 0198c46e0bd38d2a7fada9e4a1ea445218284c61 diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c index ddaa8a5a1cf..aa50d035afc 100644 --- a/drivers/net/loopback.c +++ b/drivers/net/loopback.c @@ -96,6 +96,7 @@ struct lo_driver_s { bool lo_bifup; /* true:ifup false:ifdown */ + bool lo_txdone; /* One RX packet was looped back */ WDOG_ID lo_polldog; /* TX poll timer */ struct work_s lo_work; /* For deferring work to the work queue */ @@ -111,7 +112,7 @@ struct lo_driver_s static struct lo_driver_s g_loopback; #ifdef CONFIG_NET_MULTIBUFFER -static uint8_t g_iobuffer[[MAX_NET_DEV_MTU + CONFIG_NET_GUARDSIZE]; +static uint8_t g_iobuffer[MAX_NET_DEV_MTU + CONFIG_NET_GUARDSIZE]; #endif /**************************************************************************** @@ -146,8 +147,8 @@ static int lo_rmmac(FAR struct net_driver_s *dev, FAR const uint8_t *mac); * * Description: * Check if the network has any outgoing packets ready to send. This is - * a callback from devif_poll(). devif_poll() will be called only during - * normal TX polling. + * a callback from devif_poll() or devif_timer(). devif_poll() will be + * called only during normal TX polling. * * Parameters: * dev - Reference to the NuttX driver state structure @@ -201,6 +202,8 @@ static int lo_txpoll(FAR struct net_driver_s *dev) ndbg("WARNING: Unrecognized packet type dropped: %02x\n", IPv4BUF->vhl); priv->lo_dev.d_len = 0; } + + priv->lo_txdone = true; } return 0; @@ -231,8 +234,19 @@ static void lo_poll_work(FAR void *arg) /* Perform the poll */ state = net_lock(); + priv->lo_txdone = false; (void)devif_timer(&priv->lo_dev, lo_txpoll, LO_POLLHSEC); + /* Was something received and looped back? */ + + while (priv->lo_txdone) + { + /* Yes, poll again for more TX data */ + + priv->lo_txdone = false; + (void)devif_poll(&priv->lo_dev, lo_txpoll); + } + /* Setup the watchdog poll timer again */ (void)wd_start(priv->lo_polldog, LO_WDDELAY, lo_poll_expiry, 1, priv); @@ -379,9 +393,14 @@ static void lo_txavail_work(FAR void *arg) state = net_lock(); if (priv->lo_bifup) { - /* If so, then poll the network for new XMIT data */ + do + { + /* If so, then poll the network for new XMIT data */ - (void)devif_poll(&priv->lo_dev, lo_txpoll); + priv->lo_txdone = false; + (void)devif_poll(&priv->lo_dev, lo_txpoll); + } + while (priv->lo_txdone); } net_unlock(state); diff --git a/net/Kconfig b/net/Kconfig index f7e5f67cfa6..b13a38351cb 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -102,7 +102,7 @@ config NET_SLIP_MTU bytes of data: 40 + 128. I believe that is to allow for the 2x worst cast packet expansion. Ideally we would like to advertise the 256 MSS, but restrict transfers to 128 bytes (possibly by modifying - the tcp_mss() macro). + the MSS value in the TCP connection structure). config NET_SLIP_TCP_RECVWNDO diff --git a/net/devif/devif.h b/net/devif/devif.h index d4ecdd023f5..a7c26d34371 100644 --- a/net/devif/devif.h +++ b/net/devif/devif.h @@ -423,8 +423,8 @@ uint16_t devif_dev_event(FAR struct net_driver_s *dev, void *pvconn, * The amount of data that actually is sent out after a call to this * function is determined by the maximum amount of data TCP allows. uIP * will automatically crop the data so that only the appropriate - * amount of data is sent. The function tcp_mss() can be used to query - * uIP for the amount of data that actually will be sent. + * amount of data is sent. The mss field of the TCP connection structure + * can be used to determine the amount of data that actually will be sent. * * Note: This function does not guarantee that the sent data will * arrive at the destination. If the data is lost in the network, the diff --git a/net/netdev/netdev_rxnotify.c b/net/netdev/netdev_rxnotify.c index 824c78f89c5..afef07e545d 100644 --- a/net/netdev/netdev_rxnotify.c +++ b/net/netdev/netdev_rxnotify.c @@ -79,7 +79,7 @@ * * Description: * Notify the device driver that forwards the IPv4 address that the - * application waits for RX data. + * application waits for RX data. * * Parameters: * lipaddr - The local board IPv6 address of the socket diff --git a/net/socket/connect.c b/net/socket/connect.c index 77443a775e6..00e2db1e05a 100644 --- a/net/socket/connect.c +++ b/net/socket/connect.c @@ -278,22 +278,32 @@ static uint16_t psock_connect_interrupt(FAR struct net_driver_s *dev, #ifdef CONFIG_NET_IPv4 #ifdef CONFIG_NET_IPv6 - if (pstate->tc_conn->domain == PF_INET) + if (pstate->tc_conn->domain == PF_INET) #endif - { - pstate->tc_conn->mss = TCP_IPv4_INITIAL_MSS(dev); - } + { + pstate->tc_conn->mss = TCP_IPv4_INITIAL_MSS(dev); + } #endif /* CONFIG_NET_IPv4 */ #ifdef CONFIG_NET_IPv6 #ifdef CONFIG_NET_IPv4 - else + else #endif - { - pstate->tc_conn->mss = TCP_IPv4_INITIAL_MSS(dev); - } + { + pstate->tc_conn->mss = TCP_IPv4_INITIAL_MSS(dev); + } #endif /* CONFIG_NET_IPv6 */ +#ifdef CONFIG_NETDEV_MULTINIC + /* We now have to filter all outgoing transfers so that they use only + * the MSS of this device. + */ + + DEBUGASSERT(pstate->tc_conn->dev == NULL || + pstate->tc_conn->dev == dev); + pstate->tc_conn->dev = dev; + +#endif /* CONFIG_NETDEV_MULTINIC */ #endif /* CONFIG_NET_MULTILINK */ /* Wake up the waiting thread */ diff --git a/net/socket/net_monitor.c b/net/socket/net_monitor.c index a57308cbcc7..ed2f0acc53e 100644 --- a/net/socket/net_monitor.c +++ b/net/socket/net_monitor.c @@ -163,6 +163,24 @@ static uint16_t connection_event(FAR struct net_driver_s *dev, else if ((flags & TCP_CONNECTED) != 0) { +#if 0 /* REVISIT: Assertion fires. Why? */ +#ifdef CONFIG_NETDEV_MULTINIC + FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)psock->s_conn; + + /* Make sure that this is the device bound to the connection */ + + DEBUGASSERT(conn->dev == NULL || conn->dev == dev); + conn->dev = dev; +#endif +#endif + + /* If there is no local address assigned to the socket (perhaps + * because it was INADDR_ANY), then assign it the address of the + * connecting device. + * + * TODO: Implement this. + */ + /* Indicate that the socket is now connected */ psock->s_flags |= _SF_CONNECTED; diff --git a/net/socket/net_sendfile.c b/net/socket/net_sendfile.c index 2f6c3c9bb52..9bb413cefdb 100644 --- a/net/socket/net_sendfile.c +++ b/net/socket/net_sendfile.c @@ -324,6 +324,18 @@ static uint16_t sendfile_interrupt(FAR struct net_driver_s *dev, FAR void *pvcon FAR struct sendfile_s *pstate = (FAR struct sendfile_s *)pvpriv; int ret; +#ifdef CONFIG_NETDEV_MULTINIC + /* The TCP socket is connected and, hence, should be bound to a device. + * Make sure that the polling device is the own that we are bound to. + */ + + DEBUGASSERT(conn->dev != NULL); + if (dev != conn->dev) + { + return flags; + } +#endif + nllvdbg("flags: %04x acked: %d sent: %d\n", flags, pstate->snd_acked, pstate->snd_sent); @@ -354,9 +366,9 @@ static uint16_t sendfile_interrupt(FAR struct net_driver_s *dev, FAR void *pvcon uint32_t sndlen = pstate->snd_flen - pstate->snd_sent; - if (sndlen > tcp_mss(conn)) + if (sndlen > conn->mss) { - sndlen = tcp_mss(conn); + sndlen = conn->mss; } /* Check if we have "space" in the window */ diff --git a/net/socket/recvfrom.c b/net/socket/recvfrom.c index f6519de1866..d7094d21e03 100644 --- a/net/socket/recvfrom.c +++ b/net/socket/recvfrom.c @@ -157,7 +157,7 @@ static inline void recvfrom_add_recvlen(FAR struct recvfrom_s *pstate, * The number of bytes taken from the packet. * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ @@ -205,7 +205,7 @@ static size_t recvfrom_newdata(FAR struct net_driver_s *dev, * None. * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ @@ -249,7 +249,7 @@ static void recvfrom_newpktdata(FAR struct net_driver_s *dev, * None. * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ @@ -321,7 +321,7 @@ static inline void recvfrom_newtcpdata(FAR struct net_driver_s *dev, * None. * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ @@ -353,7 +353,7 @@ static inline void recvfrom_newudpdata(FAR struct net_driver_s *dev, * None * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ @@ -520,7 +520,7 @@ out: * TRUE:timeout FALSE:no timeout * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ @@ -616,7 +616,7 @@ static inline void recvfrom_pktsender(FAR struct net_driver_s *dev, #ifdef CONFIG_NET_PKT static uint16_t recvfrom_pktinterrupt(FAR struct net_driver_s *dev, - FAR void *conn, FAR void *pvpriv, + FAR void *pvconn, FAR void *pvpriv, uint16_t flags) { struct recvfrom_s *pstate = (struct recvfrom_s *)pvpriv; @@ -744,24 +744,40 @@ static inline void recvfrom_tcpsender(FAR struct net_driver_s *dev, * * Parameters: * dev The structure of the network driver that caused the interrupt - * conn The connection structure associated with the socket + * pvconn The connection structure associated with the socket * flags Set of events describing why the callback was invoked * * Returned Value: * None * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ #ifdef CONFIG_NET_TCP static uint16_t recvfrom_tcpinterrupt(FAR struct net_driver_s *dev, - FAR void *conn, FAR void *pvpriv, + FAR void *pvconn, FAR void *pvpriv, uint16_t flags) { FAR struct recvfrom_s *pstate = (struct recvfrom_s *)pvpriv; +#if 0 /* REVISIT: The assertion fires. Why? */ +#ifdef CONFIG_NETDEV_MULTINIC + FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn; + + /* The TCP socket is connected and, hence, should be bound to a device. + * Make sure that the polling device is the own that we are bound to. + */ + + DEBUGASSERT(conn->dev == NULL || conn->dev == dev); + if (conn->dev != NULL && conn->dev != dev) + { + return flags; + } +#endif +#endif + nllvdbg("flags: %04x\n", flags); /* 'priv' might be null in some race conditions (?) */ @@ -956,7 +972,7 @@ static uint16_t recvfrom_tcpinterrupt(FAR struct net_driver_s *dev, * None * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ @@ -1049,7 +1065,8 @@ static inline void recvfrom_udpsender(struct net_driver_s *dev, struct recvfrom_ * Terminate the UDP transfer. * * Parameters: - * conn The connection structure associated with the socket + * pstate - The recvfrom state structure + * result - The result of the operation * * Returned Value: * None @@ -1086,14 +1103,14 @@ static void recvfrom_udp_terminate(FAR struct recvfrom_s *pstate, int result) * * Parameters: * dev The structure of the network driver that caused the interrupt - * conn The connection structure associated with the socket + * pvconn The connection structure associated with the socket * flags Set of events describing why the callback was invoked * * Returned Value: * None * * Assumptions: - * Running at the interrupt level + * The network is locked. * ****************************************************************************/ diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index 7e958b60068..759b50deb8b 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -101,12 +101,6 @@ devif_conn_callback_free(g_netdevices, cb, NULL) #endif -/* Get the current maximum segment size that can be sent on the current - * TCP connection. - */ - -#define tcp_mss(conn) ((conn)->mss) - #ifdef CONFIG_NET_TCP_WRITE_BUFFERS /* TCP write buffer access macros */ diff --git a/net/tcp/tcp_conn.c b/net/tcp/tcp_conn.c index fd411cf8dc7..f6e33e6ac0e 100644 --- a/net/tcp/tcp_conn.c +++ b/net/tcp/tcp_conn.c @@ -1003,6 +1003,13 @@ FAR struct tcp_conn_s *tcp_alloc_accept(FAR struct net_driver_s *dev, net_ipv6addr_copy(conn->u.ipv6.raddr, ip->srcipaddr); #ifdef CONFIG_NETDEV_MULTINIC net_ipv6addr_copy(conn->u.ipv6.laddr, ip->destipaddr); + + /* We now have to filter all outgoing transfers so that they use + * only the MSS of this device. + */ + + DEBUGASSERT(conn->dev == NULL || conn->dev == dev); + conn->dev = dev; #endif /* Find the device that can receive packets on the network @@ -1028,6 +1035,13 @@ FAR struct tcp_conn_s *tcp_alloc_accept(FAR struct net_driver_s *dev, #ifdef CONFIG_NETDEV_MULTINIC net_ipv4addr_copy(conn->u.ipv4.laddr, net_ip4addr_conv32(ip->destipaddr)); + + /* We now have to filter all outgoing transfers so that they use + * only the MSS of this device. + */ + + DEBUGASSERT(conn->dev == NULL || conn->dev == dev); + conn->dev = dev; #endif /* Find the device that can receive packets on the network @@ -1238,8 +1252,9 @@ int tcp_connect(FAR struct tcp_conn_s *conn, FAR const struct sockaddr *addr) goto errout_with_lock; } - /* Set up the local address (laddr) and the remote address (raddr) that describes the TCP connection. - */ + /* Set up the local address (laddr) and the remote address (raddr) that + * describes the TCP connection. + */ #ifdef CONFIG_NET_IPv4 #ifdef CONFIG_NET_IPv6 diff --git a/net/tcp/tcp_listen.c b/net/tcp/tcp_listen.c index dbe56b9f2a6..b4990ba1663 100644 --- a/net/tcp/tcp_listen.c +++ b/net/tcp/tcp_listen.c @@ -243,7 +243,7 @@ bool tcp_islistener(uint16_t portno) * Accept the new connection for the specified listening port. * * Assumptions: - * Called at interrupt level + * Called with the network locked. * ****************************************************************************/ diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index d8ea4a66b7a..34e9100ad07 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -338,6 +338,18 @@ static uint16_t psock_send_interrupt(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn; FAR struct socket *psock = (FAR struct socket *)pvpriv; +#ifdef CONFIG_NETDEV_MULTINIC + /* The TCP socket is connected and, hence, should be bound to a device. + * Make sure that the polling device is the own that we are bound to. + */ + + DEBUGASSERT(conn->dev != NULL); + if (dev != conn->dev) + { + return flags; + } +#endif + nllvdbg("flags: %04x\n", flags); /* If this packet contains an acknowledgement, then update the count of @@ -697,9 +709,9 @@ static uint16_t psock_send_interrupt(FAR struct net_driver_s *dev, */ sndlen = WRB_PKTLEN(wrb) - WRB_SENT(wrb); - if (sndlen > tcp_mss(conn)) + if (sndlen > conn->mss) { - sndlen = tcp_mss(conn); + sndlen = conn->mss; } if (sndlen > conn->winsize) diff --git a/net/tcp/tcp_send_unbuffered.c b/net/tcp/tcp_send_unbuffered.c index 95263324244..efffe4793f5 100644 --- a/net/tcp/tcp_send_unbuffered.c +++ b/net/tcp/tcp_send_unbuffered.c @@ -290,6 +290,18 @@ static uint16_t tcpsend_interrupt(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn; FAR struct send_s *pstate = (FAR struct send_s *)pvpriv; +#ifdef CONFIG_NETDEV_MULTINIC + /* The TCP socket is connected and, hence, should be bound to a device. + * Make sure that the polling device is the own that we are bound to. + */ + + DEBUGASSERT(conn->dev != NULL); + if (dev != conn->dev) + { + return flags; + } +#endif + nllvdbg("flags: %04x acked: %d sent: %d\n", flags, pstate->snd_acked, pstate->snd_sent); @@ -442,12 +454,12 @@ static uint16_t tcpsend_interrupt(FAR struct net_driver_s *dev, if (sndlen >= CONFIG_NET_TCP_SPLIT_SIZE) { /* sndlen is the number of bytes remaining to be sent. - * tcp_mss(conn) will return the number of bytes that can sent + * conn->mss will provide the number of bytes that can sent * in one packet. The difference, then, is the number of bytes * that would be sent in the next packet after this one. */ - int32_t next_sndlen = sndlen - tcp_mss(conn); + int32_t next_sndlen = sndlen - conn->mss; /* Is this the even packet in the packet pair transaction? */ @@ -474,13 +486,13 @@ static uint16_t tcpsend_interrupt(FAR struct net_driver_s *dev, { /* Will there be another (even) packet afer this one? * (next_sndlen > 0) Will the split condition occur on that - * next, even packet? ((next_sndlen - tcp_mss(conn)) < 0) If + * next, even packet? ((next_sndlen - conn->mss) < 0) If * so, then perform the split now to avoid the case where the * byte count is less than CONFIG_NET_TCP_SPLIT_SIZE on the * next pair. */ - if (next_sndlen > 0 && (next_sndlen - tcp_mss(conn)) < 0) + if (next_sndlen > 0 && (next_sndlen - conn->mss) < 0) { /* Here, we know that sndlen must be MSS < sndlen <= 2*MSS * and so (sndlen / 2) is <= MSS. @@ -497,9 +509,9 @@ static uint16_t tcpsend_interrupt(FAR struct net_driver_s *dev, #endif /* CONFIG_NET_TCP_SPLIT */ - if (sndlen > tcp_mss(conn)) + if (sndlen > conn->mss) { - sndlen = tcp_mss(conn); + sndlen = conn->mss; } /* Check if we have "space" in the window */