On Thu, Jun 30, 2022 at 09:08:46PM +0200, Horatiu Vultur wrote:

quoted

I've downloaded and applied your patches and I have some general feedback.
Some of it relates to changes which were not made and hence I couldn't
have commented on the patches themselves, so I'm posting it here.

1. switchdev_bridge_port_offload() returns an error code if object
replay failed, or if it couldn't get the port parent id, or if the user
tries to join a lan966x port and a port belonging to another switchdev
driver to the same LAG. It would be good to propagate this error and not
ignore it.

Yes, I will do that.

What about the case when the other port is not a switchdev port. For
example:
ip link set dev eth0 master bond0
ip link set dev dummy master bond0
ip link set dev bond0 master br0

At the last line, I was expecting to get an error.

switchdev_bridge_port_offload() currently only detects mismatched port
parent IDs, so it will not detect this condition where one bond slave is
switchdev and the other isn't. This is because the non-switchdev bond
slave does not even call switchdev_bridge_port_offload(), it's completely
silent from the perspective of the bridge.
Fact is that we don't have a common layer that enforces all common sense
netdev adjacency restrictions with switchdev, and that is one of the big
problems of the system.

quoted

6. You are missing LAG FDB migration logic in lan966x_lag_port_join().
Specifically, you assume that the lan966x_lag_first_port() will never
change, probably because you just make the switch ports join the LAG in
the order 1, 2, 3. But they can also join in the order 3, 2, 1.

It would work, but there will be problems when the ports start to leave
the LAG.
It would work because all the ports under the LAG will have the same
value in PGID_ID for DST_IDX. So if the MAC entry points to any of
this entries will be OK.

OK, I forgot DEST_IDX selects PGID and not logical port ID directly.

The problem is when the port leaves the LAG, if the MAC entry points
to the port that left the LAG then is not working anymore.
I will fix this in the next series.