Re: [PATCH net-next 08/10] net: lan966x: add PCIe FDMA XDP support

[Daniel Machon]

Hi Mohsin,

> > +static int lan966x_xdp_pci_run(struct lan966x_port *port, void *data,
> > +                            u32 data_len)
> > +{
> > +     struct bpf_prog *xdp_prog = port->xdp_prog;
> > +     struct lan966x *lan966x = port->lan966x;
> > +     struct xdp_buff xdp;
> > +     u32 act;
> > +
> > +     xdp_init_buff(&xdp, lan966x->rx.max_mtu, &port->xdp_rxq);
> > +
> > +     xdp_prepare_buff(&xdp,
> > +                      data - XDP_PACKET_HEADROOM,
> > +                      IFH_LEN_BYTES + XDP_PACKET_HEADROOM,
> > +                      data_len - IFH_LEN_BYTES,
> > +                      false);
> > +
> > +     act = bpf_prog_run_xdp(xdp_prog, &xdp);
> > +     switch (act) {
> > +     case XDP_PASS:
> > +             return FDMA_PASS;
> > +     case XDP_TX:
> > +             return lan966x_fdma_pci_xmit_xdpf(port, data, data_len) ?
> > +                    FDMA_DROP : FDMA_TX;
> 
> What if the BPF program modifies packet boundaries (e.g.,
> headroom/tailroom adjustment)? After bpf_prog_run_xdp(), xdp.data and
> xdp.data_end may differ from the original data and data_len, but
> lan966x_fdma_pci_xmit_xdpf() is called with the original values.
> Wouldn't any adjustments made by the XDP program be silently lost?
>

I think you are absolutely right. Thanks for catching this. Actually, we might
have the same issue in the platform XDP path also.

I will fix this in v2.

> > +     default:
> > +             bpf_warn_invalid_xdp_action(port->dev, xdp_prog, act);
> > +             fallthrough;
> > +     case XDP_ABORTED:
> > +             trace_xdp_exception(port->dev, xdp_prog, act);
> > +             fallthrough;
> > +     case XDP_DROP:
> > +             return FDMA_DROP;
> > +     }
> > +}
> > +

/Daniel