lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.02.1508172358470.13562@rocker1.rocker.net>
Date:	Tue, 18 Aug 2015 00:17:52 -0700 (PDT)
From:	Scott Feldman <sfeldma@...il.com>
To:	Premkumar Jonnala <pjonnala@...adcom.com>
cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] bridge: Enable configuration of ageing interval for
 bridges and switch devices.



On Fri, 14 Aug 2015, Premkumar Jonnala wrote:

> Bridge devices have ageing interval used to age out MAC addresses
> from FDB.  This ageing interval was not configuratble.
>
> Enable netlink based configuration of ageing interval for bridges and
> switch devices.  The ageing interval changes the timer used to purge
> inactive FDB entries in bridges.  The ageing interval config is
> propagated to switch devices, so that platform or hardware based
> ageing works according to configuration.
>
> Signed-off-by: Premkumar Jonnala <pjonnala@...adcom.com>

Hi Premkumar,

I agree with Roopa that we should use existing IFLA_BR_AGEING_TIME.

I hope you don't mind if share a patch for setting bridge-level attributes 
down to the switch hardware ports.  It uses the switchdev_port_attr_set() 
call on the bridge interface itself when any IFLA_BR_xxx attribute is set 
via netlink.   switchdev_port_attr_set() will do a recursive walk of all 
lower devs from the bridge, stopping at each to set the attribute.  I 
added one small tweak to switchdev_port_attr_set() to allow skipping over 
ports that return -EOPNOTSUPP.  The advantage to using 
switchdev_port_attr_set() is it automatically knows how to find the leaf 
ports in a stacked driver setup, and it uses the prepare/commit 
transaction model to make sure all ports can support new attr setting 
before committing setting to hardware.

I gave an example using rocker to set IFLA_BR_AGEING_TIME.  It's stubbed 
out, but you get the idea.  Other IFLA_BR_xxx attrs can be added to this 
model.

Does this look like something that would work?  If so, would you mind 
running with this patch and maybe even extending it for other IFLA_BR_xxx 
attrs?

Again, I hope I'm not stepping on toes here by rewriting your patch, but 
it was the best way to communicate the idea of how to use the switchdev 
frame work for these bridge attrs.

-scott




diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 0cc9e8f..830f8e6 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4680,6 +4680,24 @@ static int rocker_port_brport_flags_set(struct rocker_port *rocker_port,
  	return err;
  }

