On Thu, Jun 6, 2024 at 9:49 AM Mina Almasry almasrymina@google.com wrote:
On Tue, Jun 4, 2024 at 3:46 AM Paolo Abeni pabeni@redhat.com wrote:
On Thu, 2024-05-30 at 20:16 +0000, Mina Almasry wrote:
diff --git a/net/core/gro.c b/net/core/gro.c index 26f09c3e830b7..7b9d018f552bd 100644 --- a/net/core/gro.c +++ b/net/core/gro.c @@ -422,6 +422,9 @@ static void gro_pull_from_frag0(struct sk_buff *skb, int grow) { struct skb_shared_info *pinfo = skb_shinfo(skb);
if (WARN_ON_ONCE(!skb_frags_readable(skb)))return;BUG_ON(skb->end - skb->tail < grow); memcpy(skb_tail_pointer(skb), NAPI_GRO_CB(skb)->frag0, grow);@@ -443,7 +446,7 @@ static void gro_try_pull_from_frag0(struct sk_buff *skb) { int grow = skb_gro_offset(skb) - skb_headlen(skb);
if (grow > 0)
if (grow > 0 && skb_frags_readable(skb)) gro_pull_from_frag0(skb, grow);}
I'm unsure if this was already mentioned, so please pardon the eventual duplicate...
The above code is quite critical performance wise, and the previous patch already prevent frag0 from being set to a non paged frag,
Hi Paolo!
The last patch, d4d25dd237a61 ("net: support non paged skb frags"), AFAICT doesn't prevent frag0 from being a non-paged frag. What we do is set ->frag0=skb->data, then prevent it from being reset to skb_frag_address() for non-paged skbs. ->frag0 will likely actually be a bad value for non-paged frags, so we need to check in gro_pul_from_frag0() so that we don't accidentally pull from a bad ->frag0 value.
What I think I should do here is what you said. I should make sure frag0 and frag0_len is not set if it's a non-paged frag. Then, we don't need special checks in gro_pull_from_frag0 I think, because skb_gro_may_pull() should detect that frag0_len is 0 and should prevent a pull.
I will apply this fix to the next iteration for your review. Let me know if I missed something.
Actually, sorry you're right. As written, d4d25dd237a61 ("net: support non paged skb frags") prevents frag0 from being a non-paged frag. I can just drop these excessive checks with no downside. Sorry for the noise!