Re: [PATCH net v4 1/7] net: dsa: mt7530: fix trapping frames with multiple CPU ports on MT7531

[Arınç ÜNAL]

On 15 June 2023 00:13:52 EEST, Vladimir Oltean <olteanv@xxxxxxxxx> wrote:
>On Wed, Jun 14, 2023 at 11:56:44PM +0300, Arınç ÜNAL wrote:
>> On 14.06.2023 22:43, Vladimir Oltean wrote:
>> > On Mon, Jun 12, 2023 at 10:59:39AM +0300, arinc9.unal@xxxxxxxxx wrote:
>> > > From: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
>> > > 
>> > > Every bit of the CPU port bitmap for MT7531 and the switch on the MT7988
>> > > SoC represents a CPU port to trap frames to. These switches trap frames
>> > > received from a user port to the CPU port that is affine to the user port
>> > > from which the frames are received.
>> > > 
>> > > Currently, only the bit that corresponds to the first found CPU port is set
>> > > on the bitmap. When multiple CPU ports are being used, the trapped frames
>> > > from the user ports not affine to the first CPU port will be dropped as the
>> > > other CPU port is not set on the bitmap. The switch on the MT7988 SoC is
>> > > not affected as there's only one port to be used as a CPU port.
>> > > 
>> > > To fix this, introduce the MT7531_CPU_PMAP macro to individually set the
>> > > bits of the CPU port bitmap. Set the CPU port bitmap for MT7531 and the
>> > > switch on the MT7988 SoC on mt753x_cpu_port_enable() which runs on a loop
>> > > for each CPU port.
>> > > 
>> > > Add a comment to explain frame trapping for these switches.
>> > > 
>> > > According to the document MT7531 Reference Manual for Development Board
>> > > v1.0, the MT7531_CPU_PMAP bits are unset after reset so no need to clear it
>> > > beforehand. Since there's currently no public document for the switch on
>> > > the MT7988 SoC, I assume this is also the case for this switch.
>> > > 
>> > > Fixes: c288575f7810 ("net: dsa: mt7530: Add the support of MT7531 switch")
>> > > Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
>> > > ---
>> > 
>> > Would you agree that this is just preparatory work for change "net: dsa:
>> > introduce preferred_default_local_cpu_port and use on MT7530" and not a
>> > fix to an existing problem in the code base?
>> 
>> Makes sense. Pre-preferred_default_local_cpu_port patch, there isn't a case
>> where there's a user port affine to a CPU port that is not enabled on the
>> CPU port bitmap. So yeah, this is just preparatory work for "net: dsa:
>> introduce preferred_default_local_cpu_port and use on MT7530".
>> 
>> So how do I change the patch to reflect this?
>> 
>> Arınç
>
>net: dsa: mt7530: set all CPU ports in MT7531_CPU_PMAP
>
>MT7531_CPU_PMAP represents the destination port mask for trapped-to-CPU
>frames (further restricted by PCR_MATRIX).
>
>Currently the driver sets the first CPU port as the single port in this
>bit mask, which works fine regardless of whether the device tree defines
>port 5, 6 or 5+6 as CPU ports. This is because the logic coincides with
>DSA's logic of picking the first CPU port as the CPU port that all user
>ports are affine to, by default.
>
>An upcoming change would like to influence DSA's selection of the
>default CPU port to no longer be the first one, and in that case, this
>logic needs adaptation.
>
>Since there is no observed leakage or duplication of frames if all CPU
>ports are defined in this bit mask, simply include them all.
>
>Note that there is no Fixes tag

Thanks a lot for making it easier for me.

Arınç