netdev - Re: [net-next-2.6 V7 PATCH 1/2] Add netlink support for virtual port management (was iovnl)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201005141412.01578.arnd@arndb.de>
Date:	Fri, 14 May 2010 14:12:01 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	Scott Feldman <scofeldm@...co.com>, davem@...emloft.net,
	netdev@...r.kernel.org, chrisw@...hat.com
Subject: Re: [net-next-2.6 V7 PATCH 1/2] Add netlink support for virtual port management (was iovnl)

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
--
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