On Thu, Jan 19, 2017 at 6:00 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Thu, 2017-01-19 at 17:29 -0800, Tom Herbert wrote: >> Hi Herbert, >> >> I am looking at "net: Fix skb csum races when peeking". The context is >> that I'm seeing checksum faults after backporting mlx5 from driver. >> The handling of the checksum faults in interesting in itself. One side >> effect is that we end up recalculating and doing the dump twice when >> GRO is enabled. Another is that we validate checksum complete in a >> shared skbuf with a knowingly bad skb->csum value. >> >> In the checksum fault case we recalculate the checksum and find it was >> actually correct in the packet. If the skb is not shared then we set >> the skb->csum to the correct value and also set skb->csum_complete_sw >> and skb->csum_valid. But if skb is shared we don't do any of that. >> This is kind of odd since __skb_checksum_complete_head then returns >> zero for an skb with checksum_complete where the checksum complete >> value is not correct. I'm not sure what the consequences of doing that >> are (I don't think it's related to issues I'm seeing) but this doesn't >> seem like a good thing. Maybe we should set skb->csum in this case >> since we know it's already wrong and the fact that skbuf is shared >> can't make it more wrong... > > Why GRO can not change skb fields ? > Sure, I just noted this fact. We don't typically expect the device to be giving us miscomputed checksum complete values ;-)
> skb can not be shared at GRO layer, so really this should be done there > if you want to avoid a double checksumming later. > > You added all these skb fields, you know better than us ;) > > >
