RE: git: f74352fbcf15 - main - tcp: use enum for all congestion control signals

From: Scheffenegger, Richard <Richard.Scheffenegger_at_netapp.com>
Date: Sat, 24 Feb 2024 20:30:43 UTC
Thanks for the heads-up. Is fixed with 5e248c23d995a059d24f4784d5a256cddd42e557


Richard

From: Baptiste Daroussin <bapt@FreeBSD.org>
Sent: Samstag, 24. Februar 2024 19:24
To: Richard Scheffenegger <rscheff@FreeBSD.org>; src-committers@FreeBSD.org; dev-commits-src-all@FreeBSD.org; dev-commits-src-main@FreeBSD.org
Subject: Re: git: f74352fbcf15 - main - tcp: use enum for all congestion control signals

EXTERNAL EMAIL - USE CAUTION when clicking links or attachments


Le 24 février 2024 17:15:38 GMT+01:00, Richard Scheffenegger <rscheff@FreeBSD.org<mailto:rscheff@FreeBSD.org>> a écrit :

The branch main has been updated by rscheff:

URL: https://cgit.FreeBSD.org/src/commit/?id=f74352fbcf15341accaf5a92240871f98323215d<https://cgit.freebsd.org/src/commit/?id=f74352fbcf15341accaf5a92240871f98323215d>

commit f74352fbcf15341accaf5a92240871f98323215d
Author:     Richard Scheffenegger <rscheff@FreeBSD.org<mailto:rscheff@FreeBSD.org>>
AuthorDate: 2024-02-24 15:41:31 +0000
Commit:     Richard Scheffenegger <rscheff@FreeBSD.org<mailto:rscheff@FreeBSD.org>>
CommitDate: 2024-02-24 15:41:48 +0000

    tcp: use enum for all congestion control signals

    Facilitate easier troubleshooting by enumerating
    all congestion control signals. Typecast the
    enum to int, when a congestion control module uses
    private signals.

    No external change.

    Reviewed By:            glebius, tuexen, #transport
    Sponsored by:           NetApp, Inc.
    Differential Revision:  https://reviews.freebsd.org/D43838

________________________________

 sys/netinet/cc/cc.c         |  2 +-
 sys/netinet/cc/cc.h         | 42 +++++++++++++++++++++---------------------
 sys/netinet/cc/cc_cdg.c     | 10 +++++-----
 sys/netinet/cc/cc_chd.c     | 11 ++++++-----
 sys/netinet/cc/cc_cubic.c   | 10 ++++++----
 sys/netinet/cc/cc_dctcp.c   | 10 ++++++----
 sys/netinet/cc/cc_hd.c      |  4 ++--
 sys/netinet/cc/cc_htcp.c    | 10 ++++++----
 sys/netinet/cc/cc_newreno.c | 10 ++++++----
 sys/netinet/cc/cc_vegas.c   | 13 +++++++------
 10 files changed, 66 insertions(+), 56 deletions(-)

diff --git a/sys/netinet/cc/cc.c b/sys/netinet/cc/cc.c
index c2965b1e6a48..9308b5f8d764 100644
--- a/sys/netinet/cc/cc.c
+++ b/sys/netinet/cc/cc.c
@@ -505,7 +505,7 @@ newreno_cc_cong_signal(struct cc_var *ccv, uint32_t type)
 }

 void
