netdev - Re: [PATCH net RESEND] net: ethernet: ti: am65-cpsw: Add IFF_UNICAST

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] [day] [month] [year] [list]
Message-ID: <8a8b4320-924a-4dd3-973b-ca941489f19b@kernel.org>
Date: Wed, 6 Mar 2024 15:48:20 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Sanjuán García, Jorge
 <Jorge.SanjuanGarcia@...gon.com>, "olteanv@...il.com" <olteanv@...il.com>,
 "r-gunasekaran@...com" <r-gunasekaran@...com>
Cc: "s-vadapalli@...com" <s-vadapalli@...com>,
 "davem@...emloft.net" <davem@...emloft.net>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "andrew@...n.ch" <andrew@...n.ch>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "f.fainelli@...il.com" <f.fainelli@...il.com>,
 "kuba@...nel.org" <kuba@...nel.org>,
 "edumazet@...gle.com" <edumazet@...gle.com>,
 "pabeni@...hat.com" <pabeni@...hat.com>, Pekka Varis <p-varis@...com>
Subject: Re: [PATCH net RESEND] net: ethernet: ti: am65-cpsw: Add
 IFF_UNICAST_FLT flag to port device



On 04/03/2024 12:27, Sanjuán García, Jorge wrote:
> On Fri, 2024-03-01 at 17:49 +0200, Vladimir Oltean wrote:
>> [No suele recibir correo electrónico de olteanv@...il.com. 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

MAC_ONLY and MAC_ONLY_CAF are different bits in the
CPSW_ALE_PORT_CONTROL_REG_y register [1].

Promiscuous mode sets the MAC_ONLY_CAF bit.

>From TRM [1] , MAC =
"
Mac Only Copy All Frames.
When set a Mac Only port will transfer all received good frames to
the host.
When clear a Mac Only port will transfer packets to the host based
on ALE destination address lookup operation (which operates more
like an Ethernet Mac).
A Mac Only port is a port with maconly set.
"

Since you are operating the CPSW in MAC mode I believe MAC_ONLY is
set as well for you. That is fine.

Now, from [2], the CPSW hardware seems to poke around VLAN tags
only if VLAN_AWARE bit in CPSW_CONTROL_REG is set which seems
to be the case by default.

> static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common)
> {
>         struct am65_cpsw_host *host_p = am65_common_get_host(common);
>         int port_idx, i, ret, tx;
>         struct sk_buff *skb;
>         u32 val, port_mask;
> 
>         if (common->usage_count)
>                 return 0;
> 
>         /* 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);

One thing you could try is to not set AM65_CPSW_CTL_VLAN_AWARE here
and see if it resolves your case.

There was a patch sent recently [3] to play around this bit but it was
not clear why it was required. If AM65_CPSW_CTL_VLAN_AWARE is indeed the
cause of trouble here then it should be disabled by default.

[1] - https://www.ti.com/lit/pdf/spruid7
12.2.1.6.10.12 CPSW_ALE_PORT_CONTROL_REG_y Register

[2] - https://www.ti.com/lit/pdf/spruid7
12.2.1.4.6.4.1 Transmit VLAN Processing

[3] - https://lore.kernel.org/all/20240227082815.2073826-1-s-vadapalli@ti.com/

> 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

-- 
cheers,
-roger