linux-kernel - Re: [PATCH net-next v3 1/2] net: stmmac: Refactor VLAN implementation

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: <20250410161912.0000168a@gmail.com>
Date: Thu, 10 Apr 2025 16:19:12 +0800
From: Furong Xu <0x1207@...il.com>
To: Boon Khai Ng <boon.khai.ng@...era.com>
Cc: netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 bpf@...r.kernel.org, Andrew Lunn <andrew+netdev@...n.ch>, "David S .
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Maxime
 Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue
 <alexandre.torgue@...s.st.com>, Russell King <linux@...linux.org.uk>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
 <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>, John
 Fastabend <john.fastabend@...il.com>, Matthew Gerlach
 <matthew.gerlach@...era.com>, Tien Sung Ang <tien.sung.ang@...era.com>, Mun
 Yew Tham <mun.yew.tham@...era.com>, G Thomas Rohan
 <rohan.g.thomas@...era.com>
Subject: Re: [PATCH net-next v3 1/2] net: stmmac: Refactor VLAN
 implementation

On Tue,  8 Apr 2025 16:13:53 +0800, Boon Khai Ng <boon.khai.ng@...era.com> wrote:

> Refactor VLAN implementation by moving common code for DWMAC4 and
> DWXGMAC IPs into a separate VLAN module. VLAN implementation for
> DWMAC4 and DWXGMAC differs only for CSR base address, the descriptor
> for the VLAN ID and VLAN VALID bit field.
> 
> Signed-off-by: Boon Khai Ng <boon.khai.ng@...era.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@...era.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   2 +-
>  drivers/net/ethernet/stmicro/stmmac/common.h  |   1 +
>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h  |  40 ---
>  .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 295 +-----------------
>  .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |  13 -
>  .../ethernet/stmicro/stmmac/dwxgmac2_core.c   |  87 ------
>  drivers/net/ethernet/stmicro/stmmac/hwif.c    |   8 +
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  61 ++--
>  .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 294 +++++++++++++++++
>  .../net/ethernet/stmicro/stmmac/stmmac_vlan.h |  63 ++++
>  10 files changed, 401 insertions(+), 463 deletions(-)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.h
> 
[...]
> +static void vlan_update_hash(struct mac_device_info *hw, u32 hash,
> +			     __le16 perfect_match, bool is_double)
> +{
> +	void __iomem *ioaddr = hw->pcsr;
> +	u32 value;
> +
> +	writel(hash, ioaddr + VLAN_HASH_TABLE);
> +
> +	value = readl(ioaddr + VLAN_TAG);
> +
> +	if (hash) {
> +		value |= VLAN_VTHM | VLAN_ETV;
> +		if (is_double) {
> +			value |= VLAN_EDVLP;
> +			value |= VLAN_ESVL;
> +			value |= VLAN_DOVLTC;

I can confirm that 802.1ad (QinQ) has been broken on stmmac for years,
and it will be so nice if this refactoring includes some fixes for QinQ

> +		}
> +
> +		writel(value, ioaddr + VLAN_TAG);
> +	} else if (perfect_match) {
> +		u32 value = VLAN_ETV;
> +
> +		if (is_double) {
> +			value |= VLAN_EDVLP;
> +			value |= VLAN_ESVL;
> +			value |= VLAN_DOVLTC;
> +		}
> +
> +		writel(value | perfect_match, ioaddr + VLAN_TAG);
> +	} else {
> +		value &= ~(VLAN_VTHM | VLAN_ETV);
> +		value &= ~(VLAN_EDVLP | VLAN_ESVL);
> +		value &= ~VLAN_DOVLTC;
> +		value &= ~VLAN_VID;
> +
> +		writel(value, ioaddr + VLAN_TAG);
> +	}
> +}
> +
> +static void vlan_enable(struct mac_device_info *hw, u32 type)
> +{
> +	void __iomem *ioaddr = hw->pcsr;
> +	u32 value;
> +
> +	value = readl(ioaddr + VLAN_INCL);
> +	value |= VLAN_VLTI;
> +	value |= VLAN_CSVL; /* Only use SVLAN */
> +	value &= ~VLAN_VLC;
> +	value |= (type << VLAN_VLC_SHIFT) & VLAN_VLC;
> +	writel(value, ioaddr + VLAN_INCL);
> +}
> +
> +static void vlan_rx_hw(struct mac_device_info *hw,
> +		       struct dma_desc *rx_desc, struct sk_buff *skb)
> +{
> +	if (hw->desc->get_rx_vlan_valid(rx_desc)) {
> +		u16 vid = hw->desc->get_rx_vlan_tci(rx_desc);
> +
> +		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);

So, as the comment above, ETH_P_8021AD or ETH_P_8021Q shall be set selectively
depend on the frame type.

> +	}
> +}