On Mon, Mar 04, 2013 at 09:01:35AM -0600, Matt Domsch wrote:
On Mon, Mar 04, 2013 at 08:44:55AM -0600, Domsch, Matt wrote:
> On Mon, Feb 25, 2013 at 11:02:55AM -0600, Andy Gospodarek wrote:
> > On Sat, Feb 23, 2013 at 10:28:21AM +0100, Bill Nottingham wrote:
> > > Matt Domsch (Matt_Domsch(a)dell.com) said:
> > > > On Wed, Feb 13, 2013 at 01:57:42PM -0600, Bill Nottingham wrote:
> > > > > > If we can solve the installtime naming convention choice to
not
> > > > > > eliminate biosdevname, be able to disable systemd/udevd
naming, and
> > > > > > have the default be possible on a per-system-vendor basis,
and solve
> > > > > > the NPAR/SR-IOV/Mellanox naming problems, then I can
support this
> > > > > > proposal. Without fixing these shortcomings though, my
customers will
> > > > > > have a fit at me.
> > > > >
> > > > > If you're agreeing that biosdevname should be limited to
type9/type41
> > > > > (if I'm reading you right), and if the systemd/udev names
still use those
> > > > > fields, what parts of biosdevname are you still requiring? The
actual
> > > > > namespace used, or something else?
> > > >
> > > > I'd prefer if we didn't force users through another namespace
change.
> > > > I took a lot of flack for changing the namespace once. From their
> > > > perspective, it's still udev doing the renaming, only now the
code
> > > > moved from a udev helper into udev itself.
> > >
> > > True; the users likely don't care other than how they enable/disable
it.
> > >
> > > > I'd like to see the SR-IOV & NPAR devices handled. Those
aren't
> > > > represented in type9/type41, and their commonplace on Dell systems.
> > > >
> > > > I'd like to see the Dell-specific PCI VPD-R code from
biosdevname
> > > > included in udev, as that's used to map multi-port devices to
port
> > > > numbers. Those aren't represented in type9/type41, and
they're
> > > > commonplace on Dell systems.
> > >
> > > OK.
> > >
> > > > I'd like to see kernel driver work to be sure every multi-port
driver
> > > > with the same PCI b/d/b/f sets dev_id. That isn't necessarily
true
> > > > today, which makes it hard to trust. biosdevname needs this too,
> > > > until such a time as it's dead.
> > >
> > > Do we have a list of these, or is it a matter of doing a driver audit?
> > > CC'ing gospo.
> >
> > (Thanks, Bill, though I've been following this thread as I am on the
> > list).
> >
> > Right now almost no drivers actually set dev_id. In fact mlx4_en,
> > cxgb4, and sfc seem to be the only ones right now. If we feel like this
> > a requirement there will be work on the kernel side to add this.
>
> The challenge here is that because struct netdev is initialized to all
> zeros, code that consumes dev_id can't tell the difference between a
> driver _setting_ the value to 0 because it has 2 ports 0 and 1, and a
> default (unset) value of 0. I think the only solution here is to make
> sure the kernel drivers, if they need to set it, always set this
> non-zero, so udev, seeing a non-zero value, knows it's a valid value
> to use.
These seem to be the 4 drivers currently setting dev_id:
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c: adap->port[i]->dev_id = j;
drivers/net/ethernet/freescale/fec.c: fep->dev_id = dev_id++;
Look up a few lines:
/* setup board info structure */
fep = netdev_priv(ndev);
fep is not a pointer to the net_device, but to the private device
structure. This is why I didn't include it in the list.
drivers/net/ethernet/mellanox/mlx4/en_netdev.c: dev->dev_id =
port - 1;
drivers/net/ethernet/sfc/siena.c: efx->net_dev->dev_id = EFX_OWORD_FIELD(reg,
FRF_CZ_CS_PORT_NUM) - 1;
of these, both the mlx4 and siena drivers set them starting at 0. I
think the fec driver might be doing so as well, but it's not as
obvious. I'm pretty sure cxgb4 starts numbering at 1 just from
reading the code. So, as-is in the 3.9rc1 kernel, I wouldn't be
comfortable relying on the value of dev_id.
I agree it looks like some cleanup is needed due to the inconsistency.
This is still only an issue when a single function supports multiple
ethernet devices, right?