netdev - Re: [PATCH net-next] net: ethernet: ti: am65-cpsw: Add priv-flag for Switch VLAN Aware mode

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: <0106ce78-c83f-4552-a234-1bf7a33f1ed1@kernel.org>
Date: Thu, 29 Feb 2024 12:52:07 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Siddharth Vadapalli <s-vadapalli@...com>, Andrew Lunn <andrew@...n.ch>
Cc: Jiri Pirko <jiri@...nulli.us>, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, vladimir.oltean@....com,
 hkallweit1@...il.com, dan.carpenter@...aro.org, horms@...nel.org,
 yuehaibing@...wei.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, srk@...com,
 Pekka Varis <p-varis@...com>
Subject: Re: [PATCH net-next] net: ethernet: ti: am65-cpsw: Add priv-flag for
 Switch VLAN Aware mode



On 29/02/2024 11:27, Siddharth Vadapalli wrote:
> On Wed, Feb 28, 2024 at 02:36:55PM +0100, Andrew Lunn wrote:
>>> What if there is no kernel behavior associated with it? How can it be mimicked
>>> then?
>>
>> Simple. Implement the feature in software in the kernel for
>> everybody. Then offload it to your hardware.
>>
>> Your hardware is an accelerator. You use it to accelerate what linux
>> can already do. If Linux does not have the feature your accelerator
>> has, that accelerator feature goes unused.
> 
> Is it acceptable to have a macro in the Ethernet Driver to conditionally
> disable/enable the feature (via setting the corresponding bit in the
> register)?
> 
> The current implementation is:
> 
> 	/* Control register */
> 	writel(AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE |
> 	       AM65_CPSW_CTL_VLAN_AWARE | AM65_CPSW_CTL_P0_RX_PAD,
> 	       common->cpsw_base + AM65_CPSW_REG_CTL);
> 
> which sets the "AM65_CPSW_CTL_VLAN_AWARE" bit by default.
> 
> Could it be changed to:
> 
> #define TI_K3_CPSW_VLAN_AWARE 1
> 
> ....
> 
> 	/* Control register */
> 	val = AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE |
> 	      AM65_CPSW_CTL_P0_RX_PAD;
> 
> #ifdef TI_K3_CPSW_VLAN_AWARE
> 	val |= AM65_CPSW_CTL_VLAN_AWARE;
> #endif
> 
> 	writel(val, common->cpsw_base + AM65_CPSW_REG_CTL);
> 
> Since no additional configuration is necessary to disable/enable the
> functionality except clearing/setting a bit in a register, I am unsure of
> the implementation for the offloading part being suggested. Please let me
> know if the above implementation is an acceptable alternative.

This doesn't really solve the problem as it leaves the question open as to
who will set TI_K3_CPSW_VLAN_AWARE. And the configuration is then fixed at build.

Can you please explain in which scenario the default case does not work for you?
Why would end user want to disable VLAN_AWARE mode?

TRM states
"Transmit packets are NOT modified during switch egress when the VLAN_AWARE bit in the
CPSW_CONTROL_REG register is cleared to 0h. This means that the switch is not in VLAN-aware mode."

The same problem would also apply to cpsw.c and cpsw_new.c correct?

A bit later the driver does this

        /* switch to vlan unaware mode */
        cpsw_ale_control_set(common->ale, HOST_PORT_NUM, ALE_VLAN_AWARE, 1);
        cpsw_ale_control_set(common->ale, HOST_PORT_NUM,
                             ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);

The comment says vlan unaware but code is setting ALE_VLAN_AWARE to 1.
Is the comment wrong?

-- 
cheers,
-roger