netdev - Re: [PATCH net-next 02/15] net: dsa: tag_brcm: use the dsa_xmit_port

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: <CAOiHx=miR4JAbnYeRJwcHowgBUmvCn4X19syCxuwk8N7=xAXRQ@mail.gmail.com>
Date: Thu, 27 Nov 2025 14:29:27 +0100
From: Jonas Gorski <jonas.gorski@...il.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Florian Fainelli <florian.fainelli@...adcom.com>
Subject: Re: [PATCH net-next 02/15] net: dsa: tag_brcm: use the
 dsa_xmit_port_mask() helper

On Thu, Nov 27, 2025 at 1:09 PM Vladimir Oltean <vladimir.oltean@....com> wrote:
>
> The "brcm" and "brcm-prepend" tagging protocols populate a bit mask for
> the TX ports, so we can use dsa_xmit_port_mask() to centralize the
> decision of how to set that field. The port mask is written u8 by u8,
> first the high octet and then the low octet.
>
> Cc: Florian Fainelli <florian.fainelli@...adcom.com>
> Cc: Jonas Gorski <jonas.gorski@...il.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  net/dsa/tag_brcm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
> index eadb358179ce..cf9420439054 100644
> --- a/net/dsa/tag_brcm.c
> +++ b/net/dsa/tag_brcm.c
> @@ -92,6 +92,7 @@ static struct sk_buff *brcm_tag_xmit_ll(struct sk_buff *skb,
>  {
>         struct dsa_port *dp = dsa_user_to_port(dev);
>         u16 queue = skb_get_queue_mapping(skb);
> +       u16 port_mask;
>         u8 *brcm_tag;
>
>         /* The Ethernet switch we are interfaced with needs packets to be at
> @@ -119,10 +120,9 @@ static struct sk_buff *brcm_tag_xmit_ll(struct sk_buff *skb,
>         brcm_tag[0] = (1 << BRCM_OPCODE_SHIFT) |
>                        ((queue & BRCM_IG_TC_MASK) << BRCM_IG_TC_SHIFT);
>         brcm_tag[1] = 0;
> -       brcm_tag[2] = 0;
> -       if (dp->index == 8)
> -               brcm_tag[2] = BRCM_IG_DSTMAP2_MASK;
> -       brcm_tag[3] = (1 << dp->index) & BRCM_IG_DSTMAP1_MASK;
> +       port_mask = dsa_xmit_port_mask(skb, dev);
> +       brcm_tag[2] = (port_mask >> 8) & BRCM_IG_DSTMAP2_MASK;
> +       brcm_tag[3] = port_mask & BRCM_IG_DSTMAP1_MASK;

Since this is a consecutive bitmask (actually [22:0]), I wonder if doing

put_unaligned_be16(port_mask, &brcm_tag[2]);

would be a bit more readable.

Or even more correct put_unaligned_be24(port_mask, &brcm_tag[1]), but
we don't support any switches with that many ports.

Best regards,
Jonas