[RFC PATCH net-next 1/1] net: Support for switch port configuration

[RFC PATCH net-next 1/1] net: Support for switch port configuration

[Varlese, Marco]

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 extends iproute2 to call the
new NDO.

Signed-off-by: Marco Varlese <marco.varlese@intel.com>
---
 include/linux/netdevice.h    |  5 +++++
 include/uapi/linux/if_link.h |  1 +
 net/core/rtnetlink.c         | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 39 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..b35c314 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
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index eaa057f..e1be9ca 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1389,6 +1389,25 @@ 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 = nla_get_u64(v);
+
+		err = ops->ndo_switch_port_set_cfg(dev, op, value);
+		if (err)
+			break;
+	}
+	return err;
+}
+#endif
+
 static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
 {
 	int rem, err = -EINVAL;
@@ -1740,6 +1759,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

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Jiri Pirko]

Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the
>new NDO.


What are these attributes? Can you give some examples. I'm asking
because there is a plan to pass generic attributes to switch ports
replacing current specific ndo_switch_port_stp_update. In this case,
bridge is setting that attribute.

Is there need to set something directly from userspace or does it make
rather sense to use involved bridge/ovs/bond ? I think that both will be
needed.


>
>Signed-off-by: Marco Varlese <marco.varlese@intel.com>
>---
> include/linux/netdevice.h    |  5 +++++
> include/uapi/linux/if_link.h |  1 +
> net/core/rtnetlink.c         | 33 +++++++++++++++++++++++++++++++++
> 3 files changed, 39 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..b35c314 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
> };
> 
>diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>index eaa057f..e1be9ca 100644
>--- a/net/core/rtnetlink.c
>+++ b/net/core/rtnetlink.c
>@@ -1389,6 +1389,25 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[])
> 	return 0;
> }
> 

I believe that it would be better to move the code below to
net/switchdev/switchdev.c


>+#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 = nla_get_u64(v);
>+
>+		err = ops->ndo_switch_port_set_cfg(dev, op, value);
>+		if (err)
>+			break;
>+	}
>+	return err;
>+}
>+#endif
>+
> static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
> {
> 	int rem, err = -EINVAL;
>@@ -1740,6 +1759,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
>

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[John Fastabend]

On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the
>> new NDO.
>
>
> What are these attributes? Can you give some examples. I'm asking
> because there is a plan to pass generic attributes to switch ports
> replacing current specific ndo_switch_port_stp_update. In this case,
> bridge is setting that attribute.
>
> Is there need to set something directly from userspace or does it make
> rather sense to use involved bridge/ovs/bond ? I think that both will be
> needed.

+1

I think for many attributes it would be best to have both. The in
kernel callers and netlink userspace can use the same driver ndo_ops.

But then we don't _require_ any specific bridge/ovs/etc module. And we
may have some attributes that are not specific to any existing software
module. I'm guessing Marco has some examples of these.

[...]


-- 
John Fastabend         Intel Corporation

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Varlese, Marco]

> -----Original Message-----
> From: John Fastabend [mailto:john.fastabend@gmail.com]
> Sent: Wednesday, December 10, 2014 5:04 PM
> To: Jiri Pirko
> Cc: Varlese, Marco; netdev@vger.kernel.org;
> stephen@networkplumber.org; Fastabend, John R;
> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> configuration
> 
> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> > Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the new
> >> NDO.
> >
> >
> > What are these attributes? Can you give some examples. I'm asking
> > because there is a plan to pass generic attributes to switch ports
> > replacing current specific ndo_switch_port_stp_update. In this case,
> > bridge is setting that attribute.
> >
> > Is there need to set something directly from userspace or does it make
> > rather sense to use involved bridge/ovs/bond ? I think that both will
> > be needed.
> 
> +1
> 
> I think for many attributes it would be best to have both. The in kernel callers
> and netlink userspace can use the same driver ndo_ops.
> 
> But then we don't _require_ any specific bridge/ovs/etc module. And we
> may have some attributes that are not specific to any existing software
> module. I'm guessing Marco has some examples of these.
> 
> [...]
> 
> 
> --
> John Fastabend         Intel Corporation

We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.

An example of attributes are:
* enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
* internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
* flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);

Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.

One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.

I hope this clarifies some points.

-----------------------------------------------------------
Marco Varlese		-	Intel Corporation
-----------------------------------------------------------

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Jiri Pirko]

Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>> -----Original Message-----
>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>> Sent: Wednesday, December 10, 2014 5:04 PM
>> To: Jiri Pirko
>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>> stephen@networkplumber.org; Fastabend, John R;
>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>> configuration
>> 
>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>> > Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the new
>> >> NDO.
>> >
>> >
>> > What are these attributes? Can you give some examples. I'm asking
>> > because there is a plan to pass generic attributes to switch ports
>> > replacing current specific ndo_switch_port_stp_update. In this case,
>> > bridge is setting that attribute.
>> >
>> > Is there need to set something directly from userspace or does it make
>> > rather sense to use involved bridge/ovs/bond ? I think that both will
>> > be needed.
>> 
>> +1
>> 
>> I think for many attributes it would be best to have both. The in kernel callers
>> and netlink userspace can use the same driver ndo_ops.
>> 
>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>> may have some attributes that are not specific to any existing software
>> module. I'm guessing Marco has some examples of these.
>> 
>> [...]
>> 
>> 
>> --
>> John Fastabend         Intel Corporation
>
>We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>
>An example of attributes are:
>* enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>* internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>* flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>
>Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>
>One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>
>I hope this clarifies some points.

It does. Makes sense. We need to expose this attr set/get for both
in-kernel and userspace use cases.

Please adjust you patch for this. Also, as a second patch, it would be
great if you can convert ndo_switch_port_stp_update to this new ndo.

Thanks.


>
>-----------------------------------------------------------
>Marco Varlese		-	Intel Corporation
>-----------------------------------------------------------
>
>

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Varlese, Marco]

> -----Original Message-----
> From: Jiri Pirko [mailto:jiri@resnulli.us]
> Sent: Thursday, December 11, 2014 11:01 AM
> To: Varlese, Marco
> Cc: John Fastabend; netdev@vger.kernel.org;
> stephen@networkplumber.org; Fastabend, John R;
> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> configuration
> 
> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
> >> -----Original Message-----
> >> From: John Fastabend [mailto:john.fastabend@gmail.com]
> >> Sent: Wednesday, December 10, 2014 5:04 PM
> >> To: Jiri Pirko
> >> Cc: Varlese, Marco; netdev@vger.kernel.org;
> >> stephen@networkplumber.org; Fastabend, John R;
> >> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> >> kernel@vger.kernel.org
> >> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> >> configuration
> >>
> >> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> >> > Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the
> >> >> new NDO.
> >> >
> >> >
> >> > What are these attributes? Can you give some examples. I'm asking
> >> > because there is a plan to pass generic attributes to switch ports
> >> > replacing current specific ndo_switch_port_stp_update. In this
> >> > case, bridge is setting that attribute.
> >> >
> >> > Is there need to set something directly from userspace or does it
> >> > make rather sense to use involved bridge/ovs/bond ? I think that
> >> > both will be needed.
> >>
> >> +1
> >>
> >> I think for many attributes it would be best to have both. The in
> >> kernel callers and netlink userspace can use the same driver ndo_ops.
> >>
> >> But then we don't _require_ any specific bridge/ovs/etc module. And
> >> we may have some attributes that are not specific to any existing
> >> software module. I'm guessing Marco has some examples of these.
> >>
> >> [...]
> >>
> >>
> >> --
> >> John Fastabend         Intel Corporation
> >
> >We do have a need to configure the attributes directly from user-space and
> I have identified the tool to do that in iproute2.
> >
> >An example of attributes are:
> >* enabling/disabling of learning of source addresses on a given port
> >(you can imagine the attribute called LEARNING for example);
> >* internal loopback control (i.e. LOOPBACK) which will control how the
> >flow of traffic behaves from the switch fabric towards an egress port;
> >* flooding for broadcast/multicast/unicast type of packets (i.e.
> >BFLOODING, MFLOODING, UFLOODING);
> >
> >Some attributes would be of the type enabled/disabled while other will
> allow specific values to allow the user to configure different behaviours of
> that feature on that particular port on that platform.
> >
> >One thing to mention - as John stated as well - there might be some
> attributes that are not specific to any software module but rather have to do
> with the actual hardware/platform to configure.
> >
> >I hope this clarifies some points.
> 
> It does. Makes sense. We need to expose this attr set/get for both in-kernel
> and userspace use cases.
> 
> Please adjust you patch for this. Also, as a second patch, it would be great if
> you can convert ndo_switch_port_stp_update to this new ndo.
> 
> Thanks.
> 
> 

I was thinking of leaving the get side of things implemented via sysfs rather than implementing an NDO for it. Would this not be appropriate?

- - -
Marco Varlese

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Jiri Pirko]

