Re: [PATCH net RESEND] net: ethernet: ti: am65-cpsw: Add IFF_UNICAST_FLT flag to port device

[Sanjuán García, Jorge]

On Fri, 2024-03-01 at 17:49 +0200, Vladimir Oltean wrote:
> [No suele recibir correo electrónico de olteanv@xxxxxxxxx. Descubra
> por qué esto es importante en
> https://aka.ms/LearnAboutSenderIdentification ;]
> 
> On Fri, Mar 01, 2024 at 04:39:50PM +0530, Ravi Gunasekaran wrote:
> > On 2/29/24 9:35 AM, Jakub Kicinski wrote:
> > > On Wed, 28 Feb 2024 11:13:23 +0000 Sanjuán García, Jorge wrote:
> > > > Since commit 8940e6b669ca ("net: dsa: avoid call to
> > > > __dev_set_promiscuity()
> > > > while rtnl_mutex isn't held") when conecting one of this
> > > > switch's port
> > > > to a DSA switch as the conduit interface, the network interface
> > > > is set to
> > > > promiscuous mode by default and cannot be set to not
> > > > promiscuous mode again
> > > > from userspace. The reason for this is that the cpsw ports net
> > > > devices
> > > > do not have the flag IFF_UNICAST_FLT set in their private
> > > > flags.
> > > > 
> > > > The cpsw switch should be able to set not promiscuous mode as
> > > > otherwise
> > > > a '1' is written to bit ALE_PORT_MACONLY_CAF which makes
> > > > ethernet frames
> > > > get an additional VLAN tag when entering the port connected to
> > > > the DSA
> > > > switch. Setting the IFF_UNICAST_FLT flag to all ports allows us
> > > > to have
> > > > the conduit interface on the DSA subsystem set as not
> > > > promiscuous.
> > > 
> > > It doesn't look like am65-cpsw-nuss supports unicast filtering,
> > > tho, does it? So we're lying about support to work around some
> > > CPSW weirdness (additional VLAN tag thing)?
> > 
> > CPSW driver does not support unicast filtering.
> 
> Then the driver can't declare IFF_UNICAST_FLT.
> 
> Why does enabling promiscuous mode cause Ethernet frames to get an
> additional VLAN tag? 802.3 clause 4.2.4.1.1 Address recognition only
> says "The MAC sublayer may also provide the capability of operating
> in
> the promiscuous receive mode. In this mode of operation, the MAC
> sublayer recognizes and accepts all valid frames, regardless of their
> Destination Address field values.". Absolutely nothing about VLAN.

Hi,

Thank you all very much for the reviews. It is clear now we should not
add this IFF_UNICAST_FLT flag to this driver.

I may do some new investigations to find out exactly why this CPSW
driver is adding VLAN tags when set to promiscuous mode. The CPSW HW is
definetly adding VLAN tags whenever bit Iy_REG_Py_MACONLY of register
CPSW_Iy_ALE_PORTCTL0_y gets a "1". Maybe there is some extra
configuration needed but as far a the current am65-cpsw-nuss.c
implementation goes, am65_cpsw_slave_set_promisc() only sets that bit.

Best regards,
Jorge