Re: [ovs-dev] [PATCH ovn] expr: Combine multiple ipv4 with wildcard mask.

Wed, 10 Feb 2021 17:26:49 -0800

Hi Gongming. I saw Dumitru's review, so I won't repeat anything he said. I have a few comments of my own below. 

On 2/7/21 7:25 AM, gmingchen(陈供明) wrote:

From: Gongming Chen <gmingc...@tencent.com>

In the ovn security group, each host ip corresponds to at least 4 flow
tables (different connection states). As the scale of hosts using the
security group increases, the ovs security group flow table will
increase sharply, especially when it is applied  the remote group
feature in OpenStack.

This patch merges ipv4 addresses with wildcard masks, and replaces this
ipv4 addresses with the merged ip/mask. This will greatly reduce the
entries in the ovs security group flow table, especially when the host
size is large. After being used in a production environment, the number
of ovs flow tables will be reduced by at least 50% in most scenarios,
when the remote group in OpenStack is applied.

Analysis in the simplest scenario, a network 1.1.1.0/24 network, enable
the OpenStack security group remote group feature, create 253 virtual
machine ports(1.1.1.2-1.1.1.254).

Only focus on the number of ip addresses, in the table=44 table:
./configure --disable-combine-ipv4:
1.1.1.2-1.1.1.254(253 flow meters) * 4(connection status) *
1(local net of localport) = 1012

./configure --enable-combine-ipv4(default):
1.1.1.2/31
1.1.1.4/30
1.1.1.8/29
1.1.1.16/28
1.1.1.32/27
1.1.1.64/26
1.1.1.128/26
1.1.1.192/27
1.1.1.224/28
1.1.1.240/29
1.1.1.248/30
1.1.1.252/31
1.1.1.254
13 flow tables * 4(connection status) * 1(local net of localport) = 52

Reduced from 1012 flow meters to 52, a 19.4 times reduction.

Some scenes are similar to the following:
1.1.1.2, 1.1.1.6
After the combine:
1.1.1.2/255.255.255.251
This will slightly increase the difficulty of finding the flow table
corresponding to a single address.
such as:
ovs-ofctl dump-flows br-int | grep 1.1.1.6
The result is empty.
1.1.1.6 will match 1.1.1.2/255.255.255.251

Signed-off-by: Gongming Chen <gmingc...@tencent.com>
---
  configure.ac     |   1 +
  lib/expr.c       | 217 +++++++++++++++++++++++++++++++++++
  m4/ovn.m4        |  21 ++++
  tests/atlocal.in |   1 +
  tests/ovn.at     | 286 +++++++++++++++++++++++++++++++++--------------
  5 files changed, 443 insertions(+), 83 deletions(-)

diff --git a/configure.ac b/configure.ac
index b2d084318..6dc51a5cc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -131,6 +131,7 @@ OVS_LIBTOOL_VERSIONS
  OVS_CHECK_CXX
  AX_FUNC_POSIX_MEMALIGN
  OVN_CHECK_UNBOUND
+OVN_CHECK_COMBINE_IPV4
OVS_CHECK_INCLUDE_NEXT([stdio.h string.h]) 
  AC_CONFIG_FILES([lib/libovn.sym])
diff --git a/lib/expr.c b/lib/expr.c
index 796e88ac7..0b6dee3b3 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -1030,6 +1030,194 @@ expr_constant_set_destroy(struct expr_constant_set *cs)
      }
  }
+struct ip_v 
+{
+    uint32_t *ip;
+    uint32_t size;
+};
+
+struct ip_r
+{
+    uint32_t *ip;
+    uint32_t *mask;
+    uint32_t used;
+    uint32_t size;
+    bool *masked;
+};
Since the ip, mask, and masked arrays all have the same size, it would make memory management easier if they were combined into a single struct. For example: 

struct ip_r_entry {
    uint32_t ip;
    uint32_t mask;
    bool masked;
};

struct ip_r {
    struct ip_r_entry *entries;
    uint32_t used;
    uint32_t size;
};
This reduces the number of malloced pointers from 3 to 1, making it easier to reallocate and free data when necessary.
Also what do the names ip_v and ip_r mean? I'm guessing that ip_v might mean "IP vector"? But I have no idea what ip_r means. I think these structs could use better names. 