Thu, Dec 11, 2014 at 01:02:36PM CET, marco.varlese@intel.com wrote:
>> -----Original Message-----
>> From: Jiri Pirko [mailto:jiri@resnulli.us]
>> Sent: Thursday, December 11, 2014 11:01 AM
>> To: Varlese, Marco
>> Cc: John Fastabend; netdev@vger.kernel.org;
>> stephen@networkplumber.org; Fastabend, John R;
>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>> configuration
>> 
>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>> >> -----Original Message-----
>> >> From: John Fastabend [mailto:john.fastabend@gmail.com]
>> >> Sent: Wednesday, December 10, 2014 5:04 PM
>> >> To: Jiri Pirko
>> >> Cc: Varlese, Marco; netdev@vger.kernel.org;
>> >> stephen@networkplumber.org; Fastabend, John R;
>> >> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>> >> kernel@vger.kernel.org
>> >> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>> >> configuration
>> >>
>> >> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>> >> > Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the
>> >> >> new NDO.
>> >> >
>> >> >
>> >> > What are these attributes? Can you give some examples. I'm asking
>> >> > because there is a plan to pass generic attributes to switch ports
>> >> > replacing current specific ndo_switch_port_stp_update. In this
>> >> > case, bridge is setting that attribute.
>> >> >
>> >> > Is there need to set something directly from userspace or does it
>> >> > make rather sense to use involved bridge/ovs/bond ? I think that
>> >> > both will be needed.
>> >>
>> >> +1
>> >>
>> >> I think for many attributes it would be best to have both. The in
>> >> kernel callers and netlink userspace can use the same driver ndo_ops.
>> >>
>> >> But then we don't _require_ any specific bridge/ovs/etc module. And
>> >> we may have some attributes that are not specific to any existing
>> >> software module. I'm guessing Marco has some examples of these.
>> >>
>> >> [...]
>> >>
>> >>
>> >> --
>> >> John Fastabend         Intel Corporation
>> >
>> >We do have a need to configure the attributes directly from user-space and
>> I have identified the tool to do that in iproute2.
>> >
>> >An example of attributes are:
>> >* enabling/disabling of learning of source addresses on a given port
>> >(you can imagine the attribute called LEARNING for example);
>> >* internal loopback control (i.e. LOOPBACK) which will control how the
>> >flow of traffic behaves from the switch fabric towards an egress port;
>> >* flooding for broadcast/multicast/unicast type of packets (i.e.
>> >BFLOODING, MFLOODING, UFLOODING);
>> >
>> >Some attributes would be of the type enabled/disabled while other will
>> allow specific values to allow the user to configure different behaviours of
>> that feature on that particular port on that platform.
>> >
>> >One thing to mention - as John stated as well - there might be some
>> attributes that are not specific to any software module but rather have to do
>> with the actual hardware/platform to configure.
>> >
>> >I hope this clarifies some points.
>> 
>> It does. Makes sense. We need to expose this attr set/get for both in-kernel
>> and userspace use cases.
>> 
>> Please adjust you patch for this. Also, as a second patch, it would be great if
>> you can convert ndo_switch_port_stp_update to this new ndo.
>> 
>> Thanks.
>> 
>> 
>
>I was thinking of leaving the get side of things implemented via sysfs rather than implementing an NDO for it. Would this not be appropriate?

I believe that it is preferred to have both get and set exposed via ndo
and netlink. It can be exposed via sysfs as well, but it is "nice to have"
not "must have"

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Varlese, Marco]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Jiri Pirko
> Sent: Thursday, December 11, 2014 1:09 PM
> To: Varlese, Marco
> Cc: John Fastabend; netdev@vger.kernel.org;
> stephen@networkplumber.org; Fastabend, John R;
> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> configuration
> 
> Thu, Dec 11, 2014 at 01:02:36PM CET, marco.varlese@intel.com wrote:
> >> -----Original Message-----
> >> From: Jiri Pirko [mailto:jiri@resnulli.us]
> >> Sent: Thursday, December 11, 2014 11:01 AM
> >> To: Varlese, Marco
> >> Cc: John Fastabend; netdev@vger.kernel.org;
> >> stephen@networkplumber.org; Fastabend, John R;
> >> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> >> kernel@vger.kernel.org
> >> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> >> configuration
> >>
> >> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
> >> >> -----Original Message-----
> >> >> From: John Fastabend [mailto:john.fastabend@gmail.com]
> >> >> Sent: Wednesday, December 10, 2014 5:04 PM
> >> >> To: Jiri Pirko
> >> >> Cc: Varlese, Marco; netdev@vger.kernel.org;
> >> >> stephen@networkplumber.org; Fastabend, John R;
> >> >> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> >> >> kernel@vger.kernel.org
> >> >> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> >> >> configuration
> >> >>
> >> >> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> >> >> > Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com
> 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 extends iproute2 to call
> >> >> >> the new NDO.
> >> >> >
> >> >> >
> >> >> > What are these attributes? Can you give some examples. I'm
> >> >> > asking because there is a plan to pass generic attributes to
> >> >> > switch ports replacing current specific
> >> >> > ndo_switch_port_stp_update. In this case, bridge is setting that
> attribute.
> >> >> >
> >> >> > Is there need to set something directly from userspace or does
> >> >> > it make rather sense to use involved bridge/ovs/bond ? I think
> >> >> > that both will be needed.
> >> >>
> >> >> +1
> >> >>
> >> >> I think for many attributes it would be best to have both. The in
> >> >> kernel callers and netlink userspace can use the same driver ndo_ops.
> >> >>
> >> >> But then we don't _require_ any specific bridge/ovs/etc module.
> >> >> And we may have some attributes that are not specific to any
> >> >> existing software module. I'm guessing Marco has some examples of
> these.
> >> >>
> >> >> [...]
> >> >>
> >> >>
> >> >> --
> >> >> John Fastabend         Intel Corporation
> >> >
> >> >We do have a need to configure the attributes directly from
> >> >user-space and
> >> I have identified the tool to do that in iproute2.
> >> >
> >> >An example of attributes are:
> >> >* enabling/disabling of learning of source addresses on a given port
> >> >(you can imagine the attribute called LEARNING for example);
> >> >* internal loopback control (i.e. LOOPBACK) which will control how
> >> >the flow of traffic behaves from the switch fabric towards an egress
> >> >port;
> >> >* flooding for broadcast/multicast/unicast type of packets (i.e.
> >> >BFLOODING, MFLOODING, UFLOODING);
> >> >
> >> >Some attributes would be of the type enabled/disabled while other
> >> >will
> >> allow specific values to allow the user to configure different
> >> behaviours of that feature on that particular port on that platform.
> >> >
> >> >One thing to mention - as John stated as well - there might be some
> >> attributes that are not specific to any software module but rather
> >> have to do with the actual hardware/platform to configure.
> >> >
> >> >I hope this clarifies some points.
> >>
> >> It does. Makes sense. We need to expose this attr set/get for both
> >> in-kernel and userspace use cases.
> >>
> >> Please adjust you patch for this. Also, as a second patch, it would
> >> be great if you can convert ndo_switch_port_stp_update to this new ndo.
> >>
> >> Thanks.
> >>
> >>
> >
> >I was thinking of leaving the get side of things implemented via sysfs rather
> than implementing an NDO for it. Would this not be appropriate?
> 
> I believe that it is preferred to have both get and set exposed via ndo and
> netlink. It can be exposed via sysfs as well, but it is "nice to have"
> not "must have"
> 

I'll add the get ndo to my patch now. Thanks.

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Roopa Prabhu]

On 12/11/14, 3:01 AM, Jiri Pirko wrote:
> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>> -----Original Message-----
>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>> To: Jiri Pirko
>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>> stephen@networkplumber.org; Fastabend, John R;
>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>> kernel@vger.kernel.org
>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>> configuration
>>>
>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the new
>>>>> NDO.
>>>>
>>>> What are these attributes? Can you give some examples. I'm asking
>>>> because there is a plan to pass generic attributes to switch ports
>>>> replacing current specific ndo_switch_port_stp_update. In this case,
>>>> bridge is setting that attribute.
>>>>
>>>> Is there need to set something directly from userspace or does it make
>>>> rather sense to use involved bridge/ovs/bond ? I think that both will
>>>> be needed.
>>> +1
>>>
>>> I think for many attributes it would be best to have both. The in kernel callers
>>> and netlink userspace can use the same driver ndo_ops.
>>>
>>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>>> may have some attributes that are not specific to any existing software
>>> module. I'm guessing Marco has some examples of these.
>>>
>>> [...]
>>>
>>>
>>> --
>>> John Fastabend         Intel Corporation
>> We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>
>> An example of attributes are:
>> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>
>> Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>
>> One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>
>> I hope this clarifies some points.
> It does. Makes sense. We need to expose this attr set/get for both
> in-kernel and userspace use cases.
>
> Please adjust you patch for this. Also, as a second patch, it would be
> great if you can convert ndo_switch_port_stp_update to this new ndo.

Why are we exposing generic switch attribute get/set from userspace ?. 
We already have specific attributes for learning/flooding which can be 
extended further.
And for in kernel api....i had a sample patch in my RFC series (Which i 
was going to resubmit, until it was decided that we will use existing 
api around ndo_bridge_setlink/ndo_bridge_getlink):
http://www.spinics.net/lists/netdev/msg305473.html

