Ethernet NIC drivers depending unconditionally on INET

[Bjoern A. Zeeb]

Hi,

as announced in my other mail to current@ and net@  I added the
mandatory "INET" dependency to 12 Ethernet NIC drivers.  Additionally
I am adding if_bridge here as well.

Those drivers should be fixed and once done the (mandatory) inet
dependency should be removed again from sys/conf/files for that
driver.

If you have questions, need help, compile testing, ... feel free to
mail me.  I won't be able to test things as I do not own all of the
different cases but I can review patches, ..


Generally speaking upfront: if one of the drivers mentioned here or
not, supportes offload capabilities for rx or tx of v4 or v6, for tcp,
udp, sctp, .. they need to be properly hidden under the apropriate
#ifdefs.

Everything that needs v4 belongs under #ifdef INET.
Everything that needs v6 belongs under #ifdef INET6.
Upper layer protocols like TCP, UDP, SCTP should be at least
 	#if defined (INET) || defined(INET6).
In case of SCTP probebly #ifdef (SCTP) as well.

You will need to include opt_inet.h, opt_inet6.h, opt_sctp.h
in those cases and at least the buildkernel (not building the module
alone) part should properly check them.  For single module builds
we are kind of lost here and I think we usually enabled things by
default as they are in GENERIC.

Another general note that scared me while looking at CSUM_TSO and
IFCAP_TSO4|IFCAP_TSO6.  Almost none of the drivers actually checks
that it is an IPv4 packet before grabiing the IP header, TCP header,
...  How do we actually know that it is an IPv4 packet in all those
drivers?  Should we do a IP Version fiel check?  I think em(4) is one
of those that does.  I hope this will improve with tuexen's work on
the CSUM flags.

In case you are maintaining another driver but cannot find it on the
list - be sure we'll find you during the 9.x lifecycle in case you do
not properly handle things;-)


if_age:
----------------------------------------
I identifed two in_pseudo() calls that are INET specific. The problem
is that thi is part of the (IPv4) TSO code and it seems to be
mandatory for that case.  The proper fix seems to be to but that block
under #ifdef INET and in case INET is not defined to also disable
announcing or permitting to set these features entirely as we won't be
able to handle them or use them anyway.

if_alc:
----------------------------------------
There is one in_pseudo(); Apart from that things seem identical to if_age.

if_ale:
----------------------------------------
There is one in_pseudo(); Apart from that things seem identical to if_age.

if_em:
----------------------------------------
There is one in_pseudo() in em_tso_setup() that prevented the driver
from being linked into the kernel;  the entire code unfortunately has
at least one more place of IP/IPV6 distinction for cksum offloading
em_transmit_checksum_setup() that is not properly handled either.
In either case the feature should be disbaled and not be announced if
the address family is not there to handle the code.
Note: tuexen is working on a proper set of v4/v6 definitions for the
csum offloading, so we will be able to do this more fine grained in
the future.  For now not having INET6 would penalize INET as well.

if_igb:
----------------------------------------
igb_tso_setup() has one in_pseudo() call.   See if_age for that.
It also has dependencies on the entire LRO framework like
tcp_lro_free(), tcp_lro_rx(), tcp_lro_flush(), tcp_lro_init().
The LRO framework is on the IPv6TODO list.  For the moment if there
is no INET LRO should be completely compiled out and not advertised
removing the TUNABLE and SYSCTL and changing the default to 0.
Be prepared in case LRO will arrive for IPv6.
Side note: calling tcp_lro_free() on something that hadn't been
initialized for example because lro was enabled by the sysctl after
itniialization, can that be a problem? Maybe tcp_lro_free() should
just return in case cntl is NULL?

if_fxp:
----------------------------------------
There is one in_pseudo(); Apart from that things seem identical to if_age.

ixgbe:
----------------------------------------
There is one in_pseudo() in ixgbe_tso_setup(). See if_igb for that.
There is an arp_ifinit() call in ixgbe_ioctl() that already seems to
check for the proper address family and only needs the #ifdefs around
like the ioctl in if_em has.
The same that applies for if_igb about LRO applies here as well.

if_jme:
----------------------------------------
Two in_pseudo() calls; see if_age.

if_msk:
----------------------------------------
An in_cksum_skip() call. Almost like if_age but this seems to be
further conditional on driver flags and CSUM_TCP that seem to be set
independently of TSO. I am not entirely sure if there is a proper
solution or if we might be forced to return an error in that case in
case there is not INET support.

if_mxge:
----------------------------------------
mxge_rx_csum() has one in_pseudo(). The function and callers
already seem to know how do deal with results in case the csum can't
be validated. So this should be a simple #ifdef INET wrapping here;
side note: the tcpudp_csum variables in the callers are not needed.
side note: huge inlining going on there;)
mxge_lro_flush() has another call to in_pseudo(). As with if_igb/ixgbe
if there is no INET there should be no LRO for now, the capabilities
not advertised, etc.  Be prepared in case LRO will arrive for IPv6.

if_sk:
----------------------------------------
sk_rxcksum() has an in_addword() call. It seems that sk_rxcksum is
only used if IFCAP_RXCSUM is on. So similar things like to TSO should
be done; hiding it under #ifdef INET and not advertising the feature
etc.

if_txp.c:
----------------------------------------
Now this is an interesting case as txp_download_fw_section()
is using in_cksum() to validate the firmware csum before downloading
it to the card it seems.  I am not sure how to bes work around that
one.



if_bridge:
----------------------------------------
if_bridge is porobably mostly INET6 clean but doesn't have any
INEt checks. It's just a matter of going through the code and adding
proper #ifdef INET handling the simple cases and see what's left.
This is just work; in case you are bored, read this mail to the end
and would like to do something, know the difference between IPv4 and
IPv6 is 2 and thinking of C doesn't make you think of scale of C major
in first place, you are welcome to send patches:-)


Regards,
Bjoern

-- 
Bjoern A. Zeeb                      The greatest risk is not taking one.