Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752134AbaLRLlt (ORCPT ); Thu, 18 Dec 2014 06:41:49 -0500 Received: from casper.infradead.org ([85.118.1.10]:34219 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751265AbaLRLlr (ORCPT ); Thu, 18 Dec 2014 06:41:47 -0500 Date: Thu, 18 Dec 2014 11:41:45 +0000 From: Thomas Graf To: "Varlese, Marco" Cc: "netdev@vger.kernel.org" , "Fastabend, John R" , Jiri Pirko , "roopa@cumulusnetworks.com" , "sfeldma@gmail.com" , "linux-kernel@vger.kernel.org" Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration Message-ID: <20141218114145.GH28766@casper.infradead.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/18/14 at 11:29am, Varlese, Marco wrote: > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h > index f7d0d2d..19cb51a 100644 > --- a/include/uapi/linux/if_link.h > +++ b/include/uapi/linux/if_link.h > @@ -146,6 +146,7 @@ enum { > IFLA_PHYS_PORT_ID, > IFLA_CARRIER_CHANGES, > IFLA_PHYS_SWITCH_ID, > + IFLA_SWITCH_PORT_CFG, > __IFLA_MAX > }; Needs an entry in ifla_policy[] [IFLA_SWITCH_PORT_CFG] = { .type = NLA_NESTED }, > @@ -603,4 +604,18 @@ enum { > > #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1) > > +/* Switch Port Attributes section */ > + > +enum { > + IFLA_ATTR_UNSPEC, > + IFLA_ATTR_LEARNING, > + IFLA_ATTR_LOOPBACK, > + IFLA_ATTR_BCAST_FLOODING, > + IFLA_ATTR_UCAST_FLOODING, > + IFLA_ATTR_MCAST_FLOODING, > + __IFLA_ATTR_MAX > +}; > + > +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1) Change the prefix to IFLA_SW_* since it's switch specific? > > +#ifdef CONFIG_NET_SWITCHDEV > +static int do_setswcfg(struct net_device *dev, struct nlattr *attr) > +{ > + int rem, err = -EINVAL; > + struct nlattr *v; > + const struct net_device_ops *ops = dev->netdev_ops; > + > + nla_for_each_nested(v, attr, rem) { > + u32 op = nla_type(v); > + u64 value = 0; > + > + switch (op) { > + case IFLA_ATTR_LEARNING: > + case IFLA_ATTR_LOOPBACK: > + case IFLA_ATTR_BCAST_FLOODING: > + case IFLA_ATTR_UCAST_FLOODING: > + case IFLA_ATTR_MCAST_FLOODING: { > + if (nla_len(v) < sizeof(value)) { > + err = -EINVAL; > + break; > + } You should validate the message before you start applying the changes. Otherwise if the 3rd attribute is too short you've already applied some changes and the user has not idea how much has been applied. nla_parse_nested() can help here. > + > + value = nla_get_u64(v); > + err = ops->ndo_switch_port_set_cfg(dev, > + op, > + value); This avoids having individual ndos but wastes a lot of space in the Netlink message. Not a problem when setting configuration but you likely want to dump these attributes as well and we need 12 bytes for each attribute even though some are merely flags which could fit in 4 bytes. > static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) > { > int rem, err = -EINVAL; > @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb, > status |= DO_SETLINK_NOTIFY; > } > } > +#ifdef CONFIG_NET_SWITCHDEV > + if (tb[IFLA_SWITCH_PORT_CFG]) { > + err = -EOPNOTSUPP; > + if (!ops->ndo_switch_port_set_cfg) > + goto errout; > + if (!ops->ndo_switch_parent_id_get) > + goto errout; > + err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]); > + if (err < 0) > + goto errout; > + > + status |= DO_SETLINK_NOTIFY; > + } > +#endif Should return -EOPNOTSUPP if IFLA_SWITCH_PORT_CFG is provided but CONFIG_NET_SWITCHDEV is not set. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/