Thanks,
Roopa

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Jiri Pirko]

Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>-----Original Message-----
>>>>From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>Sent: Wednesday, December 10, 2014 5:04 PM
>>>>To: Jiri Pirko
>>>>Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>stephen@networkplumber.org; Fastabend, John R;
>>>>roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>kernel@vger.kernel.org
>>>>Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>configuration
>>>>
>>>>On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the new
>>>>>>NDO.
>>>>>
>>>>>What are these attributes? Can you give some examples. I'm asking
>>>>>because there is a plan to pass generic attributes to switch ports
>>>>>replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>bridge is setting that attribute.
>>>>>
>>>>>Is there need to set something directly from userspace or does it make
>>>>>rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>be needed.
>>>>+1
>>>>
>>>>I think for many attributes it would be best to have both. The in kernel callers
>>>>and netlink userspace can use the same driver ndo_ops.
>>>>
>>>>But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>may have some attributes that are not specific to any existing software
>>>>module. I'm guessing Marco has some examples of these.
>>>>
>>>>[...]
>>>>
>>>>
>>>>--
>>>>John Fastabend         Intel Corporation
>>>We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>
>>>An example of attributes are:
>>>* enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>* internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>* flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>
>>>Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>
>>>One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>
>>>I hope this clarifies some points.
>>It does. Makes sense. We need to expose this attr set/get for both
>>in-kernel and userspace use cases.
>>
>>Please adjust you patch for this. Also, as a second patch, it would be
>>great if you can convert ndo_switch_port_stp_update to this new ndo.
>
>Why are we exposing generic switch attribute get/set from userspace ?. We
>already have specific attributes for learning/flooding which can be extended
>further.

Yes, but that is for PF_BRIDGE and bridge specific attributes. There
might be another generic attrs, no?

>And for in kernel api....i had a sample patch in my RFC series (Which i was
>going to resubmit, until it was decided that we will use existing api around
>ndo_bridge_setlink/ndo_bridge_getlink):
>http://www.spinics.net/lists/netdev/msg305473.html

Yes, this might become handy for other generic non-bridge attrs.

>
>Thanks,
>Roopa
>
>
>

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Roopa Prabhu]

On 12/11/14, 8:56 AM, Jiri Pirko wrote:
> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>> -----Original Message-----
>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>>>> To: Jiri Pirko
>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>> stephen@networkplumber.org; Fastabend, John R;
>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>> kernel@vger.kernel.org
>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>> configuration
>>>>>
>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the new
>>>>>>> NDO.
>>>>>> What are these attributes? Can you give some examples. I'm asking
>>>>>> because there is a plan to pass generic attributes to switch ports
>>>>>> replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>> bridge is setting that attribute.
>>>>>>
>>>>>> Is there need to set something directly from userspace or does it make
>>>>>> rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>> be needed.
>>>>> +1
>>>>>
>>>>> I think for many attributes it would be best to have both. The in kernel callers
>>>>> and netlink userspace can use the same driver ndo_ops.
>>>>>
>>>>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>> may have some attributes that are not specific to any existing software
>>>>> module. I'm guessing Marco has some examples of these.
>>>>>
>>>>> [...]
>>>>>
>>>>>
>>>>> --
>>>>> John Fastabend         Intel Corporation
>>>> We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>>
>>>> An example of attributes are:
>>>> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>>
>>>> Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>>
>>>> One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>>
>>>> I hope this clarifies some points.
>>> It does. Makes sense. We need to expose this attr set/get for both
>>> in-kernel and userspace use cases.
>>>
>>> Please adjust you patch for this. Also, as a second patch, it would be
>>> great if you can convert ndo_switch_port_stp_update to this new ndo.
>> Why are we exposing generic switch attribute get/set from userspace ?. We
>> already have specific attributes for learning/flooding which can be extended
>> further.
> Yes, but that is for PF_BRIDGE and bridge specific attributes. There
> might be another generic attrs, no?
I cant think of any. And plus, the whole point of switchdev l2 offloads 
was to map these to existing
bridge attributes. And we already have a match for some of the 
attributes that marco wants.

If there is a need for such attributes, i don't see why it is needed for 
switch devices only.
It is needed for any hw (nics etc). And, a precedence to this is to do 
it via ethtool.

Having said that, am sure we will find a need for this in the future. 
And having a netlink attribute always helps.

Today, it seems like these can be mapped to existing attributes that are 
settable via ndo_bridge_setlink/getlink.

>
>> And for in kernel api....i had a sample patch in my RFC series (Which i was
>> going to resubmit, until it was decided that we will use existing api around
>> ndo_bridge_setlink/ndo_bridge_getlink):
>> http://www.spinics.net/lists/netdev/msg305473.html
> Yes, this might become handy for other generic non-bridge attrs.
>
>> Thanks,
>> Roopa
>>
>>
>>

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Jiri Pirko]

Thu, Dec 11, 2014 at 06:41:13PM CET, roopa@cumulusnetworks.com wrote:
>On 12/11/14, 8:56 AM, Jiri Pirko wrote:
>>Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>>>On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>>>Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>>>-----Original Message-----
>>>>>>From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>>>Sent: Wednesday, December 10, 2014 5:04 PM
>>>>>>To: Jiri Pirko
>>>>>>Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>>>stephen@networkplumber.org; Fastabend, John R;
>>>>>>roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>>>kernel@vger.kernel.org
>>>>>>Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>>>configuration
>>>>>>
>>>>>>On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>>>Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the new
>>>>>>>>NDO.
>>>>>>>What are these attributes? Can you give some examples. I'm asking
>>>>>>>because there is a plan to pass generic attributes to switch ports
>>>>>>>replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>>>bridge is setting that attribute.
>>>>>>>
>>>>>>>Is there need to set something directly from userspace or does it make
>>>>>>>rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>>>be needed.
>>>>>>+1
>>>>>>
>>>>>>I think for many attributes it would be best to have both. The in kernel callers
>>>>>>and netlink userspace can use the same driver ndo_ops.
>>>>>>
>>>>>>But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>>>may have some attributes that are not specific to any existing software
>>>>>>module. I'm guessing Marco has some examples of these.
>>>>>>
>>>>>>[...]
>>>>>>
>>>>>>
>>>>>>--
>>>>>>John Fastabend         Intel Corporation
>>>>>We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>>>
>>>>>An example of attributes are:
>>>>>* enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>>>* internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>>>* flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>>>
>>>>>Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>>>
>>>>>One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>>>
>>>>>I hope this clarifies some points.
>>>>It does. Makes sense. We need to expose this attr set/get for both
>>>>in-kernel and userspace use cases.
>>>>
>>>>Please adjust you patch for this. Also, as a second patch, it would be
>>>>great if you can convert ndo_switch_port_stp_update to this new ndo.
>>>Why are we exposing generic switch attribute get/set from userspace ?. We
>>>already have specific attributes for learning/flooding which can be extended
>>>further.
>>Yes, but that is for PF_BRIDGE and bridge specific attributes. There
>>might be another generic attrs, no?
>I cant think of any. And plus, the whole point of switchdev l2 offloads was
>to map these to existing
>bridge attributes. And we already have a match for some of the attributes
>that marco wants.
>
>If there is a need for such attributes, i don't see why it is needed for
>switch devices only.
>It is needed for any hw (nics etc). And, a precedence to this is to do it via
>ethtool.

Fair enough.

>
>Having said that, am sure we will find a need for this in the future. And
>having a netlink attribute always helps.
>
>Today, it seems like these can be mapped to existing attributes that are
>settable via ndo_bridge_setlink/getlink.

Okay. That makes sense so far for bridge.

>
>>
>>>And for in kernel api....i had a sample patch in my RFC series (Which i was
>>>going to resubmit, until it was decided that we will use existing api around
>>>ndo_bridge_setlink/ndo_bridge_getlink):
>>>http://www.spinics.net/lists/netdev/msg305473.html
>>Yes, this might become handy for other generic non-bridge attrs.
>>
>>>Thanks,
>>>Roopa
>>>
>>>
>>>
>

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[John Fastabend]

