[ovs-dev] [PATCH ovn v1 1/1] NAT: Enhancements to external port range support
Flavio Fernandes
flavio at flaviof.com
Wed Apr 15 22:10:06 UTC 2020
This change is a mix of minor fixes to the external port range
support recently merged to OVN [1], as well as some enhancements.
- Added external port range column to ovn-nbctl lr-nat-list output;
- Added external port range to NAT section of "ovn-nbctl show" (if needed);
- Minor fix to is_valid_port_range() checker, to catch cases when string
with multiple '-' tokens are provided. An example would be "12-34-56";
- Minor fix to actions parser, to ensure that value "0" is not allowed;
- Updated tests as needed to account for the changes mentioned above;
- Added line in NEWS to mention about this update to the NB schema;
- Minor typo in description of external port range;
- Instead of using "if (strlen(str))", use "if (str[0])" code convention.
[1]: https://patchwork.ozlabs.org/project/openvswitch/list/?series=169084
Signed-off-by: Flavio Fernandes <flavio at flaviof.com>
---
NEWS | 1 +
lib/actions.c | 5 +-
northd/ovn-northd.c | 8 +--
ovn-nb.xml | 2 +-
tests/ovn-nbctl.at | 130 ++++++++++++++++++++++++++----------------
tests/ovn.at | 4 +-
utilities/ovn-nbctl.c | 27 ++++++---
7 files changed, 112 insertions(+), 65 deletions(-)
diff --git a/NEWS b/NEWS
index 623c8810e..765b79f1c 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,6 @@
Post-v20.03.0
--------------------------
+ - Added support for external_port_range in NAT table.
OVN v20.03.0 - xx xxx xxxx
diff --git a/lib/actions.c b/lib/actions.c
index b3bf98106..c19813de0 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -779,6 +779,9 @@ parse_ct_nat(struct action_context *ctx, const char *name,
}
cn->port_range.port_lo = ntohll(ctx->lexer->token.value.integer);
+ if (cn->port_range.port_lo == 0) {
+ lexer_syntax_error(ctx->lexer, "range can't be 0");
+ }
lexer_get(ctx->lexer);
if (lexer_match(ctx->lexer, LEX_T_HYPHEN)) {
@@ -792,7 +795,7 @@ parse_ct_nat(struct action_context *ctx, const char *name,
if (cn->port_range.port_hi <= cn->port_range.port_lo) {
lexer_syntax_error(ctx->lexer, "range high should be "
- "greater than range lo");
+ "greater than range low");
}
lexer_get(ctx->lexer);
} else {
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 198028c50..25fca20e1 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8917,7 +8917,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
ds_put_format(&actions, "flags.loopback = 1; "
"ct_dnat(%s", nat->logical_ip);
- if (strlen(nat->external_port_range)) {
+ if (nat->external_port_range[0]) {
ds_put_format(&actions, ",%s",
nat->external_port_range);
}
@@ -8950,7 +8950,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
is_v6 ? "6" : "4", nat->logical_ip);
} else {
ds_put_format(&actions, "ct_dnat(%s", nat->logical_ip);
- if (strlen(nat->external_port_range)) {
+ if (nat->external_port_range[0]) {
ds_put_format(&actions, ",%s",
nat->external_port_range);
}
@@ -9061,7 +9061,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
ds_put_format(&actions, "ct_snat(%s",
nat->external_ip);
- if (strlen(nat->external_port_range)) {
+ if (nat->external_port_range[0]) {
ds_put_format(&actions, ",%s",
nat->external_port_range);
}
@@ -9104,7 +9104,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
} else {
ds_put_format(&actions, "ct_snat(%s",
nat->external_ip);
- if (strlen(nat->external_port_range)) {
+ if (nat->external_port_range[0]) {
ds_put_format(&actions, ",%s",
nat->external_port_range);
}
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 4dfdffdd7..163dd12ee 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2579,7 +2579,7 @@
</p>
<p>
- Range of port, from which a port number will be picked that will
+ Range of ports, from which a port number will be picked that will
replace the source port of to be NATed packet. This is basically
PAT (port address translation).
</p>
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 0f98c70d0..66fbcc748 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -473,15 +473,15 @@ AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat fd01::1 fd11::2])
AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3 lp0 00:00:00:01:02:03])
AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat fd01::2 fd11::3 lp0 00:00:00:01:02:03])
AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
-TYPE EXTERNAL_IP LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
-dnat 30.0.0.1 192.168.1.2
-dnat fd01::1 fd11::2
-dnat_and_snat 30.0.0.1 192.168.1.2
-dnat_and_snat 30.0.0.2 192.168.1.3 00:00:00:01:02:03 lp0
-dnat_and_snat fd01::1 fd11::2
-dnat_and_snat fd01::2 fd11::3 00:00:00:01:02:03 lp0
-snat 30.0.0.1 192.168.1.0/24
-snat fd01::1 fd11::/64
+TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
+dnat 30.0.0.1 192.168.1.2
+dnat fd01::1 fd11::2
+dnat_and_snat 30.0.0.1 192.168.1.2
+dnat_and_snat 30.0.0.2 192.168.1.3 00:00:00:01:02:03 lp0
+dnat_and_snat fd01::1 fd11::2
+dnat_and_snat fd01::2 fd11::3 00:00:00:01:02:03 lp0
+snat 30.0.0.1 192.168.1.0/24
+snat fd01::1 fd11::/64
])
AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.0/24], [1], [],
[ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and logical_ip already exists
@@ -509,28 +509,28 @@ AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.1 192.168.1.3], [1], [],
])
AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3 lp0 00:00:00:04:05:06])
AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
-TYPE EXTERNAL_IP LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
-dnat 30.0.0.1 192.168.1.2
-dnat fd01::1 fd11::2
-dnat_and_snat 30.0.0.1 192.168.1.2
-dnat_and_snat 30.0.0.2 192.168.1.3 00:00:00:04:05:06 lp0
-dnat_and_snat fd01::1 fd11::2
-dnat_and_snat fd01::2 fd11::3 00:00:00:01:02:03 lp0
-snat 30.0.0.1 192.168.1.0/24
-snat fd01::1 fd11::/64
+TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
+dnat 30.0.0.1 192.168.1.2
+dnat fd01::1 fd11::2
+dnat_and_snat 30.0.0.1 192.168.1.2
+dnat_and_snat 30.0.0.2 192.168.1.3 00:00:00:04:05:06 lp0
+dnat_and_snat fd01::1 fd11::2
+dnat_and_snat fd01::2 fd11::3 00:00:00:01:02:03 lp0
+snat 30.0.0.1 192.168.1.0/24
+snat fd01::1 fd11::/64
])
AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3])
AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat fd01::2 fd11::3])
AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
-TYPE EXTERNAL_IP LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
-dnat 30.0.0.1 192.168.1.2
-dnat fd01::1 fd11::2
-dnat_and_snat 30.0.0.1 192.168.1.2
-dnat_and_snat 30.0.0.2 192.168.1.3
-dnat_and_snat fd01::1 fd11::2
-dnat_and_snat fd01::2 fd11::3
-snat 30.0.0.1 192.168.1.0/24
-snat fd01::1 fd11::/64
+TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
+dnat 30.0.0.1 192.168.1.2
+dnat fd01::1 fd11::2
+dnat_and_snat 30.0.0.1 192.168.1.2
+dnat_and_snat 30.0.0.2 192.168.1.3
+dnat_and_snat fd01::1 fd11::2
+dnat_and_snat fd01::2 fd11::3
+snat 30.0.0.1 192.168.1.0/24
+snat fd01::1 fd11::/64
])
AT_CHECK([ovn-nbctl --bare --columns=options list nat | grep stateless=true| wc -l], [0],
@@ -581,30 +581,30 @@ AT_CHECK([ovn-nbctl --if-exists lr-nat-del lr0 snat 192.168.10.0/24])
AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat 30.0.0.1])
AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat fd01::1])
AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
-TYPE EXTERNAL_IP LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
-dnat 30.0.0.1 192.168.1.2
-dnat fd01::1 fd11::2
-dnat_and_snat 30.0.0.2 192.168.1.3
-dnat_and_snat 40.0.0.2 192.168.1.4
-dnat_and_snat fd01::2 fd11::3
-snat 30.0.0.1 192.168.1.0/24
-snat 40.0.0.3 192.168.1.6
-snat fd01::1 fd11::/64
+TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
+dnat 30.0.0.1 192.168.1.2
+dnat fd01::1 fd11::2
+dnat_and_snat 30.0.0.2 192.168.1.3
+dnat_and_snat 40.0.0.2 192.168.1.4
+dnat_and_snat fd01::2 fd11::3
+snat 30.0.0.1 192.168.1.0/24
+snat 40.0.0.3 192.168.1.6
+snat fd01::1 fd11::/64
])
AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])
AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
-TYPE EXTERNAL_IP LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
-dnat_and_snat 30.0.0.2 192.168.1.3
-dnat_and_snat 40.0.0.2 192.168.1.4
-dnat_and_snat fd01::2 fd11::3
-snat 30.0.0.1 192.168.1.0/24
-snat 40.0.0.3 192.168.1.6
-snat fd01::1 fd11::/64
+TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
+dnat_and_snat 30.0.0.2 192.168.1.3
+dnat_and_snat 40.0.0.2 192.168.1.4
+dnat_and_snat fd01::2 fd11::3
+snat 30.0.0.1 192.168.1.0/24
+snat 40.0.0.3 192.168.1.6
+snat fd01::1 fd11::/64
])
AT_CHECK([ovn-nbctl lr-nat-del lr0])
-AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 snat 40.0.0.3 192.168.1.6 1-3000])
+AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 snat 40.0.0.3 192.168.1.6 21-65535])
AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.4 192.168.1.7 1-3000])
AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.5 192.168.1.10 1])
AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.5 192.168.1.8 1-3000])
@@ -616,6 +616,10 @@ AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.
[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac.
])
+AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 0], [1], [],
+[ovn-nbctl: invalid port range 0.
+])
+
AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 1-], [1], [],
[ovn-nbctl: invalid port range 1-.
])
@@ -624,6 +628,14 @@ AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.
[ovn-nbctl: invalid port range -300.
])
+AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 1-2-3], [1], [],
+[ovn-nbctl: invalid port range 1-2-3.
+])
+
+AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 300-300], [1], [],
+[ovn-nbctl: invalid port range 300-300.
+])
+
AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 500-300], [1], [],
[ovn-nbctl: invalid port range 500-300.
])
@@ -640,13 +652,31 @@ AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.
[ovn-nbctl: invalid port range 0-10.
])
+AT_CHECK([ovn-nbctl show lr0 | grep -c 'external port(s): "1-3000"'], [0], [dnl
+3
+])
+AT_CHECK([ovn-nbctl show lr0 | grep -C2 'external port(s): "21-65535"' | uuidfilt], [0], [dnl
+ nat <0>
+ external ip: "40.0.0.3"
+ external port(s): "21-65535"
+ logical ip: "192.168.1.6"
+ type: "snat"
+])
+AT_CHECK([ovn-nbctl show lr0 | grep -C2 'external port(s): "1"' | uuidfilt], [0], [dnl
+ nat <0>
+ external ip: "40.0.0.5"
+ external port(s): "1"
+ logical ip: "192.168.1.10"
+ type: "dnat"
+])
+
AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
-TYPE EXTERNAL_IP LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
-dnat 40.0.0.4 192.168.1.7
-dnat 40.0.0.5 192.168.1.10
-dnat_and_snat 40.0.0.5 192.168.1.8
-dnat_and_snat 40.0.0.6 192.168.1.9 00:00:00:04:05:06 lp0
-snat 40.0.0.3 192.168.1.6
+TYPE EXTERNAL_IP EXTERNAL_PORT LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT
+dnat 40.0.0.4 1-3000 192.168.1.7
+dnat 40.0.0.5 1 192.168.1.10
+dnat_and_snat 40.0.0.5 1-3000 192.168.1.8
+dnat_and_snat 40.0.0.6 1-3000 192.168.1.9 00:00:00:04:05:06 lp0
+snat 40.0.0.3 21-65535 192.168.1.6
])
AT_CHECK([ovn-nbctl lr-nat-del lr0])
diff --git a/tests/ovn.at b/tests/ovn.at
index f83d3f536..cf695bb41 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1073,7 +1073,7 @@ ct_dnat(192.168.1.2, 1000);
encodes as ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1000))
has prereqs ip
ct_dnat(192.168.1.2, 1000-100);
- Syntax error at `100' range high should be greater than range lo.
+ Syntax error at `100' range high should be greater than range low.
# ct_snat
ct_snat;
@@ -1107,7 +1107,7 @@ ct_snat(192.168.1.2, 1000);
encodes as ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1000))
has prereqs ip
ct_snat(192.168.1.2, 1000-100);
- Syntax error at `100' range high should be greater than range lo.
+ Syntax error at `100' range high should be greater than range low.
# ct_clear
ct_clear;
encodes as ct_clear
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 63e5b0405..df13b9054 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -1034,6 +1034,10 @@ print_lr(const struct nbrec_logical_router *lr, struct ds *s)
UUID_ARGS(&nat->header_.uuid));
ds_put_cstr(s, " external ip: ");
ds_put_format(s, "\"%s\"\n", nat->external_ip);
+ if (nat->external_port_range[0]) {
+ ds_put_cstr(s, " external port(s): ");
+ ds_put_format(s, "\"%s\"\n", nat->external_port_range);
+ }
ds_put_cstr(s, " logical ip: ");
ds_put_format(s, "\"%s\"\n", nat->logical_ip);
ds_put_cstr(s, " type: ");
@@ -3990,7 +3994,7 @@ is_valid_port_range(const char *port_range)
goto done;
}
- char *range_hi = strtok_r(NULL, "", &save_ptr);
+ char *range_hi = strtok_r(NULL, "-", &save_ptr);
if (!range_hi) {
goto done;
}
@@ -3999,11 +4003,16 @@ is_valid_port_range(const char *port_range)
goto done;
}
+ /* Check that there is nothing after range_hi. */
+ if (strtok_r(NULL, "", &save_ptr)) {
+ goto done;
+ }
+
if (range_lo_int >= range_hi_int) {
goto done;
}
- if (range_lo_int <= 0 || range_hi_int > 65535) {
+ if (range_hi_int > 65535) {
goto done;
}
@@ -4320,20 +4329,24 @@ nbctl_lr_nat_list(struct ctl_context *ctx)
const struct nbrec_nat *nat = lr->nat[i];
char *key = xasprintf("%-17.13s%s", nat->type, nat->external_ip);
if (nat->external_mac && nat->logical_port) {
- smap_add_format(&lr_nats, key, "%-22.18s%-21.17s%s",
+ smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%-21.17s%s",
+ nat->external_port_range,
nat->logical_ip, nat->external_mac,
nat->logical_port);
} else {
- smap_add_format(&lr_nats, key, "%s", nat->logical_ip);
+ smap_add_format(&lr_nats, key, "%-17.13s%s",
+ nat->external_port_range,
+ nat->logical_ip);
}
free(key);
}
const struct smap_node **nodes = smap_sort(&lr_nats);
if (nodes) {
- ds_put_format(&ctx->output, "%-17.13s%-19.15s%-22.18s%-21.17s%s\n",
- "TYPE", "EXTERNAL_IP", "LOGICAL_IP", "EXTERNAL_MAC",
- "LOGICAL_PORT");
+ ds_put_format(&ctx->output,
+ "%-17.13s%-19.15s%-17.13s%-22.18s%-21.17s%s\n",
+ "TYPE", "EXTERNAL_IP", "EXTERNAL_PORT", "LOGICAL_IP",
+ "EXTERNAL_MAC", "LOGICAL_PORT");
for (size_t i = 0; i < smap_count(&lr_nats); i++) {
const struct smap_node *node = nodes[i];
ds_put_format(&ctx->output, "%-36.32s%s\n",
--
2.17.1
More information about the dev
mailing list