Re: cvs commit: src/sys/i386/i386 nexus.c src/sys/i386/include atomic.h src/sys/kern kern_poll.c lwkt_serialize.c src/sys/net if.c if_var.h rtsock.c src/sbin/ifconfig ifconfig.c src/sys/dev/netif/dc if_dc.c src/sys/dev/netif/em if_em.c if_em.h ...

[Joerg Sonnenberger <joerg@xxxxxxxxxxxxxxxxx>]

On Thu, May 26, 2005 at 01:36:41PM -0700, Matthew Dillon wrote:
>     This doesn't solve any of the problems I brought up.  First, turning on 
>     and off polling should NOT be done with the capabilities bitmask.  I don't
>     know why you keep trying to revert the ifconfig change.  Operational
>     flags are if_flags, *NOT* if_capabilities.

Exactly that is the point. IFF_POLLING is the operational flag, it means
"this interface uses polling". That is completely different from "this
interface supports polling" and "this interface should use polling".
We would have to add yet another interface to indicate the first.

>     The capabilities bitmask is
>     a list of capabilities, *period*.  It is NOT what controls whether
>     a capability is turned on or off.

There are *two* capability bitmasks. The first describes which
capabilities are supported (if_capabilities), that's what you removed
in the first patch set. The second is if_capenable, which describes
the desired interface state. It contains the *administrative* state,
which is modified by ifconfig. if_capabilities is *never* modified
via ifconfig.

>     Secondly, the ethernet driver
>     should *NOT* have all sorts of code randomly spread through it to
>     make polling work.  It should make a single registration call to 
>     indicate that it is capable of polling and the IF layer should
>     do the rest.  e.g.  instead of:

Well, whether we want to hide this with a yet another function
is a completely different issue. I don't have a problem with that,
but it doesn't save that much.

>     As for the rest of the changes... ether_ioctl() should simply be called
>     unconditionally for anything that device itself can't handle, including
>     any SIOCSIFFLAGS that the device itself doesn't handle on its own
>     (which is most of them).  We should probably vectorize if_up and if_down
>     inside struct ifnet and have ether_ioctl() handle that flag as well.

Adding if_stop and calling that and if_init based on the new state is
enough for almost all drivers. The rest can override it.

Joerg