From: Arnd Bergmann <arnd@arndb•de>
To: Patrick McHardy <kaber@trash•net>
Cc: Scott Feldman <scofeldm@cisco•com>,
davem@davemloft•net, netdev@vger•kernel.org, chrisw@redhat•com
Subject: Re: [net-next-2.6 V7 PATCH 1/2] Add netlink support for virtual port management (was iovnl)
Date: Fri, 14 May 2010 14:12:01 +0200 [thread overview]
Message-ID: <201005141412.01578.arnd@arndb.de> (raw)
In-Reply-To: <4BED2CD8.4020209@trash.net>
On Friday 14 May 2010, Patrick McHardy wrote:
> 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?
Yes, the request field of the VDP message shows the status (e.g. associated or
disassociated).
> > +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_*], ...
> ...
Ah, I was wondering about this already. Does this mean that IFLA_VFINFO
does this incorrectly as well?
> > 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?
No, but if parent is NULL, we must not call dev_num_vf(). The way that enic
needs the attributes, they can be either for the VF of dev->dev.parent (the
PCI PF), or for the PF itself, even if it does not have VFs, in which case
it would be interesting to have IFLA_NUM_VF = 0 in the output.
Maybe a better structure would be to separate the two cases, also allowing
a port profile to be associated with both the PF and with each of its VFs?
Something like this:
[IFLA_NUM_VF]
[IFLA_VF_PORTS]
[IFLA_VF_PORT]
[IFLA_VF_PORT_*], ...
[IFLA_VF_PORT]
[IFLA_VF_PORT_*], ...
[IFLA_PORT_SELF]
[IFLA_VF_PORT_*], ...
Arnd
next prev parent reply other threads:[~2010-05-14 12:12 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-14 1:35 [net-next-2.6 V7 PATCH 0/2] Add virtual port netlink support Scott Feldman
2010-05-14 1:35 ` [net-next-2.6 V7 PATCH 1/2] Add netlink support for virtual port management (was iovnl) Scott Feldman
2010-05-14 10:58 ` Patrick McHardy
2010-05-14 12:12 ` Arnd Bergmann [this message]
2010-05-14 16:42 ` Patrick McHardy
2010-05-14 17:19 ` Scott Feldman
2010-05-14 17:26 ` Patrick McHardy
2010-05-14 17:29 ` Arnd Bergmann
2010-05-14 17:46 ` Scott Feldman
2010-05-14 18:09 ` Patrick McHardy
2010-05-14 18:25 ` Chris Wright
2010-05-14 18:46 ` Patrick McHardy
2010-05-14 18:48 ` Chris Wright
2010-05-14 18:50 ` Patrick McHardy
2010-05-14 20:56 ` Williams, Mitch A
2010-05-14 23:43 ` Chris Wright
2010-05-14 17:35 ` Chris Wright
2010-05-14 17:54 ` Scott Feldman
2010-05-14 1:35 ` [net-next-2.6 V7 PATCH 2/2] Add ndo_{set|get}_vf_port support for enic dynamic vnics Scott Feldman
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=201005141412.01578.arnd@arndb.de \
--to=arnd@arndb$(echo .)de \
--cc=chrisw@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=kaber@trash$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=scofeldm@cisco$(echo .)com \
/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