[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4BED2CD8.4020209@trash.net>
Date: Fri, 14 May 2010 12:58:32 +0200
From: Patrick McHardy <kaber@...sh.net>
To: Scott Feldman <scofeldm@...co.com>
CC: davem@...emloft.net, netdev@...r.kernel.org, chrisw@...hat.com,
arnd@...db.de
Subject: Re: [net-next-2.6 V7 PATCH 1/2] Add netlink support for virtual port
management (was iovnl)
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 "
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists