[ovs-dev] [PATCH ovn] expr: Combine multiple ipv4 with wildcard mask.
gmingchen(陈供明)
gmingchen at tencent.com
Mon Feb 15 10:51:29 UTC 2021
Hi Mark,
Sorry for getting back to you late. Thanks for your review. I totally agree with
your very detailed suggestion. I think it is very useful, especially x2nrealloc.
I will resubmit later.
Thanks,
Gongming
On 2021/2/11, 9:26 AM, "Mark Michelson" <mmichels at redhat.com> wrote:
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 at 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 at 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`
>
More information about the dev
mailing list