+
+/* Macro to check ip return data and xrealloc. */
+#define CHECK_REALLOC_IP_R_DATA(IP_R_DATA) \
+    if (IP_R_DATA->used >= IP_R_DATA->size) { \
+        IP_R_DATA->ip = xrealloc(IP_R_DATA->ip, \
+            2 * IP_R_DATA->size * sizeof(uint32_t)); \
+        IP_R_DATA->mask = xrealloc(IP_R_DATA->mask, \
+            2 * IP_R_DATA->size * sizeof(uint32_t)); \
+        IP_R_DATA->masked = xrealloc(IP_R_DATA->masked, \
+            2 * IP_R_DATA->size * sizeof(bool)); \
+        IP_R_DATA->size = IP_R_DATA->size * 2; \
+    }
OVS provides a function called x2nrealloc() that will double the size of an array and update the size. If you use my suggestion above, then this macro becomes: 

#define CHECK_REALLOC_IP_R_DATA(IP_R_DATA) \
    if (IP_R_DATA->used >= IP_R_DATA->size) { \
        IP_R_DATA->entries = x2nrealloc(IP_R_DATA->entries,
                                        &IP_R_DATA->size,
                                        sizeof *IP_R_DATA->entries); \
    }


+
+static void
+combine_ipv4_in_mask(struct ip_v *ip_data, struct ip_r *ip_r_data){
I have to admit, I'm having trouble understanding this algorithm. As a result, I've only made structural suggestions below. 


+    int i = 0, recorded =0;
I noticed that "recorded" is always set to either 0 or 1, and it's never set based on some mathematical operation. So it probably would make more sense to change it to a bool. 


+    uint32_t diff, connect, start, end, continuous_size, mask_base;
+
+    start = 0;
+    continuous_size = 0;
+    mask_base = 0;
+    end = 0;
+
+    memset(ip_r_data, 0, sizeof(struct ip_r));
+    ip_r_data->ip = xmalloc(4 * sizeof(uint32_t));
+    ip_r_data->mask = xmalloc(4 * sizeof(uint32_t));
+    ip_r_data->masked = xmalloc(4 * sizeof(bool));
+    ip_r_data->size = 4;
+
+    while (i < ip_data->size) {
Suggestion: I think this while loop could be replaced with a for loop. The reason is that every continue statement in the loop has "i++" before it. So I think the while could be replaced with: 

for (i = 0; i < ip_data->size; i++)

Then you can remove all the "i++" calls before the continue statements.


+        if (i + 1 >= ip_data->size) {
+            goto end_segment;
+        }
+
+        diff = ip_data->ip[i + 1] - ip_data->ip[i];
+        /* Ignore equal node. */
+        if (0 == diff) {
+            i++;
+            continue;
+        }
+        /* Continuous in the segment. */
+        if ((diff & (diff - 1)) == 0) {
+            /* New segment. */
+            if (0 == recorded) {
+                connect = ip_data->ip[i + 1] ^ ip_data->ip[i];
+                /* Only one bit different. */
+                if (0 == (connect & (connect - 1))) {
+                    recorded = 1;
+                    start = ip_data->ip[i];
+                    continuous_size = 2;
+                    mask_base = connect;
+
+                    int j = 0;
+                    end = start;
+                    /* The first non-zero place in the high direction is
+                     * the end of the segment. */
+                    while (j < 32 && (0 == (start & (mask_base << j)))) {
+                        end |= (mask_base << j);
+                        j++;
+                    }
+
+                    i++;
+                    continue;
+                /* Different segments and different bit, dnot merge. */
+                }else {
+                    CHECK_REALLOC_IP_R_DATA(ip_r_data)
+
+                    ip_r_data->ip[ip_r_data->used] = ip_data->ip[i];
+                    ip_r_data->masked[ip_r_data->used] = false;
+                    ip_r_data->used++;
+
+                    i++;
+                    continue;
+                }
+
+            /* Recording in the current segment. */
+            }else{
+                /* Stop and merge mask. */
+                if (ip_data->ip[i + 1] > end) {
+                    recorded = 0;
+                    while (continuous_size) {
+                        CHECK_REALLOC_IP_R_DATA(ip_r_data)
+
+                        int segment_power, pow_base;
+                        if (0 == continuous_size) {
+                            segment_power = 0;
+                        } else {
+                            segment_power = 31 - clz32(continuous_size);
+                        }
+
+                        if (0 == mask_base) {
+                            pow_base = 0;
+                        } else {
+                            pow_base = 31 - clz32(mask_base);
+                        }
+
+                        ip_r_data->mask[ip_r_data->used] =
+                            ~(((1 << segment_power) - 1) << pow_base);
+                        ip_r_data->ip[ip_r_data->used] =
+                            ip_r_data->mask[ip_r_data->used] & start;
+                        ip_r_data->masked[ip_r_data->used] = true;
+                        ip_r_data->used++;
+
+                        continuous_size &= (~(1 << segment_power));
+                        start = ip_r_data->ip[ip_r_data->used - 1]
+                                + (1 << (segment_power + pow_base));
+                    }
+
+                    i++;
+                    continue;
+                }
+
+                continuous_size++;
+                i++;
+                continue;
+            }
+        /* Not continuous in segment or is the end of the ip data. */
+        } else {
+end_segment:
+            if (recorded) {
+                recorded = 0;
+                while (continuous_size) {
+                    CHECK_REALLOC_IP_R_DATA(ip_r_data)
+
+                    int segment_power, pow_base;
+                    if (0 == continuous_size) {
+                        segment_power = 0;
+                    } else {
+                        segment_power = 31 - clz32(continuous_size);
+                    }
+
+                    if (0 == mask_base) {
+                        pow_base = 0;
+                    } else {
+                        pow_base = 31 - clz32(mask_base);
+                    }
+
+                    ip_r_data->mask[ip_r_data->used] =
+                        ~(((1 << segment_power) - 1) << pow_base);
+                    ip_r_data->ip[ip_r_data->used] =
+                        ip_r_data->mask[ip_r_data->used] & start;
+                    ip_r_data->masked[ip_r_data->used] = true;
+                    ip_r_data->used++;
+
+                    continuous_size &= (~(1 << segment_power));
+                    start = ip_r_data->ip[ip_r_data->used - 1]
+                            + (1 << (segment_power + pow_base));
+                }
+
+                i++;
+                continue;
+            } else {
+                CHECK_REALLOC_IP_R_DATA(ip_r_data)
+
+                ip_r_data->ip[ip_r_data->used] = ip_data->ip[i];
+                ip_r_data->masked[ip_r_data->used] = false;
+                ip_r_data->used++;
+
+                i++;
+                continue;
+            }
+        }
+    }
+}
+
+static int
+compare_mask_ip(const void *a, const void *b)
+{
+    uint32_t a_ = *(uint32_t *)a;
+    uint32_t b_ = *(uint32_t *)b;
+
+    return a_ < b_ ? -1 : a_ > b_;
+}
+
  /* Adds an constant set named 'name' to 'const_sets', replacing any existing
   * constant set entry with the given name. */
  void