On 12/11/2014 09:41 AM, Roopa Prabhu wrote:
> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
>> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>>> -----Original Message-----
>>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>>>>> To: Jiri Pirko
>>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>>> stephen@networkplumber.org; Fastabend, John R;
>>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>>> kernel@vger.kernel.org
>>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>>> configuration
>>>>>>
>>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com 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 extends iproute2 to call the new
>>>>>>>> NDO.
>>>>>>> What are these attributes? Can you give some examples. I'm asking
>>>>>>> because there is a plan to pass generic attributes to switch ports
>>>>>>> replacing current specific ndo_switch_port_stp_update. In this case,
>>>>>>> bridge is setting that attribute.
>>>>>>>
>>>>>>> Is there need to set something directly from userspace or does it make
>>>>>>> rather sense to use involved bridge/ovs/bond ? I think that both will
>>>>>>> be needed.
>>>>>> +1
>>>>>>
>>>>>> I think for many attributes it would be best to have both. The in kernel callers
>>>>>> and netlink userspace can use the same driver ndo_ops.
>>>>>>
>>>>>> But then we don't _require_ any specific bridge/ovs/etc module. And we
>>>>>> may have some attributes that are not specific to any existing software
>>>>>> module. I'm guessing Marco has some examples of these.
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>> -- 
>>>>>> John Fastabend         Intel Corporation
>>>>> We do have a need to configure the attributes directly from user-space and I have identified the tool to do that in iproute2.
>>>>>
>>>>> An example of attributes are:
>>>>> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
>>>>> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
>>>>> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);
>>>>>
>>>>> Some attributes would be of the type enabled/disabled while other will allow specific values to allow the user to configure different behaviours of that feature on that particular port on that platform.
>>>>>
>>>>> One thing to mention - as John stated as well - there might be some attributes that are not specific to any software module but rather have to do with the actual hardware/platform to configure.
>>>>>
>>>>> I hope this clarifies some points.
>>>> It does. Makes sense. We need to expose this attr set/get for both
>>>> in-kernel and userspace use cases.
>>>>
>>>> Please adjust you patch for this. Also, as a second patch, it would be
>>>> great if you can convert ndo_switch_port_stp_update to this new ndo.
>>> Why are we exposing generic switch attribute get/set from userspace ?. We
>>> already have specific attributes for learning/flooding which can be extended
>>> further.
>> Yes, but that is for PF_BRIDGE and bridge specific attributes. There
>> might be another generic attrs, no?
> I cant think of any. And plus, the whole point of switchdev l2 offloads was to map these to existing
> bridge attributes. And we already have a match for some of the attributes that marco wants.
> 
> If there is a need for such attributes, i don't see why it is needed for switch devices only.
> It is needed for any hw (nics etc). And, a precedence to this is to do it via ethtool.

I would prefer to _not_ add more attributes to ethtool. 'ethtool' is in general
harder to work with then netlink for all but the most static attributes.

> 
> Having said that, am sure we will find a need for this in the future. And having a netlink attribute always helps.
> 
> Today, it seems like these can be mapped to existing attributes that are settable via ndo_bridge_setlink/getlink.

Absolutely I view this as an RFC patch noting we may/will need some extensions
in the future. .We can evaluate the attributes on a case by case basis as they
come in. And if they all fit in setlink/getlink that is great. 

> 
>>
>>> And for in kernel api....i had a sample patch in my RFC series (Which i was
>>> going to resubmit, until it was decided that we will use existing api around
>>> ndo_bridge_setlink/ndo_bridge_getlink):
>>> http://www.spinics.net/lists/netdev/msg305473.html
>> Yes, this might become handy for other generic non-bridge attrs.
>>
>>> Thanks,
>>> Roopa
>>>
>>>
>>>
> 

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Varlese, Marco]

> -----Original Message-----
> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
> Sent: Thursday, December 11, 2014 5:41 PM
> To: Jiri Pirko
> Cc: Varlese, Marco; John Fastabend; netdev@vger.kernel.org;
> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> configuration
> 
> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
> > Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
> >> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
> >>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
> >>>>> -----Original Message-----
> >>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
> >>>>> Sent: Wednesday, December 10, 2014 5:04 PM
> >>>>> To: Jiri Pirko
> >>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
> >>>>> stephen@networkplumber.org; Fastabend, John R;
> >>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> >>>>> kernel@vger.kernel.org
> >>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> >>>>> configuration
> >>>>>
> >>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> >>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com
> 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 extends iproute2 to call the
> >>>>>>> new NDO.
> >>>>>> What are these attributes? Can you give some examples. I'm asking
> >>>>>> because there is a plan to pass generic attributes to switch
> >>>>>> ports replacing current specific ndo_switch_port_stp_update. In
> >>>>>> this case, bridge is setting that attribute.
> >>>>>>
> >>>>>> Is there need to set something directly from userspace or does it
> >>>>>> make rather sense to use involved bridge/ovs/bond ? I think that
> >>>>>> both will be needed.
> >>>>> +1
> >>>>>
> >>>>> I think for many attributes it would be best to have both. The in
> >>>>> kernel callers and netlink userspace can use the same driver ndo_ops.
> >>>>>
> >>>>> But then we don't _require_ any specific bridge/ovs/etc module.
> >>>>> And we may have some attributes that are not specific to any
> >>>>> existing software module. I'm guessing Marco has some examples of
> these.
> >>>>>
> >>>>> [...]
> >>>>>
> >>>>>
> >>>>> --
> >>>>> John Fastabend         Intel Corporation
> >>>> We do have a need to configure the attributes directly from user-space
> and I have identified the tool to do that in iproute2.
> >>>>
> >>>> An example of attributes are:
> >>>> * enabling/disabling of learning of source addresses on a given
> >>>> port (you can imagine the attribute called LEARNING for example);
> >>>> * internal loopback control (i.e. LOOPBACK) which will control how
> >>>> the flow of traffic behaves from the switch fabric towards an
> >>>> egress port;
> >>>> * flooding for broadcast/multicast/unicast type of packets (i.e.
> >>>> BFLOODING, MFLOODING, UFLOODING);
> >>>>
> >>>> Some attributes would be of the type enabled/disabled while other will
> allow specific values to allow the user to configure different behaviours of
> that feature on that particular port on that platform.
> >>>>
> >>>> One thing to mention - as John stated as well - there might be some
> attributes that are not specific to any software module but rather have to do
> with the actual hardware/platform to configure.
> >>>>
> >>>> I hope this clarifies some points.
> >>> It does. Makes sense. We need to expose this attr set/get for both
> >>> in-kernel and userspace use cases.
> >>>
> >>> Please adjust you patch for this. Also, as a second patch, it would
> >>> be great if you can convert ndo_switch_port_stp_update to this new
> ndo.
> >> Why are we exposing generic switch attribute get/set from userspace
> >> ?. We already have specific attributes for learning/flooding which
> >> can be extended further.
> > Yes, but that is for PF_BRIDGE and bridge specific attributes. There
> > might be another generic attrs, no?
> I cant think of any. And plus, the whole point of switchdev l2 offloads was to
> map these to existing bridge attributes. And we already have a match for
> some of the attributes that marco wants.
> 
> If there is a need for such attributes, i don't see why it is needed for switch
> devices only.
> It is needed for any hw (nics etc). And, a precedence to this is to do it via
> ethtool.
> 
> Having said that, am sure we will find a need for this in the future.
> And having a netlink attribute always helps.
> 
> Today, it seems like these can be mapped to existing attributes that are
> settable via ndo_bridge_setlink/getlink.
> 
> >
> >> And for in kernel api....i had a sample patch in my RFC series (Which
> >> i was going to resubmit, until it was decided that we will use
> >> existing api around
> >> ndo_bridge_setlink/ndo_bridge_getlink):
> >> http://www.spinics.net/lists/netdev/msg305473.html
> > Yes, this might become handy for other generic non-bridge attrs.
> >
> >> Thanks,
> >> Roopa
> >>
> >>
> >>

The list I provided is only a subset of the attributes we will need to be exposed. I do have more and I'm sure that more will come in the future. As I mentioned in few posts earlier, some attributes are generic and some are not. 

I did not consider ethtool for few reasons but the main one is that I was under the impression that netlink was preferred in many circumstances over the ethotool_ops. Plus, all the cases I have identified so far are going to nicely fit into the setlink set of operations.

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Roopa Prabhu]

