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

From: Jiri Pirko
Date: Wed Dec 10 2014 - 11:50:26 EST


Wed, Dec 10, 2014 at 05:23:40PM CET, marco.varlese@xxxxxxxxx wrote:
>From: Marco Varlese <marco.varlese@xxxxxxxxx>
>
>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@xxxxxxxxx>
>---
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/