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

From: Jiri Pirko
Date: Thu Dec 18 2014 - 11:30:16 EST


Thu, Dec 18, 2014 at 04:30:00PM 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 adds the "get" functionality via another
>NDO and another patch that extends iproute2 to call the two new NDOs.
>
>Signed-off-by: Marco Varlese <marco.varlese@xxxxxxxxx>
>---
> include/linux/netdevice.h | 5 +++
> include/uapi/linux/if_link.h | 15 +++++++++
> net/core/rtnetlink.c | 73 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 93 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);

How about get? Userspace should be able to read the values as well.

> #endif
> };
>
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index f7d0d2d..6ad9b91 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -146,6 +146,7 @@ enum {
> IFLA_PHYS_PORT_ID,
> IFLA_CARRIER_CHANGES,
> IFLA_PHYS_SWITCH_ID,
>+ IFLA_SWITCH_PORT_CFG,
> __IFLA_MAX
> };
>
>@@ -603,4 +604,18 @@ enum {
>
> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1)
>
>+/* Switch Port Attributes section */
>+
>+enum {
>+ IFLA_SW_UNSPEC,
>+ IFLA_SW_LEARNING,
>+ IFLA_SW_LOOPBACK,
>+ IFLA_SW_BCAST_FLOODING,
>+ IFLA_SW_UCAST_FLOODING,
>+ IFLA_SW_MCAST_FLOODING,
>+ __IFLA_SW_ATTR_MAX
>+};
>+
>+#define IFLA_SW_ATTR_MAX (__IFLA_SW_ATTR_MAX - 1)
>+
> #endif /* _UAPI_LINUX_IF_LINK_H */
>diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>index eaa057f..d50ca71 100644
>--- a/net/core/rtnetlink.c
>+++ b/net/core/rtnetlink.c
>@@ -1223,6 +1223,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
> [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN },
> [IFLA_CARRIER_CHANGES] = { .type = NLA_U32 }, /* ignored */
> [IFLA_PHYS_SWITCH_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN },
>+ [IFLA_SWITCH_PORT_CFG] = { .type = NLA_NESTED },
> };
>
> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>@@ -1265,6 +1266,14 @@ static const struct nla_policy ifla_port_policy[IFLA_PORT_MAX+1] = {
> [IFLA_PORT_RESPONSE] = { .type = NLA_U16, },
> };
>
>+static const struct nla_policy ifla_sw_attr_policy[IFLA_SW_ATTR_MAX+1] = {
>+ [IFLA_SW_LEARNING] = { .type = NLA_U64 },
>+ [IFLA_SW_LOOPBACK] = { .type = NLA_U64 },
>+ [IFLA_SW_BCAST_FLOODING] = { .type = NLA_U64 },
>+ [IFLA_SW_UCAST_FLOODING] = { .type = NLA_U64 },
>+ [IFLA_SW_MCAST_FLOODING] = { .type = NLA_U64 },

Please maintain namespaces. Use IFLA_SWITCH_PORT_LEARNING for example.
That gives us place for possible future whole-switch-cfg.


>+};
>+
> static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
> {
> struct net *net = sock_net(skb->sk);
>@@ -1389,6 +1398,41 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[])
> return 0;
> }
>
>+#ifdef CONFIG_NET_SWITCHDEV
>+static int do_setswcfg(struct net_device *dev, struct nlattr *attr)
>+{
>+ int rem, err = -EINVAL;
>+ struct nlattr *v;
>+ const struct net_device_ops *ops = dev->netdev_ops;
>+
>+ nla_for_each_nested(v, attr, rem) {
>+ u32 op = nla_type(v);
>+ u64 value = 0;
>+
>+ switch (op) {
>+ case IFLA_SW_LEARNING:
>+ case IFLA_SW_LOOPBACK:
>+ case IFLA_SW_BCAST_FLOODING:
>+ case IFLA_SW_UCAST_FLOODING:
>+ case IFLA_SW_MCAST_FLOODING: {
>+ value = nla_get_u64(v);
>+ err = ops->ndo_switch_port_set_cfg(dev,
>+ op,
>+ value);
>+ break;
>+ }
>+ default:
>+ err = -EINVAL;
>+ break;
>+ }
>+ if (err)
>+ break;
>+ }
>+ return err;
>+}
>+
>+#endif
>+
> static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
> {
> int rem, err = -EINVAL;
>@@ -1740,6 +1784,35 @@ static int do_setlink(const struct sk_buff *skb,
> status |= DO_SETLINK_NOTIFY;
> }
> }
>+#ifdef CONFIG_NET_SWITCHDEV
>+ if (tb[IFLA_SWITCH_PORT_CFG]) {
>+ struct nlattr *attrs[IFLA_SW_ATTR_MAX+1];
>+
>+ err = -EOPNOTSUPP;
>+ if (!ops->ndo_switch_port_set_cfg)
>+ goto errout;
>+ if (!ops->ndo_switch_parent_id_get)
>+ goto errout;
>+
>+ err = nla_parse_nested(attrs, IFLA_SW_ATTR_MAX,
>+ tb[IFLA_SWITCH_PORT_CFG],
>+ ifla_sw_attr_policy);
>+ if (err < 0)
>+ return err;
>+
>+ err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]);
>+ if (err < 0)
>+ goto errout;
>+


Would make sense to me to move this including do_setswcfg into
net/switchdev/switchdev.c


>+ status |= DO_SETLINK_NOTIFY;
>+ }
>+#else
>+ if (tb[IFLA_SWITCH_PORT_CFG]) {
>+ err = -EOPNOTSUPP;
>+ goto errout;
>+ }
>+#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/