Messages in this thread | | | From | Vladimir Oltean <> | Subject | Re: [PATCH net-next v2 0/7] net: lan966x: Add lag support | Date | Fri, 1 Jul 2022 16:34:15 +0000 |
| |
On Thu, Jun 30, 2022 at 09:08:46PM +0200, Horatiu Vultur wrote: > > 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.
> > 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.
| |