| Message ID | 1585351104-106546-2-git-send-email-svc.mail.git@nutanix.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | NAT port range support | expand |
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: WARNING: Line is 90 characters long (recommended limit is 79) WARNING: Line lacks whitespace around operator #119 FILE: utilities/ovn-nbctl.c:706: lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [PORT_RANGE][LOGICAL_PORT EXTERNAL_MAC]\n\ Lines checked: 222, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On 3/27/20 7:18 PM, Ankur Sharma wrote: > From: Ankur Sharma <ankur.sharma@nutanix.com> > > This patch enhances the NB OVSSCHEMA to > add an additional comuln in NAT table. > > external_port_range: Specifies the range of port numbers > to translate source port to. > > Changes also add corresponding ovn-nbctl cli. > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > --- > ovn-nb.ovsschema | 5 +++-- > ovn-nb.xml | 23 +++++++++++++++++++++++ > tests/ovn-nbctl.at | 20 ++++++++++++++++++++ > utilities/ovn-nbctl.c | 50 +++++++++++++++++++++++++++++++++++++------------- > 4 files changed, 83 insertions(+), 15 deletions(-) > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > index 843e979..c7c28fd 100644 > --- a/ovn-nb.ovsschema > +++ b/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > - "version": "5.20.0", > - "cksum": "2846067333 25243", > + "version": "5.20.1", > + "cksum": "3966465842 25302", > "tables": { > "NB_Global": { > "columns": { > @@ -379,6 +379,7 @@ > "external_ip": {"type": "string"}, > "external_mac": {"type": {"key": "string", > "min": 0, "max": 1}}, > + "external_port_range": {"type": "string"}, > "logical_ip": {"type": "string"}, > "logical_port": {"type": {"key": "string", > "min": 0, "max": 1}}, > diff --git a/ovn-nb.xml b/ovn-nb.xml > index f4ecc1c..70e0c65 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2552,6 +2552,29 @@ > </p> > </column> > > + <column name="external_port_range"> > + <p> > + L4 source port range > + </p> > + > + <p> > + Range of port, 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> > + > + <p> > + Value of the column is in the format, port_lo-port_hi. > + For example: > + external_port_range : "1-30000" > + </p> > + > + <p> > + Valid range of ports is 1-65535. > + </p> > + > + </column> > + > <column name="logical_ip"> > An IPv4 network (e.g 192.168.1.0/24) or an IPv4 address. > </column> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > index fcb6ad7..34d75b7 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -604,6 +604,26 @@ 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 dnat 40.0.0.4 192.168.1.7 1-3000]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.5 192.168.1.8 1-3000]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 1-3000 lp0 00:00:00:04:05:06]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 1-3000 lp0], [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.6 192.168.1.9 1-3000 00:00:00:04:05:06], [1], [], > +[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. > +]) > + > +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_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 > +]) > + > +AT_CHECK([ovn-nbctl lr-nat-del lr0]) > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], []) > AT_CHECK([ovn-nbctl lr-nat-del lr0]) > AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])]) > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index e80058e..e84e1fc 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -702,7 +702,8 @@ Policy commands:\n\ > \n\ > NAT commands:\n\ > [--stateless]\n\ > - lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]\n\ > + [--portrange]\n\ > + lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [PORT_RANGE][LOGICAL_PORT EXTERNAL_MAC]\n\ A couple of suggestions: * Refer to this as "EXTERNAL_PORT_RANGE" to be more clear. * Why is the port range this point in the command? I suggest either placing it directly after the EXTERNAL_IP argument or as the last argument. Placing it after EXTERNAL_IP groups it nicely with the other external option. Placing it at the end makes the code changes in ovn-nbctl much simpler. Placing it at the end would also get rid of the need for the --portrange option. > add a NAT to ROUTER\n\ > lr-nat-del ROUTER [TYPE [IP]]\n\ > remove NATs from ROUTER\n\ > @@ -3969,6 +3970,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > const char *external_ip = ctx->argv[3]; > const char *logical_ip = ctx->argv[4]; > char *new_logical_ip = NULL; > + bool is_portrange = shash_find(&ctx->options, "--portrange") != NULL; > > char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr); > if (error) { > @@ -4032,14 +4034,23 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > } > } > > - const char *logical_port; > - const char *external_mac; > + const char *logical_port = NULL; > + const char *external_mac = NULL; > + const char *port_range = NULL; > + > + if (is_portrange) { > + port_range = ctx->argv[5]; > + } You should perform some validation of the supplied port range here. If the port range is not valid (e.g. 9000-8000 or hello-goodbye), then print an error and do not set anything. > + > if (ctx->argc == 6) { > - ctl_error(ctx, "lr-nat-add with logical_port " > - "must also specify external_mac."); > - free(new_logical_ip); > - return; > - } else if (ctx->argc == 7) { > + This blank line looks weird once the patch is applied. > + if (!is_portrange) { > + ctl_error(ctx, "lr-nat-add with logical_port " > + "must also specify external_mac."); > + free(new_logical_ip); > + return; > + } > + } else if (ctx->argc >= 7) { > if (strcmp(nat_type, "dnat_and_snat")) { > ctl_error(ctx, "logical_port and external_mac are only valid when " > "type is \"dnat_and_snat\"."); > @@ -4047,7 +4058,14 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > return; > } > > - logical_port = ctx->argv[5]; > + if (ctx->argc == 7 && is_portrange) { > + ctl_error(ctx, "lr-nat-add with logical_port " > + "must also specify external_mac."); > + free(new_logical_ip); > + return; > + } > + > + logical_port = ctx->argv[is_portrange ? 6: 5]; > const struct nbrec_logical_switch_port *lsp; > error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); > if (error) { > @@ -4056,7 +4074,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > return; > } > > - external_mac = ctx->argv[6]; > + external_mac = ctx->argv[is_portrange? 7: 6]; > struct eth_addr ea; > if (!eth_addr_from_string(external_mac, &ea)) { > ctl_error(ctx, "invalid mac address %s.", external_mac); > @@ -4064,6 +4082,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > return; > } > } else { > + port_range = NULL; > logical_port = NULL; > external_mac = NULL; > } > @@ -4135,6 +4154,10 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > nbrec_nat_set_external_mac(nat, external_mac); > } > > + if (port_range) { > + nbrec_nat_set_external_port_range(nat, port_range); > + } > + > smap_add(&nat_options, "stateless", stateless ? "true":"false"); > nbrec_nat_set_options(nat, &nat_options); > > @@ -6131,9 +6154,10 @@ static const struct ctl_command_syntax nbctl_commands[] = { > "", RO }, > > /* NAT commands. */ > - { "lr-nat-add", 4, 6, > - "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]", NULL, > - nbctl_lr_nat_add, NULL, "--may-exist,--stateless", RW }, > + { "lr-nat-add", 4, 7, > + "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [PORT_RANGE]" > + "[LOGICAL_PORT EXTERNAL_MAC]", NULL, > + nbctl_lr_nat_add, NULL, "--may-exist,--stateless,--portrange", RW }, > { "lr-nat-del", 1, 3, "ROUTER [TYPE [IP]]", NULL, > nbctl_lr_nat_del, NULL, "--if-exists", RW }, > { "lr-nat-list", 1, 1, "ROUTER", NULL, nbctl_lr_nat_list, NULL, "", RO }, >
Hi Mark, Thanks a lot for review. Please find my replies inline. Regards, Ankur -----Original Message----- From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Mark Michelson Sent: Monday, March 30, 2020 12:21 PM To: svc.mail.git <svc.mail.git@nutanix.com>; ovs-dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1 1/3 ovn] NAT: Provide port range in input On 3/27/20 7:18 PM, Ankur Sharma wrote: > From: Ankur Sharma <ankur.sharma@nutanix.com> > > This patch enhances the NB OVSSCHEMA to add an additional comuln in > NAT table. > > external_port_range: Specifies the range of port numbers > to translate source port to. > > Changes also add corresponding ovn-nbctl cli. > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > --- > ovn-nb.ovsschema | 5 +++-- > ovn-nb.xml | 23 +++++++++++++++++++++++ > tests/ovn-nbctl.at | 20 ++++++++++++++++++++ > utilities/ovn-nbctl.c | 50 +++++++++++++++++++++++++++++++++++++------------- > 4 files changed, 83 insertions(+), 15 deletions(-) > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index > 843e979..c7c28fd 100644 > --- a/ovn-nb.ovsschema > +++ b/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > - "version": "5.20.0", > - "cksum": "2846067333 25243", > + "version": "5.20.1", > + "cksum": "3966465842 25302", > "tables": { > "NB_Global": { > "columns": { > @@ -379,6 +379,7 @@ > "external_ip": {"type": "string"}, > "external_mac": {"type": {"key": "string", > "min": 0, "max": 1}}, > + "external_port_range": {"type": "string"}, > "logical_ip": {"type": "string"}, > "logical_port": {"type": {"key": "string", > "min": 0, "max": 1}}, diff > --git a/ovn-nb.xml b/ovn-nb.xml index f4ecc1c..70e0c65 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2552,6 +2552,29 @@ > </p> > </column> > > + <column name="external_port_range"> > + <p> > + L4 source port range > + </p> > + > + <p> > + Range of port, 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> > + > + <p> > + Value of the column is in the format, port_lo-port_hi. > + For example: > + external_port_range : "1-30000" > + </p> > + > + <p> > + Valid range of ports is 1-65535. > + </p> > + > + </column> > + > <column name="logical_ip"> > An IPv4 network (e.g 192.168.1.0/24) or an IPv4 address. > </column> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index > fcb6ad7..34d75b7 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -604,6 +604,26 @@ 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 > +dnat 40.0.0.4 192.168.1.7 1-3000]) AT_CHECK([ovn-nbctl --portrange > +lr-nat-add lr0 dnat_and_snat 40.0.0.5 192.168.1.8 1-3000]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 > +192.168.1.9 1-3000 lp0 00:00:00:04:05:06]) AT_CHECK([ovn-nbctl > +--portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 1-3000 > +lp0], [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.6 > +192.168.1.9 1-3000 00:00:00:04:05:06], [1], [], > +[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. > +]) > + > +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_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 > +]) > + > +AT_CHECK([ovn-nbctl lr-nat-del lr0]) > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], []) > AT_CHECK([ovn-nbctl lr-nat-del lr0]) > AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])]) diff --git > a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index e80058e..e84e1fc > 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -702,7 +702,8 @@ Policy commands:\n\ > \n\ > NAT commands:\n\ > [--stateless]\n\ > - lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT > EXTERNAL_MAC]\n\ > + [--portrange]\n\ > + lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP > + [PORT_RANGE][LOGICAL_PORT EXTERNAL_MAC]\n\ A couple of suggestions: * Refer to this as "EXTERNAL_PORT_RANGE" to be more clear. [ANKUR]: Sure, v2 will have this change. * Why is the port range this point in the command? I suggest either placing it directly after the EXTERNAL_IP argument or as the last argument. Placing it after EXTERNAL_IP groups it nicely with the other external option. Placing it at the end makes the code changes in ovn-nbctl much simpler. Placing it at the end would also get rid of the need for the --portrange option. [ANKUR]: Port range is specified here, since all the optional arguments start after LOGICAL_IP. I thought of having it after EXTERNAL_IP, but did not do so, ideally LOGICAL_IP should be before EXTERNAL IP 😊 (to keep all the external arguments together), plus now we would have an optional argument b/w mandatory ones and that would complicate the implementation quite a bit. However, I agree that another option is placing it in the end. V2 will have this change. "--portrange" would be still needed, since all the arguments after LOGICAL_IP are optional. If there is only 1 argument after LOGICAL_IP, then --portrange helps in classifying if next argument is port_range or it is an invalid input. > add a NAT to ROUTER\n\ > lr-nat-del ROUTER [TYPE [IP]]\n\ > remove NATs from ROUTER\n\ @@ -3969,6 > +3970,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > const char *external_ip = ctx->argv[3]; > const char *logical_ip = ctx->argv[4]; > char *new_logical_ip = NULL; > + bool is_portrange = shash_find(&ctx->options, "--portrange") != > + NULL; > > char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr); > if (error) { > @@ -4032,14 +4034,23 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > } > } > > - const char *logical_port; > - const char *external_mac; > + const char *logical_port = NULL; > + const char *external_mac = NULL; > + const char *port_range = NULL; > + > + if (is_portrange) { > + port_range = ctx->argv[5]; > + } You should perform some validation of the supplied port range here. If the port range is not valid (e.g. 9000-8000 or hello-goodbye), then print an error and do not set anything. [ANKUR]: Sure, V2 will have this change. > + > if (ctx->argc == 6) { > - ctl_error(ctx, "lr-nat-add with logical_port " > - "must also specify external_mac."); > - free(new_logical_ip); > - return; > - } else if (ctx->argc == 7) { > + This blank line looks weird once the patch is applied. [ANKUR]: Sure, V2 will have this change. > + if (!is_portrange) { > + ctl_error(ctx, "lr-nat-add with logical_port " > + "must also specify external_mac."); > + free(new_logical_ip); > + return; > + } > + } else if (ctx->argc >= 7) { > if (strcmp(nat_type, "dnat_and_snat")) { > ctl_error(ctx, "logical_port and external_mac are only valid when " > "type is \"dnat_and_snat\"."); @@ -4047,7 > +4058,14 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > return; > } > > - logical_port = ctx->argv[5]; > + if (ctx->argc == 7 && is_portrange) { > + ctl_error(ctx, "lr-nat-add with logical_port " > + "must also specify external_mac."); > + free(new_logical_ip); > + return; > + } > + > + logical_port = ctx->argv[is_portrange ? 6: 5]; > const struct nbrec_logical_switch_port *lsp; > error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); > if (error) { > @@ -4056,7 +4074,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > return; > } > > - external_mac = ctx->argv[6]; > + external_mac = ctx->argv[is_portrange? 7: 6]; > struct eth_addr ea; > if (!eth_addr_from_string(external_mac, &ea)) { > ctl_error(ctx, "invalid mac address %s.", external_mac); > @@ -4064,6 +4082,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > return; > } > } else { > + port_range = NULL; > logical_port = NULL; > external_mac = NULL; > } > @@ -4135,6 +4154,10 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > nbrec_nat_set_external_mac(nat, external_mac); > } > > + if (port_range) { > + nbrec_nat_set_external_port_range(nat, port_range); > + } > + > smap_add(&nat_options, "stateless", stateless ? "true":"false"); > nbrec_nat_set_options(nat, &nat_options); > > @@ -6131,9 +6154,10 @@ static const struct ctl_command_syntax nbctl_commands[] = { > "", RO }, > > /* NAT commands. */ > - { "lr-nat-add", 4, 6, > - "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]", NULL, > - nbctl_lr_nat_add, NULL, "--may-exist,--stateless", RW }, > + { "lr-nat-add", 4, 7, > + "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [PORT_RANGE]" > + "[LOGICAL_PORT EXTERNAL_MAC]", NULL, > + nbctl_lr_nat_add, NULL, "--may-exist,--stateless,--portrange", > + RW }, > { "lr-nat-del", 1, 3, "ROUTER [TYPE [IP]]", NULL, > nbctl_lr_nat_del, NULL, "--if-exists", RW }, > { "lr-nat-list", 1, 1, "ROUTER", NULL, nbctl_lr_nat_list, NULL, > "", RO }, >
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index 843e979..c7c28fd 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "5.20.0", - "cksum": "2846067333 25243", + "version": "5.20.1", + "cksum": "3966465842 25302", "tables": { "NB_Global": { "columns": { @@ -379,6 +379,7 @@ "external_ip": {"type": "string"}, "external_mac": {"type": {"key": "string", "min": 0, "max": 1}}, + "external_port_range": {"type": "string"}, "logical_ip": {"type": "string"}, "logical_port": {"type": {"key": "string", "min": 0, "max": 1}}, diff --git a/ovn-nb.xml b/ovn-nb.xml index f4ecc1c..70e0c65 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -2552,6 +2552,29 @@ </p> </column> + <column name="external_port_range"> + <p> + L4 source port range + </p> + + <p> + Range of port, 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> + + <p> + Value of the column is in the format, port_lo-port_hi. + For example: + external_port_range : "1-30000" + </p> + + <p> + Valid range of ports is 1-65535. + </p> + + </column> + <column name="logical_ip"> An IPv4 network (e.g 192.168.1.0/24) or an IPv4 address. </column> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index fcb6ad7..34d75b7 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -604,6 +604,26 @@ 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 dnat 40.0.0.4 192.168.1.7 1-3000]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.5 192.168.1.8 1-3000]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 1-3000 lp0 00:00:00:04:05:06]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 1-3000 lp0], [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.6 192.168.1.9 1-3000 00:00:00:04:05:06], [1], [], +[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. +]) + +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_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 +]) + +AT_CHECK([ovn-nbctl lr-nat-del lr0]) AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], []) AT_CHECK([ovn-nbctl lr-nat-del lr0]) AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])]) diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index e80058e..e84e1fc 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -702,7 +702,8 @@ Policy commands:\n\ \n\ NAT commands:\n\ [--stateless]\n\ - lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]\n\ + [--portrange]\n\ + lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [PORT_RANGE][LOGICAL_PORT EXTERNAL_MAC]\n\ add a NAT to ROUTER\n\ lr-nat-del ROUTER [TYPE [IP]]\n\ remove NATs from ROUTER\n\ @@ -3969,6 +3970,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) const char *external_ip = ctx->argv[3]; const char *logical_ip = ctx->argv[4]; char *new_logical_ip = NULL; + bool is_portrange = shash_find(&ctx->options, "--portrange") != NULL; char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr); if (error) { @@ -4032,14 +4034,23 @@ nbctl_lr_nat_add(struct ctl_context *ctx) } } - const char *logical_port; - const char *external_mac; + const char *logical_port = NULL; + const char *external_mac = NULL; + const char *port_range = NULL; + + if (is_portrange) { + port_range = ctx->argv[5]; + } + if (ctx->argc == 6) { - ctl_error(ctx, "lr-nat-add with logical_port " - "must also specify external_mac."); - free(new_logical_ip); - return; - } else if (ctx->argc == 7) { + + if (!is_portrange) { + ctl_error(ctx, "lr-nat-add with logical_port " + "must also specify external_mac."); + free(new_logical_ip); + return; + } + } else if (ctx->argc >= 7) { if (strcmp(nat_type, "dnat_and_snat")) { ctl_error(ctx, "logical_port and external_mac are only valid when " "type is \"dnat_and_snat\"."); @@ -4047,7 +4058,14 @@ nbctl_lr_nat_add(struct ctl_context *ctx) return; } - logical_port = ctx->argv[5]; + if (ctx->argc == 7 && is_portrange) { + ctl_error(ctx, "lr-nat-add with logical_port " + "must also specify external_mac."); + free(new_logical_ip); + return; + } + + logical_port = ctx->argv[is_portrange ? 6: 5]; const struct nbrec_logical_switch_port *lsp; error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); if (error) { @@ -4056,7 +4074,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) return; } - external_mac = ctx->argv[6]; + external_mac = ctx->argv[is_portrange? 7: 6]; struct eth_addr ea; if (!eth_addr_from_string(external_mac, &ea)) { ctl_error(ctx, "invalid mac address %s.", external_mac); @@ -4064,6 +4082,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) return; } } else { + port_range = NULL; logical_port = NULL; external_mac = NULL; } @@ -4135,6 +4154,10 @@ nbctl_lr_nat_add(struct ctl_context *ctx) nbrec_nat_set_external_mac(nat, external_mac); } + if (port_range) { + nbrec_nat_set_external_port_range(nat, port_range); + } + smap_add(&nat_options, "stateless", stateless ? "true":"false"); nbrec_nat_set_options(nat, &nat_options); @@ -6131,9 +6154,10 @@ static const struct ctl_command_syntax nbctl_commands[] = { "", RO }, /* NAT commands. */ - { "lr-nat-add", 4, 6, - "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]", NULL, - nbctl_lr_nat_add, NULL, "--may-exist,--stateless", RW }, + { "lr-nat-add", 4, 7, + "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [PORT_RANGE]" + "[LOGICAL_PORT EXTERNAL_MAC]", NULL, + nbctl_lr_nat_add, NULL, "--may-exist,--stateless,--portrange", RW }, { "lr-nat-del", 1, 3, "ROUTER [TYPE [IP]]", NULL, nbctl_lr_nat_del, NULL, "--if-exists", RW }, { "lr-nat-list", 1, 1, "ROUTER", NULL, nbctl_lr_nat_list, NULL, "", RO },