On Mon, Dec 19, 2022 at 5:45 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > On Mon, 19 Dec 2022 17:21:27 -0800 Stanislav Fomichev wrote: > > > - skb_postpull_rcsum(skb, skb->data + off, len); > > > - memmove(skb->data + len, skb->data, off); > > > + old_data = skb->data; > > > __skb_pull(skb, len); > > > > [..] > > Very counter-productively trimmed ;) > > > > + skb_postpull_rcsum(skb, old_data + off, len); > > > > Are you sure about the 'old_data + off' part here (for > > CHECKSUM_COMPLETE)? Shouldn't it be old_data? > > AFAIU before: > > old_data (aka skb->data before) > / > / off len > V-----><---> > [ .=======xxxxx... buffer ...... ] > ^ > \ > the xxx part is what we delete > > after: > skb->data (after) > / > v > [ .yyyyy=======... buffer ...... ] > <---><-----> > len off > ^ > \ > the yyy part is technically the old values of === but now "outside" > of the valid packet data > > > I'm assuming we need to negate the old parts that we've pulled? > > Yes. > > > Maybe safer/more correct to do the following? > > > > skb_pull_rcsum(skb, off); > > This just pulls from the front, we need to skip over various L2/L3 > headers thanks to off. Hopefully the diagrams help, LMK if they are > wrong. Ah, yeah, you're totally correct, thanks for the pics :-) Not sure how I mixed up off/len in skb_pull_rcsum.. Acked-by: Stanislav Fomichev <sdf@xxxxxxxxxx> > > memmove(skb->data, skb->data-off, off);
