* [RFC PATCH net-next v2 1/1] net: Support for switch port configuration @ 2014-12-18 11:29 Varlese, Marco 2014-12-18 11:41 ` Thomas Graf 2014-12-18 14:44 ` Roopa Prabhu 0 siblings, 2 replies; 33+ messages in thread From: Varlese, Marco @ 2014-12-18 11:29 UTC (permalink / raw) To: netdev Cc: Fastabend, John R, Thomas Graf, Jiri Pirko, roopa, sfeldma, linux-kernel From: Marco Varlese <marco.varlese@intel.com> Switch hardware offers a list of attributes that are configurable on a per port basis. This patch provides a mechanism to configure switch ports by adding an NDO for setting specific values to specific attributes. There will be a separate patch that adds the "get" functionality via another NDO and another patch that extends iproute2 to call the two new NDOs. Signed-off-by: Marco Varlese <marco.varlese@intel.com> --- include/linux/netdevice.h | 5 ++++ include/uapi/linux/if_link.h | 15 ++++++++++++ net/core/rtnetlink.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index c31f74d..4881c7b 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev, * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state); * Called to notify switch device port of bridge port STP * state change. + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, + * u32 attr, u64 value); + * Called to set specific switch ports attributes. */ struct net_device_ops { int (*ndo_init)(struct net_device *dev); @@ -1185,6 +1188,8 @@ struct net_device_ops { struct netdev_phys_item_id *psid); int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state); + int (*ndo_switch_port_set_cfg)(struct net_device *dev, + u32 attr, u64 value); #endif }; diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -146,6 +146,7 @@ enum { IFLA_PHYS_PORT_ID, IFLA_CARRIER_CHANGES, IFLA_PHYS_SWITCH_ID, + IFLA_SWITCH_PORT_CFG, __IFLA_MAX }; @@ -603,4 +604,18 @@ enum { #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) +/* Switch Port Attributes section */ + +enum { + IFLA_ATTR_UNSPEC, + IFLA_ATTR_LEARNING, + IFLA_ATTR_LOOPBACK, + IFLA_ATTR_BCAST_FLOODING, + IFLA_ATTR_UCAST_FLOODING, + IFLA_ATTR_MCAST_FLOODING, + __IFLA_ATTR_MAX +}; + +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) + #endif /* _UAPI_LINUX_IF_LINK_H */ diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index eaa057f..c0d1c45 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[]) return 0; } +#ifdef CONFIG_NET_SWITCHDEV +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) +{ + int rem, err = -EINVAL; + struct nlattr *v; + const struct net_device_ops *ops = dev->netdev_ops; + + nla_for_each_nested(v, attr, rem) { + u32 op = nla_type(v); + u64 value = 0; + + switch (op) { + case IFLA_ATTR_LEARNING: + case IFLA_ATTR_LOOPBACK: + case IFLA_ATTR_BCAST_FLOODING: + case IFLA_ATTR_UCAST_FLOODING: + case IFLA_ATTR_MCAST_FLOODING: { + if (nla_len(v) < sizeof(value)) { + err = -EINVAL; + break; + } + + value = nla_get_u64(v); + err = ops->ndo_switch_port_set_cfg(dev, + op, + value); + break; + } + default: + err = -EINVAL; + break; + } + if (err) + break; + } + return err; +} + +#endif + static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) { int rem, err = -EINVAL; @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, status |= DO_SETLINK_NOTIFY; } } +#ifdef CONFIG_NET_SWITCHDEV + if (tb[IFLA_SWITCH_PORT_CFG]) { + err = -EOPNOTSUPP; + if (!ops->ndo_switch_port_set_cfg) + goto errout; + if (!ops->ndo_switch_parent_id_get) + goto errout; + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); + if (err < 0) + goto errout; + + status |= DO_SETLINK_NOTIFY; + } +#endif err = 0; errout: -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 11:29 [RFC PATCH net-next v2 1/1] net: Support for switch port configuration Varlese, Marco @ 2014-12-18 11:41 ` Thomas Graf 2014-12-18 15:20 ` Varlese, Marco 2014-12-18 14:44 ` Roopa Prabhu 1 sibling, 1 reply; 33+ messages in thread From: Thomas Graf @ 2014-12-18 11:41 UTC (permalink / raw) To: Varlese, Marco Cc: netdev, Fastabend, John R, Jiri Pirko, roopa, sfeldma, linux-kernel On 12/18/14 at 11:29am, Varlese, Marco wrote: > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h > index f7d0d2d..19cb51a 100644 > --- a/include/uapi/linux/if_link.h > +++ b/include/uapi/linux/if_link.h > @@ -146,6 +146,7 @@ enum { > IFLA_PHYS_PORT_ID, > IFLA_CARRIER_CHANGES, > IFLA_PHYS_SWITCH_ID, > + IFLA_SWITCH_PORT_CFG, > __IFLA_MAX > }; Needs an entry in ifla_policy[] [IFLA_SWITCH_PORT_CFG] = { .type = NLA_NESTED }, > @@ -603,4 +604,18 @@ enum { > > #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > > +/* Switch Port Attributes section */ > + > +enum { > + IFLA_ATTR_UNSPEC, > + IFLA_ATTR_LEARNING, > + IFLA_ATTR_LOOPBACK, > + IFLA_ATTR_BCAST_FLOODING, > + IFLA_ATTR_UCAST_FLOODING, > + IFLA_ATTR_MCAST_FLOODING, > + __IFLA_ATTR_MAX > +}; > + > +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) Change the prefix to IFLA_SW_* since it's switch specific? > > +#ifdef CONFIG_NET_SWITCHDEV > +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) > +{ > + int rem, err = -EINVAL; > + struct nlattr *v; > + const struct net_device_ops *ops = dev->netdev_ops; > + > + nla_for_each_nested(v, attr, rem) { > + u32 op = nla_type(v); > + u64 value = 0; > + > + switch (op) { > + case IFLA_ATTR_LEARNING: > + case IFLA_ATTR_LOOPBACK: > + case IFLA_ATTR_BCAST_FLOODING: > + case IFLA_ATTR_UCAST_FLOODING: > + case IFLA_ATTR_MCAST_FLOODING: { > + if (nla_len(v) < sizeof(value)) { > + err = -EINVAL; > + break; > + } You should validate the message before you start applying the changes. Otherwise if the 3rd attribute is too short you've already applied some changes and the user has not idea how much has been applied. nla_parse_nested() can help here. > + > + value = nla_get_u64(v); > + err = ops->ndo_switch_port_set_cfg(dev, > + op, > + value); This avoids having individual ndos but wastes a lot of space in the Netlink message. Not a problem when setting configuration but you likely want to dump these attributes as well and we need 12 bytes for each attribute even though some are merely flags which could fit in 4 bytes. > static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > { > int rem, err = -EINVAL; > @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, > status |= DO_SETLINK_NOTIFY; > } > } > +#ifdef CONFIG_NET_SWITCHDEV > + if (tb[IFLA_SWITCH_PORT_CFG]) { > + err = -EOPNOTSUPP; > + if (!ops->ndo_switch_port_set_cfg) > + goto errout; > + if (!ops->ndo_switch_parent_id_get) > + goto errout; > + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > + if (err < 0) > + goto errout; > + > + status |= DO_SETLINK_NOTIFY; > + } > +#endif Should return -EOPNOTSUPP if IFLA_SWITCH_PORT_CFG is provided but CONFIG_NET_SWITCHDEV is not set. ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 11:41 ` Thomas Graf @ 2014-12-18 15:20 ` Varlese, Marco 0 siblings, 0 replies; 33+ messages in thread From: Varlese, Marco @ 2014-12-18 15:20 UTC (permalink / raw) To: Thomas Graf Cc: netdev, Fastabend, John R, Jiri Pirko, roopa, sfeldma, linux-kernel > -----Original Message----- > From: Thomas Graf [mailto:tgr@infradead.org] On Behalf Of Thomas Graf > Sent: Thursday, December 18, 2014 11:42 AM > To: Varlese, Marco > Cc: netdev@vger.kernel.org; Fastabend, John R; Jiri Pirko; > roopa@cumulusnetworks.com; sfeldma@gmail.com; linux- > kernel@vger.kernel.org > Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port > configuration > > On 12/18/14 at 11:29am, Varlese, Marco wrote: > > diff --git a/include/uapi/linux/if_link.h > > b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 > > --- a/include/uapi/linux/if_link.h > > +++ b/include/uapi/linux/if_link.h > > @@ -146,6 +146,7 @@ enum { > > IFLA_PHYS_PORT_ID, > > IFLA_CARRIER_CHANGES, > > IFLA_PHYS_SWITCH_ID, > > + IFLA_SWITCH_PORT_CFG, > > __IFLA_MAX > > }; > > Needs an entry in ifla_policy[] > > [IFLA_SWITCH_PORT_CFG] = { .type = NLA_NESTED }, > > > @@ -603,4 +604,18 @@ enum { > > > > #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > > > > +/* Switch Port Attributes section */ > > + > > +enum { > > + IFLA_ATTR_UNSPEC, > > + IFLA_ATTR_LEARNING, > > + IFLA_ATTR_LOOPBACK, > > + IFLA_ATTR_BCAST_FLOODING, > > + IFLA_ATTR_UCAST_FLOODING, > > + IFLA_ATTR_MCAST_FLOODING, > > + __IFLA_ATTR_MAX > > +}; > > + > > +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) > > Change the prefix to IFLA_SW_* since it's switch specific? > > > > > +#ifdef CONFIG_NET_SWITCHDEV > > +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { > > + int rem, err = -EINVAL; > > + struct nlattr *v; > > + const struct net_device_ops *ops = dev->netdev_ops; > > + > > + nla_for_each_nested(v, attr, rem) { > > + u32 op = nla_type(v); > > + u64 value = 0; > > + > > + switch (op) { > > + case IFLA_ATTR_LEARNING: > > + case IFLA_ATTR_LOOPBACK: > > + case IFLA_ATTR_BCAST_FLOODING: > > + case IFLA_ATTR_UCAST_FLOODING: > > + case IFLA_ATTR_MCAST_FLOODING: { > > + if (nla_len(v) < sizeof(value)) { > > + err = -EINVAL; > > + break; > > + } > > You should validate the message before you start applying the changes. > Otherwise if the 3rd attribute is too short you've already applied some > changes and the user has not idea how much has been applied. > > nla_parse_nested() can help here. > > > > + > > + value = nla_get_u64(v); > > + err = ops->ndo_switch_port_set_cfg(dev, > > + op, > > + value); > > This avoids having individual ndos but wastes a lot of space in the Netlink > message. Not a problem when setting configuration but you likely want to > dump these attributes as well and we need 12 bytes for each attribute even > though some are merely flags which could fit in 4 bytes. > > > static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > > { > > int rem, err = -EINVAL; > > @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, > > status |= DO_SETLINK_NOTIFY; > > } > > } > > +#ifdef CONFIG_NET_SWITCHDEV > > + if (tb[IFLA_SWITCH_PORT_CFG]) { > > + err = -EOPNOTSUPP; > > + if (!ops->ndo_switch_port_set_cfg) > > + goto errout; > > + if (!ops->ndo_switch_parent_id_get) > > + goto errout; > > + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > > + if (err < 0) > > + goto errout; > > + > > + status |= DO_SETLINK_NOTIFY; > > + } > > +#endif > > Should return -EOPNOTSUPP if IFLA_SWITCH_PORT_CFG is provided but > CONFIG_NET_SWITCHDEV is not set. > I think I've addressed your comments Thomas. Thanks for your suggestions. I'm going to resubmit the patch now. Thanks, Marco ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 11:29 [RFC PATCH net-next v2 1/1] net: Support for switch port configuration Varlese, Marco 2014-12-18 11:41 ` Thomas Graf @ 2014-12-18 14:44 ` Roopa Prabhu 2014-12-18 14:55 ` Varlese, Marco 1 sibling, 1 reply; 33+ messages in thread From: Roopa Prabhu @ 2014-12-18 14:44 UTC (permalink / raw) To: Varlese, Marco Cc: netdev, Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/14, 3:29 AM, Varlese, Marco wrote: > From: Marco Varlese <marco.varlese@intel.com> > > Switch hardware offers a list of attributes that are configurable > on a per port basis. > This patch provides a mechanism to configure switch ports by adding > an NDO for setting specific values to specific attributes. > There will be a separate patch that adds the "get" functionality via > another NDO and another patch that extends iproute2 to call the two > new NDOs. > > Signed-off-by: Marco Varlese <marco.varlese@intel.com> > --- > include/linux/netdevice.h | 5 ++++ > include/uapi/linux/if_link.h | 15 ++++++++++++ > net/core/rtnetlink.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 74 insertions(+) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index c31f74d..4881c7b 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev, > * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state); > * Called to notify switch device port of bridge port STP > * state change. > + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, > + * u32 attr, u64 value); > + * Called to set specific switch ports attributes. > */ > struct net_device_ops { > int (*ndo_init)(struct net_device *dev); > @@ -1185,6 +1188,8 @@ struct net_device_ops { > struct netdev_phys_item_id *psid); > int (*ndo_switch_port_stp_update)(struct net_device *dev, > u8 state); > + int (*ndo_switch_port_set_cfg)(struct net_device *dev, > + u32 attr, u64 value); > #endif > }; > > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h > index f7d0d2d..19cb51a 100644 > --- a/include/uapi/linux/if_link.h > +++ b/include/uapi/linux/if_link.h > @@ -146,6 +146,7 @@ enum { > IFLA_PHYS_PORT_ID, > IFLA_CARRIER_CHANGES, > IFLA_PHYS_SWITCH_ID, > + IFLA_SWITCH_PORT_CFG, > __IFLA_MAX > }; > > @@ -603,4 +604,18 @@ enum { > > #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > > +/* Switch Port Attributes section */ > + > +enum { > + IFLA_ATTR_UNSPEC, > + IFLA_ATTR_LEARNING, Any reason you want learning here ?. This is covered as part of the bridge setlink attributes. > + IFLA_ATTR_LOOPBACK, > + IFLA_ATTR_BCAST_FLOODING, > + IFLA_ATTR_UCAST_FLOODING, > + IFLA_ATTR_MCAST_FLOODING, > + __IFLA_ATTR_MAX > +}; > + > +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) > + > #endif /* _UAPI_LINUX_IF_LINK_H */ > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index eaa057f..c0d1c45 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[]) > return 0; > } > > +#ifdef CONFIG_NET_SWITCHDEV > +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) > +{ > + int rem, err = -EINVAL; > + struct nlattr *v; > + const struct net_device_ops *ops = dev->netdev_ops; > + > + nla_for_each_nested(v, attr, rem) { > + u32 op = nla_type(v); > + u64 value = 0; > + > + switch (op) { > + case IFLA_ATTR_LEARNING: > + case IFLA_ATTR_LOOPBACK: > + case IFLA_ATTR_BCAST_FLOODING: > + case IFLA_ATTR_UCAST_FLOODING: > + case IFLA_ATTR_MCAST_FLOODING: { > + if (nla_len(v) < sizeof(value)) { > + err = -EINVAL; > + break; > + } > + > + value = nla_get_u64(v); > + err = ops->ndo_switch_port_set_cfg(dev, > + op, > + value); > + break; > + } > + default: > + err = -EINVAL; > + break; > + } > + if (err) > + break; > + } > + return err; > +} > + > +#endif > + > static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > { > int rem, err = -EINVAL; > @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, > status |= DO_SETLINK_NOTIFY; > } > } > +#ifdef CONFIG_NET_SWITCHDEV > + if (tb[IFLA_SWITCH_PORT_CFG]) { > + err = -EOPNOTSUPP; > + if (!ops->ndo_switch_port_set_cfg) > + goto errout; > + if (!ops->ndo_switch_parent_id_get) > + goto errout; > + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > + if (err < 0) > + goto errout; > + > + status |= DO_SETLINK_NOTIFY; > + } > +#endif > err = 0; > > errout: ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 14:44 ` Roopa Prabhu @ 2014-12-18 14:55 ` Varlese, Marco 2014-12-18 15:16 ` Roopa Prabhu 2014-12-18 15:47 ` Arad, Ronen 0 siblings, 2 replies; 33+ messages in thread From: Varlese, Marco @ 2014-12-18 14:55 UTC (permalink / raw) To: Roopa Prabhu Cc: netdev, Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel > -----Original Message----- > From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com] > Sent: Thursday, December 18, 2014 2:45 PM > To: Varlese, Marco > Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri Pirko; > sfeldma@gmail.com; linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port > configuration > > On 12/18/14, 3:29 AM, Varlese, Marco wrote: > > From: Marco Varlese <marco.varlese@intel.com> > > > > Switch hardware offers a list of attributes that are configurable on a > > per port basis. > > This patch provides a mechanism to configure switch ports by adding an > > NDO for setting specific values to specific attributes. > > There will be a separate patch that adds the "get" functionality via > > another NDO and another patch that extends iproute2 to call the two > > new NDOs. > > > > Signed-off-by: Marco Varlese <marco.varlese@intel.com> > > --- > > include/linux/netdevice.h | 5 ++++ > > include/uapi/linux/if_link.h | 15 ++++++++++++ > > net/core/rtnetlink.c | 54 > ++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 74 insertions(+) > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > index c31f74d..4881c7b 100644 > > --- a/include/linux/netdevice.h > > +++ b/include/linux/netdevice.h > > @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct > net_device *dev, > > * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state); > > * Called to notify switch device port of bridge port STP > > * state change. > > + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, > > + * u32 attr, u64 value); > > + * Called to set specific switch ports attributes. > > */ > > struct net_device_ops { > > int (*ndo_init)(struct net_device *dev); > > @@ -1185,6 +1188,8 @@ struct net_device_ops { > > struct > netdev_phys_item_id *psid); > > int (*ndo_switch_port_stp_update)(struct > net_device *dev, > > u8 state); > > + int (*ndo_switch_port_set_cfg)(struct > net_device *dev, > > + u32 attr, u64 value); > > #endif > > }; > > > > diff --git a/include/uapi/linux/if_link.h > > b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 > > --- a/include/uapi/linux/if_link.h > > +++ b/include/uapi/linux/if_link.h > > @@ -146,6 +146,7 @@ enum { > > IFLA_PHYS_PORT_ID, > > IFLA_CARRIER_CHANGES, > > IFLA_PHYS_SWITCH_ID, > > + IFLA_SWITCH_PORT_CFG, > > __IFLA_MAX > > }; > > > > @@ -603,4 +604,18 @@ enum { > > > > #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > > > > +/* Switch Port Attributes section */ > > + > > +enum { > > + IFLA_ATTR_UNSPEC, > > + IFLA_ATTR_LEARNING, > Any reason you want learning here ?. This is covered as part of the bridge > setlink attributes. > Yes, because the user may _not_ want to go through a bridge interface necessarily. > > + IFLA_ATTR_LOOPBACK, > > + IFLA_ATTR_BCAST_FLOODING, > > + IFLA_ATTR_UCAST_FLOODING, > > + IFLA_ATTR_MCAST_FLOODING, > > + __IFLA_ATTR_MAX > > +}; > > + > > +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) > > + > > #endif /* _UAPI_LINUX_IF_LINK_H */ > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index > > eaa057f..c0d1c45 100644 > > --- a/net/core/rtnetlink.c > > +++ b/net/core/rtnetlink.c > > @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device > *dev, struct nlattr *tb[]) > > return 0; > > } > > > > +#ifdef CONFIG_NET_SWITCHDEV > > +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { > > + int rem, err = -EINVAL; > > + struct nlattr *v; > > + const struct net_device_ops *ops = dev->netdev_ops; > > + > > + nla_for_each_nested(v, attr, rem) { > > + u32 op = nla_type(v); > > + u64 value = 0; > > + > > + switch (op) { > > + case IFLA_ATTR_LEARNING: > > + case IFLA_ATTR_LOOPBACK: > > + case IFLA_ATTR_BCAST_FLOODING: > > + case IFLA_ATTR_UCAST_FLOODING: > > + case IFLA_ATTR_MCAST_FLOODING: { > > + if (nla_len(v) < sizeof(value)) { > > + err = -EINVAL; > > + break; > > + } > > + > > + value = nla_get_u64(v); > > + err = ops->ndo_switch_port_set_cfg(dev, > > + op, > > + value); > > + break; > > + } > > + default: > > + err = -EINVAL; > > + break; > > + } > > + if (err) > > + break; > > + } > > + return err; > > +} > > + > > +#endif > > + > > static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > > { > > int rem, err = -EINVAL; > > @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, > > status |= DO_SETLINK_NOTIFY; > > } > > } > > +#ifdef CONFIG_NET_SWITCHDEV > > + if (tb[IFLA_SWITCH_PORT_CFG]) { > > + err = -EOPNOTSUPP; > > + if (!ops->ndo_switch_port_set_cfg) > > + goto errout; > > + if (!ops->ndo_switch_parent_id_get) > > + goto errout; > > + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > > + if (err < 0) > > + goto errout; > > + > > + status |= DO_SETLINK_NOTIFY; > > + } > > +#endif > > err = 0; > > > > errout: ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 14:55 ` Varlese, Marco @ 2014-12-18 15:16 ` Roopa Prabhu 2014-12-18 17:25 ` Varlese, Marco 2014-12-18 15:47 ` Arad, Ronen 1 sibling, 1 reply; 33+ messages in thread From: Roopa Prabhu @ 2014-12-18 15:16 UTC (permalink / raw) To: Varlese, Marco Cc: netdev, Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/14, 6:55 AM, Varlese, Marco wrote: >> -----Original Message----- >> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com] >> Sent: Thursday, December 18, 2014 2:45 PM >> To: Varlese, Marco >> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri Pirko; >> sfeldma@gmail.com; linux-kernel@vger.kernel.org >> Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port >> configuration >> >> On 12/18/14, 3:29 AM, Varlese, Marco wrote: >>> From: Marco Varlese <marco.varlese@intel.com> >>> >>> Switch hardware offers a list of attributes that are configurable on a >>> per port basis. >>> This patch provides a mechanism to configure switch ports by adding an >>> NDO for setting specific values to specific attributes. >>> There will be a separate patch that adds the "get" functionality via >>> another NDO and another patch that extends iproute2 to call the two >>> new NDOs. >>> >>> Signed-off-by: Marco Varlese <marco.varlese@intel.com> >>> --- >>> include/linux/netdevice.h | 5 ++++ >>> include/uapi/linux/if_link.h | 15 ++++++++++++ >>> net/core/rtnetlink.c | 54 >> ++++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 74 insertions(+) >>> >>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >>> index c31f74d..4881c7b 100644 >>> --- a/include/linux/netdevice.h >>> +++ b/include/linux/netdevice.h >>> @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct >> net_device *dev, >>> * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state); >>> * Called to notify switch device port of bridge port STP >>> * state change. >>> + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, >>> + * u32 attr, u64 value); >>> + * Called to set specific switch ports attributes. >>> */ >>> struct net_device_ops { >>> int (*ndo_init)(struct net_device *dev); >>> @@ -1185,6 +1188,8 @@ struct net_device_ops { >>> struct >> netdev_phys_item_id *psid); >>> int (*ndo_switch_port_stp_update)(struct >> net_device *dev, >>> u8 state); >>> + int (*ndo_switch_port_set_cfg)(struct >> net_device *dev, >>> + u32 attr, u64 value); >>> #endif >>> }; >>> >>> diff --git a/include/uapi/linux/if_link.h >>> b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 >>> --- a/include/uapi/linux/if_link.h >>> +++ b/include/uapi/linux/if_link.h >>> @@ -146,6 +146,7 @@ enum { >>> IFLA_PHYS_PORT_ID, >>> IFLA_CARRIER_CHANGES, >>> IFLA_PHYS_SWITCH_ID, >>> + IFLA_SWITCH_PORT_CFG, >>> __IFLA_MAX >>> }; >>> >>> @@ -603,4 +604,18 @@ enum { >>> >>> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) >>> >>> +/* Switch Port Attributes section */ >>> + >>> +enum { >>> + IFLA_ATTR_UNSPEC, >>> + IFLA_ATTR_LEARNING, >> Any reason you want learning here ?. This is covered as part of the bridge >> setlink attributes. >> > Yes, because the user may _not_ want to go through a bridge interface necessarily. But, the bridge setlink/getlink interface was changed to accommodate 'self' for exactly such cases. I kind of understand your case for the other attributes (these are per port settings that switch asics provide). However, i don't understand the reason to pull in bridge attributes here. > >>> + IFLA_ATTR_LOOPBACK, >>> + IFLA_ATTR_BCAST_FLOODING, >>> + IFLA_ATTR_UCAST_FLOODING, >>> + IFLA_ATTR_MCAST_FLOODING, >>> + __IFLA_ATTR_MAX >>> +}; >>> + >>> +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) >>> + >>> #endif /* _UAPI_LINUX_IF_LINK_H */ >>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index >>> eaa057f..c0d1c45 100644 >>> --- a/net/core/rtnetlink.c >>> +++ b/net/core/rtnetlink.c >>> @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device >> *dev, struct nlattr *tb[]) >>> return 0; >>> } >>> >>> +#ifdef CONFIG_NET_SWITCHDEV >>> +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { >>> + int rem, err = -EINVAL; >>> + struct nlattr *v; >>> + const struct net_device_ops *ops = dev->netdev_ops; >>> + >>> + nla_for_each_nested(v, attr, rem) { >>> + u32 op = nla_type(v); >>> + u64 value = 0; >>> + >>> + switch (op) { >>> + case IFLA_ATTR_LEARNING: >>> + case IFLA_ATTR_LOOPBACK: >>> + case IFLA_ATTR_BCAST_FLOODING: >>> + case IFLA_ATTR_UCAST_FLOODING: >>> + case IFLA_ATTR_MCAST_FLOODING: { >>> + if (nla_len(v) < sizeof(value)) { >>> + err = -EINVAL; >>> + break; >>> + } >>> + >>> + value = nla_get_u64(v); >>> + err = ops->ndo_switch_port_set_cfg(dev, >>> + op, >>> + value); >>> + break; >>> + } >>> + default: >>> + err = -EINVAL; >>> + break; >>> + } >>> + if (err) >>> + break; >>> + } >>> + return err; >>> +} >>> + >>> +#endif >>> + >>> static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) >>> { >>> int rem, err = -EINVAL; >>> @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, >>> status |= DO_SETLINK_NOTIFY; >>> } >>> } >>> +#ifdef CONFIG_NET_SWITCHDEV >>> + if (tb[IFLA_SWITCH_PORT_CFG]) { >>> + err = -EOPNOTSUPP; >>> + if (!ops->ndo_switch_port_set_cfg) >>> + goto errout; >>> + if (!ops->ndo_switch_parent_id_get) >>> + goto errout; >>> + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); >>> + if (err < 0) >>> + goto errout; >>> + >>> + status |= DO_SETLINK_NOTIFY; >>> + } >>> +#endif >>> err = 0; >>> >>> errout: > -- > 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 15:16 ` Roopa Prabhu @ 2014-12-18 17:25 ` Varlese, Marco 2014-12-18 17:49 ` Roopa Prabhu 0 siblings, 1 reply; 33+ messages in thread From: Varlese, Marco @ 2014-12-18 17:25 UTC (permalink / raw) To: Roopa Prabhu Cc: netdev, Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel > -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev- > owner@vger.kernel.org] On Behalf Of Roopa Prabhu > Sent: Thursday, December 18, 2014 3:16 PM > To: Varlese, Marco > Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri Pirko; > sfeldma@gmail.com; linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port > configuration > > On 12/18/14, 6:55 AM, Varlese, Marco wrote: > >> -----Original Message----- > >> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com] > >> Sent: Thursday, December 18, 2014 2:45 PM > >> To: Varlese, Marco > >> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri > >> Pirko; sfeldma@gmail.com; linux-kernel@vger.kernel.org > >> Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port > >> configuration > >> > >> On 12/18/14, 3:29 AM, Varlese, Marco wrote: > >>> From: Marco Varlese <marco.varlese@intel.com> > >>> > >>> Switch hardware offers a list of attributes that are configurable on > >>> a per port basis. > >>> This patch provides a mechanism to configure switch ports by adding > >>> an NDO for setting specific values to specific attributes. > >>> There will be a separate patch that adds the "get" functionality via > >>> another NDO and another patch that extends iproute2 to call the two > >>> new NDOs. > >>> > >>> Signed-off-by: Marco Varlese <marco.varlese@intel.com> > >>> --- > >>> include/linux/netdevice.h | 5 ++++ > >>> include/uapi/linux/if_link.h | 15 ++++++++++++ > >>> net/core/rtnetlink.c | 54 > >> ++++++++++++++++++++++++++++++++++++++++++++ > >>> 3 files changed, 74 insertions(+) > >>> > >>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > >>> index c31f74d..4881c7b 100644 > >>> --- a/include/linux/netdevice.h > >>> +++ b/include/linux/netdevice.h > >>> @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct > >> net_device *dev, > >>> * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 > state); > >>> * Called to notify switch device port of bridge port STP > >>> * state change. > >>> + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, > >>> + * u32 attr, u64 value); > >>> + * Called to set specific switch ports attributes. > >>> */ > >>> struct net_device_ops { > >>> int (*ndo_init)(struct net_device *dev); > >>> @@ -1185,6 +1188,8 @@ struct net_device_ops { > >>> struct > >> netdev_phys_item_id *psid); > >>> int (*ndo_switch_port_stp_update)(struct > >> net_device *dev, > >>> u8 state); > >>> + int (*ndo_switch_port_set_cfg)(struct > >> net_device *dev, > >>> + u32 attr, u64 value); > >>> #endif > >>> }; > >>> > >>> diff --git a/include/uapi/linux/if_link.h > >>> b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 > >>> --- a/include/uapi/linux/if_link.h > >>> +++ b/include/uapi/linux/if_link.h > >>> @@ -146,6 +146,7 @@ enum { > >>> IFLA_PHYS_PORT_ID, > >>> IFLA_CARRIER_CHANGES, > >>> IFLA_PHYS_SWITCH_ID, > >>> + IFLA_SWITCH_PORT_CFG, > >>> __IFLA_MAX > >>> }; > >>> > >>> @@ -603,4 +604,18 @@ enum { > >>> > >>> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > >>> > >>> +/* Switch Port Attributes section */ > >>> + > >>> +enum { > >>> + IFLA_ATTR_UNSPEC, > >>> + IFLA_ATTR_LEARNING, > >> Any reason you want learning here ?. This is covered as part of the > >> bridge setlink attributes. > >> > > Yes, because the user may _not_ want to go through a bridge interface > necessarily. > But, the bridge setlink/getlink interface was changed to accommodate 'self' > for exactly such cases. > I kind of understand your case for the other attributes (these are per port > settings that switch asics provide). > > However, i don't understand the reason to pull in bridge attributes here. > Maybe, I am missing something so you might help. The learning attribute - in my case - it is like all other attributes: a port attribute (as you said, port settings that the switch provides per port). So, what I was saying is "why the user shall go through a bridge to configure the learning attribute"? From my perspective, it is as any other attribute and as such configurable on the port. > > > >>> + IFLA_ATTR_LOOPBACK, > >>> + IFLA_ATTR_BCAST_FLOODING, > >>> + IFLA_ATTR_UCAST_FLOODING, > >>> + IFLA_ATTR_MCAST_FLOODING, > >>> + __IFLA_ATTR_MAX > >>> +}; > >>> + > >>> +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) > >>> + > >>> #endif /* _UAPI_LINUX_IF_LINK_H */ diff --git > >>> a/net/core/rtnetlink.c b/net/core/rtnetlink.c index > >>> eaa057f..c0d1c45 100644 > >>> --- a/net/core/rtnetlink.c > >>> +++ b/net/core/rtnetlink.c > >>> @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device > >> *dev, struct nlattr *tb[]) > >>> return 0; > >>> } > >>> > >>> +#ifdef CONFIG_NET_SWITCHDEV > >>> +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { > >>> + int rem, err = -EINVAL; > >>> + struct nlattr *v; > >>> + const struct net_device_ops *ops = dev->netdev_ops; > >>> + > >>> + nla_for_each_nested(v, attr, rem) { > >>> + u32 op = nla_type(v); > >>> + u64 value = 0; > >>> + > >>> + switch (op) { > >>> + case IFLA_ATTR_LEARNING: > >>> + case IFLA_ATTR_LOOPBACK: > >>> + case IFLA_ATTR_BCAST_FLOODING: > >>> + case IFLA_ATTR_UCAST_FLOODING: > >>> + case IFLA_ATTR_MCAST_FLOODING: { > >>> + if (nla_len(v) < sizeof(value)) { > >>> + err = -EINVAL; > >>> + break; > >>> + } > >>> + > >>> + value = nla_get_u64(v); > >>> + err = ops->ndo_switch_port_set_cfg(dev, > >>> + op, > >>> + value); > >>> + break; > >>> + } > >>> + default: > >>> + err = -EINVAL; > >>> + break; > >>> + } > >>> + if (err) > >>> + break; > >>> + } > >>> + return err; > >>> +} > >>> + > >>> +#endif > >>> + > >>> static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > >>> { > >>> int rem, err = -EINVAL; > >>> @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, > >>> status |= DO_SETLINK_NOTIFY; > >>> } > >>> } > >>> +#ifdef CONFIG_NET_SWITCHDEV > >>> + if (tb[IFLA_SWITCH_PORT_CFG]) { > >>> + err = -EOPNOTSUPP; > >>> + if (!ops->ndo_switch_port_set_cfg) > >>> + goto errout; > >>> + if (!ops->ndo_switch_parent_id_get) > >>> + goto errout; > >>> + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > >>> + if (err < 0) > >>> + goto errout; > >>> + > >>> + status |= DO_SETLINK_NOTIFY; > >>> + } > >>> +#endif > >>> err = 0; > >>> > >>> errout: > > -- > > 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 > > -- > 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 17:25 ` Varlese, Marco @ 2014-12-18 17:49 ` Roopa Prabhu 2014-12-18 18:02 ` Varlese, Marco 0 siblings, 1 reply; 33+ messages in thread From: Roopa Prabhu @ 2014-12-18 17:49 UTC (permalink / raw) To: Varlese, Marco Cc: netdev, Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/14, 9:25 AM, Varlese, Marco wrote: >> -----Original Message----- >> From: netdev-owner@vger.kernel.org [mailto:netdev- >> owner@vger.kernel.org] On Behalf Of Roopa Prabhu >> Sent: Thursday, December 18, 2014 3:16 PM >> To: Varlese, Marco >> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri Pirko; >> sfeldma@gmail.com; linux-kernel@vger.kernel.org >> Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port >> configuration >> >> On 12/18/14, 6:55 AM, Varlese, Marco wrote: >>>> -----Original Message----- >>>> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com] >>>> Sent: Thursday, December 18, 2014 2:45 PM >>>> To: Varlese, Marco >>>> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri >>>> Pirko; sfeldma@gmail.com; linux-kernel@vger.kernel.org >>>> Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port >>>> configuration >>>> >>>> On 12/18/14, 3:29 AM, Varlese, Marco wrote: >>>>> From: Marco Varlese <marco.varlese@intel.com> >>>>> >>>>> Switch hardware offers a list of attributes that are configurable on >>>>> a per port basis. >>>>> This patch provides a mechanism to configure switch ports by adding >>>>> an NDO for setting specific values to specific attributes. >>>>> There will be a separate patch that adds the "get" functionality via >>>>> another NDO and another patch that extends iproute2 to call the two >>>>> new NDOs. >>>>> >>>>> Signed-off-by: Marco Varlese <marco.varlese@intel.com> >>>>> --- >>>>> include/linux/netdevice.h | 5 ++++ >>>>> include/uapi/linux/if_link.h | 15 ++++++++++++ >>>>> net/core/rtnetlink.c | 54 >>>> ++++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 74 insertions(+) >>>>> >>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >>>>> index c31f74d..4881c7b 100644 >>>>> --- a/include/linux/netdevice.h >>>>> +++ b/include/linux/netdevice.h >>>>> @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct >>>> net_device *dev, >>>>> * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 >> state); >>>>> * Called to notify switch device port of bridge port STP >>>>> * state change. >>>>> + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, >>>>> + * u32 attr, u64 value); >>>>> + * Called to set specific switch ports attributes. >>>>> */ >>>>> struct net_device_ops { >>>>> int (*ndo_init)(struct net_device *dev); >>>>> @@ -1185,6 +1188,8 @@ struct net_device_ops { >>>>> struct >>>> netdev_phys_item_id *psid); >>>>> int (*ndo_switch_port_stp_update)(struct >>>> net_device *dev, >>>>> u8 state); >>>>> + int (*ndo_switch_port_set_cfg)(struct >>>> net_device *dev, >>>>> + u32 attr, u64 value); >>>>> #endif >>>>> }; >>>>> >>>>> diff --git a/include/uapi/linux/if_link.h >>>>> b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 >>>>> --- a/include/uapi/linux/if_link.h >>>>> +++ b/include/uapi/linux/if_link.h >>>>> @@ -146,6 +146,7 @@ enum { >>>>> IFLA_PHYS_PORT_ID, >>>>> IFLA_CARRIER_CHANGES, >>>>> IFLA_PHYS_SWITCH_ID, >>>>> + IFLA_SWITCH_PORT_CFG, >>>>> __IFLA_MAX >>>>> }; >>>>> >>>>> @@ -603,4 +604,18 @@ enum { >>>>> >>>>> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) >>>>> >>>>> +/* Switch Port Attributes section */ >>>>> + >>>>> +enum { >>>>> + IFLA_ATTR_UNSPEC, >>>>> + IFLA_ATTR_LEARNING, >>>> Any reason you want learning here ?. This is covered as part of the >>>> bridge setlink attributes. >>>> >>> Yes, because the user may _not_ want to go through a bridge interface >> necessarily. >> But, the bridge setlink/getlink interface was changed to accommodate 'self' >> for exactly such cases. >> I kind of understand your case for the other attributes (these are per port >> settings that switch asics provide). >> >> However, i don't understand the reason to pull in bridge attributes here. >> > Maybe, I am missing something so you might help. The learning attribute - in my case - it is like all other attributes: a port attribute (as you said, port settings that the switch provides per port). > So, what I was saying is "why the user shall go through a bridge to configure the learning attribute"? From my perspective, it is as any other attribute and as such configurable on the port. Thinking about this some more, i don't see why any of these attributes (except loopback. I dont understand the loopback attribute) cant be part of the birdge port attributes. With this we will end up adding l2 attributes in two places: the general link attributes and bridge attributes. And since we have gone down the path of using ndo_bridge_setlink/getlink with 'self'....we should stick to that for all l2 attributes. The idea of overloading ndo_bridge_set/getlink, was to have the same set of attributes but support both cases where the user wants to go through the bridge driver or directly to the switch port driver. So, you are not really going through the bridge driver if you use 'self' and ndo_bridge_setlink/getlink. >>>>> + IFLA_ATTR_LOOPBACK, >>>>> + IFLA_ATTR_BCAST_FLOODING, >>>>> + IFLA_ATTR_UCAST_FLOODING, >>>>> + IFLA_ATTR_MCAST_FLOODING, >>>>> + __IFLA_ATTR_MAX >>>>> +}; >>>>> + >>>>> +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) >>>>> + >>>>> #endif /* _UAPI_LINUX_IF_LINK_H */ diff --git >>>>> a/net/core/rtnetlink.c b/net/core/rtnetlink.c index >>>>> eaa057f..c0d1c45 100644 >>>>> --- a/net/core/rtnetlink.c >>>>> +++ b/net/core/rtnetlink.c >>>>> @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device >>>> *dev, struct nlattr *tb[]) >>>>> return 0; >>>>> } >>>>> >>>>> +#ifdef CONFIG_NET_SWITCHDEV >>>>> +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { >>>>> + int rem, err = -EINVAL; >>>>> + struct nlattr *v; >>>>> + const struct net_device_ops *ops = dev->netdev_ops; >>>>> + >>>>> + nla_for_each_nested(v, attr, rem) { >>>>> + u32 op = nla_type(v); >>>>> + u64 value = 0; >>>>> + >>>>> + switch (op) { >>>>> + case IFLA_ATTR_LEARNING: >>>>> + case IFLA_ATTR_LOOPBACK: >>>>> + case IFLA_ATTR_BCAST_FLOODING: >>>>> + case IFLA_ATTR_UCAST_FLOODING: >>>>> + case IFLA_ATTR_MCAST_FLOODING: { >>>>> + if (nla_len(v) < sizeof(value)) { >>>>> + err = -EINVAL; >>>>> + break; >>>>> + } >>>>> + >>>>> + value = nla_get_u64(v); >>>>> + err = ops->ndo_switch_port_set_cfg(dev, >>>>> + op, >>>>> + value); >>>>> + break; >>>>> + } >>>>> + default: >>>>> + err = -EINVAL; >>>>> + break; >>>>> + } >>>>> + if (err) >>>>> + break; >>>>> + } >>>>> + return err; >>>>> +} >>>>> + >>>>> +#endif >>>>> + >>>>> static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) >>>>> { >>>>> int rem, err = -EINVAL; >>>>> @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, >>>>> status |= DO_SETLINK_NOTIFY; >>>>> } >>>>> } >>>>> +#ifdef CONFIG_NET_SWITCHDEV >>>>> + if (tb[IFLA_SWITCH_PORT_CFG]) { >>>>> + err = -EOPNOTSUPP; >>>>> + if (!ops->ndo_switch_port_set_cfg) >>>>> + goto errout; >>>>> + if (!ops->ndo_switch_parent_id_get) >>>>> + goto errout; >>>>> + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); >>>>> + if (err < 0) >>>>> + goto errout; >>>>> + >>>>> + status |= DO_SETLINK_NOTIFY; >>>>> + } >>>>> +#endif >>>>> err = 0; >>>>> >>>>> errout: >>> -- >>> 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 >> -- >> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 17:49 ` Roopa Prabhu @ 2014-12-18 18:02 ` Varlese, Marco 2014-12-18 18:14 ` Roopa Prabhu 2014-12-19 0:45 ` Thomas Graf 0 siblings, 2 replies; 33+ messages in thread From: Varlese, Marco @ 2014-12-18 18:02 UTC (permalink / raw) To: Roopa Prabhu Cc: netdev, Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel Removed unnecessary content for ease of reading... > >>>>> +/* Switch Port Attributes section */ > >>>>> + > >>>>> +enum { > >>>>> + IFLA_ATTR_UNSPEC, > >>>>> + IFLA_ATTR_LEARNING, > >>>> Any reason you want learning here ?. This is covered as part of > >>>> the bridge setlink attributes. > >>>> > >>> Yes, because the user may _not_ want to go through a bridge > >>> interface > >> necessarily. > >> But, the bridge setlink/getlink interface was changed to accommodate > 'self' > >> for exactly such cases. > >> I kind of understand your case for the other attributes (these are > >> per port settings that switch asics provide). > >> > >> However, i don't understand the reason to pull in bridge attributes here. > >> > > Maybe, I am missing something so you might help. The learning attribute - > in my case - it is like all other attributes: a port attribute (as you said, port > settings that the switch provides per port). > > So, what I was saying is "why the user shall go through a bridge to configure > the learning attribute"? From my perspective, it is as any other attribute and > as such configurable on the port. > > Thinking about this some more, i don't see why any of these attributes > (except loopback. I dont understand the loopback attribute) cant be part of > the birdge port attributes. > > With this we will end up adding l2 attributes in two places: the general link > attributes and bridge attributes. > > And since we have gone down the path of using ndo_bridge_setlink/getlink > with 'self'....we should stick to that for all l2 attributes. > > The idea of overloading ndo_bridge_set/getlink, was to have the same set of > attributes but support both cases where the user wants to go through the > bridge driver or directly to the switch port driver. So, you are not really going > through the bridge driver if you use 'self' and ndo_bridge_setlink/getlink. > Roopa, one of the comments I got from Thomas Graf on my v1 patch was that your patch and mine were supplementary ("I think Roopa's patches are supplementary. Not all switchdev users will be backed with a Linux Bridge. I therefore welcome your patches very much")... I also understood by others that the patch made sense for the same reason. I simply do not understand why these attributes (and maybe others in the future) could not be configured directly on a standard port but have to go through a bridge. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 18:02 ` Varlese, Marco @ 2014-12-18 18:14 ` Roopa Prabhu 2014-12-18 19:21 ` John Fastabend 2014-12-19 0:45 ` Thomas Graf 1 sibling, 1 reply; 33+ messages in thread From: Roopa Prabhu @ 2014-12-18 18:14 UTC (permalink / raw) To: Varlese, Marco Cc: netdev, Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/14, 10:02 AM, Varlese, Marco wrote: > Removed unnecessary content for ease of reading... > >>>>>>> +/* Switch Port Attributes section */ >>>>>>> + >>>>>>> +enum { >>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>> + IFLA_ATTR_LEARNING, >>>>>> Any reason you want learning here ?. This is covered as part of >>>>>> the bridge setlink attributes. >>>>>> >>>>> Yes, because the user may _not_ want to go through a bridge >>>>> interface >>>> necessarily. >>>> But, the bridge setlink/getlink interface was changed to accommodate >> 'self' >>>> for exactly such cases. >>>> I kind of understand your case for the other attributes (these are >>>> per port settings that switch asics provide). >>>> >>>> However, i don't understand the reason to pull in bridge attributes here. >>>> >>> Maybe, I am missing something so you might help. The learning attribute - >> in my case - it is like all other attributes: a port attribute (as you said, port >> settings that the switch provides per port). >>> So, what I was saying is "why the user shall go through a bridge to configure >> the learning attribute"? From my perspective, it is as any other attribute and >> as such configurable on the port. >> >> Thinking about this some more, i don't see why any of these attributes >> (except loopback. I dont understand the loopback attribute) cant be part of >> the birdge port attributes. >> >> With this we will end up adding l2 attributes in two places: the general link >> attributes and bridge attributes. >> >> And since we have gone down the path of using ndo_bridge_setlink/getlink >> with 'self'....we should stick to that for all l2 attributes. >> >> The idea of overloading ndo_bridge_set/getlink, was to have the same set of >> attributes but support both cases where the user wants to go through the >> bridge driver or directly to the switch port driver. So, you are not really going >> through the bridge driver if you use 'self' and ndo_bridge_setlink/getlink. >> > Roopa, one of the comments I got from Thomas Graf on my v1 patch was that your patch and mine were supplementary ("I think Roopa's patches are supplementary. Not all switchdev users will be backed with a Linux Bridge. I therefore welcome your patches very much")... I also understood by others that the patch made sense for the same reason. I simply do not understand why these attributes (and maybe others in the future) could not be configured directly on a standard port but have to go through a bridge. > ok, i am very confused in that case. The whole moving of bridge attributes from the bridge driver to rtnetlink.c was to make the bridge attributes accessible to any driver who wants to set l2/bridge attributes on their switch ports. So, its unclear to me why we are doing this parallel thing again. This move to rtnetlink.c was done during the recent rocker support. so, maybe scott/jiri can elaborate more. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 18:14 ` Roopa Prabhu @ 2014-12-18 19:21 ` John Fastabend 2014-12-18 22:43 ` Arad, Ronen 2014-12-18 23:07 ` Roopa Prabhu 0 siblings, 2 replies; 33+ messages in thread From: John Fastabend @ 2014-12-18 19:21 UTC (permalink / raw) To: Roopa Prabhu, Varlese, Marco Cc: netdev, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/2014 10:14 AM, Roopa Prabhu wrote: > On 12/18/14, 10:02 AM, Varlese, Marco wrote: >> Removed unnecessary content for ease of reading... >> >>>>>>>> +/* Switch Port Attributes section */ >>>>>>>> + >>>>>>>> +enum { >>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>> the bridge setlink attributes. >>>>>>> >>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>> interface >>>>> necessarily. >>>>> But, the bridge setlink/getlink interface was changed to accommodate >>> 'self' >>>>> for exactly such cases. >>>>> I kind of understand your case for the other attributes (these are >>>>> per port settings that switch asics provide). >>>>> >>>>> However, i don't understand the reason to pull in bridge attributes here. >>>>> >>>> Maybe, I am missing something so you might help. The learning attribute - >>> in my case - it is like all other attributes: a port attribute (as you said, port >>> settings that the switch provides per port). >>>> So, what I was saying is "why the user shall go through a bridge to configure >>> the learning attribute"? From my perspective, it is as any other attribute and >>> as such configurable on the port. >>> >>> Thinking about this some more, i don't see why any of these attributes >>> (except loopback. I dont understand the loopback attribute) cant be part of >>> the birdge port attributes. >>> >>> With this we will end up adding l2 attributes in two places: the general link >>> attributes and bridge attributes. >>> >>> And since we have gone down the path of using ndo_bridge_setlink/getlink >>> with 'self'....we should stick to that for all l2 attributes. >>> >>> The idea of overloading ndo_bridge_set/getlink, was to have the same set of >>> attributes but support both cases where the user wants to go through the >>> bridge driver or directly to the switch port driver. So, you are not really going >>> through the bridge driver if you use 'self' and ndo_bridge_setlink/getlink. >>> >> Roopa, one of the comments I got from Thomas Graf on my v1 patch >> was that your patch and mine were supplementary ("I think Roopa's >> patches are supplementary. Not all switchdev users will be backed >> with a Linux Bridge. I therefore welcome your patches very >> much")... I also understood by others that the patch made sense for >> the same reason. I simply do not understand why these attributes >> (and maybe others in the future) could not be configured directly >> on a standard port but have to go through a bridge. >> > ok, i am very confused in that case. The whole moving of bridge > attributes from the bridge driver to rtnetlink.c was to make the > bridge attributes accessible to any driver who wants to set l2/bridge > attributes on their switch ports. So, its unclear to me why we are > doing this parallel thing again. This move to rtnetlink.c was done > during the recent rocker support. so, maybe scott/jiri can elaborate > more. Not sure if this will add to the confusion or help. But you do not need to have the bridge.ko loaded or netdev's attached to a bridge to use the setlink/getlink ndo ops and netlink messages. This was intentionally done. Its already used with NIC devices to configure embedded bridge settings such as VEB/VEPA. I think I'm just repeating Roopa though. ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 19:21 ` John Fastabend @ 2014-12-18 22:43 ` Arad, Ronen 2014-12-19 8:14 ` Jiri Pirko 2014-12-18 23:07 ` Roopa Prabhu 1 sibling, 1 reply; 33+ messages in thread From: Arad, Ronen @ 2014-12-18 22:43 UTC (permalink / raw) To: Fastabend, John R, Roopa Prabhu, Varlese, Marco, netdev Cc: Thomas Graf, Jiri Pirko, sfeldma, linux-kernel >-----Original Message----- >From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On >Behalf Of John Fastabend >Sent: Thursday, December 18, 2014 9:21 PM >To: Roopa Prabhu; Varlese, Marco >Cc: netdev@vger.kernel.org; Thomas Graf; Jiri Pirko; sfeldma@gmail.com; linux- >kernel@vger.kernel.org >Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port >configuration > >On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >> On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>> Removed unnecessary content for ease of reading... >>> >>>>>>>>> +/* Switch Port Attributes section */ >>>>>>>>> + >>>>>>>>> +enum { >>>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>>> the bridge setlink attributes. >>>>>>>> >>>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>>> interface >>>>>> necessarily. >>>>>> But, the bridge setlink/getlink interface was changed to accommodate >>>> 'self' >>>>>> for exactly such cases. >>>>>> I kind of understand your case for the other attributes (these are >>>>>> per port settings that switch asics provide). >>>>>> >>>>>> However, i don't understand the reason to pull in bridge attributes >here. >>>>>> >>>>> Maybe, I am missing something so you might help. The learning attribute - >>>> in my case - it is like all other attributes: a port attribute (as you >said, port >>>> settings that the switch provides per port). >>>>> So, what I was saying is "why the user shall go through a bridge to >configure >>>> the learning attribute"? From my perspective, it is as any other attribute >and >>>> as such configurable on the port. >>>> >>>> Thinking about this some more, i don't see why any of these attributes >>>> (except loopback. I dont understand the loopback attribute) cant be part >of >>>> the birdge port attributes. >>>> >>>> With this we will end up adding l2 attributes in two places: the general >link >>>> attributes and bridge attributes. >>>> >>>> And since we have gone down the path of using ndo_bridge_setlink/getlink >>>> with 'self'....we should stick to that for all l2 attributes. >>>> >>>> The idea of overloading ndo_bridge_set/getlink, was to have the same set >of >>>> attributes but support both cases where the user wants to go through the >>>> bridge driver or directly to the switch port driver. So, you are not >really going >>>> through the bridge driver if you use 'self' and >ndo_bridge_setlink/getlink. >>>> > >>> Roopa, one of the comments I got from Thomas Graf on my v1 patch >>> was that your patch and mine were supplementary ("I think Roopa's >>> patches are supplementary. Not all switchdev users will be backed >>> with a Linux Bridge. I therefore welcome your patches very >>> much")... I also understood by others that the patch made sense for >>> the same reason. I simply do not understand why these attributes >>> (and maybe others in the future) could not be configured directly >>> on a standard port but have to go through a bridge. >>> >> ok, i am very confused in that case. The whole moving of bridge >> attributes from the bridge driver to rtnetlink.c was to make the >> bridge attributes accessible to any driver who wants to set l2/bridge >> attributes on their switch ports. So, its unclear to me why we are >> doing this parallel thing again. This move to rtnetlink.c was done >> during the recent rocker support. so, maybe scott/jiri can elaborate >> more. > > >Not sure if this will add to the confusion or help. But you do not >need to have the bridge.ko loaded or netdev's attached to a bridge >to use the setlink/getlink ndo ops and netlink messages. No you don't need bridge.ko to implement ndo_bridge_setlink/getlink. Rtnetlink invokes those ndos from code which does not depend on CONFIG_BRIDGE or the presence of bridge.ko. Calling some bridge exported functions such as br_fdb_external_learn_add/del requires the presence of bridge.ko and it only makes sense when the switch port device is enslaved to a bridge. > >This was intentionally done. Its already used with NIC devices to >configure embedded bridge settings such as VEB/VEPA. > >I think I'm just repeating Roopa though. > >-- >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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 22:43 ` Arad, Ronen @ 2014-12-19 8:14 ` Jiri Pirko 0 siblings, 0 replies; 33+ messages in thread From: Jiri Pirko @ 2014-12-19 8:14 UTC (permalink / raw) To: Arad, Ronen Cc: Fastabend, John R, Roopa Prabhu, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel Thu, Dec 18, 2014 at 11:43:06PM CET, ronen.arad@intel.com wrote: > > >>-----Original Message----- >>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On >>Behalf Of John Fastabend >>Sent: Thursday, December 18, 2014 9:21 PM >>To: Roopa Prabhu; Varlese, Marco >>Cc: netdev@vger.kernel.org; Thomas Graf; Jiri Pirko; sfeldma@gmail.com; linux- >>kernel@vger.kernel.org >>Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port >>configuration >> >>On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >>> On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>>> Removed unnecessary content for ease of reading... >>>> >>>>>>>>>> +/* Switch Port Attributes section */ >>>>>>>>>> + >>>>>>>>>> +enum { >>>>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>>>> the bridge setlink attributes. >>>>>>>>> >>>>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>>>> interface >>>>>>> necessarily. >>>>>>> But, the bridge setlink/getlink interface was changed to accommodate >>>>> 'self' >>>>>>> for exactly such cases. >>>>>>> I kind of understand your case for the other attributes (these are >>>>>>> per port settings that switch asics provide). >>>>>>> >>>>>>> However, i don't understand the reason to pull in bridge attributes >>here. >>>>>>> >>>>>> Maybe, I am missing something so you might help. The learning attribute - >>>>> in my case - it is like all other attributes: a port attribute (as you >>said, port >>>>> settings that the switch provides per port). >>>>>> So, what I was saying is "why the user shall go through a bridge to >>configure >>>>> the learning attribute"? From my perspective, it is as any other attribute >>and >>>>> as such configurable on the port. >>>>> >>>>> Thinking about this some more, i don't see why any of these attributes >>>>> (except loopback. I dont understand the loopback attribute) cant be part >>of >>>>> the birdge port attributes. >>>>> >>>>> With this we will end up adding l2 attributes in two places: the general >>link >>>>> attributes and bridge attributes. >>>>> >>>>> And since we have gone down the path of using ndo_bridge_setlink/getlink >>>>> with 'self'....we should stick to that for all l2 attributes. >>>>> >>>>> The idea of overloading ndo_bridge_set/getlink, was to have the same set >>of >>>>> attributes but support both cases where the user wants to go through the >>>>> bridge driver or directly to the switch port driver. So, you are not >>really going >>>>> through the bridge driver if you use 'self' and >>ndo_bridge_setlink/getlink. >>>>> >> >>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch >>>> was that your patch and mine were supplementary ("I think Roopa's >>>> patches are supplementary. Not all switchdev users will be backed >>>> with a Linux Bridge. I therefore welcome your patches very >>>> much")... I also understood by others that the patch made sense for >>>> the same reason. I simply do not understand why these attributes >>>> (and maybe others in the future) could not be configured directly >>>> on a standard port but have to go through a bridge. >>>> >>> ok, i am very confused in that case. The whole moving of bridge >>> attributes from the bridge driver to rtnetlink.c was to make the >>> bridge attributes accessible to any driver who wants to set l2/bridge >>> attributes on their switch ports. So, its unclear to me why we are >>> doing this parallel thing again. This move to rtnetlink.c was done >>> during the recent rocker support. so, maybe scott/jiri can elaborate >>> more. >> >> >>Not sure if this will add to the confusion or help. But you do not >>need to have the bridge.ko loaded or netdev's attached to a bridge >>to use the setlink/getlink ndo ops and netlink messages. > >No you don't need bridge.ko to implement ndo_bridge_setlink/getlink. Rtnetlink invokes those ndos from code which does not depend on CONFIG_BRIDGE or the presence of bridge.ko. >Calling some bridge exported functions such as br_fdb_external_learn_add/del requires the presence of bridge.ko and it only makes sense when the switch port device is enslaved to a bridge. Note I plan to change br_fdb_external_learn_add/del to (semi-)generic notifier very soon. > >> >>This was intentionally done. Its already used with NIC devices to >>configure embedded bridge settings such as VEB/VEPA. >> >>I think I'm just repeating Roopa though. >> >>-- >>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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 19:21 ` John Fastabend 2014-12-18 22:43 ` Arad, Ronen @ 2014-12-18 23:07 ` Roopa Prabhu 2014-12-18 23:26 ` Samudrala, Sridhar 1 sibling, 1 reply; 33+ messages in thread From: Roopa Prabhu @ 2014-12-18 23:07 UTC (permalink / raw) To: John Fastabend Cc: Varlese, Marco, netdev, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/14, 11:21 AM, John Fastabend wrote: > On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >> On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>> Removed unnecessary content for ease of reading... >>> >>>>>>>>> +/* Switch Port Attributes section */ >>>>>>>>> + >>>>>>>>> +enum { >>>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>>> the bridge setlink attributes. >>>>>>>> >>>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>>> interface >>>>>> necessarily. >>>>>> But, the bridge setlink/getlink interface was changed to accommodate >>>> 'self' >>>>>> for exactly such cases. >>>>>> I kind of understand your case for the other attributes (these are >>>>>> per port settings that switch asics provide). >>>>>> >>>>>> However, i don't understand the reason to pull in bridge attributes here. >>>>>> >>>>> Maybe, I am missing something so you might help. The learning attribute - >>>> in my case - it is like all other attributes: a port attribute (as you said, port >>>> settings that the switch provides per port). >>>>> So, what I was saying is "why the user shall go through a bridge to configure >>>> the learning attribute"? From my perspective, it is as any other attribute and >>>> as such configurable on the port. >>>> >>>> Thinking about this some more, i don't see why any of these attributes >>>> (except loopback. I dont understand the loopback attribute) cant be part of >>>> the birdge port attributes. >>>> >>>> With this we will end up adding l2 attributes in two places: the general link >>>> attributes and bridge attributes. >>>> >>>> And since we have gone down the path of using ndo_bridge_setlink/getlink >>>> with 'self'....we should stick to that for all l2 attributes. >>>> >>>> The idea of overloading ndo_bridge_set/getlink, was to have the same set of >>>> attributes but support both cases where the user wants to go through the >>>> bridge driver or directly to the switch port driver. So, you are not really going >>>> through the bridge driver if you use 'self' and ndo_bridge_setlink/getlink. >>>> >>> Roopa, one of the comments I got from Thomas Graf on my v1 patch >>> was that your patch and mine were supplementary ("I think Roopa's >>> patches are supplementary. Not all switchdev users will be backed >>> with a Linux Bridge. I therefore welcome your patches very >>> much")... I also understood by others that the patch made sense for >>> the same reason. I simply do not understand why these attributes >>> (and maybe others in the future) could not be configured directly >>> on a standard port but have to go through a bridge. >>> >> ok, i am very confused in that case. The whole moving of bridge >> attributes from the bridge driver to rtnetlink.c was to make the >> bridge attributes accessible to any driver who wants to set l2/bridge >> attributes on their switch ports. So, its unclear to me why we are >> doing this parallel thing again. This move to rtnetlink.c was done >> during the recent rocker support. so, maybe scott/jiri can elaborate >> more. > > Not sure if this will add to the confusion or help. But you do not > need to have the bridge.ko loaded or netdev's attached to a bridge > to use the setlink/getlink ndo ops and netlink messages. > > This was intentionally done. Its already used with NIC devices to > configure embedded bridge settings such as VEB/VEPA. that helps my case, thanks. > > I think I'm just repeating Roopa though. > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 23:07 ` Roopa Prabhu @ 2014-12-18 23:26 ` Samudrala, Sridhar 2014-12-18 23:48 ` Roopa Prabhu 0 siblings, 1 reply; 33+ messages in thread From: Samudrala, Sridhar @ 2014-12-18 23:26 UTC (permalink / raw) To: Roopa Prabhu, John Fastabend Cc: Varlese, Marco, netdev, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/2014 3:07 PM, Roopa Prabhu wrote: > On 12/18/14, 11:21 AM, John Fastabend wrote: >> On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >>> On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>>> Removed unnecessary content for ease of reading... >>>> >>>>>>>>>> +/* Switch Port Attributes section */ >>>>>>>>>> + >>>>>>>>>> +enum { >>>>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>>>> the bridge setlink attributes. >>>>>>>>> >>>>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>>>> interface >>>>>>> necessarily. >>>>>>> But, the bridge setlink/getlink interface was changed to >>>>>>> accommodate >>>>> 'self' >>>>>>> for exactly such cases. >>>>>>> I kind of understand your case for the other attributes (these are >>>>>>> per port settings that switch asics provide). >>>>>>> >>>>>>> However, i don't understand the reason to pull in bridge >>>>>>> attributes here. >>>>>>> >>>>>> Maybe, I am missing something so you might help. The learning >>>>>> attribute - >>>>> in my case - it is like all other attributes: a port attribute (as >>>>> you said, port >>>>> settings that the switch provides per port). >>>>>> So, what I was saying is "why the user shall go through a bridge >>>>>> to configure >>>>> the learning attribute"? From my perspective, it is as any other >>>>> attribute and >>>>> as such configurable on the port. >>>>> >>>>> Thinking about this some more, i don't see why any of these >>>>> attributes >>>>> (except loopback. I dont understand the loopback attribute) cant >>>>> be part of >>>>> the birdge port attributes. >>>>> >>>>> With this we will end up adding l2 attributes in two places: the >>>>> general link >>>>> attributes and bridge attributes. >>>>> >>>>> And since we have gone down the path of using >>>>> ndo_bridge_setlink/getlink >>>>> with 'self'....we should stick to that for all l2 attributes. >>>>> >>>>> The idea of overloading ndo_bridge_set/getlink, was to have the >>>>> same set of >>>>> attributes but support both cases where the user wants to go >>>>> through the >>>>> bridge driver or directly to the switch port driver. So, you are >>>>> not really going >>>>> through the bridge driver if you use 'self' and >>>>> ndo_bridge_setlink/getlink. >>>>> >>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch >>>> was that your patch and mine were supplementary ("I think Roopa's >>>> patches are supplementary. Not all switchdev users will be backed >>>> with a Linux Bridge. I therefore welcome your patches very >>>> much")... I also understood by others that the patch made sense for >>>> the same reason. I simply do not understand why these attributes >>>> (and maybe others in the future) could not be configured directly >>>> on a standard port but have to go through a bridge. >>>> >>> ok, i am very confused in that case. The whole moving of bridge >>> attributes from the bridge driver to rtnetlink.c was to make the >>> bridge attributes accessible to any driver who wants to set l2/bridge >>> attributes on their switch ports. So, its unclear to me why we are >>> doing this parallel thing again. This move to rtnetlink.c was done >>> during the recent rocker support. so, maybe scott/jiri can elaborate >>> more. >> >> Not sure if this will add to the confusion or help. But you do not >> need to have the bridge.ko loaded or netdev's attached to a bridge >> to use the setlink/getlink ndo ops and netlink messages. >> >> This was intentionally done. Its already used with NIC devices to >> configure embedded bridge settings such as VEB/VEPA. > > that helps my case, thanks. So the user interface to set/get the per-port attributes will be via 'bridge', not 'ip' bridge link set dev sw0p1 port_attr bcast_flooding 1 self bridge link get dev sw0p1 port_attr bcast_flooding self We also need an interface to set per-switch attributes. Can this work? bridge link set dev sw0 sw_attr bcast_flooding 1 master where sw0 is a bridge representing the hardware switch. >> >> I think I'm just repeating Roopa though. >> > > -- > 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 23:26 ` Samudrala, Sridhar @ 2014-12-18 23:48 ` Roopa Prabhu 2014-12-19 5:14 ` B Viswanath 2014-12-19 8:25 ` Jiri Pirko 0 siblings, 2 replies; 33+ messages in thread From: Roopa Prabhu @ 2014-12-18 23:48 UTC (permalink / raw) To: Samudrala, Sridhar Cc: John Fastabend, Varlese, Marco, netdev, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: > > On 12/18/2014 3:07 PM, Roopa Prabhu wrote: >> On 12/18/14, 11:21 AM, John Fastabend wrote: >>> On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >>>> On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>>>> Removed unnecessary content for ease of reading... >>>>> >>>>>>>>>>> +/* Switch Port Attributes section */ >>>>>>>>>>> + >>>>>>>>>>> +enum { >>>>>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>>>>> the bridge setlink attributes. >>>>>>>>>> >>>>>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>>>>> interface >>>>>>>> necessarily. >>>>>>>> But, the bridge setlink/getlink interface was changed to >>>>>>>> accommodate >>>>>> 'self' >>>>>>>> for exactly such cases. >>>>>>>> I kind of understand your case for the other attributes (these are >>>>>>>> per port settings that switch asics provide). >>>>>>>> >>>>>>>> However, i don't understand the reason to pull in bridge >>>>>>>> attributes here. >>>>>>>> >>>>>>> Maybe, I am missing something so you might help. The learning >>>>>>> attribute - >>>>>> in my case - it is like all other attributes: a port attribute >>>>>> (as you said, port >>>>>> settings that the switch provides per port). >>>>>>> So, what I was saying is "why the user shall go through a bridge >>>>>>> to configure >>>>>> the learning attribute"? From my perspective, it is as any other >>>>>> attribute and >>>>>> as such configurable on the port. >>>>>> >>>>>> Thinking about this some more, i don't see why any of these >>>>>> attributes >>>>>> (except loopback. I dont understand the loopback attribute) cant >>>>>> be part of >>>>>> the birdge port attributes. >>>>>> >>>>>> With this we will end up adding l2 attributes in two places: the >>>>>> general link >>>>>> attributes and bridge attributes. >>>>>> >>>>>> And since we have gone down the path of using >>>>>> ndo_bridge_setlink/getlink >>>>>> with 'self'....we should stick to that for all l2 attributes. >>>>>> >>>>>> The idea of overloading ndo_bridge_set/getlink, was to have the >>>>>> same set of >>>>>> attributes but support both cases where the user wants to go >>>>>> through the >>>>>> bridge driver or directly to the switch port driver. So, you are >>>>>> not really going >>>>>> through the bridge driver if you use 'self' and >>>>>> ndo_bridge_setlink/getlink. >>>>>> >>>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch >>>>> was that your patch and mine were supplementary ("I think Roopa's >>>>> patches are supplementary. Not all switchdev users will be backed >>>>> with a Linux Bridge. I therefore welcome your patches very >>>>> much")... I also understood by others that the patch made sense for >>>>> the same reason. I simply do not understand why these attributes >>>>> (and maybe others in the future) could not be configured directly >>>>> on a standard port but have to go through a bridge. >>>>> >>>> ok, i am very confused in that case. The whole moving of bridge >>>> attributes from the bridge driver to rtnetlink.c was to make the >>>> bridge attributes accessible to any driver who wants to set l2/bridge >>>> attributes on their switch ports. So, its unclear to me why we are >>>> doing this parallel thing again. This move to rtnetlink.c was done >>>> during the recent rocker support. so, maybe scott/jiri can elaborate >>>> more. >>> >>> Not sure if this will add to the confusion or help. But you do not >>> need to have the bridge.ko loaded or netdev's attached to a bridge >>> to use the setlink/getlink ndo ops and netlink messages. >>> >>> This was intentionally done. Its already used with NIC devices to >>> configure embedded bridge settings such as VEB/VEPA. >> >> that helps my case, thanks. > > So the user interface to set/get the per-port attributes will be via > 'bridge', not 'ip' > > bridge link set dev sw0p1 port_attr bcast_flooding 1 self > bridge link get dev sw0p1 port_attr bcast_flooding self yes, l2 attributes. > > We also need an interface to set per-switch attributes. Can this work? > bridge link set dev sw0 sw_attr bcast_flooding 1 master > where sw0 is a bridge representing the hardware switch. Not today. We discussed this @ LPC, and one way to do this would be to have a device representing the switch asic. This is in the works. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 23:48 ` Roopa Prabhu @ 2014-12-19 5:14 ` B Viswanath 2014-12-19 8:27 ` Jiri Pirko 2014-12-19 8:25 ` Jiri Pirko 1 sibling, 1 reply; 33+ messages in thread From: B Viswanath @ 2014-12-19 5:14 UTC (permalink / raw) To: Roopa Prabhu Cc: Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >> >> >> On 12/18/2014 3:07 PM, Roopa Prabhu wrote: >>> >>> On 12/18/14, 11:21 AM, John Fastabend wrote: >>>> >>>> On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >>>>> >>>>> On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>>>>> >>>>>> Removed unnecessary content for ease of reading... >>>>>> >>>>>>>>>>>> +/* Switch Port Attributes section */ >>>>>>>>>>>> + >>>>>>>>>>>> +enum { >>>>>>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>>>>>> >>>>>>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>>>>>> the bridge setlink attributes. >>>>>>>>>>> >>>>>>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>>>>>> interface >>>>>>>>> >>>>>>>>> necessarily. >>>>>>>>> But, the bridge setlink/getlink interface was changed to >>>>>>>>> accommodate >>>>>>> >>>>>>> 'self' >>>>>>>>> >>>>>>>>> for exactly such cases. >>>>>>>>> I kind of understand your case for the other attributes (these are >>>>>>>>> per port settings that switch asics provide). >>>>>>>>> >>>>>>>>> However, i don't understand the reason to pull in bridge attributes >>>>>>>>> here. >>>>>>>>> >>>>>>>> Maybe, I am missing something so you might help. The learning >>>>>>>> attribute - >>>>>>> >>>>>>> in my case - it is like all other attributes: a port attribute (as >>>>>>> you said, port >>>>>>> settings that the switch provides per port). >>>>>>>> >>>>>>>> So, what I was saying is "why the user shall go through a bridge to >>>>>>>> configure >>>>>>> >>>>>>> the learning attribute"? From my perspective, it is as any other >>>>>>> attribute and >>>>>>> as such configurable on the port. >>>>>>> >>>>>>> Thinking about this some more, i don't see why any of these >>>>>>> attributes >>>>>>> (except loopback. I dont understand the loopback attribute) cant be >>>>>>> part of >>>>>>> the birdge port attributes. >>>>>>> >>>>>>> With this we will end up adding l2 attributes in two places: the >>>>>>> general link >>>>>>> attributes and bridge attributes. >>>>>>> >>>>>>> And since we have gone down the path of using >>>>>>> ndo_bridge_setlink/getlink >>>>>>> with 'self'....we should stick to that for all l2 attributes. >>>>>>> >>>>>>> The idea of overloading ndo_bridge_set/getlink, was to have the same >>>>>>> set of >>>>>>> attributes but support both cases where the user wants to go through >>>>>>> the >>>>>>> bridge driver or directly to the switch port driver. So, you are not >>>>>>> really going >>>>>>> through the bridge driver if you use 'self' and >>>>>>> ndo_bridge_setlink/getlink. >>>>>>> >>>>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch >>>>>> was that your patch and mine were supplementary ("I think Roopa's >>>>>> patches are supplementary. Not all switchdev users will be backed >>>>>> with a Linux Bridge. I therefore welcome your patches very >>>>>> much")... I also understood by others that the patch made sense for >>>>>> the same reason. I simply do not understand why these attributes >>>>>> (and maybe others in the future) could not be configured directly >>>>>> on a standard port but have to go through a bridge. >>>>>> >>>>> ok, i am very confused in that case. The whole moving of bridge >>>>> attributes from the bridge driver to rtnetlink.c was to make the >>>>> bridge attributes accessible to any driver who wants to set l2/bridge >>>>> attributes on their switch ports. So, its unclear to me why we are >>>>> doing this parallel thing again. This move to rtnetlink.c was done >>>>> during the recent rocker support. so, maybe scott/jiri can elaborate >>>>> more. >>>> >>>> >>>> Not sure if this will add to the confusion or help. But you do not >>>> need to have the bridge.ko loaded or netdev's attached to a bridge >>>> to use the setlink/getlink ndo ops and netlink messages. >>>> >>>> This was intentionally done. Its already used with NIC devices to >>>> configure embedded bridge settings such as VEB/VEPA. >>> >>> >>> that helps my case, thanks. >> >> >> So the user interface to set/get the per-port attributes will be via >> 'bridge', not 'ip' >> >> bridge link set dev sw0p1 port_attr bcast_flooding 1 self >> bridge link get dev sw0p1 port_attr bcast_flooding self > > > yes, l2 attributes. >> >> >> We also need an interface to set per-switch attributes. Can this work? >> bridge link set dev sw0 sw_attr bcast_flooding 1 master >> where sw0 is a bridge representing the hardware switch. > > > Not today. We discussed this @ LPC, and one way to do this would be to have > a device > representing the switch asic. This is in the works. Can I assume that on platforms which house more than one asic (say two 24 port asics, interconnected via a 10G link or equivalent, to get a 48 port 'switch') , the 'rocker' driver (or similar) should expose them as a single set of ports, and not as two 'switch ports' ? > > > -- > 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 5:14 ` B Viswanath @ 2014-12-19 8:27 ` Jiri Pirko 2014-12-19 9:01 ` B Viswanath 0 siblings, 1 reply; 33+ messages in thread From: Jiri Pirko @ 2014-12-19 8:27 UTC (permalink / raw) To: B Viswanath Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >>> >>> >>> On 12/18/2014 3:07 PM, Roopa Prabhu wrote: >>>> >>>> On 12/18/14, 11:21 AM, John Fastabend wrote: >>>>> >>>>> On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >>>>>> >>>>>> On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>>>>>> >>>>>>> Removed unnecessary content for ease of reading... >>>>>>> >>>>>>>>>>>>> +/* Switch Port Attributes section */ >>>>>>>>>>>>> + >>>>>>>>>>>>> +enum { >>>>>>>>>>>>> + IFLA_ATTR_UNSPEC, >>>>>>>>>>>>> + IFLA_ATTR_LEARNING, >>>>>>>>>>>> >>>>>>>>>>>> Any reason you want learning here ?. This is covered as part of >>>>>>>>>>>> the bridge setlink attributes. >>>>>>>>>>>> >>>>>>>>>>> Yes, because the user may _not_ want to go through a bridge >>>>>>>>>>> interface >>>>>>>>>> >>>>>>>>>> necessarily. >>>>>>>>>> But, the bridge setlink/getlink interface was changed to >>>>>>>>>> accommodate >>>>>>>> >>>>>>>> 'self' >>>>>>>>>> >>>>>>>>>> for exactly such cases. >>>>>>>>>> I kind of understand your case for the other attributes (these are >>>>>>>>>> per port settings that switch asics provide). >>>>>>>>>> >>>>>>>>>> However, i don't understand the reason to pull in bridge attributes >>>>>>>>>> here. >>>>>>>>>> >>>>>>>>> Maybe, I am missing something so you might help. The learning >>>>>>>>> attribute - >>>>>>>> >>>>>>>> in my case - it is like all other attributes: a port attribute (as >>>>>>>> you said, port >>>>>>>> settings that the switch provides per port). >>>>>>>>> >>>>>>>>> So, what I was saying is "why the user shall go through a bridge to >>>>>>>>> configure >>>>>>>> >>>>>>>> the learning attribute"? From my perspective, it is as any other >>>>>>>> attribute and >>>>>>>> as such configurable on the port. >>>>>>>> >>>>>>>> Thinking about this some more, i don't see why any of these >>>>>>>> attributes >>>>>>>> (except loopback. I dont understand the loopback attribute) cant be >>>>>>>> part of >>>>>>>> the birdge port attributes. >>>>>>>> >>>>>>>> With this we will end up adding l2 attributes in two places: the >>>>>>>> general link >>>>>>>> attributes and bridge attributes. >>>>>>>> >>>>>>>> And since we have gone down the path of using >>>>>>>> ndo_bridge_setlink/getlink >>>>>>>> with 'self'....we should stick to that for all l2 attributes. >>>>>>>> >>>>>>>> The idea of overloading ndo_bridge_set/getlink, was to have the same >>>>>>>> set of >>>>>>>> attributes but support both cases where the user wants to go through >>>>>>>> the >>>>>>>> bridge driver or directly to the switch port driver. So, you are not >>>>>>>> really going >>>>>>>> through the bridge driver if you use 'self' and >>>>>>>> ndo_bridge_setlink/getlink. >>>>>>>> >>>>>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch >>>>>>> was that your patch and mine were supplementary ("I think Roopa's >>>>>>> patches are supplementary. Not all switchdev users will be backed >>>>>>> with a Linux Bridge. I therefore welcome your patches very >>>>>>> much")... I also understood by others that the patch made sense for >>>>>>> the same reason. I simply do not understand why these attributes >>>>>>> (and maybe others in the future) could not be configured directly >>>>>>> on a standard port but have to go through a bridge. >>>>>>> >>>>>> ok, i am very confused in that case. The whole moving of bridge >>>>>> attributes from the bridge driver to rtnetlink.c was to make the >>>>>> bridge attributes accessible to any driver who wants to set l2/bridge >>>>>> attributes on their switch ports. So, its unclear to me why we are >>>>>> doing this parallel thing again. This move to rtnetlink.c was done >>>>>> during the recent rocker support. so, maybe scott/jiri can elaborate >>>>>> more. >>>>> >>>>> >>>>> Not sure if this will add to the confusion or help. But you do not >>>>> need to have the bridge.ko loaded or netdev's attached to a bridge >>>>> to use the setlink/getlink ndo ops and netlink messages. >>>>> >>>>> This was intentionally done. Its already used with NIC devices to >>>>> configure embedded bridge settings such as VEB/VEPA. >>>> >>>> >>>> that helps my case, thanks. >>> >>> >>> So the user interface to set/get the per-port attributes will be via >>> 'bridge', not 'ip' >>> >>> bridge link set dev sw0p1 port_attr bcast_flooding 1 self >>> bridge link get dev sw0p1 port_attr bcast_flooding self >> >> >> yes, l2 attributes. >>> >>> >>> We also need an interface to set per-switch attributes. Can this work? >>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>> where sw0 is a bridge representing the hardware switch. >> >> >> Not today. We discussed this @ LPC, and one way to do this would be to have >> a device >> representing the switch asic. This is in the works. > > >Can I assume that on platforms which house more than one asic (say >two 24 port asics, interconnected via a 10G link or equivalent, to get >a 48 port 'switch') , the 'rocker' driver (or similar) should expose >them as a single set of ports, and not as two 'switch ports' ? Well that really depends on particular implementation and drivers. If you have 2 pci-e devices, I think you should expose them as 2 entities. For sure, you can have the driver to do the masking for you. I don't believe that is correct though. > >> >> >> -- >> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 8:27 ` Jiri Pirko @ 2014-12-19 9:01 ` B Viswanath 2014-12-19 9:22 ` B Viswanath 2014-12-19 9:23 ` Jiri Pirko 0 siblings, 2 replies; 33+ messages in thread From: B Viswanath @ 2014-12-19 9:01 UTC (permalink / raw) To: Jiri Pirko Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: > Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: <snipped for ease of reading> >>>> >>>> >>>> We also need an interface to set per-switch attributes. Can this work? >>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>> where sw0 is a bridge representing the hardware switch. >>> >>> >>> Not today. We discussed this @ LPC, and one way to do this would be to have >>> a device >>> representing the switch asic. This is in the works. >> >> >>Can I assume that on platforms which house more than one asic (say >>two 24 port asics, interconnected via a 10G link or equivalent, to get >>a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>them as a single set of ports, and not as two 'switch ports' ? > > Well that really depends on particular implementation and drivers. If you > have 2 pci-e devices, I think you should expose them as 2 entities. For > sure, you can have the driver to do the masking for you. I don't believe > that is correct though. > In a platform that houses two asic chips, IMO, the user is still expected to manage the router as a single entity. The configuration being applied on both asic devices need to be matching if not identical, and may not be conflicting. The FDB is to be synchronized so that (offloaded) switching can happen across the asics. Some of this stuff is asic specific anyway. Another example is that of the learning. The (hardware) learning can't be enabled on one asic, while being disabled on another one. The general use cases I have seen are all involving managing the 'router' as a single entity. That the 'router' is implemented with two asics instead of a single asic (with more ports) is to be treated as an implementation detail. This is the usual router management method that exists today. I hope I make sense. So I am trying to figure out what this single entity that will be used from a user perspective. It can be a bridge, but our bridges are more 802.1q bridges. We can use the 'self' mode, but then it means that it should reflect the entire port count, and not just an asic. So I was trying to deduce that in our switchdevice model, the best bet would be to leave the unification to the driver (i.e., to project the multiple physical asics as a single virtual switch device). This allows any 'switch' level configurations to the bridge in 'self' mode. And then we would need to consider stacking. Stacking differs from this multi-asic scenario since there would be multiple CPU involved. Thanks Vissu >> >>> >>> >>> -- >>> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:01 ` B Viswanath @ 2014-12-19 9:22 ` B Viswanath 2014-12-19 9:35 ` Jiri Pirko 2014-12-19 9:23 ` Jiri Pirko 1 sibling, 1 reply; 33+ messages in thread From: B Viswanath @ 2014-12-19 9:22 UTC (permalink / raw) To: Jiri Pirko Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel On 19 December 2014 at 14:31, B Viswanath <marichika4@gmail.com> wrote: > On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: >> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: > <snipped for ease of reading> >>>>> >>>>> >>>>> We also need an interface to set per-switch attributes. Can this work? >>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>>> where sw0 is a bridge representing the hardware switch. >>>> >>>> >>>> Not today. We discussed this @ LPC, and one way to do this would be to have >>>> a device >>>> representing the switch asic. This is in the works. >>> >>> >>>Can I assume that on platforms which house more than one asic (say >>>two 24 port asics, interconnected via a 10G link or equivalent, to get >>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>>them as a single set of ports, and not as two 'switch ports' ? >> >> Well that really depends on particular implementation and drivers. If you >> have 2 pci-e devices, I think you should expose them as 2 entities. For >> sure, you can have the driver to do the masking for you. I don't believe >> that is correct though. >> > > In a platform that houses two asic chips, IMO, the user is still > expected to manage the router as a single entity. The configuration > being applied on both asic devices need to be matching if not > identical, and may not be conflicting. The FDB is to be synchronized > so that (offloaded) switching can happen across the asics. Some of > this stuff is asic specific anyway. Another example is that of the > learning. The (hardware) learning can't be enabled on one asic, while > being disabled on another one. The general use cases I have seen are > all involving managing the 'router' as a single entity. That the > 'router' is implemented with two asics instead of a single asic (with > more ports) is to be treated as an implementation detail. This is the > usual router management method that exists today. > > I hope I make sense. > > So I am trying to figure out what this single entity that will be used > from a user perspective. It can be a bridge, but our bridges are more > 802.1q bridges. We can use the 'self' mode, but then it means that it > should reflect the entire port count, and not just an asic. > > So I was trying to deduce that in our switchdevice model, the best bet > would be to leave the unification to the driver (i.e., to project the > multiple physical asics as a single virtual switch device). This > allows any 'switch' level configurations to the bridge in 'self' mode. > > And then we would need to consider stacking. Stacking differs from > this multi-asic scenario since there would be multiple CPU involved. > > Thanks > Vissu > Another example i can provide is that of mirroring. Imagine user wanted to mirror all traffic from port 1 of asic 1 to port 2 of asic 2. This can be offloaded to hardware. However, user would be able to enter such a command only if he/she can look at a single management entity. Thanks Vissu >>> >>>> >>>> >>>> -- >>>> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:22 ` B Viswanath @ 2014-12-19 9:35 ` Jiri Pirko 0 siblings, 0 replies; 33+ messages in thread From: Jiri Pirko @ 2014-12-19 9:35 UTC (permalink / raw) To: B Viswanath Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel Fri, Dec 19, 2014 at 10:22:24AM CET, marichika4@gmail.com wrote: >On 19 December 2014 at 14:31, B Viswanath <marichika4@gmail.com> wrote: >> On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: >>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >> <snipped for ease of reading> >>>>>> >>>>>> >>>>>> We also need an interface to set per-switch attributes. Can this work? >>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>>>> where sw0 is a bridge representing the hardware switch. >>>>> >>>>> >>>>> Not today. We discussed this @ LPC, and one way to do this would be to have >>>>> a device >>>>> representing the switch asic. This is in the works. >>>> >>>> >>>>Can I assume that on platforms which house more than one asic (say >>>>two 24 port asics, interconnected via a 10G link or equivalent, to get >>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>>>them as a single set of ports, and not as two 'switch ports' ? >>> >>> Well that really depends on particular implementation and drivers. If you >>> have 2 pci-e devices, I think you should expose them as 2 entities. For >>> sure, you can have the driver to do the masking for you. I don't believe >>> that is correct though. >>> >> >> In a platform that houses two asic chips, IMO, the user is still >> expected to manage the router as a single entity. The configuration >> being applied on both asic devices need to be matching if not >> identical, and may not be conflicting. The FDB is to be synchronized >> so that (offloaded) switching can happen across the asics. Some of >> this stuff is asic specific anyway. Another example is that of the >> learning. The (hardware) learning can't be enabled on one asic, while >> being disabled on another one. The general use cases I have seen are >> all involving managing the 'router' as a single entity. That the >> 'router' is implemented with two asics instead of a single asic (with >> more ports) is to be treated as an implementation detail. This is the >> usual router management method that exists today. >> >> I hope I make sense. >> >> So I am trying to figure out what this single entity that will be used >> from a user perspective. It can be a bridge, but our bridges are more >> 802.1q bridges. We can use the 'self' mode, but then it means that it >> should reflect the entire port count, and not just an asic. >> >> So I was trying to deduce that in our switchdevice model, the best bet >> would be to leave the unification to the driver (i.e., to project the >> multiple physical asics as a single virtual switch device). This >> allows any 'switch' level configurations to the bridge in 'self' mode. >> >> And then we would need to consider stacking. Stacking differs from >> this multi-asic scenario since there would be multiple CPU involved. >> >> Thanks >> Vissu >> > >Another example i can provide is that of mirroring. Imagine user >wanted to mirror all traffic from port 1 of asic 1 to port 2 of asic >2. This can be offloaded to hardware. However, user would be able to >enter such a command only if he/she can look at a single management >entity. I understand your use case. I think this could be handled by higher entity. In this sase tome userspace agent which would be aware (by configuration) of all asics and how they are interconnected. Just a thought. Seem much nice than to do custom masking in drivers. > >Thanks >Vissu > >>>> >>>>> >>>>> >>>>> -- >>>>> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:01 ` B Viswanath 2014-12-19 9:22 ` B Viswanath @ 2014-12-19 9:23 ` Jiri Pirko 2014-12-19 9:35 ` B Viswanath 1 sibling, 1 reply; 33+ messages in thread From: Jiri Pirko @ 2014-12-19 9:23 UTC (permalink / raw) To: B Viswanath Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote: >On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: >> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: ><snipped for ease of reading> >>>>> >>>>> >>>>> We also need an interface to set per-switch attributes. Can this work? >>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>>> where sw0 is a bridge representing the hardware switch. >>>> >>>> >>>> Not today. We discussed this @ LPC, and one way to do this would be to have >>>> a device >>>> representing the switch asic. This is in the works. >>> >>> >>>Can I assume that on platforms which house more than one asic (say >>>two 24 port asics, interconnected via a 10G link or equivalent, to get >>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>>them as a single set of ports, and not as two 'switch ports' ? >> >> Well that really depends on particular implementation and drivers. If you >> have 2 pci-e devices, I think you should expose them as 2 entities. For >> sure, you can have the driver to do the masking for you. I don't believe >> that is correct though. >> > >In a platform that houses two asic chips, IMO, the user is still >expected to manage the router as a single entity. The configuration >being applied on both asic devices need to be matching if not >identical, and may not be conflicting. The FDB is to be synchronized >so that (offloaded) switching can happen across the asics. Some of >this stuff is asic specific anyway. Another example is that of the >learning. The (hardware) learning can't be enabled on one asic, while >being disabled on another one. The general use cases I have seen are >all involving managing the 'router' as a single entity. That the >'router' is implemented with two asics instead of a single asic (with >more ports) is to be treated as an implementation detail. This is the >usual router management method that exists today. > >I hope I make sense. > >So I am trying to figure out what this single entity that will be used >from a user perspective. It can be a bridge, but our bridges are more >802.1q bridges. We can use the 'self' mode, but then it means that it >should reflect the entire port count, and not just an asic. > >So I was trying to deduce that in our switchdevice model, the best bet >would be to leave the unification to the driver (i.e., to project the >multiple physical asics as a single virtual switch device). This Is it possible to have the asic as just single one? Or is it possible to connect asics being multiple chips maybe from multiple vendors together? I believe that answer is "yes" in both cases. Making two separate asics to appear as one for user is not correct in my opinion. Driver should not do such masking. It is unclean, unextendable. >allows any 'switch' level configurations to the bridge in 'self' mode. > >And then we would need to consider stacking. Stacking differs from >this multi-asic scenario since there would be multiple CPU involved. > >Thanks >Vissu > >>> >>>> >>>> >>>> -- >>>> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:23 ` Jiri Pirko @ 2014-12-19 9:35 ` B Viswanath 2014-12-19 9:55 ` Jiri Pirko 2014-12-19 14:50 ` Andy Gospodarek 0 siblings, 2 replies; 33+ messages in thread From: B Viswanath @ 2014-12-19 9:35 UTC (permalink / raw) To: Jiri Pirko Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote: > Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote: >>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: >>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >><snipped for ease of reading> >>>>>> >>>>>> >>>>>> We also need an interface to set per-switch attributes. Can this work? >>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>>>> where sw0 is a bridge representing the hardware switch. >>>>> >>>>> >>>>> Not today. We discussed this @ LPC, and one way to do this would be to have >>>>> a device >>>>> representing the switch asic. This is in the works. >>>> >>>> >>>>Can I assume that on platforms which house more than one asic (say >>>>two 24 port asics, interconnected via a 10G link or equivalent, to get >>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>>>them as a single set of ports, and not as two 'switch ports' ? >>> >>> Well that really depends on particular implementation and drivers. If you >>> have 2 pci-e devices, I think you should expose them as 2 entities. For >>> sure, you can have the driver to do the masking for you. I don't believe >>> that is correct though. >>> >> >>In a platform that houses two asic chips, IMO, the user is still >>expected to manage the router as a single entity. The configuration >>being applied on both asic devices need to be matching if not >>identical, and may not be conflicting. The FDB is to be synchronized >>so that (offloaded) switching can happen across the asics. Some of >>this stuff is asic specific anyway. Another example is that of the >>learning. The (hardware) learning can't be enabled on one asic, while >>being disabled on another one. The general use cases I have seen are >>all involving managing the 'router' as a single entity. That the >>'router' is implemented with two asics instead of a single asic (with >>more ports) is to be treated as an implementation detail. This is the >>usual router management method that exists today. >> >>I hope I make sense. >> >>So I am trying to figure out what this single entity that will be used >>from a user perspective. It can be a bridge, but our bridges are more >>802.1q bridges. We can use the 'self' mode, but then it means that it >>should reflect the entire port count, and not just an asic. >> >>So I was trying to deduce that in our switchdevice model, the best bet >>would be to leave the unification to the driver (i.e., to project the >>multiple physical asics as a single virtual switch device). Thist > > Is it possible to have the asic as just single one? Or is it possible to > connect asics being multiple chips maybe from multiple vendors together? I didn't understand the first question. Some times, it is possible to have a single asic replace two, but its a cost factor, and others that are involved. AFAIK, the answer to the second question is a No. Two asics from different vendors may not be connected together. The interconnect tends to be proprietary. > I believe that answer is "yes" in both cases. Making two separate asics > to appear as one for user is not correct in my opinion. Driver should > not do such masking. It is unclean, unextendable. > I am only looking for a single management entity. I am not thinking it needs to be at driver level. I am not sure of any other option apart from creating a 'switchdev' that Roopa was mentioning. > >>allows any 'switch' level configurations to the bridge in 'self' mode. >> >>And then we would need to consider stacking. Stacking differs from >>this multi-asic scenario since there would be multiple CPU involved. >> >>Thanks >>Vissu >> >>>> >>>>> >>>>> >>>>> -- >>>>> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:35 ` B Viswanath @ 2014-12-19 9:55 ` Jiri Pirko 2014-12-19 10:53 ` B Viswanath 2014-12-19 16:22 ` Roopa Prabhu 2014-12-19 14:50 ` Andy Gospodarek 1 sibling, 2 replies; 33+ messages in thread From: Jiri Pirko @ 2014-12-19 9:55 UTC (permalink / raw) To: B Viswanath Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel Fri, Dec 19, 2014 at 10:35:27AM CET, marichika4@gmail.com wrote: >On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote: >> Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote: >>>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: >>>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >>><snipped for ease of reading> >>>>>>> >>>>>>> >>>>>>> We also need an interface to set per-switch attributes. Can this work? >>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>>>>> where sw0 is a bridge representing the hardware switch. >>>>>> >>>>>> >>>>>> Not today. We discussed this @ LPC, and one way to do this would be to have >>>>>> a device >>>>>> representing the switch asic. This is in the works. >>>>> >>>>> >>>>>Can I assume that on platforms which house more than one asic (say >>>>>two 24 port asics, interconnected via a 10G link or equivalent, to get >>>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>>>>them as a single set of ports, and not as two 'switch ports' ? >>>> >>>> Well that really depends on particular implementation and drivers. If you >>>> have 2 pci-e devices, I think you should expose them as 2 entities. For >>>> sure, you can have the driver to do the masking for you. I don't believe >>>> that is correct though. >>>> >>> >>>In a platform that houses two asic chips, IMO, the user is still >>>expected to manage the router as a single entity. The configuration >>>being applied on both asic devices need to be matching if not >>>identical, and may not be conflicting. The FDB is to be synchronized >>>so that (offloaded) switching can happen across the asics. Some of >>>this stuff is asic specific anyway. Another example is that of the >>>learning. The (hardware) learning can't be enabled on one asic, while >>>being disabled on another one. The general use cases I have seen are >>>all involving managing the 'router' as a single entity. That the >>>'router' is implemented with two asics instead of a single asic (with >>>more ports) is to be treated as an implementation detail. This is the >>>usual router management method that exists today. >>> >>>I hope I make sense. >>> >>>So I am trying to figure out what this single entity that will be used >>>from a user perspective. It can be a bridge, but our bridges are more >>>802.1q bridges. We can use the 'self' mode, but then it means that it >>>should reflect the entire port count, and not just an asic. >>> >>>So I was trying to deduce that in our switchdevice model, the best bet >>>would be to leave the unification to the driver (i.e., to project the >>>multiple physical asics as a single virtual switch device). Thist >> >> Is it possible to have the asic as just single one? Or is it possible to >> connect asics being multiple chips maybe from multiple vendors together? > >I didn't understand the first question. Some times, it is possible to I ment that there is a design with just a single asic of this type, instead of a pair. >have a single asic replace two, but its a cost factor, and others that >are involved. > >AFAIK, the answer to the second question is a No. Two asics from >different vendors may not be connected together. The interconnect >tends to be proprietary. Okay. In that case, it might make sense to mask it on driver level. > >> I believe that answer is "yes" in both cases. Making two separate asics >> to appear as one for user is not correct in my opinion. Driver should >> not do such masking. It is unclean, unextendable. >> > >I am only looking for a single management entity. I am not thinking it >needs to be at driver level. I am not sure of any other option apart >from creating a 'switchdev' that Roopa was mentioning. Well the thing is there is a common desire to make the offloading as transparent as possible. For example, have 4 ports of same switch and put them into br0. Just like that, without need to do anything else than you would do when bridging ordinary NICs. Introducing some "management entity" would break this approach. > >> >>>allows any 'switch' level configurations to the bridge in 'self' mode. >>> >>>And then we would need to consider stacking. Stacking differs from >>>this multi-asic scenario since there would be multiple CPU involved. >>> >>>Thanks >>>Vissu >>> >>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:55 ` Jiri Pirko @ 2014-12-19 10:53 ` B Viswanath 2014-12-19 16:22 ` Roopa Prabhu 1 sibling, 0 replies; 33+ messages in thread From: B Viswanath @ 2014-12-19 10:53 UTC (permalink / raw) To: Jiri Pirko Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel On 19 December 2014 at 15:25, Jiri Pirko <jiri@resnulli.us> wrote: > Fri, Dec 19, 2014 at 10:35:27AM CET, marichika4@gmail.com wrote: >>On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote: >>> Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote: >>>>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: >>>>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>>>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>>>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >>>><snipped for ease of reading> >>>>>>>> >>>>>>>> >>>>>>>> We also need an interface to set per-switch attributes. Can this work? >>>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>>>>>> where sw0 is a bridge representing the hardware switch. >>>>>>> >>>>>>> >>>>>>> Not today. We discussed this @ LPC, and one way to do this would be to have >>>>>>> a device >>>>>>> representing the switch asic. This is in the works. >>>>>> >>>>>> >>>>>>Can I assume that on platforms which house more than one asic (say >>>>>>two 24 port asics, interconnected via a 10G link or equivalent, to get >>>>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>>>>>them as a single set of ports, and not as two 'switch ports' ? >>>>> >>>>> Well that really depends on particular implementation and drivers. If you >>>>> have 2 pci-e devices, I think you should expose them as 2 entities. For >>>>> sure, you can have the driver to do the masking for you. I don't believe >>>>> that is correct though. >>>>> >>>> >>>>In a platform that houses two asic chips, IMO, the user is still >>>>expected to manage the router as a single entity. The configuration >>>>being applied on both asic devices need to be matching if not >>>>identical, and may not be conflicting. The FDB is to be synchronized >>>>so that (offloaded) switching can happen across the asics. Some of >>>>this stuff is asic specific anyway. Another example is that of the >>>>learning. The (hardware) learning can't be enabled on one asic, while >>>>being disabled on another one. The general use cases I have seen are >>>>all involving managing the 'router' as a single entity. That the >>>>'router' is implemented with two asics instead of a single asic (with >>>>more ports) is to be treated as an implementation detail. This is the >>>>usual router management method that exists today. >>>> >>>>I hope I make sense. >>>> >>>>So I am trying to figure out what this single entity that will be used >>>>from a user perspective. It can be a bridge, but our bridges are more >>>>802.1q bridges. We can use the 'self' mode, but then it means that it >>>>should reflect the entire port count, and not just an asic. >>>> >>>>So I was trying to deduce that in our switchdevice model, the best bet >>>>would be to leave the unification to the driver (i.e., to project the >>>>multiple physical asics as a single virtual switch device). Thist >>> >>> Is it possible to have the asic as just single one? Or is it possible to >>> connect asics being multiple chips maybe from multiple vendors together? >> >>I didn't understand the first question. Some times, it is possible to > > I ment that there is a design with just a single asic of this type, > instead of a pair. > >>have a single asic replace two, but its a cost factor, and others that >>are involved. >> >>AFAIK, the answer to the second question is a No. Two asics from >>different vendors may not be connected together. The interconnect >>tends to be proprietary. > > Okay. In that case, it might make sense to mask it on driver level. > > >> >>> I believe that answer is "yes" in both cases. Making two separate asics >>> to appear as one for user is not correct in my opinion. Driver should >>> not do such masking. It is unclean, unextendable. >>> >> >>I am only looking for a single management entity. I am not thinking it >>needs to be at driver level. I am not sure of any other option apart >>from creating a 'switchdev' that Roopa was mentioning. > > Well the thing is there is a common desire to make the offloading as > transparent as possible. For example, have 4 ports of same switch and > put them into br0. Just like that, without need to do anything else > than you would do when bridging ordinary NICs. Introducing some > "management entity" would break this approach. > This is a simple and solid approach that works fine as long as the asics can be viewed as collection of ports. This is true for many usecases. However, the asics are more than a collection of ports, having shared and common infrastructure among the ports. They offer configuration options that are not specific to any port, but 'belonging' to the asic. So there may be a need in some usecases which involve in setting up these config options, to somehow identify that these belong to the asic itself. If there is a single asic, we can have implied identification, but not with multiple asics. I guess if we can manage multiple asics belonging to same vendor within the driver, we probably don't need any such identification (and that was my original question). We already have the 'self' bridge which can be enhanced for any properties. However, at this point I am not sure how the routing plays our with the 'self' bridge. IMHO, it would be neat to have a 'switchdev' which can cleanly handle such stuff. >> >>> >>>>allows any 'switch' level configurations to the bridge in 'self' mode. >>>> >>>>And then we would need to consider stacking. Stacking differs from >>>>this multi-asic scenario since there would be multiple CPU involved. >>>> >>>>Thanks >>>>Vissu >>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:55 ` Jiri Pirko 2014-12-19 10:53 ` B Viswanath @ 2014-12-19 16:22 ` Roopa Prabhu 2014-12-20 0:57 ` Williams, Kenneth 1 sibling, 1 reply; 33+ messages in thread From: Roopa Prabhu @ 2014-12-19 16:22 UTC (permalink / raw) To: Jiri Pirko Cc: B Viswanath, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel On 12/19/14, 1:55 AM, Jiri Pirko wrote: > Fri, Dec 19, 2014 at 10:35:27AM CET, marichika4@gmail.com wrote: >> On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote: >>> Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote: >>>> On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: >>>>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: >>>>>> On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >>>>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >>>> <snipped for ease of reading> >>>>>>>> >>>>>>>> We also need an interface to set per-switch attributes. Can this work? >>>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>>>>>>> where sw0 is a bridge representing the hardware switch. >>>>>>> >>>>>>> Not today. We discussed this @ LPC, and one way to do this would be to have >>>>>>> a device >>>>>>> representing the switch asic. This is in the works. >>>>>> >>>>>> Can I assume that on platforms which house more than one asic (say >>>>>> two 24 port asics, interconnected via a 10G link or equivalent, to get >>>>>> a 48 port 'switch') , the 'rocker' driver (or similar) should expose >>>>>> them as a single set of ports, and not as two 'switch ports' ? >>>>> Well that really depends on particular implementation and drivers. If you >>>>> have 2 pci-e devices, I think you should expose them as 2 entities. For >>>>> sure, you can have the driver to do the masking for you. I don't believe >>>>> that is correct though. >>>>> >>>> In a platform that houses two asic chips, IMO, the user is still >>>> expected to manage the router as a single entity. The configuration >>>> being applied on both asic devices need to be matching if not >>>> identical, and may not be conflicting. The FDB is to be synchronized >>>> so that (offloaded) switching can happen across the asics. Some of >>>> this stuff is asic specific anyway. Another example is that of the >>>> learning. The (hardware) learning can't be enabled on one asic, while >>>> being disabled on another one. The general use cases I have seen are >>>> all involving managing the 'router' as a single entity. That the >>>> 'router' is implemented with two asics instead of a single asic (with >>>> more ports) is to be treated as an implementation detail. This is the >>>> usual router management method that exists today. >>>> >>>> I hope I make sense. >>>> >>>> So I am trying to figure out what this single entity that will be used >>> >from a user perspective. It can be a bridge, but our bridges are more >>>> 802.1q bridges. We can use the 'self' mode, but then it means that it >>>> should reflect the entire port count, and not just an asic. >>>> >>>> So I was trying to deduce that in our switchdevice model, the best bet >>>> would be to leave the unification to the driver (i.e., to project the >>>> multiple physical asics as a single virtual switch device). Thist >>> Is it possible to have the asic as just single one? Or is it possible to >>> connect asics being multiple chips maybe from multiple vendors together? >> I didn't understand the first question. Some times, it is possible to > I ment that there is a design with just a single asic of this type, > instead of a pair. > >> have a single asic replace two, but its a cost factor, and others that >> are involved. >> >> AFAIK, the answer to the second question is a No. Two asics from >> different vendors may not be connected together. The interconnect >> tends to be proprietary. > Okay. In that case, it might make sense to mask it on driver level. > > >>> I believe that answer is "yes" in both cases. Making two separate asics >>> to appear as one for user is not correct in my opinion. Driver should >>> not do such masking. It is unclean, unextendable. >>> >> I am only looking for a single management entity. I am not thinking it >> needs to be at driver level. I am not sure of any other option apart > >from creating a 'switchdev' that Roopa was mentioning. > > Well the thing is there is a common desire to make the offloading as > transparent as possible. For example, have 4 ports of same switch and > put them into br0. Just like that, without need to do anything else > than you would do when bridging ordinary NICs. Introducing some > "management entity" would break this approach. > I don't think having a switchdevice breaks this approach. A software bridge is not a 1-1 mapping with the asic in all cases. When its a vlan filtering bridge, yes, it is (In which case all switch global l2 non-port specific attributes can be applied to the bridge). The switch asic can do l2 and l3 too. For a bridge, the switch asic is just accelerating l2. And a switch asic is also capable of l3, acls. A switch device (whether accessible to userspace or not) may become necessary (as discussed in other threads) where you cannot resolve a kernel object to a switch port (Global acl rules, unresolved route nexthops etc). ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 16:22 ` Roopa Prabhu @ 2014-12-20 0:57 ` Williams, Kenneth 0 siblings, 0 replies; 33+ messages in thread From: Williams, Kenneth @ 2014-12-20 0:57 UTC (permalink / raw) To: Roopa Prabhu Cc: Jiri Pirko, B Viswanath, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel On Fri, Dec 19, 2014 at 08:22:40AM -0800, Roopa Prabhu wrote: > On 12/19/14, 1:55 AM, Jiri Pirko wrote: > >Fri, Dec 19, 2014 at 10:35:27AM CET, marichika4@gmail.com wrote: > >>On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote: > >>>Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote: > >>>>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: > >>>>>Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: > >>>>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > >>>>>>>On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: > >>>><snipped for ease of reading> > >>>>>>>> > >>>>>>>>We also need an interface to set per-switch attributes. Can this work? > >>>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master > >>>>>>>>where sw0 is a bridge representing the hardware switch. > >>>>>>> > >>>>>>>Not today. We discussed this @ LPC, and one way to do this would be to have > >>>>>>>a device > >>>>>>>representing the switch asic. This is in the works. > >>>>>> > >>>>>>Can I assume that on platforms which house more than one asic (say > >>>>>>two 24 port asics, interconnected via a 10G link or equivalent, to get > >>>>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose > >>>>>>them as a single set of ports, and not as two 'switch ports' ? > >>>>>Well that really depends on particular implementation and drivers. If you > >>>>>have 2 pci-e devices, I think you should expose them as 2 entities. For > >>>>>sure, you can have the driver to do the masking for you. I don't believe > >>>>>that is correct though. > >>>>> > >>>>In a platform that houses two asic chips, IMO, the user is still > >>>>expected to manage the router as a single entity. The configuration > >>>>being applied on both asic devices need to be matching if not > >>>>identical, and may not be conflicting. The FDB is to be synchronized > >>>>so that (offloaded) switching can happen across the asics. Some of > >>>>this stuff is asic specific anyway. Another example is that of the > >>>>learning. The (hardware) learning can't be enabled on one asic, while > >>>>being disabled on another one. The general use cases I have seen are > >>>>all involving managing the 'router' as a single entity. That the > >>>>'router' is implemented with two asics instead of a single asic (with > >>>>more ports) is to be treated as an implementation detail. This is the > >>>>usual router management method that exists today. > >>>> > >>>>I hope I make sense. > >>>> > >>>>So I am trying to figure out what this single entity that will be used > >>>>from a user perspective. It can be a bridge, but our bridges are more > >>>>802.1q bridges. We can use the 'self' mode, but then it means that it > >>>>should reflect the entire port count, and not just an asic. > >>>> > >>>>So I was trying to deduce that in our switchdevice model, the best bet > >>>>would be to leave the unification to the driver (i.e., to project the > >>>>multiple physical asics as a single virtual switch device). Thist > >>>Is it possible to have the asic as just single one? Or is it possible to > >>>connect asics being multiple chips maybe from multiple vendors together? > >>I didn't understand the first question. Some times, it is possible to > >I ment that there is a design with just a single asic of this type, > >instead of a pair. > > > >>have a single asic replace two, but its a cost factor, and others that > >>are involved. > >> > >>AFAIK, the answer to the second question is a No. Two asics from > >>different vendors may not be connected together. The interconnect > >>tends to be proprietary. > >Okay. In that case, it might make sense to mask it on driver level. > > > > > >>>I believe that answer is "yes" in both cases. Making two separate asics > >>>to appear as one for user is not correct in my opinion. Driver should > >>>not do such masking. It is unclean, unextendable. > >>> > >>I am only looking for a single management entity. I am not thinking it > >>needs to be at driver level. I am not sure of any other option apart > >>from creating a 'switchdev' that Roopa was mentioning. > > > >Well the thing is there is a common desire to make the offloading as > >transparent as possible. For example, have 4 ports of same switch and > >put them into br0. Just like that, without need to do anything else > >than you would do when bridging ordinary NICs. Introducing some > >"management entity" would break this approach. > > > I don't think having a switchdevice breaks this approach. A software bridge > is not a 1-1 mapping with the asic in all cases. > When its a vlan filtering bridge, yes, it is (In which case all switch > global l2 non-port specific attributes can be applied to the bridge). > > The switch asic can do l2 and l3 too. For a bridge, the switch asic is just > accelerating l2. > And a switch asic is also capable of l3, acls. A switch device (whether > accessible to userspace or not) > may become necessary (as discussed in other threads) where you cannot > resolve a kernel object to a switch port (Global acl rules, unresolved route > nexthops etc). A switch-chip vendor that provides a proprietary mechanism of bonding two or more switch-chips into a single functional unit, also typically provides an API that allows operating on this bonded set of switch chips to be addressed as a single unit. If my understanding is correct, the question of port uniqueness, etc becomes moot. > > -- > 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-19 9:35 ` B Viswanath 2014-12-19 9:55 ` Jiri Pirko @ 2014-12-19 14:50 ` Andy Gospodarek 1 sibling, 0 replies; 33+ messages in thread From: Andy Gospodarek @ 2014-12-19 14:50 UTC (permalink / raw) To: B Viswanath Cc: Jiri Pirko, Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel On Fri, Dec 19, 2014 at 03:05:27PM +0530, B Viswanath wrote: > On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote: > > Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote: > >>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote: > >>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote: > >>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > >>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: > >><snipped for ease of reading> > >>>>>> > >>>>>> > >>>>>> We also need an interface to set per-switch attributes. Can this work? > >>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master > >>>>>> where sw0 is a bridge representing the hardware switch. > >>>>> > >>>>> > >>>>> Not today. We discussed this @ LPC, and one way to do this would be to have > >>>>> a device > >>>>> representing the switch asic. This is in the works. > >>>> > >>>> > >>>>Can I assume that on platforms which house more than one asic (say > >>>>two 24 port asics, interconnected via a 10G link or equivalent, to get > >>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose > >>>>them as a single set of ports, and not as two 'switch ports' ? > >>> > >>> Well that really depends on particular implementation and drivers. If you > >>> have 2 pci-e devices, I think you should expose them as 2 entities. For > >>> sure, you can have the driver to do the masking for you. I don't believe > >>> that is correct though. > >>> > >> > >>In a platform that houses two asic chips, IMO, the user is still > >>expected to manage the router as a single entity. The configuration > >>being applied on both asic devices need to be matching if not > >>identical, and may not be conflicting. The FDB is to be synchronized > >>so that (offloaded) switching can happen across the asics. Some of > >>this stuff is asic specific anyway. Another example is that of the > >>learning. The (hardware) learning can't be enabled on one asic, while > >>being disabled on another one. The general use cases I have seen are > >>all involving managing the 'router' as a single entity. That the > >>'router' is implemented with two asics instead of a single asic (with > >>more ports) is to be treated as an implementation detail. This is the > >>usual router management method that exists today. > >> > >>I hope I make sense. > >> > >>So I am trying to figure out what this single entity that will be used > >>from a user perspective. It can be a bridge, but our bridges are more > >>802.1q bridges. We can use the 'self' mode, but then it means that it > >>should reflect the entire port count, and not just an asic. > >> > >>So I was trying to deduce that in our switchdevice model, the best bet > >>would be to leave the unification to the driver (i.e., to project the > >>multiple physical asics as a single virtual switch device). Thist > > > > Is it possible to have the asic as just single one? Or is it possible to > > connect asics being multiple chips maybe from multiple vendors together? > > I didn't understand the first question. Some times, it is possible to > have a single asic replace two, but its a cost factor, and others that > are involved. > > AFAIK, the answer to the second question is a No. Two asics from > different vendors may not be connected together. The interconnect > tends to be proprietary. > > > I believe that answer is "yes" in both cases. Making two separate asics > > to appear as one for user is not correct in my opinion. Driver should > > not do such masking. It is unclean, unextendable. > > > > I am only looking for a single management entity. I am not thinking it > needs to be at driver level. I am not sure of any other option apart > from creating a 'switchdev' that Roopa was mentioning. This is certainly one of the possible use-cases of creating top-level switching/routing/offload dev to control all of the ASICs, but not the primary use of such a device at this point. Earlier in the thread you asked how one might handle the multi-ASIC configuration. I agree with the opinion Jiri offered (that was the concensus we reached when discussing this in person in Oct) that there were not plans to provide full support for a multi-chip configuration where the kernel tracks links between chips and keeps routing tables, FDB tables, et al up to date automatically to everything works happily without any userspace intervention. This would be a nice addition at some point. At some point there might be a need to provide something like a device-tree file or configuration on some platforms to provide a mapping between ports on an ASIC/offload device as they are referenced in the hardware, front-panel ports on a specific platform, and netdevs. Something like this could also provide a way to allow the kernel to configure the in-kernel FDB/neighbor/FIB code to write the appropriate static entries to an internal interconnect port on a specific platform. There are also some interesting use-cases for a feature like this in a single-chip configuration as well, but an in-kernel solution to that problem has not been explored at this point. > >>allows any 'switch' level configurations to the bridge in 'self' mode. > >> > >>And then we would need to consider stacking. Stacking differs from > >>this multi-asic scenario since there would be multiple CPU involved. This would also be grouped into that same TODO category, but based on the fact that these stacking protocols/frame formats appear to be vendor-specific it would seem that this is an exercise best left to userspace. > >> > >>Thanks > >>Vissu > >> > >>>> > >>>>> > >>>>> > >>>>> -- > >>>>> 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 > -- > 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 23:48 ` Roopa Prabhu 2014-12-19 5:14 ` B Viswanath @ 2014-12-19 8:25 ` Jiri Pirko 1 sibling, 0 replies; 33+ messages in thread From: Jiri Pirko @ 2014-12-19 8:25 UTC (permalink / raw) To: Roopa Prabhu Cc: Samudrala, Sridhar, John Fastabend, Varlese, Marco, netdev, Thomas Graf, sfeldma, linux-kernel Fri, Dec 19, 2014 at 12:48:28AM CET, roopa@cumulusnetworks.com wrote: >On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote: >> >>On 12/18/2014 3:07 PM, Roopa Prabhu wrote: >>>On 12/18/14, 11:21 AM, John Fastabend wrote: >>>>On 12/18/2014 10:14 AM, Roopa Prabhu wrote: >>>>>On 12/18/14, 10:02 AM, Varlese, Marco wrote: >>>>>>Removed unnecessary content for ease of reading... >>>>>> >>>>>>>>>>>>+/* Switch Port Attributes section */ >>>>>>>>>>>>+ >>>>>>>>>>>>+enum { >>>>>>>>>>>>+ IFLA_ATTR_UNSPEC, >>>>>>>>>>>>+ IFLA_ATTR_LEARNING, >>>>>>>>>>>Any reason you want learning here ?. This is covered as part of >>>>>>>>>>>the bridge setlink attributes. >>>>>>>>>>> >>>>>>>>>>Yes, because the user may _not_ want to go through a bridge >>>>>>>>>>interface >>>>>>>>>necessarily. >>>>>>>>>But, the bridge setlink/getlink interface was changed to >>>>>>>>>accommodate >>>>>>>'self' >>>>>>>>>for exactly such cases. >>>>>>>>>I kind of understand your case for the other attributes (these are >>>>>>>>>per port settings that switch asics provide). >>>>>>>>> >>>>>>>>>However, i don't understand the reason to pull in bridge >>>>>>>>>attributes here. >>>>>>>>> >>>>>>>>Maybe, I am missing something so you might help. The learning >>>>>>>>attribute - >>>>>>>in my case - it is like all other attributes: a port attribute >>>>>>>(as you said, port >>>>>>>settings that the switch provides per port). >>>>>>>>So, what I was saying is "why the user shall go through a >>>>>>>>bridge to configure >>>>>>>the learning attribute"? From my perspective, it is as any other >>>>>>>attribute and >>>>>>>as such configurable on the port. >>>>>>> >>>>>>>Thinking about this some more, i don't see why any of these >>>>>>>attributes >>>>>>>(except loopback. I dont understand the loopback attribute) cant >>>>>>>be part of >>>>>>>the birdge port attributes. >>>>>>> >>>>>>>With this we will end up adding l2 attributes in two places: the >>>>>>>general link >>>>>>>attributes and bridge attributes. >>>>>>> >>>>>>>And since we have gone down the path of using >>>>>>>ndo_bridge_setlink/getlink >>>>>>>with 'self'....we should stick to that for all l2 attributes. >>>>>>> >>>>>>>The idea of overloading ndo_bridge_set/getlink, was to have the >>>>>>>same set of >>>>>>>attributes but support both cases where the user wants to go >>>>>>>through the >>>>>>>bridge driver or directly to the switch port driver. So, you are >>>>>>>not really going >>>>>>>through the bridge driver if you use 'self' and >>>>>>>ndo_bridge_setlink/getlink. >>>>>>> >>>>>>Roopa, one of the comments I got from Thomas Graf on my v1 patch >>>>>>was that your patch and mine were supplementary ("I think Roopa's >>>>>>patches are supplementary. Not all switchdev users will be backed >>>>>>with a Linux Bridge. I therefore welcome your patches very >>>>>>much")... I also understood by others that the patch made sense for >>>>>>the same reason. I simply do not understand why these attributes >>>>>>(and maybe others in the future) could not be configured directly >>>>>>on a standard port but have to go through a bridge. >>>>>> >>>>>ok, i am very confused in that case. The whole moving of bridge >>>>>attributes from the bridge driver to rtnetlink.c was to make the >>>>>bridge attributes accessible to any driver who wants to set l2/bridge >>>>>attributes on their switch ports. So, its unclear to me why we are >>>>>doing this parallel thing again. This move to rtnetlink.c was done >>>>>during the recent rocker support. so, maybe scott/jiri can elaborate >>>>>more. >>>> >>>>Not sure if this will add to the confusion or help. But you do not >>>>need to have the bridge.ko loaded or netdev's attached to a bridge >>>>to use the setlink/getlink ndo ops and netlink messages. >>>> >>>>This was intentionally done. Its already used with NIC devices to >>>>configure embedded bridge settings such as VEB/VEPA. >>> >>>that helps my case, thanks. >> >>So the user interface to set/get the per-port attributes will be via >>'bridge', not 'ip' >> >> bridge link set dev sw0p1 port_attr bcast_flooding 1 self >> bridge link get dev sw0p1 port_attr bcast_flooding self > >yes, l2 attributes. >> >>We also need an interface to set per-switch attributes. Can this work? >> bridge link set dev sw0 sw_attr bcast_flooding 1 master >>where sw0 is a bridge representing the hardware switch. How can a bridge represent the hardware switch? What if you have 2 bridge on ports of the same hardware switch. Or what if you have a bridge with couple of ports from one hardware switch and couple of ports from second hardware switch. Or else, you can have a misxture of switch ports and other devices (taps for example). Bridge->hardware switch does not map 1:1. If you want to set any per switch attributes, just use one of the ports as a "gateway". port->hardware switch mapping is strict, so you should be okay doing that. There are no ndos for this purpose at the moment, but they can be easily added. > >Not today. We discussed this @ LPC, and one way to do this would be to have a >device >representing the switch asic. This is in the works. > Yep, I still do not find a need to introduce some new device class. I think it would be just confusing. Example. br0 --- sw0p0 --- sw0p1 --- sw0p2 br1 --- sw0p3 --- sw0p4 --- sw0p5 and in paralel, without any connection you would have some mysterious device "sw0" which would do something, not clear why. Btw this device would not be netdev, so it could not be used for anything netdevs can be currently used for. Yes, I still might be missing something, for sure. Nobody told me what yet. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 18:02 ` Varlese, Marco 2014-12-18 18:14 ` Roopa Prabhu @ 2014-12-19 0:45 ` Thomas Graf 1 sibling, 0 replies; 33+ messages in thread From: Thomas Graf @ 2014-12-19 0:45 UTC (permalink / raw) To: Varlese, Marco Cc: Roopa Prabhu, netdev, Fastabend, John R, Jiri Pirko, sfeldma, linux-kernel On 12/18/14 at 06:02pm, Varlese, Marco wrote: > Roopa, one of the comments I got from Thomas Graf on my v1 patch was that your patch and mine were supplementary ("I think Roopa's patches are supplementary. Not all switchdev users will be backed with a Linux Bridge. I therefore welcome your patches very much")... I also understood by others that the patch made sense for the same reason. I simply do not understand why these attributes (and maybe others in the future) could not be configured directly on a standard port but have to go through a bridge. Apologies for this confusion. I take that back. I was under the impression netdev_switch_port_bridge_setlink(), based on the "bridge" in the function name, would derive the attributes based on capabilities and assumption of the bridge module. This is not the case in the latest patchset proposed so it could indeed serve as the Netlink API for both the bridge and non bridge case. ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 14:55 ` Varlese, Marco 2014-12-18 15:16 ` Roopa Prabhu @ 2014-12-18 15:47 ` Arad, Ronen 2014-12-18 16:14 ` John Fastabend 1 sibling, 1 reply; 33+ messages in thread From: Arad, Ronen @ 2014-12-18 15:47 UTC (permalink / raw) To: Varlese, Marco, Roopa Prabhu, netdev Cc: Fastabend, John R, Thomas Graf, Jiri Pirko, sfeldma, linux-kernel > -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev- > owner@vger.kernel.org] On Behalf Of Varlese, Marco > Sent: Thursday, December 18, 2014 4:56 PM > To: Roopa Prabhu > Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri Pirko; > sfeldma@gmail.com; linux-kernel@vger.kernel.org > Subject: RE: [RFC PATCH net-next v2 1/1] net: Support for switch port > configuration > > > -----Original Message----- > > From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com] > > Sent: Thursday, December 18, 2014 2:45 PM > > To: Varlese, Marco > > Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri > > Pirko; sfeldma@gmail.com; linux-kernel@vger.kernel.org > > Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port > > configuration > > > > On 12/18/14, 3:29 AM, Varlese, Marco wrote: > > > From: Marco Varlese <marco.varlese@intel.com> > > > > > > Switch hardware offers a list of attributes that are configurable on > > > a per port basis. > > > This patch provides a mechanism to configure switch ports by adding > > > an NDO for setting specific values to specific attributes. > > > There will be a separate patch that adds the "get" functionality via > > > another NDO and another patch that extends iproute2 to call the two > > > new NDOs. > > > > > > Signed-off-by: Marco Varlese <marco.varlese@intel.com> > > > --- > > > include/linux/netdevice.h | 5 ++++ > > > include/uapi/linux/if_link.h | 15 ++++++++++++ > > > net/core/rtnetlink.c | 54 > > ++++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 74 insertions(+) > > > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > > index c31f74d..4881c7b 100644 > > > --- a/include/linux/netdevice.h > > > +++ b/include/linux/netdevice.h > > > @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct > > net_device *dev, > > > * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state); > > > * Called to notify switch device port of bridge port STP > > > * state change. > > > + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, > > > + * u32 attr, u64 value); > > > + * Called to set specific switch ports attributes. > > > */ > > > struct net_device_ops { > > > int (*ndo_init)(struct net_device *dev); > > > @@ -1185,6 +1188,8 @@ struct net_device_ops { > > > struct > > netdev_phys_item_id *psid); > > > int (*ndo_switch_port_stp_update)(struct > > net_device *dev, > > > u8 state); > > > + int (*ndo_switch_port_set_cfg)(struct > > net_device *dev, > > > + u32 attr, u64 value); > > > #endif > > > }; > > > > > > diff --git a/include/uapi/linux/if_link.h > > > b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 > > > --- a/include/uapi/linux/if_link.h > > > +++ b/include/uapi/linux/if_link.h > > > @@ -146,6 +146,7 @@ enum { > > > IFLA_PHYS_PORT_ID, > > > IFLA_CARRIER_CHANGES, > > > IFLA_PHYS_SWITCH_ID, > > > + IFLA_SWITCH_PORT_CFG, > > > __IFLA_MAX > > > }; > > > > > > @@ -603,4 +604,18 @@ enum { > > > > > > #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > > > > > > +/* Switch Port Attributes section */ > > > + > > > +enum { > > > + IFLA_ATTR_UNSPEC, > > > + IFLA_ATTR_LEARNING, > > Any reason you want learning here ?. This is covered as part of the > > bridge setlink attributes. > > > > Yes, because the user may _not_ want to go through a bridge interface > necessarily. Some bridge attributes or more accurately bridge port attributes overlap with port attributes. IFLA_BRPORT_LEARNING from IFLA_PROTINFO and BRIDGE_VLAN_INFO_PVID flag of IFLA_BRIDGE_VLAN_INFO from IFLA_AF_SPEC are two examples where bridge port properties might have to be mapped to a common or driver specific port attribute. Switch port driver that works without and explicit bridge device has to implement the bridge NDO's ndo_bridge_setlink/ndo_bridge_dellink. It could take care of mapping such attributes from the bridge netlink message to either driver-specific port attribute of to an explicit one (assuming IFLA_ATTR_LEARNING would be accepted). For example the driver could process the bridge learning information from the protinfo as such: int sw_driver_br_setlink(struct net_device *dev, struct nlmsghdr *nlh) { struct nlattr *protinfo; protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_PROTINFO); if (protinfo) { struct nlattr *attr; attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING); if (attr) { if (nla_len(attr) >= sizeof(u8)) { if (nla_get_u8(attr)) brport_flags |= BR_LEARNING; else brport_flags &= ~BR_LEARNING; } } /* * Map bridge port attributes to driver-specific port attributes */ } return 0; } > > > > + IFLA_ATTR_LOOPBACK, > > > + IFLA_ATTR_BCAST_FLOODING, > > > + IFLA_ATTR_UCAST_FLOODING, > > > + IFLA_ATTR_MCAST_FLOODING, > > > + __IFLA_ATTR_MAX > > > +}; > > > + > > > +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) > > > + > > > #endif /* _UAPI_LINUX_IF_LINK_H */ diff --git > > > a/net/core/rtnetlink.c b/net/core/rtnetlink.c index > > > eaa057f..c0d1c45 100644 > > > --- a/net/core/rtnetlink.c > > > +++ b/net/core/rtnetlink.c > > > @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device > > *dev, struct nlattr *tb[]) > > > return 0; > > > } > > > > > > +#ifdef CONFIG_NET_SWITCHDEV > > > +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { > > > + int rem, err = -EINVAL; > > > + struct nlattr *v; > > > + const struct net_device_ops *ops = dev->netdev_ops; > > > + > > > + nla_for_each_nested(v, attr, rem) { > > > + u32 op = nla_type(v); > > > + u64 value = 0; > > > + > > > + switch (op) { > > > + case IFLA_ATTR_LEARNING: > > > + case IFLA_ATTR_LOOPBACK: > > > + case IFLA_ATTR_BCAST_FLOODING: > > > + case IFLA_ATTR_UCAST_FLOODING: > > > + case IFLA_ATTR_MCAST_FLOODING: { > > > + if (nla_len(v) < sizeof(value)) { > > > + err = -EINVAL; > > > + break; > > > + } > > > + > > > + value = nla_get_u64(v); > > > + err = ops->ndo_switch_port_set_cfg(dev, > > > + op, > > > + value); > > > + break; > > > + } > > > + default: > > > + err = -EINVAL; > > > + break; > > > + } > > > + if (err) > > > + break; > > > + } > > > + return err; > > > +} > > > + > > > +#endif > > > + > > > static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > > > { > > > int rem, err = -EINVAL; > > > @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, > > > status |= DO_SETLINK_NOTIFY; > > > } > > > } > > > +#ifdef CONFIG_NET_SWITCHDEV > > > + if (tb[IFLA_SWITCH_PORT_CFG]) { > > > + err = -EOPNOTSUPP; > > > + if (!ops->ndo_switch_port_set_cfg) > > > + goto errout; > > > + if (!ops->ndo_switch_parent_id_get) > > > + goto errout; > > > + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > > > + if (err < 0) > > > + goto errout; > > > + > > > + status |= DO_SETLINK_NOTIFY; > > > + } > > > +#endif > > > err = 0; > > > > > > errout: > > -- > 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 15:47 ` Arad, Ronen @ 2014-12-18 16:14 ` John Fastabend 2014-12-18 17:17 ` Arad, Ronen 0 siblings, 1 reply; 33+ messages in thread From: John Fastabend @ 2014-12-18 16:14 UTC (permalink / raw) To: Arad, Ronen, Varlese, Marco, Roopa Prabhu, netdev Cc: Thomas Graf, Jiri Pirko, sfeldma, linux-kernel On 12/18/2014 07:47 AM, Arad, Ronen wrote: > > >> -----Original Message----- >> From: netdev-owner@vger.kernel.org [mailto:netdev- >> owner@vger.kernel.org] On Behalf Of Varlese, Marco >> Sent: Thursday, December 18, 2014 4:56 PM >> To: Roopa Prabhu >> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri Pirko; >> sfeldma@gmail.com; linux-kernel@vger.kernel.org >> Subject: RE: [RFC PATCH net-next v2 1/1] net: Support for switch port >> configuration >> >>> -----Original Message----- >>> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com] >>> Sent: Thursday, December 18, 2014 2:45 PM >>> To: Varlese, Marco >>> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri >>> Pirko; sfeldma@gmail.com; linux-kernel@vger.kernel.org >>> Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port >>> configuration >>> >>> On 12/18/14, 3:29 AM, Varlese, Marco wrote: >>>> From: Marco Varlese <marco.varlese@intel.com> >>>> >>>> Switch hardware offers a list of attributes that are configurable on >>>> a per port basis. >>>> This patch provides a mechanism to configure switch ports by adding >>>> an NDO for setting specific values to specific attributes. >>>> There will be a separate patch that adds the "get" functionality via >>>> another NDO and another patch that extends iproute2 to call the two >>>> new NDOs. >>>> >>>> Signed-off-by: Marco Varlese <marco.varlese@intel.com> >>>> --- >>>> include/linux/netdevice.h | 5 ++++ >>>> include/uapi/linux/if_link.h | 15 ++++++++++++ >>>> net/core/rtnetlink.c | 54 >>> ++++++++++++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 74 insertions(+) >>>> >>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >>>> index c31f74d..4881c7b 100644 >>>> --- a/include/linux/netdevice.h >>>> +++ b/include/linux/netdevice.h >>>> @@ -1027,6 +1027,9 @@ typedef u16 (*select_queue_fallback_t)(struct >>> net_device *dev, >>>> * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state); >>>> * Called to notify switch device port of bridge port STP >>>> * state change. >>>> + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, >>>> + * u32 attr, u64 value); >>>> + * Called to set specific switch ports attributes. >>>> */ >>>> struct net_device_ops { >>>> int (*ndo_init)(struct net_device *dev); >>>> @@ -1185,6 +1188,8 @@ struct net_device_ops { >>>> struct >>> netdev_phys_item_id *psid); >>>> int (*ndo_switch_port_stp_update)(struct >>> net_device *dev, >>>> u8 state); >>>> + int (*ndo_switch_port_set_cfg)(struct >>> net_device *dev, >>>> + u32 attr, u64 value); >>>> #endif >>>> }; >>>> >>>> diff --git a/include/uapi/linux/if_link.h >>>> b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 >>>> --- a/include/uapi/linux/if_link.h >>>> +++ b/include/uapi/linux/if_link.h >>>> @@ -146,6 +146,7 @@ enum { >>>> IFLA_PHYS_PORT_ID, >>>> IFLA_CARRIER_CHANGES, >>>> IFLA_PHYS_SWITCH_ID, >>>> + IFLA_SWITCH_PORT_CFG, >>>> __IFLA_MAX >>>> }; >>>> >>>> @@ -603,4 +604,18 @@ enum { >>>> >>>> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) >>>> >>>> +/* Switch Port Attributes section */ >>>> + >>>> +enum { >>>> + IFLA_ATTR_UNSPEC, >>>> + IFLA_ATTR_LEARNING, >>> Any reason you want learning here ?. This is covered as part of the >>> bridge setlink attributes. >>> >> >> Yes, because the user may _not_ want to go through a bridge interface >> necessarily. > > Some bridge attributes or more accurately bridge port attributes overlap with port attributes. > IFLA_BRPORT_LEARNING from IFLA_PROTINFO and BRIDGE_VLAN_INFO_PVID > flag of IFLA_BRIDGE_VLAN_INFO from IFLA_AF_SPEC are two examples > where bridge port properties might have to be mapped to a common or > driver specific port attribute. You've lost me a bit. > Switch port driver that works without and explicit bridge device has > to implement the bridge NDO's ndo_bridge_setlink/ndo_bridge_dellink. > It could take care of mapping such attributes from the bridge netlink > message to either driver-specific port attribute of to an explicit > one (assuming IFLA_ATTR_LEARNING would be accepted). For example the > driver could process the bridge learning information from the > protinfo as such: still lost unfortunately. So you want to enable learning on a port by port basis? Then we also need to define how the two bits interact. switch_learning + port_learning = port in learning mode !switch_learning + port_learning = port not in learning mode? etc. Do bridges and users actually enable learning on a per port basis? > int sw_driver_br_setlink(struct net_device *dev, struct nlmsghdr *nlh) > { > struct nlattr *protinfo; > protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), > IFLA_PROTINFO); > if (protinfo) { > struct nlattr *attr; > attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING); > if (attr) { > if (nla_len(attr) >= sizeof(u8)) { > if (nla_get_u8(attr)) > brport_flags |= BR_LEARNING; > else > brport_flags &= ~BR_LEARNING; > } > } > /* > * Map bridge port attributes to driver-specific port attributes > */ > } > return 0; > } > >> >>>> + IFLA_ATTR_LOOPBACK, >>>> + IFLA_ATTR_BCAST_FLOODING, >>>> + IFLA_ATTR_UCAST_FLOODING, >>>> + IFLA_ATTR_MCAST_FLOODING, >>>> + __IFLA_ATTR_MAX >>>> +}; >>>> + >>>> +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) >>>> + >>>> #endif /* _UAPI_LINUX_IF_LINK_H */ diff --git >>>> a/net/core/rtnetlink.c b/net/core/rtnetlink.c index >>>> eaa057f..c0d1c45 100644 >>>> --- a/net/core/rtnetlink.c >>>> +++ b/net/core/rtnetlink.c >>>> @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct net_device >>> *dev, struct nlattr *tb[]) >>>> return 0; >>>> } >>>> >>>> +#ifdef CONFIG_NET_SWITCHDEV >>>> +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { >>>> + int rem, err = -EINVAL; >>>> + struct nlattr *v; >>>> + const struct net_device_ops *ops = dev->netdev_ops; >>>> + >>>> + nla_for_each_nested(v, attr, rem) { >>>> + u32 op = nla_type(v); >>>> + u64 value = 0; >>>> + >>>> + switch (op) { >>>> + case IFLA_ATTR_LEARNING: >>>> + case IFLA_ATTR_LOOPBACK: >>>> + case IFLA_ATTR_BCAST_FLOODING: >>>> + case IFLA_ATTR_UCAST_FLOODING: >>>> + case IFLA_ATTR_MCAST_FLOODING: { >>>> + if (nla_len(v) < sizeof(value)) { >>>> + err = -EINVAL; >>>> + break; >>>> + } >>>> + >>>> + value = nla_get_u64(v); >>>> + err = ops->ndo_switch_port_set_cfg(dev, >>>> + op, >>>> + value); >>>> + break; >>>> + } >>>> + default: >>>> + err = -EINVAL; >>>> + break; >>>> + } >>>> + if (err) >>>> + break; >>>> + } >>>> + return err; >>>> +} >>>> + >>>> +#endif >>>> + >>>> static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) >>>> { >>>> int rem, err = -EINVAL; >>>> @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, >>>> status |= DO_SETLINK_NOTIFY; >>>> } >>>> } >>>> +#ifdef CONFIG_NET_SWITCHDEV >>>> + if (tb[IFLA_SWITCH_PORT_CFG]) { >>>> + err = -EOPNOTSUPP; >>>> + if (!ops->ndo_switch_port_set_cfg) >>>> + goto errout; >>>> + if (!ops->ndo_switch_parent_id_get) >>>> + goto errout; >>>> + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); >>>> + if (err < 0) >>>> + goto errout; >>>> + >>>> + status |= DO_SETLINK_NOTIFY; >>>> + } >>>> +#endif >>>> err = 0; >>>> >>>> errout: >> >> -- >> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration 2014-12-18 16:14 ` John Fastabend @ 2014-12-18 17:17 ` Arad, Ronen 0 siblings, 0 replies; 33+ messages in thread From: Arad, Ronen @ 2014-12-18 17:17 UTC (permalink / raw) To: Fastabend, John R, Varlese, Marco, Roopa Prabhu, netdev Cc: Thomas Graf, Jiri Pirko, sfeldma, linux-kernel > -----Original Message----- > From: Fastabend, John R > Sent: Thursday, December 18, 2014 6:14 PM > To: Arad, Ronen; Varlese, Marco; Roopa Prabhu; netdev@vger.kernel.org > Cc: Thomas Graf; Jiri Pirko; sfeldma@gmail.com; linux- > kernel@vger.kernel.org > Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port > configuration > > On 12/18/2014 07:47 AM, Arad, Ronen wrote: > > > > > >> -----Original Message----- > >> From: netdev-owner@vger.kernel.org [mailto:netdev- > >> owner@vger.kernel.org] On Behalf Of Varlese, Marco > >> Sent: Thursday, December 18, 2014 4:56 PM > >> To: Roopa Prabhu > >> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri > >> Pirko; sfeldma@gmail.com; linux-kernel@vger.kernel.org > >> Subject: RE: [RFC PATCH net-next v2 1/1] net: Support for switch port > >> configuration > >> > >>> -----Original Message----- > >>> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com] > >>> Sent: Thursday, December 18, 2014 2:45 PM > >>> To: Varlese, Marco > >>> Cc: netdev@vger.kernel.org; Fastabend, John R; Thomas Graf; Jiri > >>> Pirko; sfeldma@gmail.com; linux-kernel@vger.kernel.org > >>> Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch > >>> port configuration > >>> > >>> On 12/18/14, 3:29 AM, Varlese, Marco wrote: > >>>> From: Marco Varlese <marco.varlese@intel.com> > >>>> > >>>> Switch hardware offers a list of attributes that are configurable > >>>> on a per port basis. > >>>> This patch provides a mechanism to configure switch ports by adding > >>>> an NDO for setting specific values to specific attributes. > >>>> There will be a separate patch that adds the "get" functionality > >>>> via another NDO and another patch that extends iproute2 to call the > >>>> two new NDOs. > >>>> > >>>> Signed-off-by: Marco Varlese <marco.varlese@intel.com> > >>>> --- > >>>> include/linux/netdevice.h | 5 ++++ > >>>> include/uapi/linux/if_link.h | 15 ++++++++++++ > >>>> net/core/rtnetlink.c | 54 > >>> ++++++++++++++++++++++++++++++++++++++++++++ > >>>> 3 files changed, 74 insertions(+) > >>>> > >>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > >>>> index c31f74d..4881c7b 100644 > >>>> --- a/include/linux/netdevice.h > >>>> +++ b/include/linux/netdevice.h > >>>> @@ -1027,6 +1027,9 @@ typedef u16 > (*select_queue_fallback_t)(struct > >>> net_device *dev, > >>>> * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 > state); > >>>> * Called to notify switch device port of bridge port STP > >>>> * state change. > >>>> + * int (*ndo_switch_port_set_cfg)(struct net_device *dev, > >>>> + * u32 attr, u64 value); > >>>> + * Called to set specific switch ports attributes. > >>>> */ > >>>> struct net_device_ops { > >>>> int (*ndo_init)(struct net_device *dev); > >>>> @@ -1185,6 +1188,8 @@ struct net_device_ops { > >>>> struct > >>> netdev_phys_item_id *psid); > >>>> int (*ndo_switch_port_stp_update)(struct > >>> net_device *dev, > >>>> u8 state); > >>>> + int (*ndo_switch_port_set_cfg)(struct > >>> net_device *dev, > >>>> + u32 attr, u64 value); > >>>> #endif > >>>> }; > >>>> > >>>> diff --git a/include/uapi/linux/if_link.h > >>>> b/include/uapi/linux/if_link.h index f7d0d2d..19cb51a 100644 > >>>> --- a/include/uapi/linux/if_link.h > >>>> +++ b/include/uapi/linux/if_link.h > >>>> @@ -146,6 +146,7 @@ enum { > >>>> IFLA_PHYS_PORT_ID, > >>>> IFLA_CARRIER_CHANGES, > >>>> IFLA_PHYS_SWITCH_ID, > >>>> + IFLA_SWITCH_PORT_CFG, > >>>> __IFLA_MAX > >>>> }; > >>>> > >>>> @@ -603,4 +604,18 @@ enum { > >>>> > >>>> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > >>>> > >>>> +/* Switch Port Attributes section */ > >>>> + > >>>> +enum { > >>>> + IFLA_ATTR_UNSPEC, > >>>> + IFLA_ATTR_LEARNING, > >>> Any reason you want learning here ?. This is covered as part of the > >>> bridge setlink attributes. > >>> > >> > >> Yes, because the user may _not_ want to go through a bridge interface > >> necessarily. > > > > Some bridge attributes or more accurately bridge port attributes overlap > with port attributes. > > IFLA_BRPORT_LEARNING from IFLA_PROTINFO and > BRIDGE_VLAN_INFO_PVID flag > > of IFLA_BRIDGE_VLAN_INFO from IFLA_AF_SPEC are two examples where > > bridge port properties might have to be mapped to a common or driver > > specific port attribute. > > You've lost me a bit. > > > Switch port driver that works without and explicit bridge device has > > to implement the bridge NDO's ndo_bridge_setlink/ndo_bridge_dellink. > > It could take care of mapping such attributes from the bridge netlink > > message to either driver-specific port attribute of to an explicit one > > (assuming IFLA_ATTR_LEARNING would be accepted). For example the > > driver could process the bridge learning information from the protinfo > > as such: > > still lost unfortunately. > > So you want to enable learning on a port by port basis? Then we also need to > define how the two bits interact. > > switch_learning + port_learning = port in learning mode > !switch_learning + port_learning = port not in learning mode? > etc. > This is a valid issue. I don't believe there is a way for the switch port driver to figure out whether an AF_BRIDGE RTM_SETLINK message was intended to the bridge or to the specific port targeted. It could only make such decision separately for each attribute configured. When an explicit bridge instance is present, directing the netlink message to the bridge with SELF flag implies bridge property; Directing it to a bridge port netdev with MASTER implies bridge port property. > Do bridges and users actually enable learning on a per port basis? My understanding is that learning configuration is already on a port by port basis. That's what I see in the rocker driver. The LERNING and LEARNING_SYNC are stored in the brport_flags of a rocker_port structure. My statement was that synchronization is needed between bridge port configuration and equivalent switch port attribute. The switch port driver implementing bridge setlink NDOs has to take care of such synchronization. > > > int sw_driver_br_setlink(struct net_device *dev, struct nlmsghdr *nlh) > > { > > struct nlattr *protinfo; > > protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), > > IFLA_PROTINFO); > > if (protinfo) { > > struct nlattr *attr; > > attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING); > > if (attr) { > > if (nla_len(attr) >= sizeof(u8)) { > > if (nla_get_u8(attr)) > > brport_flags |= BR_LEARNING; > > else > > brport_flags &= ~BR_LEARNING; > > } > > } > > /* > > * Map bridge port attributes to driver-specific port attributes > > */ > > } > > return 0; > > } > > > >> > >>>> + IFLA_ATTR_LOOPBACK, > >>>> + IFLA_ATTR_BCAST_FLOODING, > >>>> + IFLA_ATTR_UCAST_FLOODING, > >>>> + IFLA_ATTR_MCAST_FLOODING, > >>>> + __IFLA_ATTR_MAX > >>>> +}; > >>>> + > >>>> +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) > >>>> + > >>>> #endif /* _UAPI_LINUX_IF_LINK_H */ diff --git > >>>> a/net/core/rtnetlink.c b/net/core/rtnetlink.c index > >>>> eaa057f..c0d1c45 100644 > >>>> --- a/net/core/rtnetlink.c > >>>> +++ b/net/core/rtnetlink.c > >>>> @@ -1389,6 +1389,46 @@ static int validate_linkmsg(struct > >>>> net_device > >>> *dev, struct nlattr *tb[]) > >>>> return 0; > >>>> } > >>>> > >>>> +#ifdef CONFIG_NET_SWITCHDEV > >>>> +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) { > >>>> + int rem, err = -EINVAL; > >>>> + struct nlattr *v; > >>>> + const struct net_device_ops *ops = dev->netdev_ops; > >>>> + > >>>> + nla_for_each_nested(v, attr, rem) { > >>>> + u32 op = nla_type(v); > >>>> + u64 value = 0; > >>>> + > >>>> + switch (op) { > >>>> + case IFLA_ATTR_LEARNING: > >>>> + case IFLA_ATTR_LOOPBACK: > >>>> + case IFLA_ATTR_BCAST_FLOODING: > >>>> + case IFLA_ATTR_UCAST_FLOODING: > >>>> + case IFLA_ATTR_MCAST_FLOODING: { > >>>> + if (nla_len(v) < sizeof(value)) { > >>>> + err = -EINVAL; > >>>> + break; > >>>> + } > >>>> + > >>>> + value = nla_get_u64(v); > >>>> + err = ops->ndo_switch_port_set_cfg(dev, > >>>> + op, > >>>> + value); > >>>> + break; > >>>> + } > >>>> + default: > >>>> + err = -EINVAL; > >>>> + break; > >>>> + } > >>>> + if (err) > >>>> + break; > >>>> + } > >>>> + return err; > >>>> +} > >>>> + > >>>> +#endif > >>>> + > >>>> static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > >>>> { > >>>> int rem, err = -EINVAL; > >>>> @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff > *skb, > >>>> status |= DO_SETLINK_NOTIFY; > >>>> } > >>>> } > >>>> +#ifdef CONFIG_NET_SWITCHDEV > >>>> + if (tb[IFLA_SWITCH_PORT_CFG]) { > >>>> + err = -EOPNOTSUPP; > >>>> + if (!ops->ndo_switch_port_set_cfg) > >>>> + goto errout; > >>>> + if (!ops->ndo_switch_parent_id_get) > >>>> + goto errout; > >>>> + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > >>>> + if (err < 0) > >>>> + goto errout; > >>>> + > >>>> + status |= DO_SETLINK_NOTIFY; > >>>> + } > >>>> +#endif > >>>> err = 0; > >>>> > >>>> errout: > >> > >> -- > >> 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2014-12-20 0:57 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-12-18 11:29 [RFC PATCH net-next v2 1/1] net: Support for switch port configuration Varlese, Marco 2014-12-18 11:41 ` Thomas Graf 2014-12-18 15:20 ` Varlese, Marco 2014-12-18 14:44 ` Roopa Prabhu 2014-12-18 14:55 ` Varlese, Marco 2014-12-18 15:16 ` Roopa Prabhu 2014-12-18 17:25 ` Varlese, Marco 2014-12-18 17:49 ` Roopa Prabhu 2014-12-18 18:02 ` Varlese, Marco 2014-12-18 18:14 ` Roopa Prabhu 2014-12-18 19:21 ` John Fastabend 2014-12-18 22:43 ` Arad, Ronen 2014-12-19 8:14 ` Jiri Pirko 2014-12-18 23:07 ` Roopa Prabhu 2014-12-18 23:26 ` Samudrala, Sridhar 2014-12-18 23:48 ` Roopa Prabhu 2014-12-19 5:14 ` B Viswanath 2014-12-19 8:27 ` Jiri Pirko 2014-12-19 9:01 ` B Viswanath 2014-12-19 9:22 ` B Viswanath 2014-12-19 9:35 ` Jiri Pirko 2014-12-19 9:23 ` Jiri Pirko 2014-12-19 9:35 ` B Viswanath 2014-12-19 9:55 ` Jiri Pirko 2014-12-19 10:53 ` B Viswanath 2014-12-19 16:22 ` Roopa Prabhu 2014-12-20 0:57 ` Williams, Kenneth 2014-12-19 14:50 ` Andy Gospodarek 2014-12-19 8:25 ` Jiri Pirko 2014-12-19 0:45 ` Thomas Graf 2014-12-18 15:47 ` Arad, Ronen 2014-12-18 16:14 ` John Fastabend 2014-12-18 17:17 ` Arad, Ronen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).