| 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: <tjc6uc74j3add7bzh7of32i5topeenzv64y3hudne2lioqwqzb@qlhi4gdfn6ww>
Date: Fri, 10 Jan 2025 17:32:30 +0530
From: "s-vadapalli@...com" <s-vadapalli@...com>
To: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
CC: "s-vadapalli@...com" <s-vadapalli@...com>,
"c-vankar@...com"
<c-vankar@...com>,
"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
"davem@...emloft.net" <davem@...emloft.net>,
"jpanis@...libre.com"
<jpanis@...libre.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"rogerq@...nel.org"
<rogerq@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>
Subject: Re: [PATCH net-next v2] net: ethernet: ti: am65-cpsw: VLAN-aware
CPSW only if !DSA
On Fri, Jan 10, 2025 at 11:48:45AM +0000, Sverdlin, Alexander wrote:
> Hi Siddharth!
Hello Alexander,
>
> On Fri, 2025-01-10 at 17:12 +0530, Siddharth Vadapalli wrote:
> > On Fri, Jan 10, 2025 at 12:27:17PM +0100, A. Sverdlin wrote:
> > > From: Alexander Sverdlin <alexander.sverdlin@...mens.com>
> > >
> > > Only configure VLAN-aware CPSW mode if no port is used as DSA CPU port.
> > > VLAN-aware mode interferes with some DSA tagging schemes and makes stacking
> > > DSA switches downstream of CPSW impossible. Previous attempts to address
> > > the issue linked below.
> > >
> > > Link: https://lore.kernel.org/netdev/20240227082815.2073826-1-s-vadapalli@ti.com/
> > > Link: https://lore.kernel.org/linux-arm-kernel/4699400.vD3TdgH1nR@localhost/
> > > Co-developed-by: Siddharth Vadapalli <s-vadapalli@...com>
> >
> > A Co-developed-by tag should be followed by a Signed-off-by tag of the
> > same individual.
>
> You are right, thanks!
>
> > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...mens.com>
> > > ---
> > > Changelog:
> > > v2: Thanks to Siddharth it does look much clearer now (conditionally clear
> > > AM65_CPSW_CTL_VLAN_AWARE instead of setting)
> > >
> > > drivers/net/ethernet/ti/am65-cpsw-nuss.c | 18 ++++++++++++++----
> > > 1 file changed, 14 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > index 5465bf872734..58c840fb7e7e 100644
> > > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > @@ -32,6 +32,7 @@
> > > #include <linux/dma/ti-cppi5.h>
> > > #include <linux/dma/k3-udma-glue.h>
> > > #include <net/page_pool/helpers.h>
> > > +#include <net/dsa.h>
> > > #include <net/switchdev.h>
> > >
> > > #include "cpsw_ale.h"
> > > @@ -724,13 +725,22 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common)
> > > u32 val, port_mask;
> > > struct page *page;
> > >
> > > + /* Control register */
> > > + val = AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE |
> > > + AM65_CPSW_CTL_VLAN_AWARE | AM65_CPSW_CTL_P0_RX_PAD;
> > > + /* VLAN aware CPSW mode is incompatible with some DSA tagging schemes.
> > > + * Therefore disable VLAN_AWARE mode if any of the ports is a DSA Port.
> > > + */
> > > + for (port_idx = 0; port_idx < common->port_num; port_idx++)
> > > + if (netdev_uses_dsa(common->ports[port_idx].ndev)) {
> > > + val &= ~AM65_CPSW_CTL_VLAN_AWARE;
> > > + break;
> > > + }
> > > + writel(val, common->cpsw_base + AM65_CPSW_REG_CTL);
> > > +
> > > if (common->usage_count)
> > > return 0;
> >
> > The changes above should be present HERE, i.e. below the
> > "common->usage_count" check, as was the case earlier.
>
> It has been moved deliberately, consider first port is being brought up
> and only then the second port is being brought up (as part of
> dsa_conduit_setup(), which sets dev->dsa_ptr right before opening the
> port). As this isn't RMW operation and actually happens under RTNL lock,
> moving in front of "if" looks safe to me... What do you think?
I understand the issue now. Does the following work?
1. am65_cpsw_nuss_common_open() can be left as-is i.e. no changes to be
made.
2. Interfaces being brought up will invoke am65_cpsw_nuss_ndo_slave_open()
which then invokes am65_cpsw_nuss_common_open().
3. Within am65_cpsw_nuss_ndo_slave_open(), immediately after the call to
am65_cpsw_nuss_common_open() returns, clear AM65_CPSW_CTL_VLAN_AWARE
bit within AM65_CPSW_REG_CTL register if the interface is DSA.
The patch then effectively is the DSA.h include plus the following diff:
------------------------------------------------------------------------------------------------
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 5465bf872734..7819a5674f9c 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1014,6 +1014,15 @@ static int am65_cpsw_nuss_ndo_slave_open(struct net_device *ndev)
common->usage_count++;
+ /* VLAN aware CPSW mode is incompatible with some DSA tagging schemes.
+ * Therefore disable VLAN_AWARE mode if any of the ports is a DSA Port.
+ */
+ if (netdev_uses_dsa(port->ndev) {
+ reg = readl(common->cpsw_base + AM65_CPSW_REG_CTL);
+ reg &= ~AM65_CPSW_CTL_VLAN_AWARE;
+ writel(reg, common->cpsw_base + AM65_CPSW_REG_CTL);
+ }
+
am65_cpsw_port_set_sl_mac(port, ndev->dev_addr);
am65_cpsw_port_enable_dscp_map(port);
------------------------------------------------------------------------------------------------
>
> > > - /* 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);
> > > /* Max length register */
> > > writel(AM65_CPSW_MAX_PACKET_SIZE,
> > > host_p->port_base + AM65_CPSW_PORT_REG_RX_MAXLEN);
>
Regards,
Siddharth.
Powered by blists - more mailing lists