On 12/12/14, 1:19 AM, Varlese, Marco wrote:
>> -----Original Message-----
>> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
>> Sent: Thursday, December 11, 2014 5:41 PM
>> To: Jiri Pirko
>> Cc: Varlese, Marco; John Fastabend; netdev@vger.kernel.org;
>> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
>> linux-kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>> configuration
>>
>> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
>>> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
>>>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>>>>>> To: Jiri Pirko
>>>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>>>> stephen@networkplumber.org; Fastabend, John R;
>>>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>>>> kernel@vger.kernel.org
>>>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>>>>> configuration
>>>>>>>
>>>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com
>> 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 extends iproute2 to call the
>>>>>>>>> new NDO.
>>>>>>>> What are these attributes? Can you give some examples. I'm asking
>>>>>>>> because there is a plan to pass generic attributes to switch
>>>>>>>> ports replacing current specific ndo_switch_port_stp_update. In
>>>>>>>> this case, bridge is setting that attribute.
>>>>>>>>
>>>>>>>> Is there need to set something directly from userspace or does it
>>>>>>>> make rather sense to use involved bridge/ovs/bond ? I think that
>>>>>>>> both will be needed.
>>>>>>> +1
>>>>>>>
>>>>>>> I think for many attributes it would be best to have both. The in
>>>>>>> kernel callers and netlink userspace can use the same driver ndo_ops.
>>>>>>>
>>>>>>> But then we don't _require_ any specific bridge/ovs/etc module.
>>>>>>> And we may have some attributes that are not specific to any
>>>>>>> existing software module. I'm guessing Marco has some examples of
>> these.
>>>>>>> [...]
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> John Fastabend         Intel Corporation
>>>>>> We do have a need to configure the attributes directly from user-space
>> and I have identified the tool to do that in iproute2.
>>>>>> An example of attributes are:
>>>>>> * enabling/disabling of learning of source addresses on a given
>>>>>> port (you can imagine the attribute called LEARNING for example);
>>>>>> * internal loopback control (i.e. LOOPBACK) which will control how
>>>>>> the flow of traffic behaves from the switch fabric towards an
>>>>>> egress port;
>>>>>> * flooding for broadcast/multicast/unicast type of packets (i.e.
>>>>>> BFLOODING, MFLOODING, UFLOODING);
>>>>>>
>>>>>> Some attributes would be of the type enabled/disabled while other will
>> allow specific values to allow the user to configure different behaviours of
>> that feature on that particular port on that platform.
>>>>>> One thing to mention - as John stated as well - there might be some
>> attributes that are not specific to any software module but rather have to do
>> with the actual hardware/platform to configure.
>>>>>> I hope this clarifies some points.
>>>>> It does. Makes sense. We need to expose this attr set/get for both
>>>>> in-kernel and userspace use cases.
>>>>>
>>>>> Please adjust you patch for this. Also, as a second patch, it would
>>>>> be great if you can convert ndo_switch_port_stp_update to this new
>> ndo.
>>>> Why are we exposing generic switch attribute get/set from userspace
>>>> ?. We already have specific attributes for learning/flooding which
>>>> can be extended further.
>>> Yes, but that is for PF_BRIDGE and bridge specific attributes. There
>>> might be another generic attrs, no?
>> I cant think of any. And plus, the whole point of switchdev l2 offloads was to
>> map these to existing bridge attributes. And we already have a match for
>> some of the attributes that marco wants.
>>
>> If there is a need for such attributes, i don't see why it is needed for switch
>> devices only.
>> It is needed for any hw (nics etc). And, a precedence to this is to do it via
>> ethtool.
>>
>> Having said that, am sure we will find a need for this in the future.
>> And having a netlink attribute always helps.
>>
>> Today, it seems like these can be mapped to existing attributes that are
>> settable via ndo_bridge_setlink/getlink.
>>
>>>> And for in kernel api....i had a sample patch in my RFC series (Which
>>>> i was going to resubmit, until it was decided that we will use
>>>> existing api around
>>>> ndo_bridge_setlink/ndo_bridge_getlink):
>>>> http://www.spinics.net/lists/netdev/msg305473.html
>>> Yes, this might become handy for other generic non-bridge attrs.
>>>
>>>> Thanks,
>>>> Roopa
>>>>
>>>>
>>>>
> The list I provided is only a subset of the attributes we will need to be exposed. I do have more and I'm sure that more will come in the future. As I mentioned in few posts earlier, some attributes are generic and some are not.
>
> I did not consider ethtool for few reasons but the main one is that I was under the impression that netlink was preferred in many circumstances over the ethotool_ops.
That is correct. I don't think anybody hinted that you should extend 
ethtool.
>   Plus, all the cases I have identified so far are going to nicely fit into the setlink set of operations.
>

Would be better if you submitted your iproute2 patch with this patch.

I do plan to resubmit my generic ndo patch soon.

Thanks,
Roopa

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Varlese, Marco]

> -----Original Message-----
> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
> Sent: Saturday, December 13, 2014 7:06 AM
> To: Varlese, Marco
> Cc: Jiri Pirko; John Fastabend; netdev@vger.kernel.org;
> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> configuration
> 
> On 12/12/14, 1:19 AM, Varlese, Marco wrote:
> >> -----Original Message-----
> >> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
> >> Sent: Thursday, December 11, 2014 5:41 PM
> >> To: Jiri Pirko
> >> Cc: Varlese, Marco; John Fastabend; netdev@vger.kernel.org;
> >> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
> >> linux-kernel@vger.kernel.org
> >> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> >> configuration
> >>
> >> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
> >>> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com
> wrote:
> >>>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
> >>>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
> >>>>>>> -----Original Message-----
> >>>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
> >>>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
> >>>>>>> To: Jiri Pirko
> >>>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
> >>>>>>> stephen@networkplumber.org; Fastabend, John R;
> >>>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> >>>>>>> kernel@vger.kernel.org
> >>>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch
> >>>>>>> port configuration
> >>>>>>>
> >>>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> >>>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com
> >> 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 extends iproute2 to call
> >>>>>>>>> the new NDO.
> >>>>>>>> What are these attributes? Can you give some examples. I'm
> >>>>>>>> asking because there is a plan to pass generic attributes to
> >>>>>>>> switch ports replacing current specific
> >>>>>>>> ndo_switch_port_stp_update. In this case, bridge is setting that
> attribute.
> >>>>>>>>
> >>>>>>>> Is there need to set something directly from userspace or does
> >>>>>>>> it make rather sense to use involved bridge/ovs/bond ? I think
> >>>>>>>> that both will be needed.
> >>>>>>> +1
> >>>>>>>
> >>>>>>> I think for many attributes it would be best to have both. The
> >>>>>>> in kernel callers and netlink userspace can use the same driver
> ndo_ops.
> >>>>>>>
> >>>>>>> But then we don't _require_ any specific bridge/ovs/etc module.
> >>>>>>> And we may have some attributes that are not specific to any
> >>>>>>> existing software module. I'm guessing Marco has some examples
> >>>>>>> of
> >> these.
> >>>>>>> [...]
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> John Fastabend         Intel Corporation
> >>>>>> We do have a need to configure the attributes directly from
> >>>>>> user-space
> >> and I have identified the tool to do that in iproute2.
> >>>>>> An example of attributes are:
> >>>>>> * enabling/disabling of learning of source addresses on a given
> >>>>>> port (you can imagine the attribute called LEARNING for example);
> >>>>>> * internal loopback control (i.e. LOOPBACK) which will control
> >>>>>> how the flow of traffic behaves from the switch fabric towards an
> >>>>>> egress port;
> >>>>>> * flooding for broadcast/multicast/unicast type of packets (i.e.
> >>>>>> BFLOODING, MFLOODING, UFLOODING);
> >>>>>>
> >>>>>> Some attributes would be of the type enabled/disabled while other
> >>>>>> will
> >> allow specific values to allow the user to configure different
> >> behaviours of that feature on that particular port on that platform.
> >>>>>> One thing to mention - as John stated as well - there might be
> >>>>>> some
> >> attributes that are not specific to any software module but rather
> >> have to do with the actual hardware/platform to configure.
> >>>>>> I hope this clarifies some points.
> >>>>> It does. Makes sense. We need to expose this attr set/get for both
> >>>>> in-kernel and userspace use cases.
> >>>>>
> >>>>> Please adjust you patch for this. Also, as a second patch, it
> >>>>> would be great if you can convert ndo_switch_port_stp_update to
> >>>>> this new
> >> ndo.
> >>>> Why are we exposing generic switch attribute get/set from userspace
> >>>> ?. We already have specific attributes for learning/flooding which
> >>>> can be extended further.
> >>> Yes, but that is for PF_BRIDGE and bridge specific attributes. There
> >>> might be another generic attrs, no?
> >> I cant think of any. And plus, the whole point of switchdev l2
> >> offloads was to map these to existing bridge attributes. And we
> >> already have a match for some of the attributes that marco wants.
> >>
> >> If there is a need for such attributes, i don't see why it is needed
> >> for switch devices only.
> >> It is needed for any hw (nics etc). And, a precedence to this is to
> >> do it via ethtool.
> >>
> >> Having said that, am sure we will find a need for this in the future.
> >> And having a netlink attribute always helps.
> >>
> >> Today, it seems like these can be mapped to existing attributes that
> >> are settable via ndo_bridge_setlink/getlink.
> >>
> >>>> And for in kernel api....i had a sample patch in my RFC series
> >>>> (Which i was going to resubmit, until it was decided that we will
> >>>> use existing api around
> >>>> ndo_bridge_setlink/ndo_bridge_getlink):
> >>>> http://www.spinics.net/lists/netdev/msg305473.html
> >>> Yes, this might become handy for other generic non-bridge attrs.
> >>>
> >>>> Thanks,
> >>>> Roopa
> >>>>
> >>>>
> >>>>
> > The list I provided is only a subset of the attributes we will need to be
> exposed. I do have more and I'm sure that more will come in the future. As I
> mentioned in few posts earlier, some attributes are generic and some are
> not.
> >
> > I did not consider ethtool for few reasons but the main one is that I was
> under the impression that netlink was preferred in many circumstances over
> the ethotool_ops.
> That is correct. I don't think anybody hinted that you should extend ethtool.
> >   Plus, all the cases I have identified so far are going to nicely fit into the
> setlink set of operations.
> >
> 
> Would be better if you submitted your iproute2 patch with this patch.
> 
> I do plan to resubmit my generic ndo patch soon.
> 
> Thanks,
> Roopa

I honestly do not understand what extra "help" the iproute2 would have brought to this RFC: that patch simply adds a new section for the iproute2 help and a new args parser for the input. From an infrastructure perspective is leveraging what netlink messages are using RTM_SETLINK hence hooking up eventually in the do_setlink(). Sure, obviously contains all the attributes I have in mind but from an infrastructure patch perspective I don't think that you would have gained much in seeing it.

