Message ID | 1444284283-33601-2-git-send-email-sfeldma@gmail.com |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
Thu, Oct 08, 2015 at 08:04:40AM CEST, sfeldma@gmail.com wrote: >From: Scott Feldman <sfeldma@gmail.com> > >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 <sfeldma@gmail.com> >--- > include/net/switchdev.h | 5 +++++ > include/uapi/linux/if_link.h | 2 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > >diff --git a/include/net/switchdev.h b/include/net/switchdev.h >index 89266a3..8d92cd0 100644 >--- a/include/net/switchdev.h >+++ b/include/net/switchdev.h >@@ -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; 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. >+ u32 val; >+ } bridge; > } u; > }; > >diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >index e3b6217..30177b3 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, >-- >1.7.10.4 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Oct. Thursday 08 (41) 10:39 AM, Jiri Pirko wrote: > Thu, Oct 08, 2015 at 08:04:40AM CEST, sfeldma@gmail.com wrote: > >From: Scott Feldman <sfeldma@gmail.com> > > > >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 <sfeldma@gmail.com> > >--- > > include/net/switchdev.h | 5 +++++ > > include/uapi/linux/if_link.h | 2 +- > > 2 files changed, 6 insertions(+), 1 deletion(-) > > > >diff --git a/include/net/switchdev.h b/include/net/switchdev.h > >index 89266a3..8d92cd0 100644 > >--- a/include/net/switchdev.h > >+++ b/include/net/switchdev.h > >@@ -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; > > 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. 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 8, 2015 at 1:39 AM, Jiri Pirko <jiri@resnulli.us> wrote: > Thu, Oct 08, 2015 at 08:04:40AM CEST, sfeldma@gmail.com wrote: >>From: Scott Feldman <sfeldma@gmail.com> >> >>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 <sfeldma@gmail.com> >>--- >> include/net/switchdev.h | 5 +++++ >> include/uapi/linux/if_link.h | 2 +- >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >>diff --git a/include/net/switchdev.h b/include/net/switchdev.h >>index 89266a3..8d92cd0 100644 >>--- a/include/net/switchdev.h >>+++ b/include/net/switchdev.h >>@@ -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; > > 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. Done, v3 sent with your suggested change. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/switchdev.h b/include/net/switchdev.h index 89266a3..8d92cd0 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -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; }; diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index e3b6217..30177b3 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,