-newreno_cc_ack_received(struct cc_var *ccv, uint16_t type)
+newreno_cc_ack_received(struct cc_var *ccv, ccsignal_t type)
 {
      if (type == CC_ACK && !IN_RECOVERY(CCV(ccv, t_flags)) &&
          (ccv->flags & CCF_CWND_LIMITED)) {
diff --git a/sys/netinet/cc/cc.h b/sys/netinet/cc/cc.h
index 9571da50b2c7..5b2cb58a24a0 100644
--- a/sys/netinet/cc/cc.h
+++ b/sys/netinet/cc/cc.h
@@ -121,25 +121,25 @@ struct cc_var {
 #define CCF_HYSTART_CAN_SH_CWND    0x0800  /* Can hystart when going CSS -> CA slam the cwnd */
 #define CCF_HYSTART_CONS_SSTH      0x1000  /* Should hystart use the more conservative ssthresh */

-/* ACK types passed to the ack_received() hook. */
-#define     CC_ACK         0x0001  /* Regular in sequence ACK. */
-#define     CC_DUPACK      0x0002  /* Duplicate ACK. */
-#define     CC_PARTIALACK  0x0004  /* Not yet. */
-#define     CC_SACK        0x0008  /* Not yet. */
+typedef enum {
+     /* ACK types passed to the ack_received() hook. */
+     CC_ACK =       0x0001, /* Regular in sequence ACK. */
+     CC_DUPACK =    0x0002, /* Duplicate ACK. */
+     CC_PARTIALACK = 0x0004, /* Not yet. */
+     CC_SACK =      0x0008, /* Not yet. */
+     /* Congestion signal types passed to the cong_signal() hook. */
+     CC_ECN =       0x0100, /* ECN marked packet received. */
+     CC_RTO =       0x0200, /* RTO fired. */
+     CC_RTO_ERR =   0x0400, /* RTO fired in error. */
+     CC_NDUPACK =   0x0800, /* Threshold of dupack's reached. */
+     /*
+      * The highest order 8 bits (0x01000000 - 0x80000000) are reserved
+      * for CC algos to declare their own congestion signal types.
+      */
+     CC_SIGPRIVMASK = 0xFF000000    /* Mask to check if sig is private. */
+} ccsignal_t;
 #endif /* defined(_KERNEL) || defined(_WANT_TCPCB) */

-/*
- * Congestion signal types passed to the cong_signal() hook. The highest order 8
- * bits (0x01000000 - 0x80000000) are reserved for CC algos to declare their own
- * congestion signal types.
- */
-#define     CC_ECN         0x00000001     /* ECN marked packet received. */
-#define     CC_RTO         0x00000002     /* RTO fired. */
-#define     CC_RTO_ERR     0x00000004     /* RTO fired in error. */
-#define     CC_NDUPACK     0x00000008     /* Threshold of dupack's reached. */
-
-#define     CC_SIGPRIVMASK 0xFF000000     /* Mask to check if sig is private. */
-
 #ifdef _KERNEL
 /*
  * Structure to hold data and function pointers that together represent a
@@ -175,10 +175,10 @@ struct cc_algo {
      void    (*conn_init)(struct cc_var *ccv);

      /* Called on receipt of an ack. */
-     void    (*ack_received)(struct cc_var *ccv, uint16_t type);
+     void    (*ack_received)(struct cc_var *ccv, ccsignal_t type);

      /* Called on detection of a congestion signal. */
-     void    (*cong_signal)(struct cc_var *ccv, uint32_t type);
+     void    (*cong_signal)(struct cc_var *ccv, ccsignal_t type);

      /* Called after exiting congestion recovery. */
      void    (*post_recovery)(struct cc_var *ccv);
@@ -236,8 +236,8 @@ extern struct rwlock cc_list_lock;
  */
 void newreno_cc_post_recovery(struct cc_var *);
 void newreno_cc_after_idle(struct cc_var *);
-void newreno_cc_cong_signal(struct cc_var *, uint32_t );
-void newreno_cc_ack_received(struct cc_var *, uint16_t);
+void newreno_cc_cong_signal(struct cc_var *, ccsignal_t);
+void newreno_cc_ack_received(struct cc_var *, ccsignal_t);

 /* Called to temporarily keep an algo from going away during change */
 void cc_refer(struct cc_algo *algo);
diff --git a/sys/netinet/cc/cc_cdg.c b/sys/netinet/cc/cc_cdg.c
index 3f23c4091170..1e9236f878d4 100644
--- a/sys/netinet/cc/cc_cdg.c
+++ b/sys/netinet/cc/cc_cdg.c
@@ -221,8 +221,8 @@ static int cdg_mod_destroy(void);
 static void cdg_conn_init(struct cc_var *ccv);
 static int cdg_cb_init(struct cc_var *ccv, void *ptr);
 static void cdg_cb_destroy(struct cc_var *ccv);
-static void cdg_cong_signal(struct cc_var *ccv, uint32_t signal_type);
-static void cdg_ack_received(struct cc_var *ccv, uint16_t ack_type);
+static void cdg_cong_signal(struct cc_var *ccv, ccsignal_t signal_type);
+static void cdg_ack_received(struct cc_var *ccv, ccsignal_t ack_type);
 static size_t cdg_data_sz(void);

 struct cc_algo cdg_cc_algo = {
@@ -450,11 +450,11 @@ cdg_window_increase(struct cc_var *ccv, int new_measurement)
 }

 static void
-cdg_cong_signal(struct cc_var *ccv, uint32_t signal_type)
+cdg_cong_signal(struct cc_var *ccv, ccsignal_t signal_type)
 {
      struct cdg *cdg_data = ccv->cc_data;

-     switch(signal_type) {
+     switch((int)signal_type) {
      case CC_CDG_DELAY:
             CCV(ccv, snd_ssthresh) = cdg_window_decrease(ccv,
                 CCV(ccv, snd_cwnd), V_cdg_beta_delay);
@@ -571,7 +571,7 @@ calc_moving_average(struct cdg *cdg_data, long qdiff_max, long qdiff_min)
 }

 static void
-cdg_ack_received(struct cc_var *ccv, uint16_t ack_type)
+cdg_ack_received(struct cc_var *ccv, ccsignal_t ack_type)
 {
      struct cdg *cdg_data;
      struct ertt *e_t;
diff --git a/sys/netinet/cc/cc_chd.c b/sys/netinet/cc/cc_chd.c
index c644d9b2cdb8..52048a7c05ae 100644
--- a/sys/netinet/cc/cc_chd.c
+++ b/sys/netinet/cc/cc_chd.c
@@ -88,10 +88,10 @@
 /* Largest possible number returned by random(). */
 #define     RANDOM_MAX     INT_MAX

-static void chd_ack_received(struct cc_var *ccv, uint16_t ack_type);
+static void chd_ack_received(struct cc_var *ccv, ccsignal_t ack_type);
 static void chd_cb_destroy(struct cc_var *ccv);
 static int  chd_cb_init(struct cc_var *ccv, void *ptr);
-static void chd_cong_signal(struct cc_var *ccv, uint32_t signal_type);
+static void chd_cong_signal(struct cc_var *ccv, ccsignal_t signal_type);
 static void chd_conn_init(struct cc_var *ccv);
 static int  chd_mod_init(void);
 static size_t       chd_data_sz(void);
@@ -235,7 +235,7 @@ chd_window_increase(struct cc_var *ccv, int new_measurement)
  * ack_type == CC_ACK.
  */
 static void
-chd_ack_received(struct cc_var *ccv, uint16_t ack_type)
+chd_ack_received(struct cc_var *ccv, ccsignal_t ack_type)
 {
      struct chd *chd_data;
      struct ertt *e_t;
@@ -336,7 +336,7 @@ chd_cb_init(struct cc_var *ccv, void *ptr)
 }

 static void
-chd_cong_signal(struct cc_var *ccv, uint32_t signal_type)
+chd_cong_signal(struct cc_var *ccv, ccsignal_t signal_type)
 {
      struct ertt *e_t;
      struct chd *chd_data;
@@ -346,7 +346,7 @@ chd_cong_signal(struct cc_var *ccv, uint32_t signal_type)
      chd_data = ccv->cc_data;
      qdly = imax(e_t->rtt, chd_data->maxrtt_in_rtt) - e_t->minrtt;

-     switch(signal_type) {
+     switch((int)signal_type) {
      case CC_CHD_DELAY:
             chd_window_decrease(ccv); /* Set new ssthresh. */
             CCV(ccv, snd_cwnd) = CCV(ccv, snd_ssthresh);
@@ -387,6 +387,7 @@ chd_cong_signal(struct cc_var *ccv, uint32_t signal_type)

      default:
             newreno_cc_cong_signal(ccv, signal_type);
+            break;
      }
 }

diff --git a/sys/netinet/cc/cc_cubic.c b/sys/netinet/cc/cc_cubic.c
index eb1587d44427..a9c7592b80ca 100644
--- a/sys/netinet/cc/cc_cubic.c
+++ b/sys/netinet/cc/cc_cubic.c
@@ -73,10 +73,10 @@
 #include <netinet/cc/cc_cubic.h>
 #include <netinet/cc/cc_module.h>

-static void cubic_ack_received(struct cc_var *ccv, uint16_t type);
+static void cubic_ack_received(struct cc_var *ccv, ccsignal_t type);
 static void cubic_cb_destroy(struct cc_var *ccv);
 static int  cubic_cb_init(struct cc_var *ccv, void *ptr);
-static void cubic_cong_signal(struct cc_var *ccv, uint32_t type);
+static void cubic_cong_signal(struct cc_var *ccv, ccsignal_t type);
 static void cubic_conn_init(struct cc_var *ccv);
 static int  cubic_mod_init(void);
 static void cubic_post_recovery(struct cc_var *ccv);
@@ -233,7 +233,7 @@ cubic_does_slow_start(struct cc_var *ccv, struct cubic *cubicd)
 }

 static void
-cubic_ack_received(struct cc_var *ccv, uint16_t type)
+cubic_ack_received(struct cc_var *ccv, ccsignal_t type)
 {
      struct cubic *cubic_data;
      unsigned long W_est, W_cubic;
@@ -417,7 +417,7 @@ cubic_cb_init(struct cc_var *ccv, void *ptr)
  * Perform any necessary tasks before we enter congestion recovery.
  */
 static void
-cubic_cong_signal(struct cc_var *ccv, uint32_t type)
+cubic_cong_signal(struct cc_var *ccv, ccsignal_t type)
 {
      struct cubic *cubic_data;
      uint32_t mss, pipe;
@@ -503,6 +503,8 @@ cubic_cong_signal(struct cc_var *ccv, uint32_t type)
             cubic_data->cwnd_epoch = cubic_data->undo_cwnd_epoch;
             cubic_data->t_epoch = cubic_data->undo_t_epoch;
             break;
+     default:
+            break;
      }
 }

diff --git a/sys/netinet/cc/cc_dctcp.c b/sys/netinet/cc/cc_dctcp.c
index ae0a56839449..374db98c5e60 100644
--- a/sys/netinet/cc/cc_dctcp.c
+++ b/sys/netinet/cc/cc_dctcp.c
@@ -79,11 +79,11 @@ struct dctcp {
      uint32_t num_cong_events; /* # of congestion events */
 };

-static void dctcp_ack_received(struct cc_var *ccv, uint16_t type);
+static void dctcp_ack_received(struct cc_var *ccv, ccsignal_t type);
 static void dctcp_after_idle(struct cc_var *ccv);
 static void dctcp_cb_destroy(struct cc_var *ccv);
 static int  dctcp_cb_init(struct cc_var *ccv, void *ptr);
-static void dctcp_cong_signal(struct cc_var *ccv, uint32_t type);
+static void dctcp_cong_signal(struct cc_var *ccv, ccsignal_t type);
 static void dctcp_conn_init(struct cc_var *ccv);
 static void dctcp_post_recovery(struct cc_var *ccv);
 static void dctcp_ecnpkt_handler(struct cc_var *ccv);
@@ -104,7 +104,7 @@ struct cc_algo dctcp_cc_algo = {
 };

 static void
-dctcp_ack_received(struct cc_var *ccv, uint16_t type)
+dctcp_ack_received(struct cc_var *ccv, ccsignal_t type)
 {
      struct dctcp *dctcp_data;
      int bytes_acked = 0;
@@ -237,7 +237,7 @@ dctcp_cb_init(struct cc_var *ccv, void *ptr)
  * Perform any necessary tasks before we enter congestion recovery.
  */
 static void
-dctcp_cong_signal(struct cc_var *ccv, uint32_t type)
+dctcp_cong_signal(struct cc_var *ccv, ccsignal_t type)
 {
      struct dctcp *dctcp_data;
      uint32_t cwin, mss, pipe;
@@ -308,6 +308,8 @@ dctcp_cong_signal(struct cc_var *ccv, uint32_t type)
                     dctcp_data->save_sndnxt += CCV(ccv, t_maxseg);
                     dctcp_data->num_cong_events++;
                     break;
+            default:
+                    break;
             }
      } else
             newreno_cc_cong_signal(ccv, type);
diff --git a/sys/netinet/cc/cc_hd.c b/sys/netinet/cc/cc_hd.c
index 1a8b62ccf426..82486563f97e 100644
--- a/sys/netinet/cc/cc_hd.c
+++ b/sys/netinet/cc/cc_hd.c
@@ -80,7 +80,7 @@
 /* Largest possible number returned by random(). */
 #define     RANDOM_MAX     INT_MAX

-static void hd_ack_received(struct cc_var *ccv, uint16_t ack_type);
+static void hd_ack_received(struct cc_var *ccv, ccsignal_t ack_type);
 static int  hd_mod_init(void);
 static size_t       hd_data_sz(void);

@@ -138,7 +138,7 @@ should_backoff(int qdly, int maxqdly)
  * as NewReno in all other circumstances.
  */
 static void
-hd_ack_received(struct cc_var *ccv, uint16_t ack_type)
+hd_ack_received(struct cc_var *ccv, ccsignal_t ack_type)
 {
      struct ertt *e_t;
      int qdly;
diff --git a/sys/netinet/cc/cc_htcp.c b/sys/netinet/cc/cc_htcp.c
index 43224446fd84..41c552a3bfa0 100644
--- a/sys/netinet/cc/cc_htcp.c
+++ b/sys/netinet/cc/cc_htcp.c
@@ -136,10 +136,10 @@
      (((diff) / hz) * (((diff) << HTCP_ALPHA_INC_SHIFT) / (4 * hz))) \
 ) >> HTCP_ALPHA_INC_SHIFT)

-static void htcp_ack_received(struct cc_var *ccv, uint16_t type);
+static void htcp_ack_received(struct cc_var *ccv, ccsignal_t type);
 static void htcp_cb_destroy(struct cc_var *ccv);
 static int  htcp_cb_init(struct cc_var *ccv, void *ptr);
-static void htcp_cong_signal(struct cc_var *ccv, uint32_t type);
+static void htcp_cong_signal(struct cc_var *ccv, ccsignal_t type);
 static int  htcp_mod_init(void);
 static void htcp_post_recovery(struct cc_var *ccv);
 static void htcp_recalc_alpha(struct cc_var *ccv);
@@ -190,7 +190,7 @@ struct cc_algo htcp_cc_algo = {
 };

 static void
-htcp_ack_received(struct cc_var *ccv, uint16_t type)
+htcp_ack_received(struct cc_var *ccv, ccsignal_t type)
 {
      struct htcp *htcp_data;

@@ -278,7 +278,7 @@ htcp_cb_init(struct cc_var *ccv, void *ptr)
  * Perform any necessary tasks before we enter congestion recovery.
  */
 static void
-htcp_cong_signal(struct cc_var *ccv, uint32_t type)
+htcp_cong_signal(struct cc_var *ccv, ccsignal_t type)
 {
      struct htcp *htcp_data;
      uint32_t mss, pipe;
@@ -345,6 +345,8 @@ htcp_cong_signal(struct cc_var *ccv, uint32_t type)
             if (CCV(ccv, t_rxtshift) >= 2)
                     htcp_data->t_last_cong = ticks;
             break;
+     default:
+            break;
      }
 }

diff --git a/sys/netinet/cc/cc_newreno.c b/sys/netinet/cc/cc_newreno.c
index 71f2764ef4bc..aa20e2c64f7d 100644
--- a/sys/netinet/cc/cc_newreno.c
+++ b/sys/netinet/cc/cc_newreno.c
@@ -84,9 +84,9 @@
 #include <netinet/cc/cc_newreno.h>

 static void newreno_cb_destroy(struct cc_var *ccv);
-static void newreno_ack_received(struct cc_var *ccv, uint16_t type);
+static void newreno_ack_received(struct cc_var *ccv, ccsignal_t type);
 static void newreno_after_idle(struct cc_var *ccv);
-static void newreno_cong_signal(struct cc_var *ccv, uint32_t type);
+static void newreno_cong_signal(struct cc_var *ccv, ccsignal_t type);
 static int newreno_ctl_output(struct cc_var *ccv, struct sockopt *sopt, void *buf);
 static void newreno_newround(struct cc_var *ccv, uint32_t round_cnt);
 static void newreno_rttsample(struct cc_var *ccv, uint32_t usec_rtt, uint32_t rxtcnt, uint32_t fas);
@@ -212,7 +212,7 @@ newreno_cb_destroy(struct cc_var *ccv)
 }

 static void
-newreno_ack_received(struct cc_var *ccv, uint16_t type)
+newreno_ack_received(struct cc_var *ccv, ccsignal_t type)
 {
      struct newreno *nreno;

@@ -363,7 +363,7 @@ newreno_after_idle(struct cc_var *ccv)
  * Perform any necessary tasks before we enter congestion recovery.
  */
 static void
-newreno_cong_signal(struct cc_var *ccv, uint32_t type)
+newreno_cong_signal(struct cc_var *ccv, ccsignal_t type)
 {
      struct newreno *nreno;
      uint32_t beta, beta_ecn, cwin, factor, mss, pipe;
@@ -442,6 +442,8 @@ newreno_cong_signal(struct cc_var *ccv, uint32_t type)
             }
             CCV(ccv, snd_cwnd) = mss;
             break;
+     default:
+            break;
      }
 }

diff --git a/sys/netinet/cc/cc_vegas.c b/sys/netinet/cc/cc_vegas.c
index aac9c9ce77ff..ecd42c1a0f53 100644
--- a/sys/netinet/cc/cc_vegas.c
+++ b/sys/netinet/cc/cc_vegas.c
@@ -84,12 +84,12 @@
  * Private signal type for rate based congestion signal.
  * See <netinet/cc.h> for appropriate bit-range to use for private signals.
  */
-#define     CC_VEGAS_RATE  0x01000000
+#define     CC_VEGAS_RATE  0x04000000

-static void vegas_ack_received(struct cc_var *ccv, uint16_t ack_type);
+static void vegas_ack_received(struct cc_var *ccv, ccsignal_t ack_type);
 static void vegas_cb_destroy(struct cc_var *ccv);
 static int  vegas_cb_init(struct cc_var *ccv, void *ptr);
-static void vegas_cong_signal(struct cc_var *ccv, uint32_t signal_type);
+static void vegas_cong_signal(struct cc_var *ccv, ccsignal_t signal_type);
 static void vegas_conn_init(struct cc_var *ccv);
 static int  vegas_mod_init(void);
 static size_t       vegas_data_sz(void);
@@ -124,7 +124,7 @@ struct cc_algo vegas_cc_algo = {
  * has been used.
  */
 static void
-vegas_ack_received(struct cc_var *ccv, uint16_t ack_type)
+vegas_ack_received(struct cc_var *ccv, ccsignal_t ack_type)
 {
      struct ertt *e_t;
      struct vegas *vegas_data;
@@ -203,7 +203,7 @@ vegas_cb_init(struct cc_var *ccv, void *ptr)
  * handled here, otherwise it falls back to newreno's congestion handling.
  */
 static void
-vegas_cong_signal(struct cc_var *ccv, uint32_t signal_type)
+vegas_cong_signal(struct cc_var *ccv, ccsignal_t signal_type)
 {
      struct vegas *vegas_data;
      int presignalrecov;
@@ -215,7 +215,7 @@ vegas_cong_signal(struct cc_var *ccv, uint32_t signal_type)
      else
             presignalrecov = 0;

-     switch(signal_type) {
+     switch((int)signal_type) {
      case CC_VEGAS_RATE:
             if (!IN_RECOVERY(CCV(ccv, t_flags))) {
                     CCV(ccv, snd_cwnd) = max(2 * CCV(ccv, t_maxseg),
@@ -228,6 +228,7 @@ vegas_cong_signal(struct cc_var *ccv, uint32_t signal_type)

      default:
             newreno_cc_cong_signal(ccv, signal_type);
+            break;
      }

      if (IN_RECOVERY(CCV(ccv, t_flags)) && !presignalrecov)

Buildworld is now broken for all arches (at least incremental build)

Among the errors

/home/bapt/worktrees/main/lib/libstats/../../sys/netinet/tcp_stats.c:138:32: error: use of undeclared identifier 'CC_NDUPACK'