Re: [ovs-dev] [PATCH v3 2/2 ovn] NAT: Northd and parser changes to support port range

2020-04-01 Thread 0-day Robot
Bleep bloop.  Greetings Ankur Sharma, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Ankur Sharma  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Ankur Sharma 
Lines checked: 372, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3 2/2 ovn] NAT: Northd and parser changes to support port range

2020-04-01 Thread Ankur Sharma
This patch has northd changes to put
port range in the logical flow based on configuration.

Port range is NOT applicable for stateless dnat_and_snat
rules.

Changes to parse the logical flow, which specifies port_range
for ct_nat action.

Example logical flow:
ct_snat(10.15.24.135,1-3)

Signed-off-by: Ankur Sharma 
---
 include/ovn/actions.h |  7 ++
 include/ovn/lex.h |  1 +
 lib/actions.c | 48 ++
 lib/lex.c |  5 +++-
 northd/ovn-northd.c   | 31 +
 tests/ovn-northd.at   | 64 +++
 tests/ovn.at  | 34 +++
 7 files changed, 180 insertions(+), 10 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 9b01492..2cec369 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -233,6 +233,13 @@ struct ovnact_ct_nat {
 struct in6_addr ipv6;
 ovs_be32 ipv4;
 };
+
+struct {
+   bool exists;
+   uint16_t port_lo;
+   uint16_t port_hi;
+} port_range;
+
 uint8_t ltable; /* Logical table ID of next table. */
 };
 
diff --git a/include/ovn/lex.h b/include/ovn/lex.h
index 8d55857..1da6ccc 100644
--- a/include/ovn/lex.h
+++ b/include/ovn/lex.h
@@ -63,6 +63,7 @@ enum lex_type {
 LEX_T_EXCHANGE, /* <-> */
 LEX_T_DECREMENT,/* -- */
 LEX_T_COLON,/* : */
+LEX_T_HYPHEN,   /* - */
 };
 
 /* Subtype for LEX_T_INTEGER and LEX_T_MASKED_INTEGER tokens.
diff --git a/lib/actions.c b/lib/actions.c
index 6351db7..5d9d93b 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -770,6 +770,38 @@ parse_ct_nat(struct action_context *ctx, const char *name,
 }
 lexer_get(ctx->lexer);
 
+if (lexer_match(ctx->lexer, LEX_T_COMMA)) {
+
+   if (ctx->lexer->token.type != LEX_T_INTEGER ||
+   ctx->lexer->token.format != LEX_F_DECIMAL) {
+  lexer_syntax_error(ctx->lexer, "expecting Integer for port "
+ "range");
+   }
+
+   cn->port_range.port_lo = ntohll(ctx->lexer->token.value.integer);
+   lexer_get(ctx->lexer);
+
+   if (lexer_match(ctx->lexer, LEX_T_HYPHEN)) {
+
+   if (ctx->lexer->token.type != LEX_T_INTEGER) {
+   lexer_syntax_error(ctx->lexer, "expecting Integer for port "
+  "range");
+   }
+   cn->port_range.port_hi = ntohll(
+ctx->lexer->token.value.integer);
+
+   if (cn->port_range.port_hi <= cn->port_range.port_lo) {
+   lexer_syntax_error(ctx->lexer, "range high should be "
+  "greater than range lo");
+   }
+   lexer_get(ctx->lexer);
+   } else {
+   cn->port_range.port_hi = 0;
+   }
+
+   cn->port_range.exists = true;
+}
+
 if (!lexer_force_match(ctx->lexer, LEX_T_RPAREN)) {
 return;
 }
@@ -799,6 +831,17 @@ format_ct_nat(const struct ovnact_ct_nat *cn, const char 
*name, struct ds *s)
 ipv6_format_addr(&cn->ipv6, s);
 ds_put_char(s, ')');
 }
+
+if (cn->port_range.exists) {
+ds_chomp(s, ')');
+ds_put_format(s, ",%d", cn->port_range.port_lo);
+
+if (cn->port_range.port_hi) {
+ds_put_format(s, "-%d", cn->port_range.port_hi);
+}
+ds_put_char(s, ')');
+}
+
 ds_put_char(s, ';');
 }
 
@@ -861,6 +904,11 @@ encode_ct_nat(const struct ovnact_ct_nat *cn,
 }
 }
 
+if (cn->port_range.exists) {
+   nat->range.proto.min = cn->port_range.port_lo;
+   nat->range.proto.max = cn->port_range.port_hi;
+}
+
 ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
 ct = ofpacts->header;
 if (cn->family == AF_INET || cn->family == AF_INET6) {
diff --git a/lib/lex.c b/lib/lex.c
index 7a2ab41..94f6c77 100644
--- a/lib/lex.c
+++ b/lib/lex.c
@@ -301,6 +301,9 @@ lex_token_format(const struct lex_token *token, struct ds 
*s)
 case LEX_T_COLON:
 ds_put_char(s, ':');
 break;
+case LEX_T_HYPHEN:
+ds_put_char(s, '-');
+break;
 default:
 OVS_NOT_REACHED();
 }
@@ -757,7 +760,7 @@ next:
 token->type = LEX_T_DECREMENT;
 p++;
 } else {
-lex_error(token, "`-' is only valid as part of `--'.");
+   token->type = LEX_T_HYPHEN;
 }
 break;
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 0762781..71d420d 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8849,7 +8849,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
   is_v6 ? "6" : "4", nat->logical_ip);
 } else {
 ds_put_format(&actions, "f