Re: Remove TCP_FACK

2017-10-25 Thread Job Snijders 
This has been committed. Since the patch changed the userland ABI, don't
forget to rebuild (at least) fstat, netstat & tcpbench.

Kind regards,

Job

Re: Remove TCP_FACK

2017-10-25 Thread Mike Belopuhov 
On Tue, Oct 24, 2017 at 23:22 +0200, Job Snijders wrote:
> Dear all,
> 
> This patch builds upon the work shared in the following email. Mike's
> patch is a prerequisite to apply this patch.
> 
>   Date: Tue, 24 Oct 2017 15:21:08 +0200
>   From: Mike Belopuhov 
>   Subject: Re: Refactor TCP partial ACK handling
> 
> TCP_FACK was disabled by provos@ in June 1999. This patch removes
> the TCP_FACK option and associated #if{,n}def code.
> 
> TCP_FACK is an algorithm that decides that when something is lost, all
> not SACKed packets until the most forward SACK are lost. It may be a
> correct estimate, if network does not reorder packets. 
> 
> The algorithm described in RFC 6675 may be a better replacement. This
> culling patch can provide guidance how and where to implement 6675.
> 
> Kind regards,
> 
> Job
> 

This makes my life that much easier so naturally I'm in favour of this
change.  OK mikeb

> @@ -2705,11 +2608,9 @@ tcp_sack_partialack(struct tcpcb *tp, struct tcphdr 
> *th)
>   /* Turn off retx. timer (will start again next segment) */
>   TCP_TIMER_DISARM(tp, TCPT_REXMT);
>   tp->t_rtttime = 0;
> -#ifndef TCP_FACK
>   /*
>* Partial window deflation.  This statement relies on the
> -  * fact that tp->snd_una has not been updated yet.  In FACK
> -  * hold snd_cwnd constant during fast recovery.
> +  * fact that tp->snd_una has not been updated yet.  
>*/

trailing white space in the '+' line above.

Re: Remove TCP_FACK

2017-10-25 Thread Martin Pieuchot 
On 24/10/17(Tue) 23:22, Job Snijders wrote:
> Dear all,
> 
> This patch builds upon the work shared in the following email. Mike's
> patch is a prerequisite to apply this patch.
> 
>   Date: Tue, 24 Oct 2017 15:21:08 +0200
>   From: Mike Belopuhov 
>   Subject: Re: Refactor TCP partial ACK handling
> 
> TCP_FACK was disabled by provos@ in June 1999. This patch removes
> the TCP_FACK option and associated #if{,n}def code.
> 
> TCP_FACK is an algorithm that decides that when something is lost, all
> not SACKed packets until the most forward SACK are lost. It may be a
> correct estimate, if network does not reorder packets. 
> 
> The algorithm described in RFC 6675 may be a better replacement. This
> culling patch can provide guidance how and where to implement 6675.

I'm happy to see fewer #ifdef in these spaghetti.  Especially now that
some refactoring is welcome for future CC and MP works.

ok mpi@

