Christos Zoulas <christos@zoulas.com> wrote:

> 1. Don't call the variable sleep. It is not a good idea to name variables
>    after commonly used functions. And you get shadow warnings too.
> 2. Probably it is time to IFQ_ENQUEUE -> ifq_enqueue()... a function.
> 3. If non-blocking I/O is set then you should probably return EWOULDBLOCK
>    or EAGAIN. How do the upper layers handle this (i.e. does the operation
>    get really retried or the packet is dropped and is handled by the prorocl
>    layer?)? Does using select work to wakeup when space is available again?

That's the whole point: if non blocking I/O is set, SUS and our man
pages say that we must block instead of failling.
 
> 4. There is a typo in "should"
> 5. if_ppp.c if_strip.c if_sl.c if_sppsubr.c open-code the ALTQ code. I don't
>    think that this is needed anymore.

What do you mean?

> 6. As far as passing in the spl level, I think that the whole thing should
>    be done in one function:
> 
>       mflags = m->m_flags;
>       len = m->m_pkthdr.len;
>       s = splnet();
>       /*
>        * Queue message on interface, and start output if interface
>        * not yet active.
>        */
>       IFQ_ENQUEUE(&ifp->if_snd, m, &pktattr, error);
>       if (error) {
>               /* mbuf is already freed */
>               splx(s);
>               return (error);
>       }
>       ifp->if_obytes += len;
>       if (mflags & M_MCAST)
>               ifp->if_omcasts++;
>       if ((ifp->if_flags & IFF_OACTIVE) == 0)
>               (*ifp->if_start)(ifp);
>       splx(s);
> 
>     Since all the non-tunneling interfaces do this, or a buggy version
>     of this. Then you can have a different version for the tunnelling
>     interfaces (bridge, gif, etc.). This will cleanup the code significantly.

Right. I call it ifq_enqueue, I expand IFQ_ENQUEUE in it, and I remove
IFQ_ENQUEUE definition?
 
>     Also the ieee1394 appears to freeing the mbuf on failure, or am I
>     mis-reading this?

Maybe a bug, who did the firewire support?

-- 
Emmanuel Dreyfus
Le cahier de l'admin BSD 2eme ed. est dans toutes les bonnes librairies
http://www.eyrolles.com/Informatique/Livre/9782212114638/livre-bsd.php
manu@netbsd.org