Re: if_ef rotten

On Thu, 23 Nov 2006, Kostik Belousov wrote:

> When trying to kldload if_ef (on yesterday CURRENT/amd64, with WITNESS and 
> almost all debugging enabled), I got a lot of "malloc(M_WAITOK) of "XXX", 
> forcing M_NOWAIT with the following non-sleepable locks held: ... ifnet ..."
>
> Is it rotten ? (No, I do not use it, just noted it when doing some other 
> testing work).

There are three general classes of problems with if_ef:

(1) We don't really have a model for handling "derived" interfaces very well,
     and this shows up in both if_ef and if_vlan.  if_ef could use many of the
     updates performed to if_vlan being done to it.

(2) if_ef lacks internal locking of its own data structures.  I believe Tom
     Rhodes was looking at this, and sent me a patch to review a while back,
     which I have neglected to review thus far, but seemed likely to require
     significant refinement before it's ready to be committed.  It ended up on
     my "exceptional case lots of attention required" slow path, and has
     effectively ended up in exactly the wrong spot on my work queue as a
     result, which is hardly Tom's fault :-).

(3) It likely interacts poorly with existing locking in the stack, hence the
     errors you've seen.  For example, calling mbuf allocation routines in the
     output path, where higher level locks may already be held.

The above list goes from "hardest" to "easiest" in order.  More generally, (1) 
is an issue we need to address in one of two ways: really working out how 
derived interfaces should work, and/or more tightly integrating our three 
ethernet link layer components into one combined system: if_ethersubr, 
if_vlan, and if_ef.  Because 802.1 link layer encapsulation is also useful for 
QoS-related stuff (I understand), this integration makes quite a bit of sense. 
We should also consider this in the same light as similar functionality in 
802.11 and see whether there are simply parallels, or also overlap.  I know 
both Sam and Bruce have had opinions on this topic in the past also.

Robert N M Watson
Computer Laboratory
University of Cambridge