Wed, Feb 28, 2024 at 08:06:39AM CET, s-vadapalli@ti.com wrote:

On 27/02/24 18:09, Jiri Pirko wrote:

quoted

Tue, Feb 27, 2024 at 09:28:15AM CET, s-vadapalli@ti.com wrote:

quoted

The CPSW Ethernet Switch on TI's K3 SoCs can be configured to operate in
VLAN Aware or VLAN Unaware modes of operation. This is different from
the ALE being VLAN Aware and Unaware. The Ethernet Switch being VLAN Aware
results in the addition/removal/replacement of VLAN tag of packets during
egress as described in section "12.2.1.4.6.4.1 Transmit VLAN Processing" of
the AM65x Technical Reference Manual available at:
https://www.ti.com/lit/ug/spruid7e/spruid7e.pdf
In VLAN Unaware mode, packets remain unmodified on egress.

The driver currently configures the Ethernet Switch in VLAN Aware mode by
default and there is no support to toggle this capability of the Ethernet
Switch at runtime. Thus, add support to toggle the capability by exporting
it via the ethtool "priv-flags" interface.

I don't follow. You have all the means to offload all bridge/vlan
configurations properly and setup your hw according to that. See mlxsw
for a reference. I don't see the need for any custom driver knobs.

Thank you for reviewing the patch. Please note that the "VLAN Aware mode" being
referred to here is different from ALE being VLAN aware. The hw offload of
bridge/vlan configurations is already supported in the context of the ALE. The
Ethernet Switch being VLAN Aware is a layer on top of that, which enables
further processing on top of the untagged/VLAN packets. This patch aims to
provide a method to enable the following use-cases:
1. ALE VLAN Aware + CPSW VLAN Aware
2. ALE VLAN Aware + CPSW VLAN Unaware

All hw offloads of bridge/vlan configurations are w.r.t. ALE VLAN Aware alone.
Currently, only use-case 1 is enabled by the driver by default and there is no
knob to toggle to use-case 2.

I am quoting sections of the Technical Reference Manual mentioned in my commit
message, in order to clarify the CPSW VLAN Unaware and CPSW VLAN Aware terminology.

CPSW VLAN Unaware:
Transmit packets are NOT modified during switch egress.

CPSW VLAN Aware:
1. Untagged Packet Operations
Untagged packets are all packets that are not a VLAN packet or a priority tagged
packet. According to the CPWS0_FORCE_UNTAGGED_EGRESS_REG[1-0] MASK bit in the
packet header the packet may exit the switch with a VLAN tag inserted or the
packet may leave the switch unchanged....
2. Priority Tagged Packet Operations (VLAN VID == 0 && EN_VID0_MODE ==0h)
Priority tagged packets are packets that contain a VLAN header with VID = 0.
According to the CPSW_ALE_FORCE_UNTAGGED_EGRESS_REG[1-0] MASK bit in the packet
header, priority tagged packets may exit the switch with their VLAN ID and
priority replaced or they may have their priority tag completely removed....
3. VLAN Tagged Packet Operations (VLAN VID != 0 || (EN_VID0_MODE ==1h && VLAN
VID ==0))
VLAN tagged packets are packets that contain a VLAN header specifying the VLAN
the packet belongs to
(VID), the packet priority (PRI), and the drop eligibility indicator (CFI).
According to the CPSW_ALE_FORCE_UNTAGGED_EGRESS_REG[1-0] MASK bit in the packet
header, VLAN tagged packets may exit the switch with their VLAN priority
replaced or they may have their VLAN header completely removed...

I hope that this clarifies that CPSW VLAN Unaware/Aware is a layer on top of the
hw offload-able bridge/vlan configuration.  Please let me know if there is
anything specific that could enable this without requiring the "priv-flag" based
implementation of this patch.

I have no clue what "ALE" is. But in general. User provided
configuration, using ip/bridge/etc tools/uapi. According to this
configuration, kernel is bahaving. When you do offload, you should just
make sure to mimic/mirror the kernel behaviour. With this in mind, why
can't you do it without adding additional knob? And if you really need
it because the know does some internal hw/fw tuning, priv flag of netdev
is most probably not the correct place to put it. If it is, make sure
you advocate for it properly in the patch description.

pw-bot: cr

-- 
Regards,
Siddharth.