@@ -1044,6 +1232,11 @@ expr_const_sets_add(struct shash *const_sets, const char 
*name,
      cs->in_curlies = true;
      cs->n_values = 0;
      cs->values = xmalloc(n_values * sizeof *cs->values);
+    struct ip_r ip_r_data;
+    struct ip_v ip_data;
+    ip_data.ip = xmalloc(n_values * sizeof(uint32_t));
+    ip_data.size = 0;
+
      if (convert_to_integer) {
          cs->type = EXPR_C_INTEGER;
          for (size_t i = 0; i < n_values; i++) {
@@ -1056,6 +1249,11 @@ expr_const_sets_add(struct shash *const_sets, const char 
*name,
                  && lex.token.type != LEX_T_MASKED_INTEGER) {
                  VLOG_WARN("Invalid constant set entry: '%s', token type: %d",
                            values[i], lex.token.type);
+#ifdef HAVE_COMBINE_IPV4
+            } else if (lex.token.type == LEX_T_INTEGER
+                       && lex.token.format == LEX_F_IPV4) {
+                ip_data.ip[ip_data.size++] = ntohl(lex.token.value.ipv4);
+#endif
              } else {
                  union expr_constant *c = &cs->values[cs->n_values++];
                  c->value = lex.token.value;
@@ -1075,6 +1273,25 @@ expr_const_sets_add(struct shash *const_sets, const char 
*name,
          }
      }
+ if (ip_data.size > 0) { 
+        qsort(ip_data.ip, ip_data.size, sizeof(uint32_t), compare_mask_ip);
+        combine_ipv4_in_mask(&ip_data, &ip_r_data);
+        for (int i = 0; i < ip_r_data.used; ++i) {
+            union expr_constant *c = &cs->values[cs->n_values++];
+            memset(&c->value, 0, sizeof c->value);
+            memset(&c->mask, 0, sizeof c->mask);
+            c->value.ipv4 = htonl(ip_r_data.ip[i]);
+            c->format = LEX_F_IPV4;
+            c->masked = ip_r_data.masked[i];
+            if (c->masked) {
+                c->mask.ipv4 = htonl(ip_r_data.mask[i]);
+            }
+        }
+        free(ip_r_data.ip);
+        free(ip_r_data.mask);
+        free(ip_r_data.masked);
+    }
+
      shash_add(const_sets, name, cs);
  }
diff --git a/m4/ovn.m4 b/m4/ovn.m4 
index dacfabb2a..49fcbef9b 100644
--- a/m4/ovn.m4
+++ b/m4/ovn.m4
@@ -576,3 +576,24 @@ AC_DEFUN([OVN_CHECK_UNBOUND],
     fi
     AM_CONDITIONAL([HAVE_UNBOUND], [test "$HAVE_UNBOUND" = yes])
     AC_SUBST([HAVE_UNBOUND])])
+
+dnl Checks for combine multiple ipv4 with wildcard mask.
+AC_DEFUN([OVN_CHECK_COMBINE_IPV4],
+  [AC_ARG_ENABLE(
+     [combine-ipv4],
+     [AC_HELP_STRING([--disable-combine-ipv4], [Disable combine multiple ipv4 
feature])],
+     [case "${enableval}" in
+        (yes) combine_ipv4=true ;;
+        (no)  combine_ipv4=false ;;
+        (*) AC_MSG_ERROR([bad value ${enableval} for --disable-combine-ipv4]) 
;;
+      esac],
+     [combine_ipv4=true])
+
+   #AM_CONDITIONAL([HAVE_COMBINE_IPV4], [test "$combine_ipv4" = true])
+   #AC_DEFINE([HAVE_COMBINE_IPV4], [1], [Define to 1 if combine-ipv4 is 
enable.])
+   if test "$combine_ipv4" = true; then
+     AC_DEFINE([HAVE_COMBINE_IPV4], [1], [Define to 1 if combine-ipv4 is 
enable.])
+     HAVE_COMBINE_IPV4=yes
+   fi
+   AC_SUBST([HAVE_COMBINE_IPV4])
+   AM_CONDITIONAL([HAVE_COMBINE_IPV4], [test "$combine_ipv4" = true])])
diff --git a/tests/atlocal.in b/tests/atlocal.in
index 5ebc8e117..3024a2d2a 100644
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -3,6 +3,7 @@ HAVE_OPENSSL='@HAVE_OPENSSL@'
  OPENSSL_SUPPORTS_SNI='@OPENSSL_SUPPORTS_SNI@'
  HAVE_UNBOUND='@HAVE_UNBOUND@'
  EGREP='@EGREP@'
