On Sun, Jul 31, 2022 at 10:49:35PM +0200, Martin Blumenstingl wrote:

While working on my patches a more practical question came up while I
was breaking the driver and then trying to make local_termination.sh
pass again.
At the start of run_tests for the standalone port scenario I am
getting the following values:
  rcv_dmac = 00:01:02:03:04:02
  MACVLAN_ADDR = 00:00:de:ad:be:ef
My expectation is that port_fdb_add() is called with these MAC
addresses. I verified that dsa_switch_supports_uc_filtering() returns
true, but still I

Unfinished phrase, I suppose you wanted to say that you don't get a call
to port_fdb_add() with $MACVLAN_ADDR. I don't see why that would be the
case. Try to put some prints in dsa_slave_sync_uc(), since that's the
path through which $MACVLAN_ADDR comes in. On the other hand, $rcv_dmac
comes from dsa_slave_open().

quoted

I'll do so if you have a specific question about something apparently
not mapping to the expectations.

I still have an issue which I believe is related to the FDB handling.

I *think* that I have implemented FDB isolation correctly in my
work-in-progress branch [0].

The GSWIP140 datasheet (page 82) has a "MAC Learning disable and MAC
Learning Limitation Description" (table 26).
In the xRX200 vendor kernel I cannot find the LNDIS bit in
PCE_PCTRL_3, so I suspect it has only been added in newer GSWIP
revisions (xRX200 is at least one major IP revision behind GSW140).
Maybe Hauke knows?
So what I'm doing to disable learning is setting the "learning limit"
(which limits the number of entries that can be learned) for that port
to zero.

 My problem is that whenever I disable learning a lot of tests from
local_termination.sh are failing, including:
- TEST: lan2: Unicast IPv4 to primary MAC address
- TEST: lan2: Unicast IPv4 to macvlan MAC address

Setting the PLIMMOD bit to 1 means that GSWIP won't drop the packet if
the learning limit is exceeded (the default value seems to be 0).

Yes, I'm not sure why you'd want to drop packets that aren't learned, so
I would expect PLIMMOD to be 1 if you're disabling learning via LRNLIM=0.

This at least works around the first failing test (Unicast IPv4 to
primary MAC address).

Not sure why "works around" is the choice of words here.

Based on your understanding of my issue: I am going in the right
direction when I'm saying that this is an FDB issue?

I don't know why the tests fail. I downloaded your code and I see that
you touch PLIMMOD from ds->ops->port_set_host_flood(). Why not just
leave it at 1? It's a global bit anyway, it affects what happens with
all ports that have source address learning disabled, it seems a very
odd choice to do what you're doing.

When you have learning disabled on standalone ports (by design), and
local_termination.sh receives packets on $swp1, a standalone port
(by design), don't you agree that the MAC SA of these packets won't be
learned, and you're telling the switch "yeah, drop them"?

[0] https://github.com/xdarklight/linux/commits/lantiq-gswip-integration-20220730

There are many things to say about the code, however it's a bit
difficult to review it just by looking at the Github commits.
For example I'm looking at gswip_port_fdb(), I don't really understand
why this:

	for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
		if (priv->vlans[i].bridge == bridge) {
			fid = priv->vlans[i].fid;
			break;
		}
	}

is not this:

	for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
		if (priv->vlans[i].bridge == bridge &&
		    priv->vlans[i].vid == vid) {
			fid = priv->vlans[i].fid;
			break;
		}
	}