+static int rocker_port_bridge_set(struct rocker_port *rocker_port,
+				  enum switchdev_trans trans,
+				  struct switchdev_attr_bridge *bridge)
+{
+	switch (bridge->attr) {
+	case IFLA_BR_AGEING_TIME:
+		{
+			u32 ageing_time = bridge->val;
+			/* XXX push ageing_time down to device */
+		}
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
  static int rocker_port_attr_set(struct net_device *dev,
  				struct switchdev_attr *attr)
  {
@@ -4707,6 +4725,10 @@ static int rocker_port_attr_set(struct net_device *dev,
  		err = rocker_port_brport_flags_set(rocker_port, attr->trans,
  						   attr->u.brport_flags);
  		break;
+	case SWITCHDEV_ATTR_BRIDGE:
+		err = rocker_port_bridge_set(rocker_port, attr->trans,
+					     &attr->u.bridge);
+		break;
  	default:
  		err = -EOPNOTSUPP;
  		break;
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 319baab..22a6dbe 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -15,6 +15,7 @@
  #include <linux/notifier.h>

  #define SWITCHDEV_F_NO_RECURSE		BIT(0)
+#define SWITCHDEV_F_SKIP_EOPNOTSUPP	BIT(1)

  enum switchdev_trans {
  	SWITCHDEV_TRANS_NONE,
@@ -28,6 +29,7 @@ enum switchdev_attr_id {
  	SWITCHDEV_ATTR_PORT_PARENT_ID,
  	SWITCHDEV_ATTR_PORT_STP_STATE,
  	SWITCHDEV_ATTR_PORT_BRIDGE_FLAGS,
+	SWITCHDEV_ATTR_BRIDGE,
  };

  struct switchdev_attr {
@@ -38,6 +40,10 @@ struct switchdev_attr {
  		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
  		u8 stp_state;				/* PORT_STP_STATE */
  		unsigned long brport_flags;		/* PORT_BRIDGE_FLAGS */
+		struct switchdev_attr_bridge {		/* BRIDGE */
+			enum ifla_br attr;
+			u32 val;
+		} bridge;
  	} u;
  };

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index ff659f0..0630053 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -222,7 +222,7 @@ enum in6_addr_gen_mode {

  /* Bridge section */

-enum {
+enum ifla_br {
  	IFLA_BR_UNSPEC,
  	IFLA_BR_FORWARD_DELAY,
  	IFLA_BR_HELLO_TIME,
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 0f2408f..01401ea 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -759,9 +759,9 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
  	}

  	if (data[IFLA_BR_AGEING_TIME]) {
-		u32 ageing_time = nla_get_u32(data[IFLA_BR_AGEING_TIME]);
-
-		br->ageing_time = clock_t_to_jiffies(ageing_time);
+		err = br_set_ageing_time(br, nla_get_u32(data[IFLA_BR_AGEING_TIME]));
+		if (err)
+			return err;
  	}

  	if (data[IFLA_BR_STP_STATE]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 3d95647..9807a6f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -807,6 +807,7 @@ void __br_set_forward_delay(struct net_bridge *br, unsigned long t);
  int br_set_forward_delay(struct net_bridge *br, unsigned long x);
  int br_set_hello_time(struct net_bridge *br, unsigned long x);
  int br_set_max_age(struct net_bridge *br, unsigned long x);
+int br_set_ageing_time(struct net_bridge *br, u32 ageing_time);


  /* br_stp_if.c */
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index ed74ffa..5de27bc 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -566,6 +566,25 @@ int br_set_max_age(struct net_bridge *br, unsigned long val)

  }

+int br_set_ageing_time(struct net_bridge *br, u32 ageing_time)
+{
+	struct switchdev_attr attr = {
+		.id = SWITCHDEV_ATTR_BRIDGE,
+		.flags = SWITCHDEV_F_SKIP_EOPNOTSUPP,
+		.u.bridge.attr = IFLA_BR_AGEING_TIME,
+		.u.bridge.val = ageing_time,
+	};
+	int err;
+
+	err = switchdev_port_attr_set(br->dev, &attr);
+	if (err)
+		return err;
+
+	br->ageing_time = clock_t_to_jiffies(ageing_time);
+
+	return 0;
+}
+
  void __br_set_forward_delay(struct net_bridge *br, unsigned long t)
  {
  	br->bridge_forward_delay = t;
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 16c1c43..b990301 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -73,7 +73,7 @@ static int __switchdev_port_attr_set(struct net_device *dev,
  		return ops->switchdev_port_attr_set(dev, attr);

  	if (attr->flags & SWITCHDEV_F_NO_RECURSE)
-		return err;
+		goto done;

  	/* Switch device port(s) may be stacked under
  	 * bond/team/vlan dev, so recurse down to set attr on
@@ -82,10 +82,17 @@ static int __switchdev_port_attr_set(struct net_device *dev,

  	netdev_for_each_lower_dev(dev, lower_dev, iter) {
  		err = __switchdev_port_attr_set(lower_dev, attr);
+		if (err == -EOPNOTSUPP &&
+		    attr->flags & SWITCHDEV_F_SKIP_EOPNOTSUPP)
+			continue;
  		if (err)
  			break;
  	}

+done:
+	if (err == -EOPNOTSUPP && attr->flags & SWITCHDEV_F_SKIP_EOPNOTSUPP)
+		err = 0;
+
  	return err;
  }

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