On Tue, Feb 09, 2021 at 08:27:24PM +0200, Vladimir Oltean wrote: > But there's an interesting side effect of allowing > br_switchdev_set_port_flag to run concurrently (notifier call chains use > a rw_semaphore and only take the read side). Basically now drivers that > cache the brport flags in their entirety are broken, because there isn't > any guarantee that bits outside the mask are valid any longer (we can > even enforce that by masking the flags with the mask when notifying > them). They would need to do the same trick of updating just the masked > part of their cached flags. Except for the fact that they would need > some sort of spinlock too, I don't think that the basic bitwise > operations are atomic or anything like that. I'm a bit reluctant to add > a spinlock in prestera, rocker, mlxsw just for this purpose. What do you > think? My take on things is that I can change those drivers to do what ocelot and sja1105 do, which is to just have some bool values like this: if (flags.mask & BR_LEARNING) ocelot_port->learn_ena = !!(flags.val & BR_LEARNING); which eliminates concurrency to the shared unsigned long brport_flags variable. No locking, no complications.
