Re: [PATCH net-next v3 5/7] net: lan966x: Update dma_dir of page_pool_params

Alexander Lobakin
2022-11-22T11:45:08Z

From: Horatiu Vultur <horatiu.vultur@microchip.com>
Date: Mon, 21 Nov 2022 22:28:48 +0100

> To add support for XDP_TX it is required to be able to write to the DMA
> area therefore it is required that the pages will be mapped using
> DMA_BIDIRECTIONAL flag.
> Therefore check if there are any xdp programs on the interfaces and in
> that case set DMA_BIDRECTIONAL otherwise use DMA_FROM_DEVICE.
> Therefore when a new XDP program is added it is required to redo the
> page_pool.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> .../ethernet/microchip/lan966x/lan966x_fdma.c | 29 ++++++++++++++----
> .../ethernet/microchip/lan966x/lan966x_main.h | 2 ++
> .../ethernet/microchip/lan966x/lan966x_xdp.c | 30 +++++++++++++++++++
> 3 files changed, 55 insertions(+), 6 deletions(-)

[...]

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
> index 8ebde1eb6a09c..05c5a28206558 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
> @@ -11,6 +11,8 @@ static int lan966x_xdp_setup(struct net_device *dev, struct netdev_bpf *xdp)
> struct lan966x_port *port = netdev_priv(dev);
> struct lan966x *lan966x = port->lan966x;
> struct bpf_prog *old_prog;
> + bool old_xdp, new_xdp;
> + int err;
>
> if (!lan966x->fdma) {
> NL_SET_ERR_MSG_MOD(xdp->extack,
> @@ -18,7 +20,20 @@ static int lan966x_xdp_setup(struct net_device *dev, struct netdev_bpf *xdp)
> return -EOPNOTSUPP;
> }
>
> + old_xdp = lan966x_xdp_present(lan966x);
> old_prog = xchg(&port->xdp_prog, xdp->prog);
> + new_xdp = lan966x_xdp_present(lan966x);
> +
> + if (old_xdp != new_xdp)
> + goto out;

Shouldn't it be the other way around? E.g. when there's no prog and
you're installing it or there is a prog and we're removing it from
the interface, DMA dir must be changed, so we reload the Pools, but
if `old_xdp == new_xdp` we should just hotswap them and goto out?

> +
> + err = lan966x_fdma_reload_page_pool(lan966x);
> + if (err) {
> + xchg(&port->xdp_prog, old_prog);
> + return err;
> + }
> +
> +out:
> if (old_prog)
> bpf_prog_put(old_prog);
>

[...]

> --
> 2.38.0

Thanks,
Olek