Date:   Tue,  9 Nov 2021 11:31:00 +0100
From:   Louis Amas <louis.amas@....link>
To:     Marcin Wojtas <mw@...ihalf.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        KP Singh <kpsingh@...nel.org>
Cc:     Louis Amas <louis.amas@....link>,
        Emmanuel Deloget <emmanuel.deloget@....link>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org
Subject: [PATCH] net: mvpp2: fix XDP rx queues registering

The registration of XDP queue information is incorrect because the RX queue id we use is invalid.
When port->id == 0 it appears to works as expected yet it's no longer the case when port->id != 0.

This is due to the fact that the value we use (rxq->id) is not the per-port queue index which
XDP expects -- it's a global index which should not be used in this case. Instead we shall use
rxq->logic_rxq which is the correct, per-port value.

Signed-off-by: Louis Amas <louis.amas@....link>
Signed-off-by: Emmanuel Deloget <emmanuel.deloget@....link>
---

As we were trying to capture packets using XDP on our mv8040-powered
MacchiatoBin, we experienced an issue related to rx queue numbering.

Before I get to the problem itself, a bit of setup:

* the Macchiato has several ports, all of them handled using the mvpp2
  ethernet driver. We are able to reproduce our issue on any device whose
  port->id != 0 (we used eth2 for our tests). When port->id == 0 (for
  example on eth0) everything works as expected ;

* we use xdp-tutorial for our tests ; more specifically, we used the
  advanced03-AF_XDP tutorial as it provides a simple testbed. We modified
  the kernel to simplify it:

        SEC("xdp_sock")
        int xdp_sock_prog(struct xdp_md *ctx)
        {
                int index = ctx->rx_queue_index;

                /* A set entry here means that the correspnding queue_id
                 * has an active AF_XDP socket bound to it. */
                if (bpf_map_lookup_elem(&xsks_map, &index))
                        return bpf_redirect_map(&xsks_map, index, 0);

                return XDP_PASS;
        }

* we tested kernel 5.10 (out target) and 5.15 (for reference) ; both kernels
  exhibits the same symptoms ; I expect kernel 5.9 (the first linux kernel
  with XDP support in the mvpp2 driver) to exhibit the same problem.

The normal invocation of this program would be:

        ./af_xdp_user -d ETHDEV

We should then capture packets on this interface. When ETHDEV is eth0
(port->id == 0) everything works as expcted ; when using ETHDEV == eth2
we fail to capture anything.

We investigated the issue and found that XDP rx queues (setup as
struct xdp_rxq_info by the mvpp2 driver) for this device were wrong. XDP
expected them to be numbered in [0..3] but we found numbers in [32..35].

The reason for this lies in mvpp2_main.c at lines 2962 and 2966 which are
of the form (symbolic notation, close to actual code, function
mvpp2_rxq_init()):

        err = xdp_rxq_info_reg(&rxq->some_xdp_rxqinfo, port->dev, rxq->id, 0);

The rxq->id value we pass to this function is incorrect - it's a virtual queue
id which is computed as (symbolic notation, not actual code):

        rxq->id = port->id * max_rxq_count + queue_id

In our case, max_rxq_count == 32 and port->id == 1 for eth2, meaning our
rxq->id are in the range [32..35] (for 4 queues).

We tried to force the rx queue id on the XDP side by using:

        ./af_xdp_user -d eth2 -Q 32

But that failed -- as expected, because we should not have more than 4
rx queues.

The computing of rxq->id is valid, but the use of rxq->id in this context is
not. What we really want here is the rx queue id for this port, and this value
is stored in rxq->logic_rxq -- as hinted by the code in mvpp2_rxq_init().
Replacing rxq->id by this value in the two xdp_rxq_info_reg() calls fixed the
issue and allowed us to use XDP on all the Macchiato ethernet ports.

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 587def69a6f7..f0ea377341c6 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -2959,11 +2959,11 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
        mvpp2_rxq_status_update(port, rxq->id, 0, rxq->size);

        if (priv->percpu_pools) {
-               err = xdp_rxq_info_reg(&rxq->xdp_rxq_short, port->dev, rxq->id, 0);
+               err = xdp_rxq_info_reg(&rxq->xdp_rxq_short, port->dev, rxq->logic_rxq, 0);
                if (err < 0)
                        goto err_free_dma;

-               err = xdp_rxq_info_reg(&rxq->xdp_rxq_long, port->dev, rxq->id, 0);
+               err = xdp_rxq_info_reg(&rxq->xdp_rxq_long, port->dev, rxq->logic_rxq, 0);
                if (err < 0)
                        goto err_unregister_rxq_short;

--
2.25.1


[eho.link event] <https://www.linkedin.com/company/eho.link/>

Powered by blists - more mailing lists