From b1e09d4847693daa480d16a12ee69cfa48febc9a Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 27 Aug 2015 09:06:46 -0600 Subject: [PATCH] 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. --- ChangeLog | 9 +++++++++ net/socket/connect.c | 26 ++++++++++++++++++-------- net/socket/net_sendfile.c | 12 ++++++++++++ net/tcp/tcp_conn.c | 14 ++++++++++++++ net/tcp/tcp_send_buffered.c | 12 ++++++++++++ net/tcp/tcp_send_unbuffered.c | 12 ++++++++++++ 6 files changed, 77 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2bda7e474b1..cc6e9cd57a7 100755 --- a/ChangeLog +++ b/ChangeLog @@ -10883,4 +10883,13 @@ SaeHie Park (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/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_sendfile.c b/net/socket/net_sendfile.c index 443a66357d3..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); diff --git a/net/tcp/tcp_conn.c b/net/tcp/tcp_conn.c index 32ea2d6901e..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 diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index 3044b5f030e..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 diff --git a/net/tcp/tcp_send_unbuffered.c b/net/tcp/tcp_send_unbuffered.c index b8052a1ec7b..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);