Re: HEADS-UP: IFLIB implementations of sys/dev/e1000 em, lem, igb pending

On Thursday, January 05, 2017 08:17:56 PM Sean Bruno wrote:
> tl;dr --> igbX devices will become emX devices
> 
> We're about to commit an update to sys/dev/e1000 that will implement and
> activate IFLIB for em(4), lem(4) & igb(4) and would appreciate all folks
> who can test and poke at the drivers to do so this week.  This will have
> some really great changes for performance and standardization that have
> been bouncing around inside of various FreeBSD shops that have been
> collaborating with Matt Macy over the last year.
> 
> This will implement multiple queues for certain em(4) devices that are
> capable of such things and add some new sysctl's for you to poke at in
> your monitoring tools.
> 
> Due to limitations of device registration, igbX devices will become emX
> devices.  So, you'll need to make a minor update to your rc.conf and
> scripts that manipulate the network devices.
> 
> UPDATING will be bumped to reflect these changes.
> 
> MFC to stable/11 will have a legacy implementation that doesn't use
> IFLIB for compatibility reasons.
> 
> A documentation and man page update will follow in the next few days
> explaining how to work with the changed driver.

This is a very invasive change, and seems unnecessary.  The only thing you
can't share between two drivers with different names is the probe routine
(which you would want to be different since they would cover different
PCI ID lists).  That is, you only need:

static int
igb_probe(device_t dev)
{
    /* check igb PCI ID list */
}

static int
em_probe(device_t dev)
{
    /* check em PCI ID list */
}

static int
foo_attach(device_t dev)
{
   ...
}

static int
foo_detach(device_t dev)
{
   ...
}

static device_method_t igb_methods[] = {
    DEVMETHOD(device_probe,   igb_probe),
    DEVMETHOD(device_attach,  foo_attach),
    /* Other methods all use foo_* */
};

static device_method_t em_methods[] = {
    DEVMETHOD(device_probe,   em_probe),
    DEVMETHOD(device_attach,  foo_attach),
    /* Other methods all use foo_* */
};

static driver_t igb_driver = {
    "igb", igb_methods, sizeof(struct foo_softc)
};

static driver_t em_driver = {
    "em", em_methods, sizeof(struct foo_softc)
};

DRIVER_MODULE(igb, pci, igb_driver, ...);
DRIVER_MODULE(em, pci, em_driver, ...);

This isn't a huge amount of code to carry around, and seems a very small
price to pay compared to the cost of breaking existing machines (and
existing documentation, so now things have to document <= 11 and >= 12
differently, etc.).

(FWIW, this approach is what cxgbe uses to have the same driver code manage
cxgbe/cxl/cc.)

-- 
John Baldwin