On Thu, Jan 16, 2025 at 05:25:49PM -0800, Tim Harvey wrote:

On Thu, Jan 16, 2025 at 5:09 PM [off-list ref] wrote:

quoted

quoted

Hi Tim,

quoted

Hi Arun,

Ok, that makes sense to me and falls in line with what my patch here
was trying to do. When you enable the reserved multicast table it
makes sense to update the entire table right? You are only updating
one address/group. Can you please review and comment on my patch
here?

During my testing of STP protocol, I found that Group 0 of reserved
multicast table needs to be updated. Since I have not worked on other
groups in the multicast table, I didn't update it.

I could not find the original patch to review, it shows "not found" in
lore.kernel.org.

Below are my comments,

- Why override bit is not set in REG_SW_ALU_VAL_B register.
- ksz9477_enable_stp_addr() can be renamed since it updates all the
table entries.

The reserved multicast table has only 8 entries that apply to 48
multicast addresses, so some addresses share one entry.

Some entries that are supposed to forward only to the host port or skip
should be updated when that host port is not the default one.

The override bit should be set for the STP address as that is required
for receiving when the port is closed.

Some entries for MVRP/MSRP should forward to the host port when the host
can process those messages and broadcast to all ports when the host does
not process those messages, but that is not controllable by the switch
driver so I do not know how to handle in this situation.

Hi Tristram,

Thanks for your feedback.

What is the behavior when the reserved multicast table is not enabled
(does it forward to all ports, drop all mcast, something else?)

quoted

The default reserved multicast table forwards to host port on entries 0,
2, and 6; skips host port on entries 4, 5, and 7; forwards to all ports
on entry 3; and drops on entry 1.

Is this behavior the desired behavior as far as the Linux DSA folks would want?

commit 331d64f752bb ("net: dsa: microchip: add the enable_stp_addr
pointer in ksz_dev_ops") enables the reserved multicast table and
adjusts the cpu port for entry 0 leaving the rest the same (and wrong
if the cpu port is not the highest port in the switch).

My patch adjusts the entries but keeps the rules the same and the
question that is posed is that the right thing to do with respect to
Linux DSA?

Best Regard,

Tim

Not sure if that's a question for the DSA maintainers or for Tristram,
because I've expressed already earlier in this thread what is the
current way in which the Linux bridge expects this mechanism to work.
This is not the Microchip KSZ SDK, and thus, following the network stack
rules is not optional, and inventing local conventions when there
already exist global ones is a no-go. If you don't like the conventions
you are of course free to challenge them, but this is not the right
audience to do that.