Author: rrs
Date: Fri May 15 14:00:12 2020
New Revision: 361080
URL: https://svnweb.freebsd.org/changeset/base/361080

Log:
  This fixes several skyzaller issues found with the
  help of Michael Tuexen. There was some accounting
  errors with TCPFO for bbr and also for both rack
  and bbr there was a FO case where we should be
  jumping to the just_return_nolock label to
  exit instead of returning 0. This of course
  caused no timer to be running and thus the
  stuck sessions.
  
  Reported by: Michael Tuexen and Skyzaller
  Sponsored by: Netflix Inc.
  Differential Revision:        https://reviews.freebsd.org/D24852

Modified:
  head/sys/netinet/tcp_stacks/bbr.c
  head/sys/netinet/tcp_stacks/rack.c
  head/sys/netinet/tcp_stacks/rack_bbr_common.c

Modified: head/sys/netinet/tcp_stacks/bbr.c
==============================================================================
--- head/sys/netinet/tcp_stacks/bbr.c   Fri May 15 13:53:10 2020        
(r361079)
+++ head/sys/netinet/tcp_stacks/bbr.c   Fri May 15 14:00:12 2020        
(r361080)
@@ -4975,6 +4975,15 @@ bbr_remxt_tmr(struct tcpcb *tp)
                        rsm->r_flags &= ~(BBR_ACKED | BBR_SACK_PASSED | 
BBR_WAS_SACKPASS);
                        bbr_log_type_rsmclear(bbr, cts, rsm, old_flags, 
__LINE__);
                } else {
+                       if ((tp->t_state < TCPS_ESTABLISHED) &&
+                           (rsm->r_start == tp->snd_una)) {
+                               /*
+                                * Special case for TCP FO. Where
+                                * we sent more data beyond the snd_max.
+                                * We don't mark that as lost and stop here.
+                                */
+                               break;
+                       }
                        if ((rsm->r_flags & BBR_MARKED_LOST) == 0) {
                                bbr->r_ctl.rc_lost += rsm->r_end - rsm->r_start;
                                bbr->r_ctl.rc_lost_bytes += rsm->r_end - 
rsm->r_start;
@@ -12315,7 +12324,8 @@ bbr_output_wtime(struct tcpcb *tp, const struct timeva
             (tp->t_state == TCPS_SYN_SENT)) &&
            SEQ_GT(tp->snd_max, tp->snd_una) && /* initial SYN or SYN|ACK sent 
*/
            (tp->t_rxtshift == 0)) {    /* not a retransmit */
-               return (0);
+               len = 0;
+               goto just_return_nolock;
        }
        /*
         * Before sending anything check for a state update. For hpts
@@ -14286,6 +14296,7 @@ nomore:
            (hw_tls == 0) &&
            (len > 0) &&
            ((flags & TH_RST) == 0) &&
+           ((flags & TH_SYN) == 0) &&
            (IN_RECOVERY(tp->t_flags) == 0) &&
            (bbr->rc_in_persist == 0) &&
            (tot_len < bbr->r_ctl.rc_pace_max_segs)) {

Modified: head/sys/netinet/tcp_stacks/rack.c
==============================================================================
--- head/sys/netinet/tcp_stacks/rack.c  Fri May 15 13:53:10 2020        
(r361079)
+++ head/sys/netinet/tcp_stacks/rack.c  Fri May 15 14:00:12 2020        
(r361080)
@@ -3873,6 +3873,7 @@ skip_measurement:
         * the next send will trigger us picking up the missing data.
         */
        if (rack->r_ctl.rc_first_appl &&
+           TCPS_HAVEESTABLISHED(tp->t_state) &&
            rack->r_ctl.rc_app_limited_cnt &&
            (SEQ_GT(rack->r_ctl.rc_first_appl->r_start, th_ack)) &&
            ((rack->r_ctl.rc_first_appl->r_start - th_ack) >
@@ -11741,6 +11742,13 @@ rack_start_gp_measurement(struct tcpcb *tp, struct tcp
        struct rack_sendmap *my_rsm = NULL;
        struct rack_sendmap fe;
 
+       if (tp->t_state < TCPS_ESTABLISHED) {
+               /*
+                * We don't start any measurements if we are
+                * not at least established.
+                */
+               return;
+       }
        tp->t_flags |= TF_GPUTINPROG;
        rack->r_ctl.rc_gp_lowrtt = 0xffffffff;
        rack->r_ctl.rc_gp_high_rwnd = rack->rc_tp->snd_wnd;
@@ -12109,8 +12117,10 @@ rack_output(struct tcpcb *tp)
            ((tp->t_state == TCPS_SYN_RECEIVED) ||
             (tp->t_state == TCPS_SYN_SENT)) &&
            SEQ_GT(tp->snd_max, tp->snd_una) && /* initial SYN or SYN|ACK sent 
*/
-           (tp->t_rxtshift == 0))              /* not a retransmit */
-               return (0);
+           (tp->t_rxtshift == 0)) {              /* not a retransmit */
+               cwnd_to_use = rack->r_ctl.cwnd_to_use = tp->snd_cwnd;
+               goto just_return_nolock;
+       }
        /*
         * Determine length of data that should be transmitted, and flags
         * that will be used. If there is some data or critical controls

Modified: head/sys/netinet/tcp_stacks/rack_bbr_common.c
==============================================================================
--- head/sys/netinet/tcp_stacks/rack_bbr_common.c       Fri May 15 13:53:10 
2020        (r361079)
+++ head/sys/netinet/tcp_stacks/rack_bbr_common.c       Fri May 15 14:00:12 
2020        (r361080)
@@ -466,7 +466,14 @@ ctf_do_queued_segments(struct socket *so, struct tcpcb
 uint32_t
 ctf_outstanding(struct tcpcb *tp)
 {
-       return(tp->snd_max - tp->snd_una);
+       uint32_t bytes_out;
+
+       bytes_out = tp->snd_max - tp->snd_una;
+       if (tp->t_state < TCPS_ESTABLISHED)
+               bytes_out++;
+       if (tp->t_flags & TF_SENTFIN)
+               bytes_out++;
+       return (bytes_out);
 }
 
 uint32_t
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to