linux-kernel - Re: [PATCH net-next v2 2/3] net: lan966x: Add PGID_FIRST and PGID

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:   Tue, 4 Jan 2022 11:21:20 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>
Subject: Re: [PATCH net-next v2 2/3] net: lan966x: Add PGID_FIRST and
 PGID_LAST

On Tue, Jan 04, 2022 at 11:18:48AM +0100, Horatiu Vultur wrote:
> The first entries in the PGID table are used by the front ports while
> the last entries are used for different purposes like flooding mask,
> copy to CPU, etc. So add these macros to define which entries can be
> used for general purpose.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> ---

Not too thrilled by the "first" and "last" names, since there are PGIDs
before the "first" PGID, and after the "last" PGID, too. I can see how
others may get confused about this. In the ocelot driver they are called
"nonreserved" PGIDs. It also doesn't help that PGID_FIRST and PGID_LAST
are defined under the "Reserved PGIDs" comment, because they aren't reserved.

>  drivers/net/ethernet/microchip/lan966x/lan966x_main.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> index f70e54526f53..190d62ced3fd 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> @@ -30,7 +30,11 @@
>  /* Reserved amount for (SRC, PRIO) at index 8*SRC + PRIO */
>  #define QSYS_Q_RSRV			95
>  
> +#define CPU_PORT			8
> +
>  /* Reserved PGIDs */
> +#define PGID_FIRST			(CPU_PORT + 1)
> +#define PGID_LAST			PGID_CPU

Since PGID_LAST is defined in terms of PGID_CPU, I would put it _below_
PGID_CPU.

>  #define PGID_CPU			(PGID_AGGR - 6)
>  #define PGID_UC				(PGID_AGGR - 5)
>  #define PGID_BC				(PGID_AGGR - 4)
> @@ -44,8 +48,6 @@
>  #define LAN966X_SPEED_100		2
>  #define LAN966X_SPEED_10		3
>  
> -#define CPU_PORT			8
> -
>  /* MAC table entry types.
>   * ENTRYTYPE_NORMAL is subject to aging.
>   * ENTRYTYPE_LOCKED is not subject to aging.
> -- 
> 2.33.0
>