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

From: Gongming Chen <gmingchen@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 <gmingchen@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(-)

On 2/7/21 1:25 PM, gmingchen(陈供明) wrote:
> From: Gongming Chen <gmingchen@tencent.com>

Hi Gongming,

First of all, thanks for the contribution!

This is not a full review, just some comments for now.

It seems that there's a memory leak with your patch applied. 
AddressSanitizer reports:

Direct leak of 12 byte(s) in 1 object(s) allocated from:
     #0 0x49644d in malloc 
(/home/runner/work/ovn/ovn/ovn-20.12.90/_build/sub/tests/ovstest+0x49644d)
     #1 0x538604 in xmalloc 
/home/runner/work/ovn/ovn/ovs_src/lib/util.c:138:15
     #2 0x62636f in expr_const_sets_add 
/home/runner/work/ovn/ovn/ovn-20.12.90/_build/sub/../../lib/expr.c:1237:18
     #3 0x4cf8f6 in create_addr_sets 
/home/runner/work/ovn/ovn/ovn-20.12.90/_build/sub/../../tests/test-ovn.c:230:5
     #4 0x4cf8f6 in test_parse_expr__ 
/home/runner/work/ovn/ovn/ovn-20.12.90/_build/sub/../../tests/test-ovn.c:296:5
     #5 0x4dfd04 in ovs_cmdl_run_command__ 
/home/runner/work/ovn/ovn/ovs_src/lib/command-line.c:247:17
     #6 0x4c6810 in test_ovn_main 
/home/runner/work/ovn/ovn/ovn-20.12.90/_build/sub/../../tests/test-ovn.c:1635:5
     #7 0x4c6810 in ovstest_wrapper_test_ovn_main__ 
/home/runner/work/ovn/ovn/ovn-20.12.90/_build/sub/../../tests/test-ovn.c:1638:1
     #8 0x4dfd04 in ovs_cmdl_run_command__ 
/home/runner/work/ovn/ovn/ovs_src/lib/command-line.c:247:17
     #9 0x4c5fe3 in main 
/home/runner/work/ovn/ovn/ovn-20.12.90/_build/sub/../../tests/ovstest.c:150:9
     #10 0x7f71aa6ddbf6 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)

Full reports are available in the ovsrobot OVN github CI run artifacts:
https://github.com/ovsrobot/ovn/actions/runs/545416492

Just a note, if you push the branch to your own fork it will trigger the 
github action to run CI and the "linux clang test asan" job will also 
enable AddressSanitizer.

There are also a few style related issues (e.g., sizeof args), please 
see the guidelines here:

https://github.com/ovn-org/ovn/blob/master/Documentation/internals/contributing/coding-style.rst

> 
> 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.

I think it would be great to describe the algorithm here, in the commit 
log, but also in the comments in the code.

> 
> 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

Would it make sense and potentially make the life of the users easier by 
also adding a utility to automatically do the IP/MASK match?

I'm thinking of something like:
$ ovs-ofctl dump-flows br-int | ovn-match-ip 1.1.1.6
[..]

I didn't look at the implementation in detail but I'm surprised by the 
following:

- change test-ovn.c to define $set1 = {1.1.1.2, .. 1.1.1.254}, basically 
the example you gave above.
- then run:

$ echo 'ip4.src == $set1' | ./tests/ovstest test-ovn expr-to-flows
ip,nw_src=1.1.1.2/31
ip,nw_src=1.1.1.4
$

Shouldn't this output actually match the 13 entries in your sample output?

> 
> Signed-off-by: Gongming Chen <gmingchen@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

Is there a specific reason to not have this enabled by default?  It 
would be great if we could avoid HAVE_COMBINE_IPV4 ifdefs in the code 
and tests.

>   
>   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;
> +};
> +
> +/* Macro to check ip return data and xrealloc. */

In my opinion it would be clearer if this would be a function.

> +#define CHECK_REALLOC_IP_R_DATA(IP_R_DATA) \
> +    if (IP_R_DATA->used >= IP_R_DATA->size) { \

Probably best to wrap IP_R_DATA as "(IP_R_DATA)"

> +        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; \
> +    }
> +
> +static void
> +combine_ipv4_in_mask(struct ip_v *ip_data, struct ip_r *ip_r_data){

Some comments describing the algorithm would help.

I'll wait for your feedback on all the above before continuing the review.

Thanks,
Dumitru

> +    int i = 0, recorded =0;
> +    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) {
> +        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`
>

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 <gmingchen@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 <gmingchen@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`
>