| 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
| ||
|
Message-ID: <20250424121604.GE3042781@horms.kernel.org>
Date: Thu, 24 Apr 2025 13:16:04 +0100
From: Simon Horman <horms@...nel.org>
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>,
Furong Xu <0x1207@...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 v4 1/2] net: stmmac: Refactor VLAN implementation
On Tue, Apr 22, 2025 at 12:29:29AM +0800, Boon Khai Ng 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.
>
> The descriptor format for VLAN is not moved to the common code due
> to hardware-specific differences between DWMAC4 and DWXGMAC.
>
> For the DWMAC4 IP, the Receive Normal Descriptor 0 (RDES0) is
> formatted as follows:
> 31 0
> ------------------------ -----------------------
> RDES0| Inner VLAN TAG [31:16] | Outer VLAN TAG [15:0] |
> ------------------------ -----------------------
>
> For the DWXGMAC IP, the RDES0 format varies based on the
> Tunneled Frame bit (TNP):
>
> a) For Non-Tunneled Frame (TNP=0)
>
> 31 0
> ------------------------ -----------------------
> RDES0| Inner VLAN TAG [31:16] | Outer VLAN TAG [15:0] |
> ------------------------ -----------------------
>
> b) For Tunneled Frame (TNP=1)
>
> 31 8 7 3 2 0
> --------------------- ------------------ -------
> RDES0| VNID/VSID | Reserved | OL2L3 |
> --------------------- ------------------ ------
>
> The logic for handling tunneled frames is not yet implemented
> in the dwxgmac2_wrback_get_rx_vlan_tci() function. Therefore,
> it is prudent to maintain separate functions within their
> respective descriptor driver files
> (dwxgmac2_descs.c and dwmac4_descs.c).
>
> Signed-off-by: Boon Khai Ng <boon.khai.ng@...era.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@...era.com>
...
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> index a6d395c6bacd..d9f41c047e5e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> @@ -614,76 +614,6 @@ static int dwxgmac2_rss_configure(struct mac_device_info *hw,
> return 0;
> }
>
> -static void dwxgmac2_update_vlan_hash(struct mac_device_info *hw, u32 hash,
> - u16 perfect_match, bool is_double)
> -{
> - void __iomem *ioaddr = hw->pcsr;
> -
> - writel(hash, ioaddr + XGMAC_VLAN_HASH_TABLE);
> -
> - if (hash) {
> - u32 value = readl(ioaddr + XGMAC_PACKET_FILTER);
> -
> - value |= XGMAC_FILTER_VTFE;
> -
> - writel(value, ioaddr + XGMAC_PACKET_FILTER);
Here the XGMAC_FILTER_VTFE bit of XGMAC_PACKET_FILTER is set.
However, this logic does not appear in vlan_update_hash()
> -
> - value = readl(ioaddr + XGMAC_VLAN_TAG);
> -
> - value |= XGMAC_VLAN_VTHM | XGMAC_VLAN_ETV;
> - if (is_double) {
> - value |= XGMAC_VLAN_EDVLP;
> - value |= XGMAC_VLAN_ESVL;
> - value |= XGMAC_VLAN_DOVLTC;
> - } else {
> - value &= ~XGMAC_VLAN_EDVLP;
> - value &= ~XGMAC_VLAN_ESVL;
> - value &= ~XGMAC_VLAN_DOVLTC;
> - }
And likewise, here value is based on reading from XGMAC_VLAN_TAG.
Whereas in vlan_update_hash is constructed without reading from
XGMAC_VLAN_TAG.
Can I clarify that this is intentional and that vlan_update_hash(),
which is based on the DWMAC4 implementation, will also work for DWXGMAC IP?
> -
> - value &= ~XGMAC_VLAN_VID;
> - writel(value, ioaddr + XGMAC_VLAN_TAG);
> - } else if (perfect_match) {
> - u32 value = readl(ioaddr + XGMAC_PACKET_FILTER);
> -
> - value |= XGMAC_FILTER_VTFE;
> -
> - writel(value, ioaddr + XGMAC_PACKET_FILTER);
> -
> - value = readl(ioaddr + XGMAC_VLAN_TAG);
> -
> - value &= ~XGMAC_VLAN_VTHM;
> - value |= XGMAC_VLAN_ETV;
> - if (is_double) {
> - value |= XGMAC_VLAN_EDVLP;
> - value |= XGMAC_VLAN_ESVL;
> - value |= XGMAC_VLAN_DOVLTC;
> - } else {
> - value &= ~XGMAC_VLAN_EDVLP;
> - value &= ~XGMAC_VLAN_ESVL;
> - value &= ~XGMAC_VLAN_DOVLTC;
> - }
> -
> - value &= ~XGMAC_VLAN_VID;
> - writel(value | perfect_match, ioaddr + XGMAC_VLAN_TAG);
> - } else {
> - u32 value = readl(ioaddr + XGMAC_PACKET_FILTER);
> -
> - value &= ~XGMAC_FILTER_VTFE;
> -
> - writel(value, ioaddr + XGMAC_PACKET_FILTER);
> -
> - value = readl(ioaddr + XGMAC_VLAN_TAG);
> -
> - value &= ~(XGMAC_VLAN_VTHM | XGMAC_VLAN_ETV);
> - value &= ~(XGMAC_VLAN_EDVLP | XGMAC_VLAN_ESVL);
> - value &= ~XGMAC_VLAN_DOVLTC;
> - value &= ~XGMAC_VLAN_VID;
> -
> - writel(value, ioaddr + XGMAC_VLAN_TAG);
> - }
> -}
...
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
...
> +static int vlan_write_filter(struct net_device *dev,
> + struct mac_device_info *hw,
> + u8 index, u32 data)
> +{
> + void __iomem *ioaddr = (void __iomem *)dev->base_addr;
> + int i, timeout = 10;
> + u32 val;
> +
> + if (index >= hw->num_vlan)
> + return -EINVAL;
> +
> + writel(data, ioaddr + VLAN_TAG_DATA);
> +
> + val = readl(ioaddr + VLAN_TAG);
> + val &= ~(VLAN_TAG_CTRL_OFS_MASK |
> + VLAN_TAG_CTRL_CT |
> + VLAN_TAG_CTRL_OB);
> + val |= (index << VLAN_TAG_CTRL_OFS_SHIFT) | VLAN_TAG_CTRL_OB;
> +
> + writel(val, ioaddr + VLAN_TAG);
> +
> + for (i = 0; i < timeout; i++) {
> + val = readl(ioaddr + VLAN_TAG);
> + if (!(val & VLAN_TAG_CTRL_OB))
> + return 0;
> + udelay(1);
> + }
I am curious to know why readl_poll_timeout() isn't used here
as was the case in dwmac4_write_vlan_filter().
> +
> + netdev_err(dev, "Timeout accessing MAC_VLAN_Tag_Filter\n");
> +
> + return -EBUSY;
> +}
...
Powered by blists - more mailing lists