| Message ID | 20100515031416.GE15313@sequoia.sous-sol.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Saturday 15 May 2010 05:14:16 Chris Wright wrote: > Now we have a set of nested attributes: > > IFLA_VFINFO_LIST (NESTED) > IFLA_VF_INFO (NESTED) > IFLA_VF_MAC > IFLA_VF_VLAN > IFLA_VF_TX_RATE > > This allows a single set to operate on multiple attributes if desired. > Among other things, it means a dump can be replayed to set state. > > The current interface has yet to be released, so this seems like > something to consider for 2.6.34. > > Signed-off-by: Chris Wright <chrisw@sous-sol.org Very nice! This would be the minimum change to make the ABI conform to the general rules, so it would be really good to have that. Acked-by: Arnd Bergmann <arnd@arndb.de> It does make the interface a bit strange (less than before), since the new IFLA_VF_INFO now contains three nested attributes that each contain their own vf number field, and we don't require that they are identical or that each of the nested attributes inside VF_INFO appears only once. How about a second patch that splits out an IFLA_VF_NUMBER attribute and makes do_setvfinfo use nla_parse_nested instead of nla_for_each_nested in order to tighten the rules on this some more? Arnd -- 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
From: Chris Wright <chrisw@sous-sol.org> Date: Fri, 14 May 2010 20:14:16 -0700 > Now we have a set of nested attributes: > > IFLA_VFINFO_LIST (NESTED) > IFLA_VF_INFO (NESTED) > IFLA_VF_MAC > IFLA_VF_VLAN > IFLA_VF_TX_RATE > > This allows a single set to operate on multiple attributes if desired. > Among other things, it means a dump can be replayed to set state. > > The current interface has yet to be released, so this seems like > something to consider for 2.6.34. > > Signed-off-by: Chris Wright <chrisw@sous-sol.org Agreed, applied to net-2.6, thanks Chris! -- 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
On Fri, 14 May 2010 20:14:16 -0700 Chris Wright <chrisw@sous-sol.org> wrote: > Now we have a set of nested attributes: > > IFLA_VFINFO_LIST (NESTED) > IFLA_VF_INFO (NESTED) > IFLA_VF_MAC > IFLA_VF_VLAN > IFLA_VF_TX_RATE > > This allows a single set to operate on multiple attributes if desired. > Among other things, it means a dump can be replayed to set state. > > The current interface has yet to be released, so this seems like > something to consider for 2.6.34. > > Signed-off-by: Chris Wright <chrisw@sous-sol.org > --- iproute2 update please? Also I would really like documentation on this.
>-----Original Message----- >From: Stephen Hemminger [mailto:shemminger@vyatta.com] >Sent: Monday, May 17, 2010 9:00 AM >To: Chris Wright >Cc: davem@davemloft.net; kaber@trash.net; Williams, Mitch A; >arnd@arndb.de; scofeldm@cisco.com; netdev@vger.kernel.org >Subject: Re: [PATCH] rtnetlink: make SR-IOV VF interface symmetric > >On Fri, 14 May 2010 20:14:16 -0700 >Chris Wright <chrisw@sous-sol.org> wrote: > >> Now we have a set of nested attributes: >> >> IFLA_VFINFO_LIST (NESTED) >> IFLA_VF_INFO (NESTED) >> IFLA_VF_MAC >> IFLA_VF_VLAN >> IFLA_VF_TX_RATE >> >> This allows a single set to operate on multiple attributes if desired. >> Among other things, it means a dump can be replayed to set state. >> >> The current interface has yet to be released, so this seems like >> something to consider for 2.6.34. >> >> Signed-off-by: Chris Wright <chrisw@sous-sol.org >> --- > >iproute2 update please? > >Also I would really like documentation on this. > > >-- Chris, have you got the iproute2 parts working, or do I need to pick it up? Thanks again for your work on this. -Mitch -- 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
* Williams, Mitch A (mitch.a.williams@intel.com) wrote: > >-----Original Message----- > >From: Stephen Hemminger [mailto:shemminger@vyatta.com] > >Sent: Monday, May 17, 2010 9:00 AM > >To: Chris Wright > >Cc: davem@davemloft.net; kaber@trash.net; Williams, Mitch A; > >arnd@arndb.de; scofeldm@cisco.com; netdev@vger.kernel.org > >Subject: Re: [PATCH] rtnetlink: make SR-IOV VF interface symmetric > > > >On Fri, 14 May 2010 20:14:16 -0700 > >Chris Wright <chrisw@sous-sol.org> wrote: > > > >> Now we have a set of nested attributes: > >> > >> IFLA_VFINFO_LIST (NESTED) > >> IFLA_VF_INFO (NESTED) > >> IFLA_VF_MAC > >> IFLA_VF_VLAN > >> IFLA_VF_TX_RATE > >> > >> This allows a single set to operate on multiple attributes if desired. > >> Among other things, it means a dump can be replayed to set state. > >> > >> The current interface has yet to be released, so this seems like > >> something to consider for 2.6.34. > >> > >> Signed-off-by: Chris Wright <chrisw@sous-sol.org > >> --- > > > >iproute2 update please? > > > >Also I would really like documentation on this. And a pony? ;-) Docs in what form? > Chris, have you got the iproute2 parts working, or do I need to pick it up? > > Thanks again for your work on this. I've got those bits, just was waiting until patches merged. Will send them out this afternoon. thanks, -chris -- 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
* Arnd Bergmann (arnd@arndb.de) wrote: > On Saturday 15 May 2010 05:14:16 Chris Wright wrote: > > Now we have a set of nested attributes: > > > > IFLA_VFINFO_LIST (NESTED) > > IFLA_VF_INFO (NESTED) > > IFLA_VF_MAC > > IFLA_VF_VLAN > > IFLA_VF_TX_RATE > > > > This allows a single set to operate on multiple attributes if desired. > > Among other things, it means a dump can be replayed to set state. > > > > The current interface has yet to be released, so this seems like > > something to consider for 2.6.34. > > > > Signed-off-by: Chris Wright <chrisw@sous-sol.org > > Very nice! This would be the minimum change to make the ABI conform > to the general rules, so it would be really good to have that. > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > It does make the interface a bit strange (less than before), since the > new IFLA_VF_INFO now contains three nested attributes that each contain their > own vf number field, and we don't require that they are identical or that > each of the nested attributes inside VF_INFO appears only once. > > How about a second patch that splits out an IFLA_VF_NUMBER attribute > and makes do_setvfinfo use nla_parse_nested instead of nla_for_each_nested > in order to tighten the rules on this some more? Yes, that's a great idea Arnd. I'll tighten that up. thanks, -chris -- 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
On Mon, 17 May 2010 09:07:32 -0700 Chris Wright <chrisw@sous-sol.org> wrote: > * Williams, Mitch A (mitch.a.williams@intel.com) wrote: > > >-----Original Message----- > > >From: Stephen Hemminger [mailto:shemminger@vyatta.com] > > >Sent: Monday, May 17, 2010 9:00 AM > > >To: Chris Wright > > >Cc: davem@davemloft.net; kaber@trash.net; Williams, Mitch A; > > >arnd@arndb.de; scofeldm@cisco.com; netdev@vger.kernel.org > > >Subject: Re: [PATCH] rtnetlink: make SR-IOV VF interface symmetric > > > > > >On Fri, 14 May 2010 20:14:16 -0700 > > >Chris Wright <chrisw@sous-sol.org> wrote: > > > > > >> Now we have a set of nested attributes: > > >> > > >> IFLA_VFINFO_LIST (NESTED) > > >> IFLA_VF_INFO (NESTED) > > >> IFLA_VF_MAC > > >> IFLA_VF_VLAN > > >> IFLA_VF_TX_RATE > > >> > > >> This allows a single set to operate on multiple attributes if desired. > > >> Among other things, it means a dump can be replayed to set state. > > >> > > >> The current interface has yet to be released, so this seems like > > >> something to consider for 2.6.34. > > >> > > >> Signed-off-by: Chris Wright <chrisw@sous-sol.org > > >> --- > > > > > >iproute2 update please? > > > > > >Also I would really like documentation on this. > > And a pony? ;-) > > Docs in what form? > The man page for ip.8 already has some pieces from other version. Just fix them. I just pushed up a couple of changes.
diff --git a/include/linux/if_link.h b/include/linux/if_link.h index c9bf92c..d94963b 100644 --- a/include/linux/if_link.h +++ b/include/linux/if_link.h @@ -79,10 +79,7 @@ enum { IFLA_NET_NS_PID, IFLA_IFALIAS, IFLA_NUM_VF, /* Number of VFs if device is SR-IOV PF */ - IFLA_VF_MAC, /* Hardware queue specific attributes */ - IFLA_VF_VLAN, - IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */ - IFLA_VFINFO, + IFLA_VFINFO_LIST, __IFLA_MAX }; @@ -203,6 +200,24 @@ enum macvlan_mode { /* SR-IOV virtual function managment section */ +enum { + IFLA_VF_INFO_UNSPEC, + IFLA_VF_INFO, + __IFLA_VF_INFO_MAX, +}; + +#define IFLA_VF_INFO_MAX (__IFLA_VF_INFO_MAX - 1) + +enum { + IFLA_VF_UNSPEC, + IFLA_VF_MAC, /* Hardware queue specific attributes */ + IFLA_VF_VLAN, + IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */ + __IFLA_VF_MAX, +}; + +#define IFLA_VF_MAX (__IFLA_VF_MAX - 1) + struct ifla_vf_mac { __u32 vf; __u8 mac[32]; /* MAX_ADDR_LEN */ diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index fe776c9..074afcd 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -602,12 +602,19 @@ static void copy_rtnl_link_stats(struct rtnl_link_stats *a, a->tx_compressed = b->tx_compressed; }; +/* All VF info */ static inline int rtnl_vfinfo_size(const struct net_device *dev) { - if (dev->dev.parent && dev_is_pci(dev->dev.parent)) - return dev_num_vf(dev->dev.parent) * - sizeof(struct ifla_vf_info); - else + if (dev->dev.parent && dev_is_pci(dev->dev.parent)) { + + int num_vfs = dev_num_vf(dev->dev.parent); + size_t size = nlmsg_total_size(sizeof(struct nlattr)); + size += nlmsg_total_size(num_vfs * sizeof(struct nlattr)); + size += num_vfs * (sizeof(struct ifla_vf_mac) + + sizeof(struct ifla_vf_vlan) + + sizeof(struct ifla_vf_tx_rate)); + return size; + } else return 0; } @@ -629,7 +636,7 @@ static inline size_t if_nlmsg_size(const struct net_device *dev) + nla_total_size(1) /* IFLA_OPERSTATE */ + nla_total_size(1) /* IFLA_LINKMODE */ + nla_total_size(4) /* IFLA_NUM_VF */ - + nla_total_size(rtnl_vfinfo_size(dev)) /* IFLA_VFINFO */ + + rtnl_vfinfo_size(dev) /* IFLA_VFINFO_LIST */ + rtnl_link_get_size(dev); /* IFLA_LINKINFO */ } @@ -700,14 +707,37 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, 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++) { + struct nlattr *vfinfo, *vf; + int num_vfs = dev_num_vf(dev->dev.parent); + + NLA_PUT_U32(skb, IFLA_NUM_VF, num_vfs); + vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST); + if (!vfinfo) + goto nla_put_failure; + for (i = 0; i < num_vfs; i++) { + struct ifla_vf_info ivi; + struct ifla_vf_mac vf_mac; + struct ifla_vf_vlan vf_vlan; + struct ifla_vf_tx_rate vf_tx_rate; if (dev->netdev_ops->ndo_get_vf_config(dev, i, &ivi)) break; - NLA_PUT(skb, IFLA_VFINFO, sizeof(ivi), &ivi); + vf_mac.vf = vf_vlan.vf = vf_tx_rate.vf = ivi.vf; + memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac)); + vf_vlan.vlan = ivi.vlan; + vf_vlan.qos = ivi.qos; + vf_tx_rate.rate = ivi.tx_rate; + vf = nla_nest_start(skb, IFLA_VF_INFO); + if (!vf) { + nla_nest_cancel(skb, vfinfo); + goto nla_put_failure; + } + NLA_PUT(skb, IFLA_VF_MAC, sizeof(vf_mac), &vf_mac); + NLA_PUT(skb, IFLA_VF_VLAN, sizeof(vf_vlan), &vf_vlan); + NLA_PUT(skb, IFLA_VF_TX_RATE, sizeof(vf_tx_rate), &vf_tx_rate); + nla_nest_end(skb, vf); } + nla_nest_end(skb, vfinfo); } if (dev->rtnl_link_ops) { if (rtnl_link_fill(skb, dev) < 0) @@ -769,12 +799,7 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = { [IFLA_LINKINFO] = { .type = NLA_NESTED }, [IFLA_NET_NS_PID] = { .type = NLA_U32 }, [IFLA_IFALIAS] = { .type = NLA_STRING, .len = IFALIASZ-1 }, - [IFLA_VF_MAC] = { .type = NLA_BINARY, - .len = sizeof(struct ifla_vf_mac) }, - [IFLA_VF_VLAN] = { .type = NLA_BINARY, - .len = sizeof(struct ifla_vf_vlan) }, - [IFLA_VF_TX_RATE] = { .type = NLA_BINARY, - .len = sizeof(struct ifla_vf_tx_rate) }, + [IFLA_VFINFO_LIST] = {. type = NLA_NESTED }, }; EXPORT_SYMBOL(ifla_policy); @@ -783,6 +808,19 @@ static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { [IFLA_INFO_DATA] = { .type = NLA_NESTED }, }; +static const struct nla_policy ifla_vfinfo_policy[IFLA_VF_INFO_MAX+1] = { + [IFLA_VF_INFO] = { .type = NLA_NESTED }, +}; + +static const struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = { + [IFLA_VF_MAC] = { .type = NLA_BINARY, + .len = sizeof(struct ifla_vf_mac) }, + [IFLA_VF_VLAN] = { .type = NLA_BINARY, + .len = sizeof(struct ifla_vf_vlan) }, + [IFLA_VF_TX_RATE] = { .type = NLA_BINARY, + .len = sizeof(struct ifla_vf_tx_rate) }, +}; + struct net *rtnl_link_get_net(struct net *src_net, struct nlattr *tb[]) { struct net *net; @@ -812,6 +850,52 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[]) return 0; } +static int do_setvfinfo(struct net_device *dev, struct nlattr *attr) +{ + int rem, err = -EINVAL; + struct nlattr *vf; + const struct net_device_ops *ops = dev->netdev_ops; + + nla_for_each_nested(vf, attr, rem) { + switch (nla_type(vf)) { + case IFLA_VF_MAC: { + struct ifla_vf_mac *ivm; + ivm = nla_data(vf); + err = -EOPNOTSUPP; + if (ops->ndo_set_vf_mac) + err = ops->ndo_set_vf_mac(dev, ivm->vf, + ivm->mac); + break; + } + case IFLA_VF_VLAN: { + struct ifla_vf_vlan *ivv; + ivv = nla_data(vf); + err = -EOPNOTSUPP; + if (ops->ndo_set_vf_vlan) + err = ops->ndo_set_vf_vlan(dev, ivv->vf, + ivv->vlan, + ivv->qos); + break; + } + case IFLA_VF_TX_RATE: { + struct ifla_vf_tx_rate *ivt; + ivt = nla_data(vf); + err = -EOPNOTSUPP; + if (ops->ndo_set_vf_tx_rate) + err = ops->ndo_set_vf_tx_rate(dev, ivt->vf, + ivt->rate); + break; + } + default: + err = -EINVAL; + break; + } + if (err) + break; + } + return err; +} + static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, struct nlattr **tb, char *ifname, int modified) { @@ -942,40 +1026,17 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, write_unlock_bh(&dev_base_lock); } - if (tb[IFLA_VF_MAC]) { - struct ifla_vf_mac *ivm; - ivm = nla_data(tb[IFLA_VF_MAC]); - err = -EOPNOTSUPP; - if (ops->ndo_set_vf_mac) - err = ops->ndo_set_vf_mac(dev, ivm->vf, ivm->mac); - if (err < 0) - goto errout; - modified = 1; - } - - if (tb[IFLA_VF_VLAN]) { - struct ifla_vf_vlan *ivv; - ivv = nla_data(tb[IFLA_VF_VLAN]); - err = -EOPNOTSUPP; - if (ops->ndo_set_vf_vlan) - err = ops->ndo_set_vf_vlan(dev, ivv->vf, - ivv->vlan, - ivv->qos); - if (err < 0) - goto errout; - modified = 1; - } - err = 0; - - if (tb[IFLA_VF_TX_RATE]) { - struct ifla_vf_tx_rate *ivt; - ivt = nla_data(tb[IFLA_VF_TX_RATE]); - err = -EOPNOTSUPP; - if (ops->ndo_set_vf_tx_rate) - err = ops->ndo_set_vf_tx_rate(dev, ivt->vf, ivt->rate); - if (err < 0) - goto errout; - modified = 1; + if (tb[IFLA_VFINFO_LIST]) { + struct nlattr *attr; + int rem; + nla_for_each_nested(attr, tb[IFLA_VFINFO_LIST], rem) { + if (nla_type(attr) != IFLA_VF_INFO) + goto errout; + err = do_setvfinfo(dev, attr); + if (err < 0) + goto errout; + modified = 1; + } } err = 0;
Now we have a set of nested attributes: IFLA_VFINFO_LIST (NESTED) IFLA_VF_INFO (NESTED) IFLA_VF_MAC IFLA_VF_VLAN IFLA_VF_TX_RATE This allows a single set to operate on multiple attributes if desired. Among other things, it means a dump can be replayed to set state. The current interface has yet to be released, so this seems like something to consider for 2.6.34. Signed-off-by: Chris Wright <chrisw@sous-sol.org --- Stephen, I'll send the iproute2 update as well include/linux/if_link.h | 23 ++++++- net/core/rtnetlink.c | 159 ++++++++++++++++++++++++++++++++-------------- 2 files changed, 129 insertions(+), 53 deletions(-)