public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash•net>
To: Jeff Kirsher <jeffrey.t.kirsher@intel•com>
Cc: davem@davemloft•net, netdev@vger•kernel.org, gospo@redhat•com,
	Mitch Williams <mitch.a.williams@intel•com>
Subject: Re: [net-next-2.6 PATCH v3 4/5] rtnetlink: Add VF config code to rtnetlink
Date: Wed, 10 Feb 2010 13:02:41 +0100	[thread overview]
Message-ID: <4B72A061.40905@trash.net> (raw)
In-Reply-To: <20100210114403.14483.11004.stgit@localhost.localdomain>

Jeff Kirsher wrote:
> @@ -665,6 +677,17 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
>  	stats = dev_get_stats(dev);
>  	copy_rtnl_link_stats(nla_data(attr), stats);
>  
> +	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);

We usually encapsulate lists of the same attribute type in another
top-level attribute. Check out the IFLA_VLAN_*_QOS attributes for
an example.

The interface should also be symetrical, IOW you should dump the
same attributes used in the userspace->kernel direction instead
of a combined "info" attribute.

> +		}
> +	}

> @@ -898,6 +927,44 @@ 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]);
> +		write_lock_bh(&dev_base_lock);

It dev_base_lock really correct here? This is running under the RTNL, so
changes to the device list can't happen.

> +		if (ops->ndo_set_vf_mac)
> +			err = ops->ndo_set_vf_mac(dev, ivm->vf, ivm->mac);

Shouldn't this indicate an error if the attributes aren't supported?

> +		write_unlock_bh(&dev_base_lock);
> +		if (err < 0)
> +			goto errout;
> +		modified = 1;
> +	}
> +
> +	if (tb[IFLA_VF_VLAN]) {
> +		struct ifla_vf_vlan *ivv;
> +		ivv = nla_data(tb[IFLA_VF_VLAN]);
> +		write_lock_bh(&dev_base_lock);
> +		if (ops->ndo_set_vf_vlan)
> +			err = ops->ndo_set_vf_vlan(dev, ivv->vf,
> +						   (u16)ivv->vlan,
> +						   (u8)ivv->qos);

The casts aren't necessary. But why does struct ifla_vf_vlan use u32
for the vlan in the first place?

> +		write_unlock_bh(&dev_base_lock);
> +		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]);
> +		write_lock_bh(&dev_base_lock);
> +		if (ops->ndo_set_vf_tx_rate)
> +			err = ops->ndo_set_vf_tx_rate(dev, ivt->vf, ivt->rate);
> +		write_unlock_bh(&dev_base_lock);
> +		if (err < 0)
> +			goto errout;
> +		modified = 1;
> +	}
>  	err = 0;
>  
>  errout:
> 
> --
> 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
> 


  reply	other threads:[~2010-02-10 12:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-10 11:43 [net-next-2.6 PATCH v3 1/5] pci: Add SR-IOV convenience functions and macros Jeff Kirsher
2010-02-10 11:43 ` [net-next-2.6 PATCH v3 2/5] if_link: Add SR-IOV configuration methods Jeff Kirsher
2010-02-13  0:56   ` David Miller
2010-02-10 11:43 ` [net-next-2.6 PATCH v3 3/5] net: Add netdev ops for SR-IOV configuration Jeff Kirsher
2010-02-13  0:56   ` David Miller
2010-02-10 11:44 ` [net-next-2.6 PATCH v3 4/5] rtnetlink: Add VF config code to rtnetlink Jeff Kirsher
2010-02-10 12:02   ` Patrick McHardy [this message]
2010-02-10 22:33     ` Williams, Mitch A
2010-02-11  6:07       ` Patrick McHardy
2010-02-24 14:15       ` Patrick McHardy
2010-02-24 18:13         ` Williams, Mitch A
2010-02-25 10:18           ` Patrick McHardy
2010-02-13  0:56   ` David Miller
2010-02-10 11:44 ` [net-next-2.6 PATCH v3 5/5] igb: support for VF configuration tools Jeff Kirsher
2010-02-13  0:57   ` David Miller
2010-02-13  0:56 ` [net-next-2.6 PATCH v3 1/5] pci: Add SR-IOV convenience functions and macros David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B72A061.40905@trash.net \
    --to=kaber@trash$(echo .)net \
    --cc=davem@davemloft$(echo .)net \
    --cc=gospo@redhat$(echo .)com \
    --cc=jeffrey.t.kirsher@intel$(echo .)com \
    --cc=mitch.a.williams@intel$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox