tcp options length with SACK

[Robert Elz <kre%munnari.OZ.AU@localhost>]

While I was (fairly ineffectively) looking to see if I could spot
a cause for PR51767 (which Martin has since fixed) I came across ...

/*      
 * Determine the length of the TCP options for this connection.
 *
 * XXX:  What do we do for SACK, when we add that?  Just reserve
 *       all of the space?  Otherwise we can't exactly be incrementing
 *       cwnd by an amount that varies depending on the amount we last
 *       had to SACK!
 */     
        
u_int           
tcp_optlen(struct tcpcb *tp)
{       
	/* ... */

(and sure enough SACK is not in any way accounted for in tcp_optlen() nor
is it handled separately at the couple of places that tcp_oplen() is called,
but the "when we add that" is now long past.)

This made me wonder if this could be related to the ocasional problems
people report when using SACK - my impression is that the effect might be
to allow packets bigger than the MSS to be transmitted (or attempted to).

Of course, I may be misreading things, in which case at the very least that
comment ought to be corrected.


Also, while I was looking at the optlen calcs (and use) for TCP, it occurred
to me that with all of SACK, TIMESTAMP, and TCP_SIGNATURE in use, the TCP
options space would be very close to (if not beyond) filled, a situation
we do not handle gracefully at all ... as a first step I'd suggest deleting
all the padding NOP options that are now always inserted for alignment
(they certainly can make processing faster, but they are not required)
rather than just bailing if the options exceed the space allowed, and if
even that is not going to allow things to work, it might be better to
disable one of the options (SACK or SIGNATURE I'd guess, TIMESTAMP is
too useful) rather than just bailing (generating a send error.)

kre