ofctrl-seqno: Do not truncate the last acked value

The requested and acked seqno values are allowed
to be uint64_t, however the values that were added
to the hmap were truncated to uint32_t. This would
lead to loss of information when the value is bigger.

Use uin64_t for the function signatures and for the
hash to prevent truncation.

Fixes: c93c626 ("controller: Implement a generic barrier based on ofctrl cur_cfg sync.")
Reported-at: https://bugzilla.redhat.com/2074019
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 292717d)

controller/ofctrl-seqno.c

@@ -59,7 +59,7 @@ static struct ofctrl_seqno_state *ofctrl_seqno_states;
 static void ofctrl_acked_seqnos_init(struct ofctrl_acked_seqnos *seqnos,
                                      uint64_t last_acked);
 static void ofctrl_acked_seqnos_add(struct ofctrl_acked_seqnos *seqnos,
-                                    uint32_t val);
+                                    uint64_t val);
 
 /* ofctrl_seqno_update related static function prototypes. */
 static void ofctrl_seqno_update_create__(size_t seqno_type, uint64_t req_cfg);
@@ -106,11 +106,11 @@ ofctrl_acked_seqnos_destroy(struct ofctrl_acked_seqnos *seqnos)
 /* Returns true if 'val' is one of the acked sequence numbers in 'seqnos'. */
 bool
 ofctrl_acked_seqnos_contains(const struct ofctrl_acked_seqnos *seqnos,
-                             uint32_t val)
+                             uint64_t val)
 {
     struct ofctrl_ack_seqno *sn;
 
-    HMAP_FOR_EACH_WITH_HASH (sn, node, hash_int(val, 0), &seqnos->acked) {
+    HMAP_FOR_EACH_WITH_HASH (sn, node, hash_uint64(val), &seqnos->acked) {
         if (sn->seqno == val) {
             return true;
         }
@@ -213,12 +213,12 @@ ofctrl_acked_seqnos_init(struct ofctrl_acked_seqnos *seqnos,
 }
 
 static void
-ofctrl_acked_seqnos_add(struct ofctrl_acked_seqnos *seqnos, uint32_t val)
+ofctrl_acked_seqnos_add(struct ofctrl_acked_seqnos *seqnos, uint64_t val)
 {
     seqnos->last_acked = val;
 
     struct ofctrl_ack_seqno *sn = xmalloc(sizeof *sn);
-    hmap_insert(&seqnos->acked, &sn->node, hash_int(val, 0));
+    hmap_insert(&seqnos->acked, &sn->node, hash_uint64(val));
     sn->seqno = val;
 }
 

controller/ofctrl-seqno.h

@@ -37,7 +37,7 @@ struct ofctrl_ack_seqno {
 struct ofctrl_acked_seqnos *ofctrl_acked_seqnos_get(size_t seqno_type);
 void ofctrl_acked_seqnos_destroy(struct ofctrl_acked_seqnos *seqnos);
 bool ofctrl_acked_seqnos_contains(const struct ofctrl_acked_seqnos *seqnos,
-                                  uint32_t val);
+                                  uint64_t val);
 
 void ofctrl_seqno_init(void);
 size_t ofctrl_seqno_add_type(void);

controller/test-ofctrl-seqno.c

@@ -124,9 +124,10 @@ test_ofctrl_seqno_ack_seqnos(struct ovs_cmdl_context *ctx)
         }
 
         for (unsigned int j = 0; j < n_app_seqnos; j++, n_reqs++) {
-            unsigned int app_seqno;
+            unsigned long long int app_seqno;
 
-            if (!test_read_uint_value(ctx, shift++, "app_seqno", &app_seqno)) {
+            if (!test_read_ullong_value(ctx, shift++, "app_seqno",
+                                        &app_seqno)) {
                 return;
             }
             ofctrl_seqno_update_create(i, app_seqno);
@@ -138,9 +139,9 @@ test_ofctrl_seqno_ack_seqnos(struct ovs_cmdl_context *ctx)
         return;
     }
     for (unsigned int i = 0; i < n_acks; i++) {
-        unsigned int ack_seqno;
+        unsigned long long int ack_seqno;
 
-        if (!test_read_uint_value(ctx, shift++, "ack_seqno", &ack_seqno)) {
+        if (!test_read_ullong_value(ctx, shift++, "ack_seqno", &ack_seqno)) {
             return;
         }
         ofctrl_seqno_run(ack_seqno);

tests/ovn-ofctrl-seqno.at

@@ -223,4 +223,37 @@ ofctrl-seqno-type: 1
   51
   52
 ])
+
+AS_BOX([Ack seqno that doesn't fit in uint32_t])
+n_types=2
+n_app_seqnos=1
+app_seqnos1="4294967296"
+app_seqnos2="4294967297"
+
+n_acks=1
+acks="1"
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
+          ${n_acks} ${acks}], [0], [dnl
+ofctrl-seqno-req-cfg: 2
+ofctrl-seqno-type: 0
+  last-acked 4294967296
+  4294967296
+ofctrl-seqno-type: 1
+  last-acked 0
+])
+
+n_acks=1
+acks="2"
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
+          ${n_acks} ${acks}], [0], [dnl
+ofctrl-seqno-req-cfg: 2
+ofctrl-seqno-type: 0
+  last-acked 4294967296
+  4294967296
+ofctrl-seqno-type: 1
+  last-acked 4294967297
+  4294967297
+])
 AT_CLEANUP

tests/test-utils.c

@@ -61,3 +61,20 @@ test_read_value(struct ovs_cmdl_context *ctx, unsigned int index,
 
     return ctx->argv[index];
 }
+
+bool
+test_read_ullong_value(struct ovs_cmdl_context *ctx, unsigned int index,
+                       const char *descr, unsigned long long int *result)
+{
+    if (index >= ctx->argc) {
+        fprintf(stderr, "Missing %s argument\n", descr);
+        return false;
+    }
+
+    const char *arg = ctx->argv[index];
+    if (!str_to_ullong(arg, 10, result)) {
+        fprintf(stderr, "Invalid %s: %s\n", descr, arg);
+        return false;
+    }
+    return true;
+}

tests/test-utils.h

@@ -24,5 +24,7 @@ bool test_read_uint_hex_value(struct ovs_cmdl_context *ctx, unsigned int index,
                               const char *descr, unsigned int *result);
 const char *test_read_value(struct ovs_cmdl_context *ctx, unsigned int index,
                             const char *descr);
+bool test_read_ullong_value(struct ovs_cmdl_context *ctx, unsigned int index,
+                            const char *descr, unsigned long long int *result);
 
 #endif /* tests/test-utils.h */