> diff --git share/man/man4/options.4 share/man/man4/options.4
> index c28d4e27896..737dc29efea 100644
> --- share/man/man4/options.4
> +++ share/man/man4/options.4
> @@ -445,11 +445,6 @@ TCP to adjust the transmission rate using this signal.
>  Both communication endpoints negotiate enabling
>  .Em ECN
>  functionality at the TCP connection establishment.
> -.It Cd option TCP_FACK
> -Turns on forward acknowledgements allowing a more precise estimate of
> -outstanding data during the fast recovery phase by using
> -.Em SACK
> -information.
>  .It Cd option TCP_SIGNATURE
>  Turns on support for the TCP MD5 Signature option (RFC 2385).
>  This is used by
> diff --git sys/conf/GENERIC sys/conf/GENERIC
> index 6df800175ed..e385b45785c 100644
> --- sys/conf/GENERIC
> +++ sys/conf/GENERIC
> @@ -47,7 +47,6 @@ option  FUSE# FUSE
>  option   SOCKET_SPLICE   # Socket Splicing for TCP and UDP
>  option   TCP_ECN # Explicit Congestion Notification for 
> TCP
>  option   TCP_SIGNATURE   # TCP MD5 Signatures, for BGP routing 
> sessions
> -#option  TCP_FACK# Forward Acknowledgements for TCP
>  
>  option   INET6   # IPv6
>  option   IPSEC   # IPsec
> diff --git sys/netinet/tcp_input.c sys/netinet/tcp_input.c
> index 8d172e2905c..4321d85854c 100644
> --- sys/netinet/tcp_input.c
> +++ sys/netinet/tcp_input.c
> @@ -974,10 +974,6 @@ findpcb:
>   if (SEQ_GT(tp->snd_una, tp->snd_last))
>  #endif
>   tp->snd_last = tp->snd_una;
> -#ifdef TCP_FACK
> - tp->snd_fack = tp->snd_una;
> - tp->retran_data = 0;
> -#endif
>   m_freem(m);
>  
>   /*
> @@ -1566,18 +1562,7 @@ trimthenstep6:
>*/
>   if (TCP_TIMER_ISARMED(tp, TCPT_REXMT) == 0)
>   tp->t_dupacks = 0;
> -#ifdef TCP_FACK
> - /*
> -  * In FACK, can enter fast rec. if the receiver
> -  * reports a reass. queue longer than 3 segs.
> -  */
> - else if (++tp->t_dupacks == tcprexmtthresh ||
> - ((SEQ_GT(tp->snd_fack, tcprexmtthresh *
> - tp->t_maxseg + tp->snd_una)) &&
> - SEQ_GT(tp->snd_una, tp->snd_last))) {
> -#else
>   else if (++tp->t_dupacks == tcprexmtthresh) {
> -#endif /* TCP_FACK */
>   tcp_seq onxt = tp->snd_nxt;
>   u_long win =
>   ulmin(tp->snd_wnd, tp->snd_cwnd) /
> @@ -1603,15 +1588,6 @@ trimthenstep6:
>  #endif
>   tcpstat_inc(tcps_cwr_frecovery);
>   
> tcpstat_inc(tcps_sack_recovery_episode);
> -#ifdef TCP_FACK
> - tp->t_dupacks = tcprexmtthresh;
> - (void) tcp_output(tp);
> - /*
> -  * During FR, snd_cwnd is held
> -  * constant for FACK.
> -  */
> - tp->snd_cwnd = tp->snd_ssthresh;
> -#else
>   /*
>* tcp_output() will send
>* oldest SACK-eligible rtx.
> @@ -1619,7 +1595,6 @@ trimthenstep6:
>   (void) tcp_output(tp);
>   tp->snd_cwnd = tp->snd_ssthresh+
>

Remove TCP_FACK

2017-10-24 Thread Job Snijders 
Dear all,

This patch builds upon the work shared in the following email. Mike's
patch is a prerequisite to apply this patch.

Date: Tue, 24 Oct 2017 15:21:08 +0200
From: Mike Belopuhov 
Subject: Re: Refactor TCP partial ACK handling

TCP_FACK was disabled by provos@ in June 1999. This patch removes
the TCP_FACK option and associated #if{,n}def code.

TCP_FACK is an algorithm that decides that when something is lost, all
not SACKed packets until the most forward SACK are lost. It may be a
correct estimate, if network does not reorder packets. 

The algorithm described in RFC 6675 may be a better replacement. This
culling patch can provide guidance how and where to implement 6675.

Kind regards,

Job

 share/man/man4/options.4 |   5 ---
 sys/conf/GENERIC |   1 -
 sys/netinet/tcp_input.c  | 111 +--
 sys/netinet/tcp_output.c |  42 --
 sys/netinet/tcp_timer.c  |   5 ---
 sys/netinet/tcp_usrreq.c |   5 ---
 sys/netinet/tcp_var.h|   6 ---
 usr.bin/netstat/inet.c   |   3 --
 8 files changed, 1 insertion(+), 177 deletions(-)

diff --git share/man/man4/options.4 share/man/man4/options.4
index c28d4e27896..737dc29efea 100644
--- share/man/man4/options.4
+++ share/man/man4/options.4
@@ -445,11 +445,6 @@ TCP to adjust the transmission rate using this signal.
 Both communication endpoints negotiate enabling
 .Em ECN
 functionality at the TCP connection establishment.
-.It Cd option TCP_FACK
-Turns on forward acknowledgements allowing a more precise estimate of
-outstanding data during the fast recovery phase by using
-.Em SACK
-information.
 .It Cd option TCP_SIGNATURE
 Turns on support for the TCP MD5 Signature option (RFC 2385).
 This is used by
diff --git sys/conf/GENERIC sys/conf/GENERIC
index 6df800175ed..e385b45785c 100644
--- sys/conf/GENERIC
+++ sys/conf/GENERIC
@@ -47,7 +47,6 @@ optionFUSE# FUSE
 option SOCKET_SPLICE   # Socket Splicing for TCP and UDP
 option TCP_ECN # Explicit Congestion Notification for TCP
 option TCP_SIGNATURE   # TCP MD5 Signatures, for BGP routing sessions
-#optionTCP_FACK# Forward Acknowledgements for TCP
 
 option INET6   # IPv6
 option IPSEC   # IPsec
diff --git sys/netinet/tcp_input.c sys/netinet/tcp_input.c
index 8d172e2905c..4321d85854c 100644
--- sys/netinet/tcp_input.c
+++ sys/netinet/tcp_input.c
@@ -974,10 +974,6 @@ findpcb:
if (SEQ_GT(tp->snd_una, tp->snd_last))
 #endif
tp->snd_last = tp->snd_una;
-#ifdef TCP_FACK
-   tp->snd_fack = tp->snd_una;
-   tp->retran_data = 0;
-#endif
m_freem(m);
 
/*
@@ -1566,18 +1562,7 @@ trimthenstep6:
 */
if (TCP_TIMER_ISARMED(tp, TCPT_REXMT) == 0)
tp->t_dupacks = 0;
-#ifdef TCP_FACK
-   /*
-* In FACK, can enter fast rec. if the receiver
-* reports a reass. queue longer than 3 segs.
-*/
-   else if (++tp->t_dupacks == tcprexmtthresh ||
-   ((SEQ_GT(tp->snd_fack, tcprexmtthresh *
-   tp->t_maxseg + tp->snd_una)) &&
-   SEQ_GT(tp->snd_una, tp->snd_last))) {
-#else
else if (++tp->t_dupacks == tcprexmtthresh) {
-#endif /* TCP_FACK */
tcp_seq onxt = tp->snd_nxt;
u_long win =
ulmin(tp->snd_wnd, tp->snd_cwnd) /
@@ -1603,15 +1588,6 @@ trimthenstep6:
 #endif
tcpstat_inc(tcps_cwr_frecovery);

tcpstat_inc(tcps_sack_recovery_episode);
-#ifdef TCP_FACK
-   tp->t_dupacks = tcprexmtthresh;
-   (void) tcp_output(tp);
-   /*
-* During FR, snd_cwnd is held
-* constant for FACK.
-*/
-   tp->snd_cwnd = tp->snd_ssthresh;
-#else
/*
 * tcp_output() will send
 * oldest SACK-eligible rtx.
@@ -1619,7 +1595,6 @@ trimthenstep6:
(void) tcp_output(tp);