Anyway, good to know you're reworking you generic patch. I'll keep an eye out for your new NDO.


Thanks,
Marco

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Arad, Ronen]

The proposed patch introduces a way for supporting device specific switch port attributes.
Can we expect user-space tools such as iproute2 to be aware of such attributes from every device?
A generic tool like iproute2 can't be aware of all the specific attributes of all the devices that will use the newly proposed ndo.
Do we need a generic mechanism for a device to expose to user-space the set of device specific attributes it supports?
Exported information should include:
- Attribute keyword - will be used by iproute2 to parse user input and display in device specific help
- Attribute type - the numeric value for the 'attr' argument of ndo_switch_port_set_cfg().
- Attribute value range - range of supported values for the attribute
- Attribute help
  Note: A generic ndo patch as suggested by Roopa requires going beyond simple range to make it useable by generic user-space tool like iproute2.

With such mechanism in place iproute2 could provide end-user friendly experience in a generic way.

-Ronen

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Varlese, Marco
> Sent: Monday, December 15, 2014 1:40 AM
> To: Roopa Prabhu
> Cc: Jiri Pirko; John Fastabend; netdev@vger.kernel.org;
> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
> linux-kernel@vger.kernel.org
> Subject: RE: [RFC PATCH net-next 1/1] net: Support for switch port
> configuration
> 
> > -----Original Message-----
> > From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
> > Sent: Saturday, December 13, 2014 7:06 AM
> > To: Varlese, Marco
> > Cc: Jiri Pirko; John Fastabend; netdev@vger.kernel.org;
> > stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> > configuration
> >
> > On 12/12/14, 1:19 AM, Varlese, Marco wrote:
> > >> -----Original Message-----
> > >> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
> > >> Sent: Thursday, December 11, 2014 5:41 PM
> > >> To: Jiri Pirko
> > >> Cc: Varlese, Marco; John Fastabend; netdev@vger.kernel.org;
> > >> stephen@networkplumber.org; Fastabend, John R;
> sfeldma@gmail.com;
> > >> linux-kernel@vger.kernel.org
> > >> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> > >> configuration
> > >>
> > >> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
> > >>> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com
> > wrote:
> > >>>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
> > >>>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com
> wrote:
> > >>>>>>> -----Original Message-----
> > >>>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
> > >>>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
> > >>>>>>> To: Jiri Pirko
> > >>>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
> > >>>>>>> stephen@networkplumber.org; Fastabend, John R;
> > >>>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> > >>>>>>> kernel@vger.kernel.org
> > >>>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch
> > >>>>>>> port configuration
> > >>>>>>>
> > >>>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> > >>>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com
> > >> 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 extends iproute2 to call
> > >>>>>>>>> the new NDO.
> > >>>>>>>> What are these attributes? Can you give some examples. I'm
> > >>>>>>>> asking because there is a plan to pass generic attributes to
> > >>>>>>>> switch ports replacing current specific
> > >>>>>>>> ndo_switch_port_stp_update. In this case, bridge is setting
> > >>>>>>>> that
> > attribute.
> > >>>>>>>>
> > >>>>>>>> Is there need to set something directly from userspace or
> > >>>>>>>> does it make rather sense to use involved bridge/ovs/bond ? I
> > >>>>>>>> think that both will be needed.
> > >>>>>>> +1
> > >>>>>>>
> > >>>>>>> I think for many attributes it would be best to have both. The
> > >>>>>>> in kernel callers and netlink userspace can use the same
> > >>>>>>> driver
> > ndo_ops.
> > >>>>>>>
> > >>>>>>> But then we don't _require_ any specific bridge/ovs/etc module.
> > >>>>>>> And we may have some attributes that are not specific to any
> > >>>>>>> existing software module. I'm guessing Marco has some examples
> > >>>>>>> of
> > >> these.
> > >>>>>>> [...]
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> --
> > >>>>>>> John Fastabend         Intel Corporation
> > >>>>>> We do have a need to configure the attributes directly from
> > >>>>>> user-space
> > >> and I have identified the tool to do that in iproute2.
> > >>>>>> An example of attributes are:
> > >>>>>> * enabling/disabling of learning of source addresses on a given
> > >>>>>> port (you can imagine the attribute called LEARNING for
> > >>>>>> example);
> > >>>>>> * internal loopback control (i.e. LOOPBACK) which will control
> > >>>>>> how the flow of traffic behaves from the switch fabric towards
> > >>>>>> an egress port;
> > >>>>>> * flooding for broadcast/multicast/unicast type of packets (i.e.
> > >>>>>> BFLOODING, MFLOODING, UFLOODING);
> > >>>>>>
> > >>>>>> Some attributes would be of the type enabled/disabled while
> > >>>>>> other will
> > >> allow specific values to allow the user to configure different
> > >> behaviours of that feature on that particular port on that platform.
> > >>>>>> One thing to mention - as John stated as well - there might be
> > >>>>>> some
> > >> attributes that are not specific to any software module but rather
> > >> have to do with the actual hardware/platform to configure.
> > >>>>>> I hope this clarifies some points.
> > >>>>> It does. Makes sense. We need to expose this attr set/get for
> > >>>>> both in-kernel and userspace use cases.
> > >>>>>
> > >>>>> Please adjust you patch for this. Also, as a second patch, it
> > >>>>> would be great if you can convert ndo_switch_port_stp_update to
> > >>>>> this new
> > >> ndo.
> > >>>> Why are we exposing generic switch attribute get/set from
> > >>>> userspace ?. We already have specific attributes for
> > >>>> learning/flooding which can be extended further.
> > >>> Yes, but that is for PF_BRIDGE and bridge specific attributes.
> > >>> There might be another generic attrs, no?
> > >> I cant think of any. And plus, the whole point of switchdev l2
> > >> offloads was to map these to existing bridge attributes. And we
> > >> already have a match for some of the attributes that marco wants.
> > >>
> > >> If there is a need for such attributes, i don't see why it is
> > >> needed for switch devices only.
> > >> It is needed for any hw (nics etc). And, a precedence to this is to
> > >> do it via ethtool.
> > >>
> > >> Having said that, am sure we will find a need for this in the future.
> > >> And having a netlink attribute always helps.
> > >>
> > >> Today, it seems like these can be mapped to existing attributes
> > >> that are settable via ndo_bridge_setlink/getlink.
> > >>
> > >>>> And for in kernel api....i had a sample patch in my RFC series
> > >>>> (Which i was going to resubmit, until it was decided that we will
> > >>>> use existing api around
> > >>>> ndo_bridge_setlink/ndo_bridge_getlink):
> > >>>> http://www.spinics.net/lists/netdev/msg305473.html
> > >>> Yes, this might become handy for other generic non-bridge attrs.
> > >>>
> > >>>> Thanks,
> > >>>> Roopa
> > >>>>
> > >>>>
> > >>>>
> > > The list I provided is only a subset of the attributes we will need
> > > to be
> > exposed. I do have more and I'm sure that more will come in the
> > future. As I mentioned in few posts earlier, some attributes are
> > generic and some are not.
> > >
> > > I did not consider ethtool for few reasons but the main one is that
> > > I was
> > under the impression that netlink was preferred in many circumstances
> > over the ethotool_ops.
> > That is correct. I don't think anybody hinted that you should extend
> ethtool.
> > >   Plus, all the cases I have identified so far are going to nicely
> > > fit into the
> > setlink set of operations.
> > >
> >
> > Would be better if you submitted your iproute2 patch with this patch.
> >
> > I do plan to resubmit my generic ndo patch soon.
> >
> > Thanks,
> > Roopa
> 
> I honestly do not understand what extra "help" the iproute2 would have
> brought to this RFC: that patch simply adds a new section for the iproute2
> help and a new args parser for the input. From an infrastructure perspective
> is leveraging what netlink messages are using RTM_SETLINK hence hooking
> up eventually in the do_setlink(). Sure, obviously contains all the attributes I
> have in mind but from an infrastructure patch perspective I don't think that
> you would have gained much in seeing it.
> 
> Anyway, good to know you're reworking you generic patch. I'll keep an eye
> out for your new NDO.
> 
> 
> Thanks,
> Marco
> 
> --
> 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

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Roopa Prabhu]

