Re: [PATCH] net: fix wrong network header length

[Lina Wang]

On Thu, 2022-02-10 at 17:02 +0100, Paolo Abeni wrote:

> > @@ -3682,6 +3682,7 @@ struct sk_buff *skb_segment_list(struct
> > sk_buff *skb,
> >  	struct sk_buff *tail = NULL;
> >  	struct sk_buff *nskb, *tmp;
> >  	int err;
> > +	unsigned int len_diff = 0;
> 
> Mintor nit: please respect the reverse x-mas tree order.
> 

Yes,v2 has change unsigned int to int

> >  
> >  	skb_push(skb, -skb_network_offset(skb) + offset);
> > @@ -3721,9 +3722,11 @@ struct sk_buff *skb_segment_list(struct
> > sk_buff *skb,
> >  		skb_push(nskb, -skb_network_offset(nskb) + offset);
> >  
> >  		skb_release_head_state(nskb);
> > +		len_diff = skb_network_header_len(nskb) -
> > skb_network_header_len(skb);
> >  		 __copy_skb_header(nskb, skb);
> >  
> >  		skb_headers_offset_update(nskb, skb_headroom(nskb) -
> > skb_headroom(skb));
> > +		nskb->transport_header += len_diff;
> 
> This does not look correct ?!? the network hdr position for nskb will
> still be uncorrect?!? and even the mac hdr likely?!? possibly you
> need
> to change the offset in skb_headers_offset_update().
> 

Network hdr position and mac hdr are both right, because bpf processing & 
skb_headers_offset_update have updated them to right position. After bpf
loading, the first skb's network header&mac_header became 44, transport
header still is 64. After skb_headers_offset_update, fraglist skb's mac
header and network header are still 24, the same with original packet.
Just fraglist skb's transport header became 44, as original is 64. 
Only transport header cannot be easily updated the same offset, because 
6to4 has different network header.

Actually,at the beginning, I want to change skb_headers_offset_update, but 
it has been called also in other place, maybe a new function should be 
needed here.
 
Skb_headers_offset_update has other wrong part in my scenary, 
inner_transport_header\inner_network_header\inner_mac_header shouldnot be 
changed, but they are been updated because of different headroom. They are
not used later, so wrong value didnot affect anything.

> Paolo
>

Thanks!