On Thu, Oct 20, 2022 at 04:02:24PM +0300, Vladimir Oltean wrote:

On Tue, Oct 18, 2022 at 06:56:12PM +0200, Hans J. Schultz wrote:

quoted

@@ -3315,6 +3316,7 @@ static int dsa_slave_fdb_event(struct net_device *dev,
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	bool host_addr = fdb_info->is_local;
 	struct dsa_switch *ds = dp->ds;
+	u16 fdb_flags = 0;
 
 	if (ctx && ctx != dp)
 		return 0;

@@ -3361,6 +3363,9 @@ static int dsa_slave_fdb_event(struct net_device *dev,
 		   orig_dev->name, fdb_info->addr, fdb_info->vid,
 		   host_addr ? " as host address" : "");
 
+	if (fdb_info->locked)
+		fdb_flags |= DSA_FDB_FLAG_LOCKED;

This is the bridge->driver direction. In which of the changes up until
now/through which mechanism will the bridge emit a
SWITCHDEV_FDB_ADD_TO_DEVICE with fdb_info->locked = true?

I believe it can happen in the following call chain:

br_handle_frame_finish
   br_fdb_update // p->flags & BR_PORT_MAB
       fdb_notify
           br_switchdev_fdb_notify

This can happen with Spectrum when a packet ingresses via a locked port
and incurs an FDB miss in hardware. The packet will be trapped and
injected to the Rx path where it should invoke the above call chain.

Don't the other switchdev drivers except DSA (search for SWITCHDEV_FDB_EVENT_TO_DEVICE
in the drivers/ folder) need to handle this new flag too, even if to reject it?

Yes, agree. At least with mlxsw it is not a big deal right now because
it ignores entries with !BR_FDB_ADDED_BY_USER and locked entries are
always like that, but it would be good to make it more explicit.

When other drivers will want to look at fdb_info->locked, they'll have
the surprise that it's impossible to maintain backwards compatibility,
because they didn't use to treat the flag at all in the past (so either
locked or unlocked, they did the same thing).

quoted

+
 	INIT_WORK(&switchdev_work->work, dsa_slave_switchdev_event_work);
 	switchdev_work->event = event;
 	switchdev_work->dev = dev;

@@ -3369,6 +3374,7 @@ static int dsa_slave_fdb_event(struct net_device *dev,
 	ether_addr_copy(switchdev_work->addr, fdb_info->addr);
 	switchdev_work->vid = fdb_info->vid;
 	switchdev_work->host_addr = host_addr;
+	switchdev_work->fdb_flags = fdb_flags;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel