Re: [RFC PATCH net-next 0/7] DSA master state tracking

On Thu, Dec 09, 2021 at 12:32:23AM +0200, Vladimir Oltean wrote:

This patch set is provided solely for review purposes (therefore not to
be applied anywhere) and for Ansuel to test whether they resolve the
slowdown reported here:
https://patchwork.kernel.org/project/netdevbpf/cover/20211207145942.7444-1-ansuelsmth@gmail.com/

It does conflict with net-next due to other patches that are in my tree,
and which were also posted here and would need to be picked ("Rework DSA
bridge TX forwarding offload API"):
https://patchwork.kernel.org/project/netdevbpf/cover/20211206165758.1553882-1-vladimir.oltean@nxp.com/

Additionally, for Ansuel's work there is also a logical dependency with
this series ("Replace DSA dp->priv with tagger-owned storage"):
https://patchwork.kernel.org/project/netdevbpf/cover/20211208200504.3136642-1-vladimir.oltean@nxp.com/

To get both dependency series, the following commands should be sufficient:
git b4 20211206165758.1553882-1-vladimir.oltean@nxp.com
git b4 20211208200504.3136642-1-vladimir.oltean@nxp.com

where "git b4" is an alias in ~/.gitconfig:
[b4]
	midmask = https://lore.kernel.org/r/%s
[alias]
	b4 = "!f() { b4 am -t -o - $@ | git am -3; }; f"

The patches posted here are mainly to offer a consistent
"master_up"/"master_going_down" chain of events to switches, without
duplicates, and always starting with "master_up" and ending with
"master_going_down". This way, drivers should know when they can perform
Ethernet-based register access.

Vladimir Oltean (7):
  net: dsa: only bring down user ports assigned to a given DSA master
  net: dsa: refactor the NETDEV_GOING_DOWN master tracking into separate
    function
  net: dsa: use dsa_tree_for_each_user_port in
    dsa_tree_master_going_down()
  net: dsa: provide switch operations for tracking the master state
  net: dsa: stop updating master MTU from master.c
  net: dsa: hold rtnl_mutex when calling dsa_master_{setup,teardown}
  net: dsa: replay master state events in
    dsa_tree_{setup,teardown}_master

 include/net/dsa.h  |  8 +++++++
 net/dsa/dsa2.c     | 52 ++++++++++++++++++++++++++++++++++++++++++++--
 net/dsa/dsa_priv.h | 11 ++++++++++
 net/dsa/master.c   | 29 +++-----------------------
 net/dsa/slave.c    | 32 +++++++++++++++-------------
 net/dsa/switch.c   | 29 ++++++++++++++++++++++++++
 6 files changed, 118 insertions(+), 43 deletions(-)

-- 
2.25.1

I applied this patch and it does work correctly. Sadly the problem is
not solved and still the packet are not tracked correctly. What I notice
is that everything starts to work as soon as the master is set to
promiiscuous mode. Wonder if we should track that event instead of
simple up?

Here is a bootlog [0]. I added some log when the function timeouts and when
master up is actually called.
Current implementation for this is just a bool that is set to true on
master up and false on master going down. (final version should use
locking to check if an Ethernet transation is in progress)

[0] https://pastebin.com/7w2kgG7a

-- 
	Ansuel