Re: [PATCH v2 net-next 0/5] Make SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE blocking

On Fri, Aug 20, 2021 at 07:09:18PM +0300, Ido Schimmel wrote:

On Fri, Aug 20, 2021 at 12:37:23PM +0300, Vladimir Oltean wrote:

quoted

On Fri, Aug 20, 2021 at 12:16:10PM +0300, Ido Schimmel wrote:

quoted

On Thu, Aug 19, 2021 at 07:07:18PM +0300, Vladimir Oltean wrote:

quoted

Problem statement:

Any time a driver needs to create a private association between a bridge
upper interface and use that association within its
SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE handler, we have an issue with FDB
entries deleted by the bridge when the port leaves. The issue is that
all switchdev drivers schedule a work item to have sleepable context,
and that work item can be actually scheduled after the port has left the
bridge, which means the association might have already been broken by
the time the scheduled FDB work item attempts to use it.

This is handled in mlxsw by telling the device to flush the FDB entries
pointing to the {port, FID} when the VLAN is deleted (synchronously).

Again, central solution vs mlxsw solution.

Again, a solution is forced on everyone regardless if it benefits them
or not. List is bombarded with version after version until patches are
applied. *EXHAUSTING*.

So if I replace "bombarded" with a more neutral word, isn't that how
it's done though? What would you do if you wanted to achieve something
but the framework stood in your way? Would you work around it to avoid
bombarding the list?

With these patches, except DSA, everyone gets another queue_work() for
each FDB entry. In some cases, it completely misses the purpose of the
patchset.

I also fail to see the point. Patch 3 will have to make things worse
before they get better. It is like that in DSA too, and made more
reasonable only in the last patch from the series.

If I saw any middle-ground way, like keeping the notifiers on the atomic
chain for unconverted drivers, I would have done it. But what do you do
if more than one driver listens for one event, one driver wants it
blocking, the other wants it atomic. Do you make the bridge emit it
twice? That's even worse than having one useless queue_work() in some
drivers.

So if you think I can avoid that please tell me how.

Want a central solution? Make sure it is properly integrated. "Don't
have the energy"? Ask for help. Do not try to force a solution on
everyone and motivate them to change the code by doing a poor conversion
yourself.

I don't accept "this will have to do".

So I can make many suppositions about what I did wrong, but I would
prefer that you tell me.

Is it the timing, as we're late in the development cycle? Maybe, and
that would make a lot of sense, but I don't want to assume anything that
has not been said.

Is it that I converted too few drivers? You said I'm bombarding the
list. Can I convert more drivers with less code? I would be absolutely
glad to. I have more driver conversions unsubmitted, some tested on
hardware.

Is it that I didn't ask for help? I still believe that it is best I
leave the driver maintainers to do the rest of the conversion, at their
own pace and with hardware to test and find issues I can not using just
code analysis and non-expert knowledge. After all, with all due respect
to the net-next tree, I sent these patches to a development git tree,
not to a production facility.

quoted

quoted

What is FDB isolation? Cover letter says: "There are use cases which
need FDB isolation between standalone ports and bridged ports, as well
as isolation between ports of different bridges".

FDB isolation means exactly what it says: that the hardware FDB lookup
of ports that are standalone, or under one bridge, is unable to find FDB entries
(same MAC address, same VID) learned on another port from another bridge.

quoted

Does it mean that DSA currently forwards packets between ports even if
they are member in different bridges or standalone?

No, that is plain forwarding isolation in my understanding of terms, and
we have had that for many years now.

So if I have {00:01:02:03:04:05, 5} in br0, but not in br1 and now a
packet with this DMAC/VID needs to be forwarded in br1 it will be
dropped instead of being flooded?

Yes.