Re: [PATCH net-next v2 12/14] net: ethernet: qualcomm: Initialize PPE L2 bridge settings

[Lei Wei <quic_leiwei@quicinc.com>]

On 1/9/2025 12:59 AM, Andrew Lunn wrote:
> On Wed, Jan 08, 2025 at 09:47:19PM +0800, Luo Jie wrote:
>> From: Lei Wei <quic_leiwei@quicinc.com>
>>
>> Configure the default L2 bridge settings for the PPE ports to
>> enable L2 frame forwarding between CPU port and PPE Ethernet
>> ports.
> 
> It would be good to have an 'only' in there:
> 
>> to _only_
>> enable L2 frame forwarding between CPU port and PPE Ethernet
>> ports
> 
> That makes it clear there is no port to port forwarding, the ports are
> isolated.
> 

Sure, I will add 'only' to make it clear.

>> The per-port L2 bridge settings are initialized as follows:
>> For PPE CPU port, the PPE bridge TX is enabled and FDB learn is
>> disabled. For PPE physical port, the PPE bridge TX is disabled
>> and FDB learn is enabled by default and the L2 forward action
>> is initialized as forward to CPU port.
> 
> Why is learning needed on physical ports? In general, switches forward
> unknown destination addresses to the CPU. Which is what you want when
> the ports are isolated from each other. Everything goes to the
> CPU. But maybe this switch does not work like this?
> 

L2 forwarding can be disabled in PPE in two ways:

1.) Keep the learning enabled (which is the default HW setting) and
configure the FDB-miss-action to redirect to CPU.

This works because even if FDB learning is enabled, we need to represent
the bridge and the physical ports using their 'virtual switch instance'
(VSI) in the PPE HW, and create the 'port membership' for the bridge VSI
(the list of slave ports), before FDB based forwarding can take place. 
Since we do not yet support switchdev, these VSI are not created and 
packets are always forwarded to CPU due to FDB miss.

(or)

2.) Explicitly disable learning either globally or on the ports.

With method 1 we can achieve packet forwarding to CPU without explicitly
disabling learning. When switchdev is enabled later, L2 forwarding can 
be enabled as a natural extension on top of this configuration. So we 
have chosen the first approach.

> 	Andrew
>