Messages in this thread | | | Date | Wed, 26 Oct 2022 00:08:30 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH 1/1] net: fec: add initial XDP support |
| |
> +#define FEC_ENET_XDP_PASS 0 > +#define FEC_ENET_XDP_CONSUMED BIT(0) > +#define FEC_ENET_XDP_TX BIT(1) > +#define FEC_ENET_XDP_REDIR BIT(2)
I don't know XDP, so maybe a silly question. Are these action mutually exclusive? Are these really bits, or should it be an enum? fec_enet_run_xdp() does not combine them as bits.
> +static int fec_enet_bpf(struct net_device *dev, struct netdev_bpf *bpf) > +{ > + struct fec_enet_private *fep = netdev_priv(dev); > + bool is_run = netif_running(dev);
You have the space, so maybe call it is_running.
> + struct bpf_prog *old_prog; > + unsigned int dsize; > + int i; > + > + switch (bpf->command) { > + case XDP_SETUP_PROG: > + if (is_run) > + fec_enet_close(dev);
fec_net_close() followed by fec_enet_open() is pretty expensive. The PHY is stopped and disconnected, and then connected and started. That will probably trigger an auto-neg, which takes around 1.5 seconds before the link is up again.
Maybe you should optimise this. I guess the real issue here is you need to resize the RX ring. You need to be careful with that anyway. If the machine is under memory pressure, you might not be able to allocate the ring, resulting in a broken interface. What is recommended for ethtool --set-ring is that you first allocate the new ring, and if that is successful, free the old ring. If the allocation fails, you still have the old ring, and you can safely return -ENOMEM and still have a working interface.
So i think you can split this patch up into a few parts:
XDP using the default ring size. Your benchmarks show it works, its just not optimal. But the resulting smaller patch will be easier to review.
Add support for ethtool set-ring, which will allow you to pick apart the bits of fec_net_close() and fec_enet_open() which are needed for changing the rings. This might actually need a refactoring patch?
And then add support for optimal ring size for XDP.
Andrew
| |