On 12/15/14, 1:39 AM, Varlese, Marco wrote:
>> -----Original Message-----
>> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
>> Sent: Saturday, December 13, 2014 7:06 AM
>> To: Varlese, Marco
>> Cc: Jiri Pirko; John Fastabend; netdev@vger.kernel.org;
>> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
>> linux-kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>> configuration
>>
>> On 12/12/14, 1:19 AM, Varlese, Marco wrote:
>>>> -----Original Message-----
>>>> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
>>>> Sent: Thursday, December 11, 2014 5:41 PM
>>>> To: Jiri Pirko
>>>> Cc: Varlese, Marco; John Fastabend; netdev@vger.kernel.org;
>>>> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com;
>>>> linux-kernel@vger.kernel.org
>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
>>>> configuration
>>>>
>>>> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
>>>>> Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com
>> wrote:
>>>>>> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
>>>>>>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>>>>>>>>> Sent: Wednesday, December 10, 2014 5:04 PM
>>>>>>>>> To: Jiri Pirko
>>>>>>>>> Cc: Varlese, Marco; netdev@vger.kernel.org;
>>>>>>>>> stephen@networkplumber.org; Fastabend, John R;
>>>>>>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
>>>>>>>>> kernel@vger.kernel.org
>>>>>>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch
>>>>>>>>> port configuration
>>>>>>>>>
>>>>>>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
>>>>>>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com
>>>> 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 extends iproute2 to call
>>>>>>>>>>> the new NDO.
>>>>>>>>>> What are these attributes? Can you give some examples. I'm
>>>>>>>>>> asking because there is a plan to pass generic attributes to
>>>>>>>>>> switch ports replacing current specific
>>>>>>>>>> ndo_switch_port_stp_update. In this case, bridge is setting that
>> attribute.
>>>>>>>>>> Is there need to set something directly from userspace or does
>>>>>>>>>> it make rather sense to use involved bridge/ovs/bond ? I think
>>>>>>>>>> that both will be needed.
>>>>>>>>> +1
>>>>>>>>>
>>>>>>>>> I think for many attributes it would be best to have both. The
>>>>>>>>> in kernel callers and netlink userspace can use the same driver
>> ndo_ops.
>>>>>>>>> But then we don't _require_ any specific bridge/ovs/etc module.
>>>>>>>>> And we may have some attributes that are not specific to any
>>>>>>>>> existing software module. I'm guessing Marco has some examples
>>>>>>>>> of
>>>> these.
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> John Fastabend         Intel Corporation
>>>>>>>> We do have a need to configure the attributes directly from
>>>>>>>> user-space
>>>> and I have identified the tool to do that in iproute2.
>>>>>>>> An example of attributes are:
>>>>>>>> * enabling/disabling of learning of source addresses on a given
>>>>>>>> port (you can imagine the attribute called LEARNING for example);
>>>>>>>> * internal loopback control (i.e. LOOPBACK) which will control
>>>>>>>> how the flow of traffic behaves from the switch fabric towards an
>>>>>>>> egress port;
>>>>>>>> * flooding for broadcast/multicast/unicast type of packets (i.e.
>>>>>>>> BFLOODING, MFLOODING, UFLOODING);
>>>>>>>>
>>>>>>>> Some attributes would be of the type enabled/disabled while other
>>>>>>>> will
>>>> allow specific values to allow the user to configure different
>>>> behaviours of that feature on that particular port on that platform.
>>>>>>>> One thing to mention - as John stated as well - there might be
>>>>>>>> some
>>>> attributes that are not specific to any software module but rather
>>>> have to do with the actual hardware/platform to configure.
>>>>>>>> I hope this clarifies some points.
>>>>>>> It does. Makes sense. We need to expose this attr set/get for both
>>>>>>> in-kernel and userspace use cases.
>>>>>>>
>>>>>>> Please adjust you patch for this. Also, as a second patch, it
>>>>>>> would be great if you can convert ndo_switch_port_stp_update to
>>>>>>> this new
>>>> ndo.
>>>>>> Why are we exposing generic switch attribute get/set from userspace
>>>>>> ?. We already have specific attributes for learning/flooding which
>>>>>> can be extended further.
>>>>> Yes, but that is for PF_BRIDGE and bridge specific attributes. There
>>>>> might be another generic attrs, no?
>>>> I cant think of any. And plus, the whole point of switchdev l2
>>>> offloads was to map these to existing bridge attributes. And we
>>>> already have a match for some of the attributes that marco wants.
>>>>
>>>> If there is a need for such attributes, i don't see why it is needed
>>>> for switch devices only.
>>>> It is needed for any hw (nics etc). And, a precedence to this is to
>>>> do it via ethtool.
>>>>
>>>> Having said that, am sure we will find a need for this in the future.
>>>> And having a netlink attribute always helps.
>>>>
>>>> Today, it seems like these can be mapped to existing attributes that
>>>> are settable via ndo_bridge_setlink/getlink.
>>>>
>>>>>> And for in kernel api....i had a sample patch in my RFC series
>>>>>> (Which i was going to resubmit, until it was decided that we will
>>>>>> use existing api around
>>>>>> ndo_bridge_setlink/ndo_bridge_getlink):
>>>>>> http://www.spinics.net/lists/netdev/msg305473.html
>>>>> Yes, this might become handy for other generic non-bridge attrs.
>>>>>
>>>>>> Thanks,
>>>>>> Roopa
>>>>>>
>>>>>>
>>>>>>
>>> The list I provided is only a subset of the attributes we will need to be
>> exposed. I do have more and I'm sure that more will come in the future. As I
>> mentioned in few posts earlier, some attributes are generic and some are
>> not.
>>> I did not consider ethtool for few reasons but the main one is that I was
>> under the impression that netlink was preferred in many circumstances over
>> the ethotool_ops.
>> That is correct. I don't think anybody hinted that you should extend ethtool.
>>>    Plus, all the cases I have identified so far are going to nicely fit into the
>> setlink set of operations.
>> Would be better if you submitted your iproute2 patch with this patch.
>>
>> I do plan to resubmit my generic ndo patch soon.
>>
>> Thanks,
>> Roopa
> I honestly do not understand what extra "help" the iproute2 would have brought to this RFC: that patch simply adds a new section for the iproute2 help and a new args parser for the input. From an infrastructure perspective is leveraging what netlink messages are using RTM_SETLINK hence hooking up eventually in the do_setlink(). Sure, obviously contains all the attributes I have in mind but from an infrastructure patch perspective I don't think that you would have gained much in seeing it.
correct. But you mentioned iproute2 changes in your patch comment. And 
since i was not getting a clear understanding of what these attributes 
were...from your current patch..., i thought your iproute2 patch might 
shed some light on how you plan to handle these attributes.

>
> Anyway, good to know you're reworking you generic patch. I'll keep an eye out for your new NDO.
>
>
> Thanks,
> Marco
>

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Rosen, Rami]

Hi, all,

Regarding preferring using netlink sockets versus ethtool IOCTLs for setting kernel network attributes from userspace, I fully agree with Marco. The netlink API is much more structured and
much more geared towards this type of operation, than the IOCTL-based ethtool. 

Regards,
Rami Rosen
Software Engineer, Intel

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Varlese, Marco
Sent: Friday, December 12, 2014 11:20
To: Roopa Prabhu; Jiri Pirko
Cc: John Fastabend; netdev@vger.kernel.org; stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com; linux-kernel@vger.kernel.org
Subject: RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