+HAVE_COMBINE_IPV4='@HAVE_COMBINE_IPV4@'
if test x"$PYTHON3" = x; then 
      PYTHON3='@PYTHON3@'
diff --git a/tests/ovn.at b/tests/ovn.at
index 80c9fe138..094a3f1e8 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -616,6 +616,24 @@ ip,nw_src=10.0.0.1
  ip,nw_src=10.0.0.2
  ip,nw_src=10.0.0.3
  ])
+
+if test "$HAVE_COMBINE_IPV4" = yes; then
+AT_CHECK([expr_to_flow 'ip4.src == $set1'], [0], [dnl
+ip,nw_src=10.0.0.1
+ip,nw_src=10.0.0.2/31
+])
+AT_CHECK([expr_to_flow 'ip4.src == {1.2.3.4, $set1}'], [0], [dnl
+ip,nw_src=1.2.3.4
+ip,nw_src=10.0.0.1
+ip,nw_src=10.0.0.2/31
+])
+AT_CHECK([expr_to_flow 'ip4.src == {1.2.0.0/20, 5.5.5.0/24, $set1}'], [0], [dnl
+ip,nw_src=1.2.0.0/20
+ip,nw_src=10.0.0.1
+ip,nw_src=10.0.0.2/31
+ip,nw_src=5.5.5.0/24
+])
+else
  AT_CHECK([expr_to_flow 'ip4.src == $set1'], [0], [dnl
  ip,nw_src=10.0.0.1
  ip,nw_src=10.0.0.2
@@ -634,6 +652,8 @@ ip,nw_src=10.0.0.2
  ip,nw_src=10.0.0.3
  ip,nw_src=5.5.5.0/24
  ])
