Subject: Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: mac-auth/MAB implementation

From: Hans Schultz <[email-masked]>
Date: Fri, 11 Mar 2022 08:59:04 +0100

On tor, mar 10, 2022 at 15:54, Andrew Lunn <[email-masked]> wrote:
>> +		if (mv88e6xxx_port_is_locked(chip, chip->ports[spid].port))
>> +			err = mv88e6xxx_switchdev_handle_atu_miss_violation(chip,
>> +									    chip->ports[spid].port,
>> +									    &entry,
>> +									    fid);
>
>> +static int mv88e6xxx_find_vid_on_matching_fid(struct mv88e6xxx_chip *chip,
>> +					      const struct mv88e6xxx_vtu_entry *entry,
>> +					      void *priv)
>> +{
>> +	struct mv88e6xxx_fid_search_ctx *ctx = priv;
>> +
>> +	if (ctx->fid_search == entry->fid) {
>> +		ctx->vid_found = entry->vid;
>> +		return 1;
>> +	}
>> +	return 0;
>> +}
>> +
>> +int mv88e6xxx_switchdev_handle_atu_miss_violation(struct mv88e6xxx_chip *chip,
>> +						  int port,
>> +						  struct mv88e6xxx_atu_entry *entry,
>> +						  u16 fid)
>> +{
>> +	struct switchdev_notifier_fdb_info info = {
>> +		.addr = entry->mac,
>> +		.vid = 0,
>> +		.added_by_user = false,
>> +		.is_local = false,
>> +		.offloaded = true,
>> +		.locked = true,
>> +	};
>> +	struct mv88e6xxx_fid_search_ctx ctx;
>> +	struct netlink_ext_ack *extack;
>> +	struct net_device *brport;
>> +	struct dsa_port *dp;
>> +	int err;
>> +
>> +	ctx.fid_search = fid;
>> +	err = mv88e6xxx_vtu_walk(chip, mv88e6xxx_find_vid_on_matching_fid, &ctx);
>
> I could be reading this code wrong, but it looks like you assume there
> is a single new entry in the ATU. But interrupts on these devices are
> slow. It would be easy for two or more devices to pop into existence
> at the same time. Don't you need to walk the whole ATU to find all the
> new entries? Have you tried this with a traffic generating populating
> the ATU with new entries at different rates, up to line rate? Do you
> get notifications for them all?
>
>     Andrew

We have not tried your said test, but if a packet doesn't manage to
trigger a ATU miss violation interrupt, not much will happen as far as I
see. The device sending the packet will not get access, but if it sends
again (maybe after a short while), it can still trigger the ATU miss
violation interrupt and get access.
I think that the normal behaviour for a device would be to try and
connect, and if that is not successfull inside a short time, it will wait
for a timeout before trying again.