Re: [ovs-dev] [PATCH ovn v1 1/1] NAT: Enhancements to external port range support
On Wed, Apr 22, 2020 at 5:56 AM Ankur Sharma wrote:
>
> Hi Flavio,
>
> Thanks for the patch, changes look good to me.
>
> Acked-by: Ankur Sharma
Thanks Flavio and Ankur. I applied this patch to master.
Numan
>
>
> Regards,
> Ankur
>
> From: dev on behalf of Flavio Fernandes
>
> Sent: Wednesday, April 15, 2020 3:10 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH ovn v1 1/1] NAT: Enhancements to external port
> range support
>
> 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://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_openvswitch_list_-3Fseries-3D169084=DwICAg=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PmpZbISfYcMxSmPgWZMlPi9qbIWzlZhH0hJuBcz9gLs=l3Oek3NF54pTpFcw0840MCrjHAS4WwxLY63RuK30_Bo=
>
> Signed-off-by: Flavio Fernandes
> ---
> 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
> 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(, "flags.loopback = 1; "
>"ct_dnat(%s", nat->logical_ip);
>
> -if (strlen(nat->external_port_range)) {
> +if (nat->external_port_range[0]) {
> ds_put_format(, ",%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(, "ct_dnat(%s",
> nat->logical_ip);
> -if (strlen(nat->external_port_range)) {
> +if (nat->external_port_range[0]) {
> ds_put_format(, ",%s",
>nat->external_port_range);
> }
> @@ -9061,7 +9061,7 @@ build_lrouter_flows(s
Re: [ovs-dev] [PATCH ovn v1 1/1] NAT: Enhancements to external port range support
Hi Flavio,
Thanks for the patch, changes look good to me.
Acked-by: Ankur Sharma
Regards,
Ankur
From: dev on behalf of Flavio Fernandes
Sent: Wednesday, April 15, 2020 3:10 PM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH ovn v1 1/1] NAT: Enhancements to external port range
support
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://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_openvswitch_list_-3Fseries-3D169084=DwICAg=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PmpZbISfYcMxSmPgWZMlPi9qbIWzlZhH0hJuBcz9gLs=l3Oek3NF54pTpFcw0840MCrjHAS4WwxLY63RuK30_Bo=
Signed-off-by: Flavio Fernandes
---
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
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(, "flags.loopback = 1; "
"ct_dnat(%s", nat->logical_ip);
-if (strlen(nat->external_port_range)) {
+if (nat->external_port_range[0]) {
ds_put_format(, ",%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(, "ct_dnat(%s", nat->logical_ip);
-if (strlen(nat->external_port_range)) {
+if (nat->external_port_range[0]) {
ds_put_format(, ",%s",
nat->external_port_range);
}
@@ -9061,7 +9061,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
ds_put_format(, "ct_snat(%s",
nat->external_ip);
-if (strlen(nat->external_port_range)) {
+if (nat->external_port_range[0]) {
ds_put_format(, ",%s",
nat->external_port_range);
}
@@ -9104,7 +9104,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
} else {
ds_put_
Re: [ovs-dev] [PATCH ovn v1 1/1] NAT: Enhancements to external port range support
On Thu, Apr 16, 2020 at 3:45 AM Flavio Fernandes wrote:
>
> 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
Thanks for the patch Flavio.
LGTM.
Acked-by: Numan Siddique
Numan
> ---
> 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
> 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(, "flags.loopback = 1; "
>"ct_dnat(%s", nat->logical_ip);
>
> -if (strlen(nat->external_port_range)) {
> +if (nat->external_port_range[0]) {
> ds_put_format(, ",%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(, "ct_dnat(%s",
> nat->logical_ip);
> -if (strlen(nat->external_port_range)) {
> +if (nat->external_port_range[0]) {
> ds_put_format(, ",%s",
>nat->external_port_range);
> }
> @@ -9061,7 +9061,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
> *ports,
> ds_put_format(, "ct_snat(%s",
>nat->external_ip);
>
> -if (strlen(nat->external_port_range)) {
> +if (nat->external_port_range[0]) {
> ds_put_format(, ",%s",
>nat->external_port_range);
> }
> @@ -9104,7 +9104,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
> *ports,
> } else {
> ds_put_format(, "ct_snat(%s",
>nat->external_ip);
> -if (strlen(nat->external_port_range)) {
> +if (nat->external_port_range[0]) {
> ds_put_format(, ",%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 @@
>
[ovs-dev] [PATCH ovn v1 1/1] NAT: Enhancements to external port range support
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
---
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
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(, "flags.loopback = 1; "
"ct_dnat(%s", nat->logical_ip);
-if (strlen(nat->external_port_range)) {
+if (nat->external_port_range[0]) {
ds_put_format(, ",%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(, "ct_dnat(%s", nat->logical_ip);
-if (strlen(nat->external_port_range)) {
+if (nat->external_port_range[0]) {
ds_put_format(, ",%s",
nat->external_port_range);
}
@@ -9061,7 +9061,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
ds_put_format(, "ct_snat(%s",
nat->external_ip);
-if (strlen(nat->external_port_range)) {
+if (nat->external_port_range[0]) {
ds_put_format(, ",%s",
nat->external_port_range);
}
@@ -9104,7 +9104,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
} else {
ds_put_format(, "ct_snat(%s",
nat->external_ip);
-if (strlen(nat->external_port_range)) {
+if (nat->external_port_range[0]) {
ds_put_format(, ",%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 @@
-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).
diff --git a/tests/ovn-nbctl.at
