On Fri, Nov 6, 2015 at 6:34 AM, [email protected] <[email protected]> wrote: > While reading through net-code i came across some code in tcp_sendpage which > i think > it is not working in the intended way all tie time. But as this code is at > that a > central place and pretty old, im suspicious if my analysis is really right. > > The code in question is this > > (from net/ipv4/tcp.c) > > if (!(sk->sk_route_caps & NETIF_F_SG) || > !(sk->sk_route_caps & NETIF_F_ALL_CSUM)) > return sock_no_sendpage(sk->sk_socket, page, offset, size, > flags); > > > especially this part. > > !(sk->sk_route_caps & NETIF_F_ALL_CSUM) > > The problem occurs if a device only supports checksumming for ipv4 or ipv6. In > this cases only NETIF_F_IP_CSUM or NETIF_F_IPV6_CSUM is set. > > Lets assume a device is only supporting checksumming in ipv6 via > NETIF_F_IPV6_CSUM, > and we are calling tcp_sendpage for a ipv4 connection. From my understanding > the intend > of the above code is that in this case sock_no_sendpage should be called. But > the bit-check > against NETIF_F_ALL_CSUM will be always > 0, as it is defined as > > (from include/linux/netdev_features.h) > > #define NETIF_F_V4_CSUM (NETIF_F_GEN_CSUM | NETIF_F_IP_CSUM) > #define NETIF_F_V6_CSUM (NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM) > #define NETIF_F_ALL_CSUM (NETIF_F_V4_CSUM | NETIF_F_V6_CSUM) > > so it will never get in that branch. > > So imo the code should be something like this (in pseudocode) > > (!(sk->sk_route_caps & NETIF_F_V4_CSUM) && proto == IPV4) || > (!(sk->sk_route_caps & NETIF_F_V6_CSUM) && proto == IPV6) > > So what am i missing?
Well, looking at the code I don't readily see how this works either, but note that tcp_sendmsg has the same check also. If your analysis is correct then checksum for IPv6 would commonly be broken (i.e. several devices support IPv4 checksum but not IPv6)-- so I find that hard to believe so I'm probably missing something too! (at least this ambiguity is one more reason why we need to get rid of NETIF_F_IP_CSUM and NETIF_F_V6_CSUM!). Can you try to verify this a bug with some testing? Thanks, Tom > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
