Messages in this thread | | | Subject | Re: [PATCH net-next v2 2/3] net: use skb_for_each_frag() helper where possible | From | Eric Dumazet <> | Date | Mon, 12 Apr 2021 09:40:13 +0200 |
| |
On 4/12/21 2:38 AM, Matteo Croce wrote: > From: Matteo Croce <mcroce@microsoft.com> > > use the new helper macro skb_for_each_frag() which allows to iterate > through all the SKB fragments. > > The patch was created with Coccinelle, this was the semantic patch: > > @@ > struct sk_buff *skb; > identifier i; > statement S; > iterator name skb_for_each_frag; > @@ > -for (i = 0; i < skb_shinfo(skb)->nr_frags; \(++i\|i++\)) > +skb_for_each_frag(skb, i) > S > @@ > struct skb_shared_info *sinfo; > struct sk_buff *skb; > identifier i; > statement S; > iterator name skb_for_each_frag; > @@
I disagree with this part :
> sinfo = skb_shinfo(skb) > ... > -for (i = 0; i < sinfo->nr_frags; \(++i\|i++\)) > +skb_for_each_frag(skb, i) > S >
> index bde781f46b41..5de00477eaf9 100644 > --- a/net/ipv4/tcp_output.c > +++ b/net/ipv4/tcp_output.c > @@ -1644,7 +1644,7 @@ static int __pskb_trim_head(struct sk_buff *skb, int len) > eat = len; > k = 0; > shinfo = skb_shinfo(skb); > - for (i = 0; i < shinfo->nr_frags; i++) { > + skb_for_each_frag(skb, i) { > int size = skb_frag_size(&shinfo->frags[i]); > > if (size <= eat) {
This will force the compiler to re-evaluate skb_shinfo(skb)->nr_frags in the loop, since atomic operations like skb_frag_unref() have a memory clobber.
skb_shinfo(skb)->nr_frags has to reload three vars.
The macro should only be used when the code had
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
| |