Re: [patch net-next 07/10] bridge: call netdev_sw_port_stp_update when bridge port STP status changes

Thu, Nov 06, 2014 at 07:53:35PM CET, sfeldma@gmail.com wrote:

On Thu, Nov 6, 2014 at 6:59 AM, Florian Fainelli [off-list ref]
wrote:

quoted

On 11/06/2014 01:20 AM, Jiri Pirko wrote:

quoted

From: Scott Feldman <redacted>

To notify switch driver of change in STP state of bridge port, add new
.ndo op and provide swdev wrapper func to call ndo op. Use it in bridge
code then.

Signed-off-by: Scott Feldman <redacted>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---

[snip]

quoted

 #endif /* _LINUX_SWITCHDEV_H_ */

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 86c239b..13fecf1 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c

@@ -17,6 +17,7 @@
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <uapi/linux/if_bridge.h>
+#include <net/switchdev.h>

 #include "br_private.h"
 #include "br_private_stp.h"

@@ -304,6 +305,7 @@ static int br_set_port_state(struct net_bridge_port

*p, u8 state)

quoted

      br_set_state(p, state);
      br_log_state(p);
+     netdev_sw_port_stp_update(p->dev, p->state);

Is there a reason netdev_sw_port_stp_update() is not folded in
br_set_state()? Are we missing calls to br_set_state() in some locations?

I put the netdev_sw call at the same level as br_log_state() and
br_ifinfo_notify(),
but now that you bring up the question, I agree it would be cleaner/safer
if netdev_sw call was from br_set_state().

Not sure about this. netdev_sw_port_stp_update is not called every time
br_set_state is called. br_log_state is in simillar position.
br_set_state is now just a simple setter. I would probably leave this
change for the possible future follow-up. It can move br_log_state call
as well.

quoted

--
Florian