Re: Much improved sosend_*() functions


On Fri, 29 Sep 2006, Andre Oppermann wrote:

I like the concept of these changes in principle -- this is the reason I broke out sosend_copyin(), so that we could start plugging bits of the send routines more easily for optimization. However, I think we need to review this really carefully. A casual glance brought up one question, and I hope to get a chance to review this in detail in the next few days. The question is with regard to the 'space' variable. When breaking out sosend_copyin(), I originally simply passed space in as an argument, which is what you do currently. However, I found I had to change it to pass in the variable by reference so that it could be updated, as later portions of sosend() depend on it being updated in order to figure out what flags to pass to pru_send() with respect to PRUS_MORETOCOME, as well as for the (resid && space > 0) condition for the main loop. In your revised version, 'space' isn't updated in sosend() after calling m_uiotombuf(), which on a casual read, suggests that PRUS_MORETOCOME will no longer get set on the last pass into pru_send(), and that the loop may go an extra time and pass more data into TCP than there is room in the socket send buffer. I may be reading wrong, I've not had time to look in any detail, but that was my experience, so you may find you need to pass send by reference, as I do in sosend_copyin(), for the same reason.

Your observation is correct. The variable 'space' is no longer updated with my changes. For sosend_dgram() this is of no consequence as PRUS_ MORETOCOME can't ever be true. For datagrams the uio must not contain more data than we have space left in the socket buffer. Otherwise we fail with EMSGSIZE. For sosend_generic() PRUS_MORETOCOME is no longer correctly set with my changes. For TCP this is of no consequence either as TCP doesn't care about it. For correctness I'll change my patch to update 'space' in sosend_generic() and remove the PRUS_MORETOCOME flag from sosend_dgram() completely.

Are you sure TCP doesn't care? I thought it used PRUS_MORETOCOME as a hint to determine whether to immediately output or wait for small segments, but admit to never having read the code in detail. Also, 'space' is not just about PRUS_MORETOCOME, it's also about the loop terminating at the right time.

I'll try to give your revised patch a closer reading this weekend.

Thanks,

Robert N M Watson
Computer Laboratory
University of Cambridge
_______________________________________________
freebsd-current@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscribe@xxxxxxxxxxx"

Relevant Pages

  • Re: Much improved sosend_*() functions
    ... For datagrams the uio must not contain more data than we have space left in the socket buffer. ... For TCP this is of no consequence either as TCP doesn't care about it. ... Also, 'space' is not just about PRUS_MORETOCOME, it's also about the loop terminating at the right time. ... I'll try to give your revised patch a closer reading this weekend. ...
    (freebsd-net)
  • Re: panic: m_copym, length > size of mbuf chain
    ... > copied from the chain than is actually present in the chain. ... > suggests a bug in socket buffer management or the TCP code. ... > It sounds like the socket buffer state may be inconsistent with the TCP ... I'm very suspicious of the SACK code. ...
    (freebsd-current)
  • Re: using ipfw seems to interfere with socket communication
    ... to break the code's assumption that TCP over the loopback interface is instantaneous. ... If my fading memory of TCP/IP Illustrated Vol 2 serves me right, that was actually the case at least back then: the sendto system call would push the data all the way down to lo0, which would immediately pass it back up until it ended up in the receiving socket buffer. ...
    (freebsd-net)
  • Re: panic: m_copym, length > size of mbuf chain
    ... the struct mbuf* argument is valid. ... out of mbufs in which to find data to copy, as it hit the end of the chain ... suggests a bug in socket buffer management or the TCP code. ... It sounds like the socket buffer state may be inconsistent with the TCP ...
    (freebsd-current)