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 ? 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 ;)
