hiren added a comment.

  In https://reviews.freebsd.org/D5872#127345, @jtl wrote:
  
  > In https://reviews.freebsd.org/D5872#127343, @mike-karels.net wrote:
  >
  > > If we get an ENOBUFS when sending data, we will already be running the 
retransmit timer.
  >
  >
  > Good point, but see below.
  >
  > > If we drop an ACK on ENOBUFS, either we will receive more data and
  > >  attempt another ACK, or the sender will time out and resend data.  Either
  > >  will get the connection started again.
  >
  > True, but it may take time if we have to wait for a retransmission time 
out. That isn't necessarily a bad thing.
  
  
  But setting retransmission timer on an ACK seems... wrong.
  
  >> I believe lines 1552-1554 should
  >>  simply be deleted.
  > 
  > On the surface, that seems like a reasonable alternative. However, its 
interesting to see why this code was originally added. It looks like it was 
added by jlemon in r61179. Here's what the commit message says:
  > 
  >> When attempting to transmit a packet, if the system fails to allocate
  >>  a mbuf, it may return without setting any timers.  If no more data is
  >>  scheduled to be transmitted (this was a FIN) the system will sit in
  >>  LAST_ACK state forever.
  >> 
  >> Thus, when mbuf allocation fails, set the retransmit timer if neither
  >>  the retransmit or persist timer is already pending.
  > 
  > Even back then, the retransmit and/or persist timers should have been 
running by this point in the code. It would be interesting to know if jlemon 
proved this code fixed the problem, or it was just speculated. We fixed a 
similar-sounding problem last year, and the code is different now, so these 
particular lines may truly no longer be necessary. But, I am a little hesitant 
to remove this code without knowing more about its origin.
  > 
  > Just my 2c.
  
  I guess this boils down to how you want to "deal" with this (possibly) 
transient memory error.
  
  I'd like to go with @mike-karels.net's suggestion as when we reach to this 
point in code, I don't see how timers couldn't have been set already. And I 
doubt we can get any more info from that old commit :/
  
  But because this is (sigh..) tcp and if we want to be really cautious, I like 
@jtl 's patch of handling this for 'acks' with delack. (In that patch, I am not 
sure why we want to drop cwnd down to 1mss only in case of data and not for 
acks. I think we should do that for both?)
  
  (fwiw, other BSDs don't have this special handling as Mark suggested.)

REVISION DETAIL
  https://reviews.freebsd.org/D5872

EMAIL PREFERENCES
  https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, network, glebius, lstewart, adrian, delphij, 
decui_microsoft.com, honzhan_microsoft.com, howard0su_gmail.com, 
freebsd-net-list, transport, jtl, hiren
Cc: mike-karels.net, jtl
_______________________________________________
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Reply via email to