linux-kernel - Re: [PATCH 1/1] net: mvpp2: fix XDP rx queues registering

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Ya4vd9+pBbVJML+K@shell.armlinux.org.uk>
Date:   Mon, 6 Dec 2021 15:42:47 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Emmanuel Deloget <emmanuel.deloget@....link>
Cc:     Louis Amas <louis.amas@....link>, andrii@...nel.org,
        ast@...nel.org, bpf@...r.kernel.org, daniel@...earbox.net,
        davem@...emloft.net, hawk@...nel.org, john.fastabend@...il.com,
        kafai@...com, kpsingh@...nel.org, kuba@...nel.org,
        linux-kernel@...r.kernel.org, mw@...ihalf.com,
        netdev@...r.kernel.org, songliubraving@...com, yhs@...com
Subject: Re: [PATCH 1/1] net: mvpp2: fix XDP rx queues registering

On Mon, Dec 06, 2021 at 04:37:20PM +0100, Emmanuel Deloget wrote:
> Hello,
> 
> On 10/11/2021 15:41, Louis Amas wrote:
> > 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.
> > 
> > When we register the XDP rx queue information (using
> > xdp_rxq_info_reg() in function mvpp2_rxq_init()) we tell them to use
> > rxq->id as the queue id. This value iscomputed as:
> > rxq->id = port->id * max_rxq_count + queue_id
> > 
> > where max_rxq_count depends on the device version. In the MB case,
> > this value is 32, meaning that rx queues on eth2 are numbered from
> > 32 to 35 - there are four of them.
> > 
> > Clearly, this is not the per-port queue id that XDP is expecting:
> > it wants a value in the range [0..3]. It shall directly use queue_id
> > which is stored in rxq->logic_rxq -- so let's use that value instead.
> > 
> > This is consistent with the remaining part of the code in
> > mvpp2_rxq_init().
> > 
> > Fixes: b27db2274ba8 ("mvpp2: use page_pool allocator")
> > Signed-off-by: Louis Amas <louis.amas@....link>
> > Signed-off-by: Emmanuel Deloget <emmanuel.deloget@....link>
> > Reviewed-by: Marcin Wojtas <mw@...ihalf.com>
> > ---
> > This is a repost of [1]. The patch itself is not changed, but the
> > commit message has been enhanced using part of the explaination in
> > order to make it clearer (hopefully) and to incorporate the
> > reviewed-by tag from Marcin.
> > 
> > v1: original patch
> > v2: revamped commit description (no change in the patch itself)
> > 
> > [1] https://lore.kernel.org/bpf/20211109103101.92382-1-louis.amas@eho.link/
> > 
> >   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;
> > 
> 
> Is there any update on this patch ? Without it, XDP only partially work on a
> MACCHIATOBin (read: it works on some ports, not on others, as described in
> our analysis sent together with the original patch).

Hi,

I suspect if you *didn't* thread your updated patch to your previous
submission, then it would end up with a separate entry in
patchwork.kernel.org, and the netdev maintainers will notice that the
patch is ready for inclusion, having been reviewed by Marcin.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!