netdev - Re: [RFC PATCH net-next] net: Add phys_port identifier to struct net

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] [day] [month] [year] [list]

Message-ID: <1371227240.1940.9.camel@bwh-desktop.uk.level5networks.com>
Date:	Fri, 14 Jun 2013 17:27:20 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	<Narendra_K@...l.com>
CC:	<netdev@...r.kernel.org>, <john.r.fastabend@...el.com>
Subject: Re: [RFC PATCH net-next] net: Add phys_port identifier to struct
 net_device and export it to sysfs

On Fri, 2013-06-14 at 08:22 -0700, Narendra_K@...l.com wrote:
> On Thu, Jun 13, 2013 at 11:31:28PM +0530, Ben Hutchings wrote:
> > 
> > On Thu, 2013-06-13 at 08:45 -0700, Narendra_K@...l.com wrote:
> > [...]
> > > The requirement is to have a generic interface using which
> > > kernel/drivers can provide information/hints to user space about the
> > > physical port number used by a network interface.
> > >
> > > The following options were explored -
> > >
> > > 1. 'dev_id' sysfs attribute:
> > >
> > > In addition to being used to differentiate between devices that share
> > > the same link layer address, it is being used to indicate the physical
> > > port number used by a network interface.
> > 
> > This last bit is no longer true.
> 
> Right. mlx4_en driver is setting dev_id. I have sent a RFC patch to
> not set dev_id. I will modify the commit message in the next version
> of the patch.

Oh, I forgot that mlx4_en also misused dev_id.

> > 
> > [...]
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -1059,6 +1059,18 @@ struct net_device_ops {
> > >                                                     bool new_carrier);
> > >  };
> > > 
> > > +/* This structure holds a universally unique identifier to
> > > + * identify the physical port used by a netdevice
> > > + */
> > > +struct port_identifier {
> > > +     union {
> > > +             u8 mac48[6];
> > > +             u8 eui64[8];
> > > +             u8 uuid[16];
> > > +     } id;
> > > +     unsigned short length;
> > > +};
> > 
> > I can't think why those three ID-spaces would not be sufficient, but I
> > also don't see any reason to restrict to them.  I think it's preferable
> > to use a simple array of bytes, like for the link-layer address:
> > 
> >         unsigned char port_id[MAX_ADDR_LEN];
> >         unsigned char port_id_len;
> > 
> 
> Ok. With the above change, the switch statement below is probably not
> required. Could it be like 
> 
> if (len < 0 || > MAX_ADDR_LEN)

I don't think any range check is needed at all.  If the port_id_len is
out of ragne that's an obvious bug in the driver which I don't think you
need to check for.  (The networking core doesn't currently check
addr_len, for example.)  And an unsigned length cannot be < 0 anyway!

> 	return -EINVAL;
> if (!len)
> 	return 0;
> 
> ret = sysfs_format_mac(buf, net->phys_port.port_id, len);
[...]

Right.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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