[Intel-wired-lan] [PATCH 2/3 net-next v5] igb: refactor XDP registration
Alexander Lobakin
alexandr.lobakin at intel.com
Tue Jan 18 15:05:12 UTC 2022
From: Corinna Vinschen <vinschen at redhat.com>
Date: Mon, 17 Jan 2022 19:29:14 +0100
> On changing the RX ring parameters igb uses a hack to avoid a warning
> when calling xdp_rxq_info_reg via igb_setup_rx_resources. It just
> clears the struct xdp_rxq_info content.
>
> Change this to unregister if we're already registered instead. Align
> code to the igc code.
>
> Fixes: 9cbc948b5a20c ("igb: add XDP support")
> Signed-off-by: Corinna Vinschen <vinschen at redhat.com>
> ---
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ----
> drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++++++---
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 51a2dcaf553d..2a5782063f4c 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -965,10 +965,6 @@ static int igb_set_ringparam(struct net_device *netdev,
> memcpy(&temp_ring[i], adapter->rx_ring[i],
> sizeof(struct igb_ring));
>
> - /* Clear copied XDP RX-queue info */
> - memset(&temp_ring[i].xdp_rxq, 0,
> - sizeof(temp_ring[i].xdp_rxq));
> -
> temp_ring[i].count = new_rx_count;
> err = igb_setup_rx_resources(&temp_ring[i]);
> if (err) {
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 38ba92022cd4..cea89d301bfd 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4352,7 +4352,7 @@ int igb_setup_rx_resources(struct igb_ring *rx_ring)
> {
> struct igb_adapter *adapter = netdev_priv(rx_ring->netdev);
> struct device *dev = rx_ring->dev;
> - int size;
> + int size, res;
>
> size = sizeof(struct igb_rx_buffer) * rx_ring->count;
>
> @@ -4376,9 +4376,15 @@ int igb_setup_rx_resources(struct igb_ring *rx_ring)
> rx_ring->xdp_prog = adapter->xdp_prog;
>
> /* XDP RX-queue info */
> - if (xdp_rxq_info_reg(&rx_ring->xdp_rxq, rx_ring->netdev,
> - rx_ring->queue_index, 0) < 0)
> + if (xdp_rxq_info_is_reg(&rx_ring->xdp_rxq))
> + xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
> + res = xdp_rxq_info_reg(&rx_ring->xdp_rxq, rx_ring->netdev,
> + rx_ring->queue_index, 0);
> + if (res < 0) {
> + dev_err(dev, "Failed to register xdp_rxq index %u\n",
> + rx_ring->queue_index);
> goto err;
Error path always returns -ENOMEM, even in this case, and reports
that it failed to allocate memory for rings. Handle this correctly
and return `res` instead and without one more error message?
> + }
As I mentioned a bit above, `res` is unused here as an error code,
only to test the value on < 0. Does it make sense to add a new
variable?
>
> return 0;
>
> --
> 2.27.0
Thanks,
Al
More information about the Intel-wired-lan
mailing list