linux-kernel - RE: [EXT] Re: [PATCH v4 net-next 10/19] net: mvpp2: add FCA RXQ non occupied descriptor threshold

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]

Date:   Wed, 27 Jan 2021 18:41:32 +0000
From:   Stefan Chulski <stefanc@...vell.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC:     Network Development <netdev@...r.kernel.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        David Miller <davem@...emloft.net>,
        "Nadav Haklai" <nadavh@...vell.com>,
        Yan Markman <ymarkman@...vell.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "mw@...ihalf.com" <mw@...ihalf.com>, Andrew Lunn <andrew@...n.ch>,
        "rmk+kernel@...linux.org.uk" <rmk+kernel@...linux.org.uk>,
        Antoine Tenart <atenart@...nel.org>
Subject: RE: [EXT] Re: [PATCH v4 net-next 10/19] net: mvpp2: add FCA RXQ non
 occupied descriptor threshold


 >
> > From: Stefan Chulski <stefanc@...vell.com>
> >
> > RXQ non occupied descriptor threshold would be used by Flow Control
> > Firmware feature to move to the XOFF mode.
> > RXQ non occupied threshold would change interrupt cause that polled by
> > CM3 Firmware.
> > Actual non occupied interrupt masked and won't trigger interrupt.
> 
> Does this mean that this change enables a feature, but it is unused due to a
> masked interrupt?

Firmware poll RXQ non occupied cause register to indicate if number of registers bellow threshold.
We do not trigger any interrupt, just poll this bit in CM3. So this cause always masked.

> >
> > Signed-off-by: Stefan Chulski <stefanc@...vell.com>
> > ---
> >  drivers/net/ethernet/marvell/mvpp2/mvpp2.h      |  3 ++
> >  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 46
> > +++++++++++++++++---
> >  2 files changed, 42 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > index 73f087c..9d8993f 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > @@ -295,6 +295,8 @@
> >  #define     MVPP2_PON_CAUSE_TXP_OCCUP_DESC_ALL_MASK
> 0x3fc00000
> >  #define     MVPP2_PON_CAUSE_MISC_SUM_MASK              BIT(31)
> >  #define MVPP2_ISR_MISC_CAUSE_REG               0x55b0
> > +#define MVPP2_ISR_RX_ERR_CAUSE_REG(port)       (0x5520 + 4 * (port))
> > +#define            MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK  0x00ff
> 
> The indentation in this file is inconsistent. Here even between the two newly
> introduced lines.

Ok, I would fix this.

> >         /* If the thread isn't used, don't do anything */
> > -       if (smp_processor_id() > port->priv->nthreads)
> > +       if (cpu >= port->priv->nthreads)
> >                 return;
> 
> Here and below, the change from greater than to greater than is really a
> (standalone) fix?

Ok, I would move this to separate patch.

> > +/* Routine set the number of non-occupied descriptors threshold that
> > +change
> > + * interrupt error cause polled by FW Flow Control  */
> 
> nit: no need for "Routine". Also, does "change .. cause" mean "that triggers
> an interrupt"?

Ok.

Thanks,
Stefan.