switchdev: push bridge attributes down
Subsystems (5)
v2 cover switchdev: push bridge attributes down
From: Scott Feldman <[email protected]>
Push bridge-level attributes down to switchdev drivers. This patchset adds the infrastructure and then pushes, as an example, ageing_time attribute down from bridge to switchdev (rocker) driver. Add some range-checking for ageing_time.
# ip link set dev br0 type bridge ageing_time 1000
# ip link set dev br0 type bridge ageing_time 999 RTNETLINK answers: Numerical result out of range
Up until now, switchdev attrs where port-level attrs, so the netdev used in switchdev_attr_set() would be a switch port or bond of switch ports. With bridge-level attrs, the netdev passed to switchdev_attr_set() is the bridge netdev. The same recusive algo is used to visit the leaves of the stacked drivers to set the attr, it's just in this case we start one layer higher in the stack. One note is not all ports in the bridge may support setting a bridge-level attribute, so rather than failing the entire set, we'll skip over those ports returning -EOPNOTSUPP.
v1->v2: rebase w/ net-next
Scott Feldman (4):
switchdev: add bridge attributes
switchdev: skip over ports returning -EOPNOTSUPP when recursing ports
bridge: push bridge setting ageing_time down to switchdev
rocker: handle setting bridge ageing_time
drivers/net/ethernet/rocker/rocker.c | 22 ++++++++++++++++++++++ include/net/switchdev.h | 6 ++++++ include/uapi/linux/if_link.h | 2 +- net/bridge/br_ioctl.c | 3 +-- net/bridge/br_netlink.c | 6 +++--- net/bridge/br_private.h | 1 + net/bridge/br_stp.c | 24 ++++++++++++++++++++++++ net/bridge/br_sysfs_br.c | 3 +-- net/switchdev/switchdev.c | 9 ++++++++- 9 files changed, 67 insertions(+), 9 deletions(-)
Signature · 1 line
1.7.10.4
PATCH v2 1/4 switchdev: add bridge attributes
From: Scott Feldman <[email protected]>
Setting the stage to push bridge-level attributes down to port driver so hardware can be programmed accordingly. Bridge-level attribute example is ageing_time. This is a per-bridge attribute, not a per-bridge-port attr.
--- include/net/switchdev.h | 5 +++++ include/uapi/linux/if_link.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)
include/net/switchdev.h +5 -0
@@ -43,6 +43,7 @@ enum switchdev_attr_id {SWITCHDEV_ATTR_ID_PORT_PARENT_ID ,SWITCHDEV_ATTR_ID_PORT_STP_STATE ,SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS , + SWITCHDEV_ATTR_ID_BRIDGE, }; struct switchdev_attr {
@@ -52,6 +53,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; };
include/uapi/linux/if_link.h +1 -1
@@ -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 ,
Signature · 1 line
1.7.10.4
Jiri Pirko
maintainerRe: [PATCH net-next v2 1/4] switchdev: add bridge attributes
Quoted 25 lines · Thu, Oct 08, 2015 at 08:04:40AM CEST, [email protected] wrote:
From: Scott Feldman <[email protected]>
Setting the stage to push bridge-level attributes down to port driver so hardware can be programmed accordingly. Bridge-level attribute example is
ageing_time. This is a per-bridge attribute, not a per-bridge-port attr.Signed-off-by Scott Feldman <[email protected]>---
include/net/switchdev.h | 5 +++++ include/uapi/linux/if_link.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)include/net/switchdev.h +1
-0@@ -43,6 +43,7 @@ enum switchdev_attr_id {SWITCHDEV_ATTR_ID_PORT_PARENT_ID ,SWITCHDEV_ATTR_ID_PORT_STP_STATE ,SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS , + SWITCHDEV_ATTR_ID_BRIDGE, }; struct switchdev_attr {
include/net/switchdev.h struct switchdev_attr { · line 57
Quoted 5 lines
include/net/switchdev.h +2
-0@@ -52,6 +53,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;
I don't like pushing down IFLA_BR_* values throught switchdev. I think it might better to just intruduce:SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME
and "u32 ageing_time" here. Something similar to stp_state. Much easier to read and does not give blank cheque for passing any bridge IFLA_BR_* down to drivers.
It also aligns with bridge code nicely, I believe.
Quoted 21 lines
include/net/switchdev.h +2
-0+u32 val; + } bridge; } u; };include/uapi/linux/if_link.h +1
-1@@ -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 ,Signature · 1 line
1.7.10.4
Re: [PATCH net-next v2 1/4] switchdev: add bridge attributes
Quoted 36 lines · Jiri Pirko
Quoted 25 lines · Thu, Oct 08, 2015 at 08:04:40AM CEST, [email protected] wrote:
From: Scott Feldman <[email protected]>
Setting the stage to push bridge-level attributes down to port driver so hardware can be programmed accordingly. Bridge-level attribute example is
ageing_time. This is a per-bridge attribute, not a per-bridge-port attr.Signed-off-by Scott Feldman <[email protected]>---
include/net/switchdev.h | 5 +++++ include/uapi/linux/if_link.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)include/net/switchdev.h +1
-0@@ -43,6 +43,7 @@ enum switchdev_attr_id {SWITCHDEV_ATTR_ID_PORT_PARENT_ID ,SWITCHDEV_ATTR_ID_PORT_STP_STATE ,SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS , + SWITCHDEV_ATTR_ID_BRIDGE, }; struct switchdev_attr {include/net/switchdev.h struct switchdev_attr { · line 57
Quoted 5 lines
include/net/switchdev.h +2
-0@@ -52,6 +53,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;I don't like pushing down IFLA_BR_* values throught switchdev. I think it might better to just intruduce:
SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME
Quoted 5 lines
and "u32
ageing_time" here. Something similar tostp_state. Much easier to read and does not give blank cheque for passing any bridge IFLA_BR_* down to drivers.It also aligns with bridge code nicely, I believe.
I would add that pushing bridge-specific aspects down to the drivers does not really make sense since such notion does not exist for them. A Linux-bridge is just an untagged-VLAN for the switch chip point-of-view.
Thanks,
-v
Re: [PATCH net-next v2 1/4] switchdev: add bridge attributes
In reply to Jiri PirkoQuoted 36 lines · Jiri Pirko
Quoted 25 lines · Thu, Oct 08, 2015 at 08:04:40AM CEST, [email protected] wrote:
From: Scott Feldman <[email protected]>
Setting the stage to push bridge-level attributes down to port driver so hardware can be programmed accordingly. Bridge-level attribute example is
ageing_time. This is a per-bridge attribute, not a per-bridge-port attr.Signed-off-by Scott Feldman <[email protected]>---
include/net/switchdev.h | 5 +++++ include/uapi/linux/if_link.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)include/net/switchdev.h +1
-0@@ -43,6 +43,7 @@ enum switchdev_attr_id {SWITCHDEV_ATTR_ID_PORT_PARENT_ID ,SWITCHDEV_ATTR_ID_PORT_STP_STATE ,SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS , + SWITCHDEV_ATTR_ID_BRIDGE, }; struct switchdev_attr {include/net/switchdev.h struct switchdev_attr { · line 57
Quoted 5 lines
include/net/switchdev.h +2
-0@@ -52,6 +53,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;I don't like pushing down IFLA_BR_* values throught switchdev. I think it might better to just intruduce:
SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME
Quoted 5 lines
and "u32
ageing_time" here. Something similar tostp_state. Much easier to read and does not give blank cheque for passing any bridge IFLA_BR_* down to drivers.It also aligns with bridge code nicely, I believe.
Done, v3 sent with your suggested change.
PATCH v2 2/4 switchdev: skip over ports returning -EOPNOTSUPP when recursing ports
In reply to [email protected]From: Scott Feldman <[email protected]>
This allows us to recurse over all the ports, skipping over unsupporting ports. Without the change, the recursion would stop at first unsupported port.
--- include/net/switchdev.h | 1 + net/switchdev/switchdev.c | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-)
include/net/switchdev.h +1 -0
@@ -16,6 +16,7 @@#include <linux/list.h> #define SWITCHDEV_F_NO_RECURSE BIT(0) +#define SWITCHDEV_F_SKIP_EOPNOTSUPP BIT(1)struct switchdev_trans_item {struct list_head list;
net/switchdev/switchdev.c +8 -1
@@ -147,7 +147,7 @@ static int __switchdev_port_attr_set(struct net_device *dev,return ops->switchdev_port_attr_set (dev, attr, trans);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
@@ -156,10 +156,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, trans); +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; }
Signature · 1 line
1.7.10.4
PATCH v2 3/4 bridge: push bridge setting ageing_time down to switchdev
In reply to [email protected]From: Scott Feldman <[email protected]>
Use SWITCHDEV_F_SKIP_EOPNOTSUPP to skip over ports in bridge that don't support setting ageing_time (or setting bridge attrs in general).
If push fails, don't update ageing_time in bridge and return err to user.
If push succeeds, update ageing_time in bridge and run gc_timer now to recalabrate when to run gc_timer next, based on new ageing_time.
--- net/bridge/br_ioctl.c | 3 +-- net/bridge/br_netlink.c | 6 +++--- net/bridge/br_private.h | 1 + net/bridge/br_stp.c | 24 ++++++++++++++++++++++++ net/bridge/br_sysfs_br.c | 3 +-- 5 files changed, 30 insertions(+), 7 deletions(-)
net/bridge/br_ioctl.c +1 -2
@@ -200,8 +200,7 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)if (!ns_capable (dev_net (dev)->user_ns,CAP_NET_ADMIN ))return -EPERM ; - br->ageing_time =clock_t_to_jiffies (args[1 ]); -return 0 ; +return br_set_ageing_time (br, args[1 ]);case BRCTL_GET_PORT_INFO : {
net/bridge/br_netlink.c +3 -3
@@ -870,9 +870,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 ]) {
net/bridge/br_private.h +1 -0
@@ -882,6 +882,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 */
net/bridge/br_stp.c +24 -0
@@ -566,6 +566,30 @@ 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_ID_BRIDGE , + .flags =SWITCHDEV_F_SKIP_EOPNOTSUPP , + .u.bridge.attr =IFLA_BR_AGEING_TIME , + .u.bridge.val = ageing_time, + }; +unsigned long t =clock_t_to_jiffies (ageing_time); +int err; + +if (t <BR_MIN_AGEING_TIME || t >BR_MAX_AGEING_TIME ) +return -ERANGE ; + + err =switchdev_port_attr_set (br->dev, &attr); +if (err) +return err; + + br->ageing_time = t; +mod_timer (&br->gc_timer, jiffies); + +return 0 ; +} +void __br_set_forward_delay (struct net_bridge *br,unsigned long t) { br->bridge_forward_delay = t;
net/bridge/br_sysfs_br.c +1 -2
@@ -102,8 +102,7 @@ static ssize_t ageing_time_show(struct device *d,static int set_ageing_time (struct net_bridge *br,unsigned long val) { - br->ageing_time =clock_t_to_jiffies (val); -return 0 ; +return br_set_ageing_time (br, val); } static ssize_t ageing_time_store(struct device *d,
Signature · 1 line
1.7.10.4
PATCH v2 4/4 rocker: handle setting bridge ageing_time
In reply to [email protected]From: Scott Feldman <[email protected]>
The FDB cleanup timer will get rescheduled to re-evaluate FDB entries based on new ageing_time.
--- drivers/net/ethernet/rocker/rocker.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
drivers/net/ethernet/rocker/rocker.c +22 -0
@@ -4361,6 +4361,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, +struct switchdev_trans *trans, +struct switchdev_attr_bridge *bridge) +{ +switch (bridge->attr) { +case IFLA_BR_AGEING_TIME : +if (switchdev_trans_ph_prepare (trans)) +return 0 ; + rocker_port->ageing_time =clock_t_to_jiffies (bridge->val); +mod_timer (&rocker_port->rocker->fdb_cleanup_timer, jiffies); +break ; +default : +return -EOPNOTSUPP ; + } + +return 0 ; +} +static int rocker_port_attr_set (struct net_device *dev,struct switchdev_attr *attr,struct switchdev_trans *trans)
@@ -4378,6 +4396,10 @@ static int rocker_port_attr_set(struct net_device *dev, err =rocker_port_brport_flags_set (rocker_port, trans, attr->u.brport_flags);break ; +case SWITCHDEV_ATTR_ID_BRIDGE : + err =rocker_port_bridge_set (rocker_port, trans, + &attr->u.bridge); +break ;default : err = -EOPNOTSUPP ;break ;
Signature · 1 line
1.7.10.4