Re: [PATCH net-next] xdp: Fixing skb->pp_recycle flag in generic XDP handling

[Liang Chen <liangchen.linux@gmail.com>]

On Thu, Aug 3, 2023 at 1:11 AM Alexander Lobakin
<aleksander.lobakin@intel.com> wrote:
>
> From: Liang Chen <liangchen.linux@gmail.com>
> Date: Wed,  2 Aug 2023 15:04:54 +0800
>
> > In the generic XDP processing flow, if an skb with a page pool page
> > (skb->pp_recycle == 1) fails to meet XDP packet requirements, it will
> > undergo head expansion and linearization of fragment data. As a result,
> > skb->head points to a reallocated buffer without any fragments. At this
> > point, the skb will not contain any page pool pages. However, the
> > skb->pp_recycle flag is still set to 1, which is inconsistent with the
> > actual situation. Although it doesn't seem to cause much real harm at the
>
> This means it must be handled in the function which replaces the head,
> i.e. pskb_expand_head(). Your change only suppresses one symptom of the
> issue.
>

I attempted to do so. But after pskb_expand_head, there may still be
skb frags with pp pages left. It is after skb_linearize those frags
are removed.

Thanks,
Liang

> > moment(a little nagetive impact on skb_try_coalesce), to avoid potential
>
>                   ^^^^^^^^
>                   negative
>

Sure, Thanks.


> > issues associated with using incorrect skb->pp_recycle information,
> > setting skb->pp_recycle to 0 to reflect the pp state of the skb.
> >
> > Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
>
> I won't say for sure, but may be a candidate for the fixes tree, not
> next. This way it would need a "Fixes:" tag here (above the SoB).
>
> > ---
> >  net/core/dev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 10e5a036c706..07baf72be7d7 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4934,6 +4934,8 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> >                       goto do_drop;
> >               if (skb_linearize(skb))
> >                       goto do_drop;
> > +             if (skb->pp_recycle)
> > +                     skb->pp_recycle = 0;
> >       }
> >
> >       act = bpf_prog_run_generic_xdp(skb, xdp, xdp_prog);
>
> Thanks,
> Olek