> -----Original Message-----
> From: Roopa Prabhu [mailto:roopa@cumulusnetworks.com]
> Sent: Thursday, December 11, 2014 5:41 PM
> To: Jiri Pirko
> Cc: Varlese, Marco; John Fastabend; netdev@vger.kernel.org; 
> stephen@networkplumber.org; Fastabend, John R; sfeldma@gmail.com; 
> linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port 
> configuration
> 
> On 12/11/14, 8:56 AM, Jiri Pirko wrote:
> > Thu, Dec 11, 2014 at 05:37:46PM CET, roopa@cumulusnetworks.com wrote:
> >> On 12/11/14, 3:01 AM, Jiri Pirko wrote:
> >>> Thu, Dec 11, 2014 at 10:59:42AM CET, marco.varlese@intel.com wrote:
> >>>>> -----Original Message-----
> >>>>> From: John Fastabend [mailto:john.fastabend@gmail.com]
> >>>>> Sent: Wednesday, December 10, 2014 5:04 PM
> >>>>> To: Jiri Pirko
> >>>>> Cc: Varlese, Marco; netdev@vger.kernel.org; 
> >>>>> stephen@networkplumber.org; Fastabend, John R; 
> >>>>> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux- 
> >>>>> kernel@vger.kernel.org
> >>>>> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch 
> >>>>> port configuration
> >>>>>
> >>>>> On 12/10/2014 08:50 AM, Jiri Pirko wrote:
> >>>>>> Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@intel.com
> 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 extends iproute2 to call 
> >>>>>>> the new NDO.
> >>>>>> What are these attributes? Can you give some examples. I'm 
> >>>>>> asking because there is a plan to pass generic attributes to 
> >>>>>> switch ports replacing current specific 
> >>>>>> ndo_switch_port_stp_update. In this case, bridge is setting that attribute.
> >>>>>>
> >>>>>> Is there need to set something directly from userspace or does 
> >>>>>> it make rather sense to use involved bridge/ovs/bond ? I think 
> >>>>>> that both will be needed.
> >>>>> +1
> >>>>>
> >>>>> I think for many attributes it would be best to have both. The 
> >>>>> in kernel callers and netlink userspace can use the same driver ndo_ops.
> >>>>>
> >>>>> But then we don't _require_ any specific bridge/ovs/etc module.
> >>>>> And we may have some attributes that are not specific to any 
> >>>>> existing software module. I'm guessing Marco has some examples 
> >>>>> of
> these.
> >>>>>
> >>>>> [...]
> >>>>>
> >>>>>
> >>>>> --
> >>>>> John Fastabend         Intel Corporation
> >>>> We do have a need to configure the attributes directly from 
> >>>> user-space
> and I have identified the tool to do that in iproute2.
> >>>>
> >>>> An example of attributes are:
> >>>> * enabling/disabling of learning of source addresses on a given 
> >>>> port (you can imagine the attribute called LEARNING for example);
> >>>> * internal loopback control (i.e. LOOPBACK) which will control 
> >>>> how the flow of traffic behaves from the switch fabric towards an 
> >>>> egress port;
> >>>> * flooding for broadcast/multicast/unicast type of packets (i.e.
> >>>> BFLOODING, MFLOODING, UFLOODING);
> >>>>
> >>>> Some attributes would be of the type enabled/disabled while other 
> >>>> will
> allow specific values to allow the user to configure different 
> behaviours of that feature on that particular port on that platform.
> >>>>
> >>>> One thing to mention - as John stated as well - there might be 
> >>>> some
> attributes that are not specific to any software module but rather 
> have to do with the actual hardware/platform to configure.
> >>>>
> >>>> I hope this clarifies some points.
> >>> It does. Makes sense. We need to expose this attr set/get for both 
> >>> in-kernel and userspace use cases.
> >>>
> >>> Please adjust you patch for this. Also, as a second patch, it 
> >>> would be great if you can convert ndo_switch_port_stp_update to 
> >>> this new
> ndo.
> >> Why are we exposing generic switch attribute get/set from userspace 
> >> ?. We already have specific attributes for learning/flooding which 
> >> can be extended further.
> > Yes, but that is for PF_BRIDGE and bridge specific attributes. There 
> > might be another generic attrs, no?
> I cant think of any. And plus, the whole point of switchdev l2 
> offloads was to map these to existing bridge attributes. And we 
> already have a match for some of the attributes that marco wants.
> 
> If there is a need for such attributes, i don't see why it is needed 
> for switch devices only.
> It is needed for any hw (nics etc). And, a precedence to this is to do 
> it via ethtool.
> 
> Having said that, am sure we will find a need for this in the future.
> And having a netlink attribute always helps.
> 
> Today, it seems like these can be mapped to existing attributes that 
> are settable via ndo_bridge_setlink/getlink.
> 
> >
> >> And for in kernel api....i had a sample patch in my RFC series 
> >> (Which i was going to resubmit, until it was decided that we will 
> >> use existing api around
> >> ndo_bridge_setlink/ndo_bridge_getlink):
> >> http://www.spinics.net/lists/netdev/msg305473.html
> > Yes, this might become handy for other generic non-bridge attrs.
> >
> >> Thanks,
> >> Roopa
> >>
> >>
> >>

The list I provided is only a subset of the attributes we will need to be exposed. I do have more and I'm sure that more will come in the future. As I mentioned in few posts earlier, some attributes are generic and some are not. 

I did not consider ethtool for few reasons but the main one is that I was under the impression that netlink was preferred in many circumstances over the ethotool_ops. Plus, all the cases I have identified so far are going to nicely fit into the setlink set of operations.

--
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
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Thomas Graf]

On 12/11/14 at 09:59am, Varlese, Marco wrote:
> An example of attributes are:
> * enabling/disabling of learning of source addresses on a given port (you can imagine the attribute called LEARNING for example);
> * internal loopback control (i.e. LOOPBACK) which will control how the flow of traffic behaves from the switch fabric towards an egress port;
> * flooding for broadcast/multicast/unicast type of packets (i.e. BFLOODING, MFLOODING, UFLOODING);

All of these are highly generic and should *not* be passed through
from user space to the driver directly but rather be properly
abstracted as Roopa proposed. The value of this API is abstraction.
If we introduce per device attributes for generic functions we lose
large portions of the value gained.

You mentioned you have additional attributes in mind, maybe you can
give a few examples which are not generic, i.e. do not apply to
multiple vendors.

RE: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Varlese, Marco]

> -----Original Message-----
> From: Thomas Graf [mailto:tgr@infradead.org] On Behalf Of Thomas Graf
> Sent: Monday, December 15, 2014 2:08 PM
> To: Varlese, Marco
> Cc: John Fastabend; Jiri Pirko; netdev@vger.kernel.org;
> stephen@networkplumber.org; Fastabend, John R;
> roopa@cumulusnetworks.com; sfeldma@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [RFC PATCH net-next 1/1] net: Support for switch port
> configuration
> 
> On 12/11/14 at 09:59am, Varlese, Marco wrote:
> > An example of attributes are:
> > * enabling/disabling of learning of source addresses on a given port
> > (you can imagine the attribute called LEARNING for example);
> > * internal loopback control (i.e. LOOPBACK) which will control how the
> > flow of traffic behaves from the switch fabric towards an egress port;
> > * flooding for broadcast/multicast/unicast type of packets (i.e.
> > BFLOODING, MFLOODING, UFLOODING);
> 
> All of these are highly generic and should *not* be passed through from user
> space to the driver directly but rather be properly abstracted as Roopa
> proposed. The value of this API is abstraction.
How would you let the user enable/disable features then? For instance, how would the user enable/disable flooding for broadcast packets (BFLOODING) on a given port? What I was proposing is to have a list of attributes (to be added in if_link.h) which can be tuned by the user using a tool like iproute2. What do you propose? 
I think I have seen Roopa posting his updated ndo patch and getting some feedback by few people already and as long as I will be able to accomplish the use case described here I am happy with his way.

> If we introduce per device attributes for generic functions we lose large
> portions of the value gained.
> You mentioned you have additional attributes in mind, maybe you can give a
> few examples which are not generic, i.e. do not apply to multiple vendors.
I do not have an example right now of a vendor specific attribute but I was just saying that might happen (i.e. someone will have a feature not implemented by others?).

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Thomas Graf]

On 12/15/14 at 02:29pm, Varlese, Marco wrote:
> > All of these are highly generic and should *not* be passed through from user
> > space to the driver directly but rather be properly abstracted as Roopa
> > proposed. The value of this API is abstraction.
> How would you let the user enable/disable features then? For instance, how would the user enable/disable flooding for broadcast packets (BFLOODING) on a given port? What I was proposing is to have a list of attributes (to be added in if_link.h) which can be tuned by the user using a tool like iproute2. What do you propose? 

Excellent, I agree with what you are saying. What set me off is that
the patch does not reflect that yet. Instead, the patch introduces
a pure Netlink pass-through API to the driver.

I would expect the patch to:
 1. Parse the Netlink messages and be aware of individual attributes
 2. Validate them
 3. Pass the configuration to the driver using an API that can also
    be consumed from in-kernel users.

> I think I have seen Roopa posting his updated ndo patch and getting some feedback by few people already and as long as I will be able to accomplish the use case described here I am happy with his way.

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.

The overlap is in the ndo. I think both the API you propose and
Roopa's bridge code should use the same NDO.

> I do not have an example right now of a vendor specific attribute but I was just saying that might happen (i.e. someone will have a feature not implemented by others?).

That's fine. Once we have them we can consider adding vendor specific
extensions.

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Roopa Prabhu]

On 12/15/14, 6:40 AM, Thomas Graf wrote:
> On 12/15/14 at 02:29pm, Varlese, Marco wrote:
>>> All of these are highly generic and should *not* be passed through from user
>>> space to the driver directly but rather be properly abstracted as Roopa
>>> proposed. The value of this API is abstraction.
>> How would you let the user enable/disable features then? For instance, how would the user enable/disable flooding for broadcast packets (BFLOODING) on a given port? What I was proposing is to have a list of attributes (to be added in if_link.h) which can be tuned by the user using a tool like iproute2. What do you propose?
> Excellent, I agree with what you are saying. What set me off is that
> the patch does not reflect that yet. Instead, the patch introduces
> a pure Netlink pass-through API to the driver.
>
> I would expect the patch to:
>   1. Parse the Netlink messages and be aware of individual attributes
>   2. Validate them
>   3. Pass the configuration to the driver using an API that can also
>      be consumed from in-kernel users.
yes, exactly.
>
>> I think I have seen Roopa posting his updated ndo patch and getting some feedback by few people already and as long as I will be able to accomplish the use case described here I am happy with his way.
> 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.
>
> The overlap is in the ndo. I think both the API you propose and
> Roopa's bridge code should use the same NDO.
>> I do not have an example right now of a vendor specific attribute but I was just saying that might happen (i.e. someone will have a feature not implemented by others?).
> That's fine. Once we have them we can consider adding vendor specific
> extensions.

Re: [RFC PATCH net-next 1/1] net: Support for switch port configuration

[Thomas Graf]

On 12/10/14 at 04:23pm, Varlese, Marco wrote:
> +#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 = nla_get_u64(v);
> +
> +		err = ops->ndo_switch_port_set_cfg(dev, op, value);
> +		if (err)
> +			break;
> +	}
> +	return err;
> +}
> +#endif

A strictly technical feedback first: I suggest to split the above into
a validation and commit part to keep Netlink operations atomic. Doing
commit & rollback for the deeply nested configuration we are heading
to will be difficult and error prone. Let's keep updates atomic for as
long as possible, i.e. individual set operations can't fail.