On Mar 27, 12:28am, manu@netbsd.org (Emmanuel Dreyfus) wrote:
-- Subject: Re: fixing send(2) semantics (kern/29750)

| Here is a first attempt at dealing with the problem:
| http://ftp.espci.fr/shadow/manu/send.diff
| 
| It's not complete and indeed it needs more work: I need feedback. But at
| least it makes NetBSD pass the test program attached to kern/29750 =20
| 
| - I had to pass the spl value obtained from splnet() to IFQ_ENQUEUE().
| I'm not very satisfied with that, but I don't see any other way around.
| Suggestions are welcome.
| 
| - ip_output() and ip6_output() are the only place where struct socket is
| available, so I did not fix other protocols, I just changed them to
| match new prototypes. Is it okay to pull struct socket from optional
| arguments in {ddp|ns|whatever}_ouput()?

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? 
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.
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.

    Also the ieee1394 appears to freeing the mbuf on failure, or am I
    mis-reading this?

christos