+fi
+
  AT_CHECK([expr_to_flow 'ip6.src == {::1, ::2, ::3}'], [0], [dnl
  ipv6,ipv6_src=::1
  ipv6,ipv6_src=::2
@@ -13580,26 +13600,46 @@ cat 2.expected > expout
  $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
  AT_CHECK([cat 2.packets], [0], [expout])
-# There should be total of 9 flows present with conjunction action and 2 flows 
-# with conj match. Eg.
-# table=44, priority=2001,conj_id=2,metadata=0x1 actions=resubmit(,45)
-# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(2,2/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,2/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(2,2/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(3,2/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(3,2/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(3,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(2,1/2),conjunction(3,1/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(2,1/2),conjunction(3,1/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(2,1/2),conjunction(3,1/2)
-
-OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conj_id | wc -l`])
+if test "$HAVE_COMBINE_IPV4" = yes; then
+    # There should be total of 6 flows present with conjunction action and 2 
flows
+    # with conj match. Eg.
+    #table=45, n_packets=1, n_bytes=42, idle_age=211, 
priority=2001,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
+    #table=45, n_packets=0, n_bytes=0, idle_age=211, 
priority=2001,conj_id=3,ip,metadata=0x1 actions=drop
+    #priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(2,2/2),conjunction(3,2/2)
+    #priority=2001,ip,metadata=0x1,nw_src=10.0.0.4/31 
actions=conjunction(2,2/2),conjunction(3,2/2)
+    #priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4/31 
actions=conjunction(2,1/2)
+    #priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(2,1/2)
+    #priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(3,1/2)
+    #priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8/31 
actions=conjunction(3,1/2)
+
+    OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conj_id | wc -l`])
+else
+    # There should be total of 9 flows present with conjunction action and 2 
flows
+    # with conj match. Eg.
+    # table=44, priority=2001,conj_id=2,metadata=0x1 actions=resubmit(,45)
+    # table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(2,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(2,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(3,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(3,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(3,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(2,1/2),conjunction(3,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(2,1/2),conjunction(3,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(2,1/2),conjunction(3,1/2)
+
+    OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conj_id | wc -l`])
+fi
as hv1 ovs-ofctl dump-flows br-int @@ -13621,25 +13661,44 @@ AT_CHECK([cat 2.packets], [0], []) # Remove the first ACL, and verify that the conjunction flows are updated 
  # properly.
-# There should be total of 6 flows present with conjunction action and 1 flow
-# with conj match. Eg.
-# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(4,2/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(4,2/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(4,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(4,1/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 actions=conjunction(4,1/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 actions=conjunction(4,1/2)
ovn-nbctl acl-del ls1 to-lport 1001 \ 
  'ip4 && ip4.src == $set1 && ip4.dst == $set1'
-OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \ 
-grep conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conj_id | wc -l`])
+
+if test "$HAVE_COMBINE_IPV4" = yes; then
+    # There should be total of 6 flows present with conjunction action and 1 
flow
+    # with conj match. Eg.
+    # table=45, n_packets=1, n_bytes=42, idle_age=34, 
priority=2001,conj_id=3,ip,metadata=0x1 actions=drop
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(3,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4/31 
actions=conjunction(3,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8/31 
actions=conjunction(3,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(3,1/2)
+
+    OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conj_id | wc -l`])
+else
+    # There should be total of 6 flows present with conjunction action and 1 
flow
+    # with conj match. Eg.
+    # table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(4,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(4,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(4,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 actions=conjunction(4,1/2)
+
+    OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conj_id | wc -l`])
+fi
# Add the ACL back 
  ovn-nbctl acl-add ls1 to-lport 1001 \
@@ -13648,44 +13707,80 @@ ovn-nbctl acl-add ls1 to-lport 1001 \
  ovn-nbctl acl-add ls1 to-lport 1001 \
  'ip4 && ip4.src == $set1 && ip4.dst == {10.0.0.9, 10.0.0.10}' drop
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(4,1/2) 
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(4,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(5,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(5,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(5,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 
actions=conjunction(4,1/2),conjunction(6,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(6,1/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2)
-
-OVS_WAIT_UNTIL([test 10 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction.*conjunction | wc -l`])
+if test "$HAVE_COMBINE_IPV4" = yes; then
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(3,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8/31 
actions=conjunction(3,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4/31 
actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4/31 
actions=conjunction(3,2/2),conjunction(4,2/2),conjunction(5,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(3,2/2),conjunction(4,2/2),conjunction(5,2/2)
+
+    OVS_WAIT_UNTIL([test 8 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction.*conjunction | wc -l`])
+else
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 
actions=conjunction(4,1/2),conjunction(6,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(6,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2)
+
+    OVS_WAIT_UNTIL([test 10 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction.*conjunction | wc -l`])
+fi
# Remove 10.0.0.7 from address set2. All flows should be updated properly. 
  ovn-nbctl set Address_Set set2 \
  addresses=\"10.0.0.8\",\"10.0.0.9\"
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(9,1/2) 
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(7,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(8,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(9,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 
actions=conjunction(7,1/2),conjunction(8,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(9,1/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2)
-
-OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction.*conjunction | wc -l`])
+if test "$HAVE_COMBINE_IPV4" = yes; then
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4/31,nw_dst=10.0.0.8/31 
actions=drop
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6,nw_dst=10.0.0.8/31 
actions=drop
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4/31 
actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(4,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(5,2/2),conjunction(4,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4/31 
actions=conjunction(5,2/2),conjunction(4,2/2)
+
+    OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction.*conjunction | wc -l`])
+else
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(9,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(7,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(8,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(9,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 
actions=conjunction(7,1/2),conjunction(8,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(9,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2)
+
+    OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction.*conjunction | wc -l`])
+fi
# Remove an ACL again 
  ovn-nbctl acl-del ls1 to-lport 1001 \
@@ -13693,19 +13788,36 @@ ovn-nbctl acl-del ls1 to-lport 1001 \
wait_for_ports_up 
  ovn-nbctl --wait=hv sync
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(10,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(11,1/2)
-# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 
actions=conjunction(10,1/2),conjunction(11,1/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(10,2/2),conjunction(11,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(10,2/2),conjunction(11,2/2)
-# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(10,2/2),conjunction(11,2/2)
-
-OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction | wc -l`])
-OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
-grep conjunction.*conjunction.*conjunction | wc -l`])
+
+if test "$HAVE_COMBINE_IPV4" = yes; then
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6,nw_dst=10.0.0.8/31 
actions=drop
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4/31,nw_dst=10.0.0.8/31 
actions=drop
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(5,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4/31 
actions=conjunction(5,2/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(5,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(5,1/2)
+
+    OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction.*conjunction | wc -l`])
+else
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 
actions=conjunction(10,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(11,1/2)
+    # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 
actions=conjunction(10,1/2),conjunction(11,1/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 
actions=conjunction(10,2/2),conjunction(11,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 
actions=conjunction(10,2/2),conjunction(11,2/2)
+    # priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 
actions=conjunction(10,2/2),conjunction(11,2/2)
+
+    OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction | wc -l`])
+    OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \
+    grep conjunction.*conjunction.*conjunction | wc -l`])
+fi
OVN_CLEANUP([hv1]) 
  AT_CLEANUP
@@ -15350,11 +15462,19 @@ for i in 1 2 3; do
# Update address set as1 
      ovn-nbctl --wait=hv set addr as1 addresses="10.1.2.10 10.1.2.11"
-    AT_CHECK([ovs-ofctl dump-flows br-int | grep "10.1.2.11"], [0], [ignore])
+    if test "$HAVE_COMBINE_IPV4" = yes; then
+        AT_CHECK([ovs-ofctl dump-flows br-int | grep "10.1.2.10/31"], [0], 
[ignore])
+    else
+        AT_CHECK([ovs-ofctl dump-flows br-int | grep "10.1.2.11"], [0], 
[ignore])
+    fi
# Update address set as2 
      ovn-nbctl --wait=hv set addr as2 addresses="10.1.2.12 10.1.2.13"
-    AT_CHECK([ovs-ofctl dump-flows br-int | grep "10.1.2.12"], [0], [ignore])
+    if test "$HAVE_COMBINE_IPV4" = yes; then
+        AT_CHECK([ovs-ofctl dump-flows br-int | grep "10.1.2.12/31"], [0], 
[ignore])
+    else
+        AT_CHECK([ovs-ofctl dump-flows br-int | grep "10.1.2.12"], [0], 
[ignore])
+    fi
# Add another ACL referencing as1 
      n_flows_before=`ovs-ofctl dump-flows br-int | grep "10.1.2.10" | wc -l`




_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to