On Mon, Oct 26, 2015, at 20:39, Tom Herbert wrote: > On Mon, Oct 26, 2015 at 11:44 AM, Hannes Frederic Sowa > <han...@stressinduktion.org> wrote: > > > > > > On Mon, Oct 26, 2015, at 15:19, Tom Herbert wrote: > >> > We already concluded that drivers do have this problem and not the stack > >> > above ip6_fragment. The places I am aware of I fixed in this patch. Also > >> > IPv4 to me seems unaffected, albeit one can certainly clean up the logic > >> > in net-next. > >> > > >> I don't understand why checksum for IP fragments is a driver problem. > >> When fragments are sent to driver they should never have > >> CHECKSUM_PARTIAL set (or maybe that is what you are seeing?). > > > > Because either the drivers or the hardware does not correctly iterate > > over the extension headers to fetch the final nexthdr field which is > > used to compute the checksum. This is different from IPv4. > > > > I can only guess e.g. from the e1000e driver: > > > > case cpu_to_be16(ETH_P_IPV6): > > /* XXX not handling all IPV6 headers */ > > if (ipv6_hdr(skb)->nexthdr == IPPROTO_TCP) > > cmd_len |= E1000_TXD_CMD_TCP; > > break; > > > Yes, but in the case of a fragment that code should never be hit since > ip_summed shouldn't be CHECKSUM_PARTIAL for a fragment (maybe after > the fix in ip_output). For other cases of extension headers the e1000e > is broken since it apparently does call skb_checksum_help for > protocols it doesn't understand (the /* XXX not handling all IPV6 > headers */ comment is worrisome!)
Agreed! I am testing with WARN_ON_ONCE in ip6_fragment if I can hit another path where we would have to call skb_checksum_help. I need to review IPv4 tomorrow if we need to do according changes there, probably. Bye, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
