From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick McHardy Subject: Re: [net-next-2.6 V7 PATCH 1/2] Add netlink support for virtual port management (was iovnl) Date: Fri, 14 May 2010 12:58:32 +0200 Message-ID: <4BED2CD8.4020209@trash.net> References: <20100514013144.1816.31191.stgit@savbu-pc100.cisco.com> <20100514013526.1816.45104.stgit@savbu-pc100.cisco.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, netdev@vger.kernel.org, chrisw@redhat.com, arnd@arndb.de To: Scott Feldman Return-path: Received: from stinky.trash.net ([213.144.137.162]:42334 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753538Ab0ENK6e (ORCPT ); Fri, 14 May 2010 06:58:34 -0400 In-Reply-To: <20100514013526.1816.45104.stgit@savbu-pc100.cisco.com> Sender: netdev-owner@vger.kernel.org List-ID: Scott Feldman wrote: > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -653,6 +653,26 @@ static inline int rtnl_vfinfo_size(const struct net_device *dev) > return 0; > } > > +static size_t rtnl_vf_port_size(const struct net_device *dev) > +{ > + size_t vf_port_size = nla_total_size(sizeof(struct nlattr)) > + /* VF_PORT_VF */ > + + nla_total_size(VF_PORT_PROFILE_MAX)/* VF_PORT_PROFILE */ > + + nla_total_size(sizeof(struct ifla_vf_port_vsi)) > + /* VF_PORT_VSI_TYPE */ > + + nla_total_size(VF_PORT_UUID_MAX) /* VF_PORT_VSI_INSTANCE */ > + + nla_total_size(VF_PORT_UUID_MAX) /* VF_PORT_HOST_UUID */ > + + nla_total_size(1) /* VF_PROT_VDP_REQUEST */ Do messages generated by the kernel really contain a request? > + + nla_total_size(2); /* VF_PORT_VDP_RESPONSE */ > + > + if (!dev->netdev_ops->ndo_get_vf_port || !dev->dev.parent) > + return 0; > + if (dev_num_vf(dev->dev.parent)) > + return vf_port_size * dev_num_vf(dev->dev.parent); > + else > + return vf_port_size; > +} > + > +static int rtnl_vf_port_fill_nest(struct sk_buff *skb, struct net_device *dev, > + int vf) > +{ > + struct nlattr *data; > + int err; > + > + data = nla_nest_start(skb, IFLA_VF_PORT); We usually use a top-level attribute to encapsulate lists of identical attributes. The other iflink attributes may only occur once and are usually parsed using nla_parse_nested(), which will parse all IFLA_VF_PORT attributes, but only return the last one. Something like: iflink message: ... [IFLA_VF_PORTS] [IFLA_VF_PORT] [IFLA_VF_PORT_*], ... [IFLA_VF_PORT] [IFLA_VF_PORT_*], ... ... > + if (!data) > + return -EMSGSIZE; > + > + if (vf != VF_PORT_VF_NOT_USED) > + nla_put_u32(skb, IFLA_VF_PORT_VF, vf); This should be checking for errors or use NLA_PUT_U32. > + > + err = dev->netdev_ops->ndo_get_vf_port(dev, vf, skb); > + if (err) { > + nla_nest_cancel(skb, data); > + return err; > + } > + > + nla_nest_end(skb, data); > + > + return 0; > +} > + > static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, > int type, u32 pid, u32 seq, u32 change, > unsigned int flags) > @@ -747,17 +819,23 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, > goto nla_put_failure; > copy_rtnl_link_stats64(nla_data(attr), stats); > > + if (dev->dev.parent) > + NLA_PUT_U32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)); Just wondering, is the only case where dev.parent is non-NULL really when virtual ports are present? > + > if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent) { > int i; > struct ifla_vf_info ivi; > > - NLA_PUT_U32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)); > for (i = 0; i < dev_num_vf(dev->dev.parent); i++) { > if (dev->netdev_ops->ndo_get_vf_config(dev, i, &ivi)) > break; > NLA_PUT(skb, IFLA_VFINFO, sizeof(ivi), &ivi); > } > } > + > + if (rtnl_vf_port_fill(skb, dev)) > + goto nla_put_failure; > + > if (dev->rtnl_link_ops) { > if (rtnl_link_fill(skb, dev) < 0) > goto nla_put_failure; > @@ -824,6 +902,7 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = { > .len = sizeof(struct ifla_vf_vlan) }, > [IFLA_VF_TX_RATE] = { .type = NLA_BINARY, > .len = sizeof(struct ifla_vf_tx_rate) }, > + [IFLA_VF_PORT] = { .type = NLA_NESTED }, > }; > EXPORT_SYMBOL(ifla_policy); > > @@ -832,6 +911,20 @@ static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { > [IFLA_INFO_DATA] = { .type = NLA_NESTED }, > }; > > +static const struct nla_policy ifla_vf_port_policy[IFLA_VF_PORT_MAX+1] = { > + [IFLA_VF_PORT_VF] = { .type = NLA_U32 }, > + [IFLA_VF_PORT_PROFILE] = { .type = NLA_STRING, > + .len = VF_PORT_PROFILE_MAX }, > + [IFLA_VF_PORT_VSI_TYPE] = { .type = NLA_BINARY, > + .len = sizeof(struct ifla_vf_port_vsi)}, > + [IFLA_VF_PORT_INSTANCE_UUID]= { .type = NLA_BINARY, > + .len = VF_PORT_UUID_MAX }, > + [IFLA_VF_PORT_HOST_UUID] = { .type = NLA_STRING, > + .len = VF_PORT_UUID_MAX }, > + [IFLA_VF_PORT_REQUEST] = { .type = NLA_U8, }, > + [IFLA_VF_PORT_RESPONSE] = { .type = NLA_U16, }, > +}; > + > struct net *rtnl_link_get_net(struct net *src_net, struct nlattr *tb[]) > { > struct net *net; > @@ -1028,6 +1121,27 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, > } > err = 0; > > + if (tb[IFLA_VF_PORT]) { > + struct nlattr *vf_port[IFLA_VF_PORT_MAX+1]; > + int vf = VF_PORT_VF_NOT_USED; > + > + err = nla_parse_nested(vf_port, IFLA_VF_PORT_MAX, > + tb[IFLA_VF_PORT], ifla_vf_port_policy); > + if (err < 0) > + goto errout; > + > + if (vf_port[IFLA_VF_PORT_VF]) > + vf = nla_get_u32(vf_port[IFLA_VF_PORT_VF]); > + > + err = -EOPNOTSUPP; > + if (ops->ndo_set_vf_port) > + err = ops->ndo_set_vf_port(dev, vf, vf_port); > + if (err < 0) > + goto errout; > + modified = 1; > + } > + err = 0; > + > errout: > if (err < 0 && modified && net_ratelimit()) > printk(KERN_WARNING "A link change request failed with " >