| Message ID | 1600309019-99938-2-git-send-email-svc.mail.git@nutanix.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | NAT port range support | expand |
On 17/09/2020 03:16, Ankur Sharma wrote: > From: Ankur Sharma <ankur.sharma@nutanix.com> > > This patch enhances the NB OVSSCHEMA to > add an additional column in NAT table. > > external_port_hash: Specifies the hashing mechanism > if port range is specified. > > Changes also add corresponding ovn-nbctl cli. > Thanks for the patch. > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > --- > ovn-nb.ovsschema | 5 +- > ovn-nb.xml | 15 ++++++ > tests/ovn-nbctl.at | 136 +++++++++++++++++++++++++++++++------------------- > utilities/ovn-nbctl.c | 102 ++++++++++++++++++++++++++++--------- > 4 files changed, 182 insertions(+), 76 deletions(-) > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > index 092322a..9b8e070 100644 > --- a/ovn-nb.ovsschema > +++ b/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > - "version": "5.27.0", > - "cksum": "3507518247 26773", > + "version": "5.28.0", > + "cksum": "2621169942 26831", > "tables": { > "NB_Global": { > "columns": { > @@ -398,6 +398,7 @@ > "external_mac": {"type": {"key": "string", > "min": 0, "max": 1}}, > "external_port_range": {"type": "string"}, > + "external_port_hash": {"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 0bfe626..142d934 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2729,6 +2729,21 @@ > > </column> > > + <column name="external_port_hash"> > + <p> > + Hashing algorithm to hash a packet to specified port range > + </p> > + > + <p> > + Applicable only if port range is also specified. > + </p> > + > + <p> > + Takes one of the 2 values "Random" and "Hash" > + </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 baf7a87..6ce1ecf 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -476,15 +476,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 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 > +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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 > @@ -512,28 +512,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 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 > +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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 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 > +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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], > @@ -584,26 +584,26 @@ 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 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 > +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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 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 > +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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]) > @@ -613,10 +613,10 @@ 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]) > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 lp0 00:00:00:04:05:06 1-3000]) > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 lp0 1-3000], [1], [], > -[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. > +[ovn-nbctl: invalid port range lp0. > ]) > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 00:00:00:04:05:06 1-3000], [1], [], > -[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. > +[ovn-nbctl: invalid port range 00:00:00:04:05:06. > ]) > > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 0], [1], [], > @@ -674,12 +674,46 @@ AT_CHECK([ovn-nbctl show lr0 | grep -C2 'external port(s): "1"' | uuidfilt], [0] > ]) > > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > -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 > +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 snat 40.0.0.3 192.168.1.6 21-65535 hash]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.4 192.168.1.7 1-3000 random]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.5 192.168.1.10 1 hash]) > +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 lp0 00:00:00:04:05:06 1-3000]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 lp0 1-3000 hash], [1], [], > +[ovn-nbctl: invalid mac address 1-3000. > +]) > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 00:00:00:04:05:06 1-3000 hash], [1], [], > +[ovn-nbctl: 00:00:00:04:05:06: port name not found > +]) > + > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 0 random], [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-300 abcd], [1], [], > +[ovn-nbctl: invalid port hash abcd. > +]) > + > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 abcd], [1], [], > +[ovn-nbctl: invalid port range abcd. > +]) > + > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT > +dnat 40.0.0.4 1-3000 random 192.168.1.7 > +dnat 40.0.0.5 1 hash 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 hash 192.168.1.6 > ]) > > AT_CHECK([ovn-nbctl lr-nat-del lr0]) > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index c54e639..3a1b158 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -1088,6 +1088,11 @@ print_lr(const struct nbrec_logical_router *lr, struct ds *s) > if (nat->external_port_range[0]) { > ds_put_cstr(s, " external port(s): "); > ds_put_format(s, "\"%s\"\n", nat->external_port_range); > + > + if (nat->external_port_hash[0]) { > + ds_put_cstr(s, " external port_hash: "); > + ds_put_format(s, "\"%s\"\n", nat->external_port_hash); > + } > } > ds_put_cstr(s, " logical ip: "); > ds_put_format(s, "\"%s\"\n", nat->logical_ip); > @@ -4129,6 +4134,16 @@ out: > free(nexthop); > } > > +static inline bool > +is_valid_port_hash(const char *port_hash) > +{ > + if (!strcmp(port_hash, "hash") || !strcmp(port_hash, "random")) { > + return true; > + } > + > + return false; > +} > + > static bool > is_valid_port_range(const char *port_range) > { > @@ -4246,6 +4261,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > const char *logical_port = NULL; > const char *external_mac = NULL; > const char *port_range = NULL; > + const char *port_hash = NULL; > > if (ctx->argc == 6) { > if (!is_portrange) { > @@ -4259,19 +4275,46 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > goto cleanup; > } > > - } 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\"."); > - goto cleanup; > - } > + } else if (ctx->argc == 7) { > + if (is_portrange) { > + port_range = ctx->argv[5]; > + if (!is_valid_port_range(port_range)) { > + ctl_error(ctx, "invalid port range %s.", port_range); > + goto cleanup; > + } > > - if (ctx->argc == 7 && is_portrange) { > - ctl_error(ctx, "lr-nat-add with logical_port " > - "must also specify external_mac."); > - goto cleanup; > + /* No need to validate the hash value, NBDB set will fail, > + * If value is not valid */ This comment is little confusing. We state that there is no need to validate the hash value, yet we still validate it? Also "If" should not be capitalized > + port_hash = ctx->argv[6]; > + if (!is_valid_port_hash(port_hash)) { > + ctl_error(ctx, "invalid port hash %s.", port_hash); > + goto cleanup; > + } > + } else { > + if (strcmp(nat_type, "dnat_and_snat")) { > + ctl_error(ctx, "logical_port and external_mac are only valid " > + "when type is \"dnat_and_snat\"."); > + goto cleanup; > + } > + > + logical_port = ctx->argv[5]; > + const struct nbrec_logical_switch_port *lsp; > + error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); > + if (error) { > + ctx->error = error; > + goto cleanup; > + } > + > + external_mac = ctx->argv[6]; > + struct eth_addr ea; > + if (!eth_addr_from_string(external_mac, &ea)) { > + ctl_error(ctx, "invalid mac address %s.", external_mac); > + goto cleanup; > + } > } > > + } else if (ctx->argc >= 8) { > + > logical_port = ctx->argv[5]; > const struct nbrec_logical_switch_port *lsp; > error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); > @@ -4286,11 +4329,17 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > ctl_error(ctx, "invalid mac address %s.", external_mac); > goto cleanup; > } > - > - if (ctx->argc > 7) { > - port_range = ctx->argv[7]; > - if (!is_valid_port_range(port_range)) { > - ctl_error(ctx, "invalid port range %s.", port_range); > + port_range = ctx->argv[7]; > + if (!is_valid_port_range(port_range)) { > + ctl_error(ctx, "invalid port range %s.", port_range); > + goto cleanup; > + } > + if (ctx->argc > 8) { > + /* No need to validate the hash value, NBDB set will fail, > + * If value is not valid */ And here, same comment as above. > + port_hash = ctx->argv[8]; > + if (!is_valid_port_hash(port_hash)) { > + ctl_error(ctx, "invalid port hash %s.", port_hash); > goto cleanup; > } > } > @@ -4299,6 +4348,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > port_range = NULL; > logical_port = NULL; > external_mac = NULL; > + port_hash = NULL; > } > > bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL; > @@ -4387,6 +4437,9 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > > if (port_range) { > nbrec_nat_set_external_port_range(nat, port_range); > + if (port_hash) { > + nbrec_nat_set_external_port_hash(nat, port_hash); > + } > } > > smap_add(&nat_options, "stateless", stateless ? "true":"false"); > @@ -4507,13 +4560,15 @@ 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, "%-17.13s%-22.18s%-21.17s%s", > - nat->external_port_range, > + smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%-" > + "22.18s%-21.17s%s",nat->external_port_range, > + nat->external_port_hash, > nat->logical_ip, nat->external_mac, > nat->logical_port); > } else { > - smap_add_format(&lr_nats, key, "%-17.13s%s", > + smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%s", > nat->external_port_range, > + nat->external_port_hash, > nat->logical_ip); > } > free(key); > @@ -4522,9 +4577,9 @@ nbctl_lr_nat_list(struct ctl_context *ctx) > const struct smap_node **nodes = smap_sort(&lr_nats); > if (nodes) { > 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"); > + "%-17.13s%-19.15s%-17.13s%-22.18s%-22.18s%-21.17s%s\n", > + "TYPE", "EXTERNAL_IP", "EXTERNAL_PORT", "EXTERNAL_PORT_HASH", > + "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", > @@ -6538,8 +6593,9 @@ static const struct ctl_command_syntax nbctl_commands[] = { > /* NAT commands. */ > { "lr-nat-add", 4, 7, Should this be { "lr-nat-add", 4, 8, > "ROUTER TYPE EXTERNAL_IP LOGICAL_IP" > - "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE]", NULL, > - nbctl_lr_nat_add, NULL, "--may-exist,--stateless,--portrange", RW }, > + "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE EXTERNAL_PORT_HASH]", Is EXTERNAL_PORT_HASH mandatory now when specifying EXTERNAL_PORT_RANGE? Perhaps it could be "[EXTERNAL_PORT_RANGE [EXTERNAL_PORT_HASH]]"? Is there a default value? > + 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 }, >
On Mon, Sep 21, 2020 at 7:49 PM Mark Gray <mark.d.gray@redhat.com> wrote: > On 17/09/2020 03:16, Ankur Sharma wrote: > > From: Ankur Sharma <ankur.sharma@nutanix.com> > > > > This patch enhances the NB OVSSCHEMA to > > add an additional column in NAT table. > > > > external_port_hash: Specifies the hashing mechanism > > if port range is specified. > > > > Changes also add corresponding ovn-nbctl cli. > > > > Thanks for the patch. > > > Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com> > > --- > > ovn-nb.ovsschema | 5 +- > > ovn-nb.xml | 15 ++++++ > > tests/ovn-nbctl.at | 136 > +++++++++++++++++++++++++++++++------------------- > > utilities/ovn-nbctl.c | 102 ++++++++++++++++++++++++++++--------- > > 4 files changed, 182 insertions(+), 76 deletions(-) > > > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > > index 092322a..9b8e070 100644 > > --- a/ovn-nb.ovsschema > > +++ b/ovn-nb.ovsschema > > @@ -1,7 +1,7 @@ > > { > > "name": "OVN_Northbound", > > - "version": "5.27.0", > > - "cksum": "3507518247 26773", > > + "version": "5.28.0", > > + "cksum": "2621169942 26831", > > "tables": { > > "NB_Global": { > > "columns": { > > @@ -398,6 +398,7 @@ > > "external_mac": {"type": {"key": "string", > > "min": 0, "max": 1}}, > > "external_port_range": {"type": "string"}, > > + "external_port_hash": {"type": "string"}, > I think it's better to set an enum here and restrict the values to "hash" and "random". And also suggest to set the type with "min":0 , "max": 1 ? For NAT entries without port range and port hash set, this column would be an empty array. > > "logical_ip": {"type": "string"}, > > "logical_port": {"type": {"key": "string", > > "min": 0, "max": 1}}, > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index 0bfe626..142d934 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -2729,6 +2729,21 @@ > > > > </column> > > > > + <column name="external_port_hash"> > > + <p> > > + Hashing algorithm to hash a packet to specified port range > > + </p> > > + > > + <p> > > + Applicable only if port range is also specified. > > + </p> > > + > > + <p> > > + Takes one of the 2 values "Random" and "Hash" > Is the first letter capitalized for the port hash value ? > > + </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 baf7a87..6ce1ecf 100644 > > --- a/tests/ovn-nbctl.at > > +++ b/tests/ovn-nbctl.at > > @@ -476,15 +476,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 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 > > +TYPE EXTERNAL_IP EXTERNAL_PORT > EXTERNAL_PORT_HASH 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 > > @@ -512,28 +512,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 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 > > +TYPE EXTERNAL_IP EXTERNAL_PORT > EXTERNAL_PORT_HASH 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 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 > > +TYPE EXTERNAL_IP EXTERNAL_PORT > EXTERNAL_PORT_HASH 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], > > @@ -584,26 +584,26 @@ 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 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 > > +TYPE EXTERNAL_IP EXTERNAL_PORT > EXTERNAL_PORT_HASH 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 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 > > +TYPE EXTERNAL_IP EXTERNAL_PORT > EXTERNAL_PORT_HASH 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]) > > @@ -613,10 +613,10 @@ 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]) > > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 > 192.168.1.9 lp0 00:00:00:04:05:06 1-3000]) > > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 > 192.168.1.9 lp0 1-3000], [1], [], > > -[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. > > +[ovn-nbctl: invalid port range lp0. > > ]) > > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 > 192.168.1.9 00:00:00:04:05:06 1-3000], [1], [], > > -[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. > > +[ovn-nbctl: invalid port range 00:00:00:04:05:06. > > ]) > > > > AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 > 192.168.1.10 0], [1], [], > > @@ -674,12 +674,46 @@ AT_CHECK([ovn-nbctl show lr0 | grep -C2 'external > port(s): "1"' | uuidfilt], [0] > > ]) > > > > AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > > -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 > > +TYPE EXTERNAL_IP EXTERNAL_PORT > EXTERNAL_PORT_HASH 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]) > > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 snat 40.0.0.3 > 192.168.1.6 21-65535 hash]) > > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.4 > 192.168.1.7 1-3000 random]) > > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.5 > 192.168.1.10 1 hash]) > > +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 lp0 00:00:00:04:05:06 1-3000]) > > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 > 192.168.1.9 lp0 1-3000 hash], [1], [], > > +[ovn-nbctl: invalid mac address 1-3000. > > +]) > > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 > 192.168.1.9 00:00:00:04:05:06 1-3000 hash], [1], [], > > +[ovn-nbctl: 00:00:00:04:05:06: port name not found > > +]) > > + > > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 > 192.168.1.10 0 random], [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-300 abcd], [1], [], > > +[ovn-nbctl: invalid port hash abcd. > > +]) > > + > > +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 > 192.168.1.10 abcd], [1], [], > > +[ovn-nbctl: invalid port range abcd. > > +]) > > + > > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl > > +TYPE EXTERNAL_IP EXTERNAL_PORT > EXTERNAL_PORT_HASH LOGICAL_IP EXTERNAL_MAC > LOGICAL_PORT > > +dnat 40.0.0.4 1-3000 random > 192.168.1.7 > > +dnat 40.0.0.5 1 hash > 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 hash > 192.168.1.6 > > ]) > > > > AT_CHECK([ovn-nbctl lr-nat-del lr0]) > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > > index c54e639..3a1b158 100644 > > --- a/utilities/ovn-nbctl.c > > +++ b/utilities/ovn-nbctl.c > > @@ -1088,6 +1088,11 @@ print_lr(const struct nbrec_logical_router *lr, > struct ds *s) > > if (nat->external_port_range[0]) { > > ds_put_cstr(s, " external port(s): "); > > ds_put_format(s, "\"%s\"\n", nat->external_port_range); > > + > > + if (nat->external_port_hash[0]) { > > + ds_put_cstr(s, " external port_hash: "); > > + ds_put_format(s, "\"%s\"\n", nat->external_port_hash); > > + } > > } > > ds_put_cstr(s, " logical ip: "); > > ds_put_format(s, "\"%s\"\n", nat->logical_ip); > > @@ -4129,6 +4134,16 @@ out: > > free(nexthop); > > } > > > > +static inline bool > > +is_valid_port_hash(const char *port_hash) > > +{ > > + if (!strcmp(port_hash, "hash") || !strcmp(port_hash, "random")) { > > + return true; > > + } > > + > > + return false; > > +} > > + > > static bool > > is_valid_port_range(const char *port_range) > > { > > @@ -4246,6 +4261,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > > const char *logical_port = NULL; > > const char *external_mac = NULL; > > const char *port_range = NULL; > > + const char *port_hash = NULL; > > > > if (ctx->argc == 6) { > > if (!is_portrange) { > > @@ -4259,19 +4275,46 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > > goto cleanup; > > } > > > > - } 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\"."); > > - goto cleanup; > > - } > > + } else if (ctx->argc == 7) { > > + if (is_portrange) { > > + port_range = ctx->argv[5]; > > + if (!is_valid_port_range(port_range)) { > > + ctl_error(ctx, "invalid port range %s.", port_range); > > + goto cleanup; > > + } > > > > - if (ctx->argc == 7 && is_portrange) { > > - ctl_error(ctx, "lr-nat-add with logical_port " > > - "must also specify external_mac."); > > - goto cleanup; > > + /* No need to validate the hash value, NBDB set will fail, > > + * If value is not valid */ > > This comment is little confusing. We state that there is no need to > validate the hash value, yet we still validate it? > Agree. The comment is a little confusing. > > Also "If" should not be capitalized > > > + port_hash = ctx->argv[6]; > > + if (!is_valid_port_hash(port_hash)) { > Instead of 'is_valid_port_hash', how about 'is_allowed_port_hash()' ? I don't have any strong preference. I am fine if you want to stick with 'is_valid_port_hash()'. > + ctl_error(ctx, "invalid port hash %s.", port_hash); > > + goto cleanup; > > + } > > + } else { > > + if (strcmp(nat_type, "dnat_and_snat")) { > > + ctl_error(ctx, "logical_port and external_mac are only > valid " > > + "when type is \"dnat_and_snat\"."); > > + goto cleanup; > > + } > > + > > + logical_port = ctx->argv[5]; > > + const struct nbrec_logical_switch_port *lsp; > > + error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); > > + if (error) { > > + ctx->error = error; > > + goto cleanup; > > + } > > + > > + external_mac = ctx->argv[6]; > > + struct eth_addr ea; > > + if (!eth_addr_from_string(external_mac, &ea)) { > > + ctl_error(ctx, "invalid mac address %s.", external_mac); > > + goto cleanup; > > + } > > } > > > > + } else if (ctx->argc >= 8) { > > + > > logical_port = ctx->argv[5]; > > const struct nbrec_logical_switch_port *lsp; > > error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); > > @@ -4286,11 +4329,17 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > > ctl_error(ctx, "invalid mac address %s.", external_mac); > > goto cleanup; > > } > > - > > - if (ctx->argc > 7) { > > - port_range = ctx->argv[7]; > > - if (!is_valid_port_range(port_range)) { > > - ctl_error(ctx, "invalid port range %s.", port_range); > > + port_range = ctx->argv[7]; > > + if (!is_valid_port_range(port_range)) { > > + ctl_error(ctx, "invalid port range %s.", port_range); > > + goto cleanup; > > + } > > + if (ctx->argc > 8) { > > + /* No need to validate the hash value, NBDB set will fail, > > + * If value is not valid */ > > And here, same comment as above. > > > + port_hash = ctx->argv[8]; > > + if (!is_valid_port_hash(port_hash)) { > > + ctl_error(ctx, "invalid port hash %s.", port_hash); > > goto cleanup; > > } > > } > > @@ -4299,6 +4348,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > > port_range = NULL; > > logical_port = NULL; > > external_mac = NULL; > > + port_hash = NULL; > > } > > > > bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL; > > @@ -4387,6 +4437,9 @@ nbctl_lr_nat_add(struct ctl_context *ctx) > > > > if (port_range) { > > nbrec_nat_set_external_port_range(nat, port_range); > > + if (port_hash) { > > + nbrec_nat_set_external_port_hash(nat, port_hash); > > + } > > } > > > > smap_add(&nat_options, "stateless", stateless ? "true":"false"); > > @@ -4507,13 +4560,15 @@ 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, "%-17.13s%-22.18s%-21.17s%s", > > - nat->external_port_range, > > + smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%-" > > + "22.18s%-21.17s%s",nat->external_port_range, > > + nat->external_port_hash, > > nat->logical_ip, nat->external_mac, > > nat->logical_port); > > } else { > > - smap_add_format(&lr_nats, key, "%-17.13s%s", > > + smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%s", > > nat->external_port_range, > > + nat->external_port_hash, > > nat->logical_ip); > > } > > free(key); > > @@ -4522,9 +4577,9 @@ nbctl_lr_nat_list(struct ctl_context *ctx) > > const struct smap_node **nodes = smap_sort(&lr_nats); > > if (nodes) { > > 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"); > > + "%-17.13s%-19.15s%-17.13s%-22.18s%-22.18s%-21.17s%s\n", > > + "TYPE", "EXTERNAL_IP", "EXTERNAL_PORT", > "EXTERNAL_PORT_HASH", > > + "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", > > @@ -6538,8 +6593,9 @@ static const struct ctl_command_syntax > nbctl_commands[] = { > > /* NAT commands. */ > > { "lr-nat-add", 4, 7, > > Should this be > > { "lr-nat-add", 4, 8, > > > "ROUTER TYPE EXTERNAL_IP LOGICAL_IP" > > - "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE]", NULL, > > - nbctl_lr_nat_add, NULL, "--may-exist,--stateless,--portrange", RW > }, > > + "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE > EXTERNAL_PORT_HASH]", > > Is EXTERNAL_PORT_HASH mandatory now when specifying EXTERNAL_PORT_RANGE? > Perhaps it could be "[EXTERNAL_PORT_RANGE [EXTERNAL_PORT_HASH]]"? Is > there a default value? > > Thanks Numan > > + 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 }, > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index 092322a..9b8e070 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "5.27.0", - "cksum": "3507518247 26773", + "version": "5.28.0", + "cksum": "2621169942 26831", "tables": { "NB_Global": { "columns": { @@ -398,6 +398,7 @@ "external_mac": {"type": {"key": "string", "min": 0, "max": 1}}, "external_port_range": {"type": "string"}, + "external_port_hash": {"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 0bfe626..142d934 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -2729,6 +2729,21 @@ </column> + <column name="external_port_hash"> + <p> + Hashing algorithm to hash a packet to specified port range + </p> + + <p> + Applicable only if port range is also specified. + </p> + + <p> + Takes one of the 2 values "Random" and "Hash" + </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 baf7a87..6ce1ecf 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -476,15 +476,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 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 +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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 @@ -512,28 +512,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 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 +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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 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 +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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], @@ -584,26 +584,26 @@ 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 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 +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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 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 +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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]) @@ -613,10 +613,10 @@ 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]) AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 lp0 00:00:00:04:05:06 1-3000]) AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 lp0 1-3000], [1], [], -[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. +[ovn-nbctl: invalid port range lp0. ]) AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 00:00:00:04:05:06 1-3000], [1], [], -[ovn-nbctl: lr-nat-add with logical_port must also specify external_mac. +[ovn-nbctl: invalid port range 00:00:00:04:05:06. ]) AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 0], [1], [], @@ -674,12 +674,46 @@ AT_CHECK([ovn-nbctl show lr0 | grep -C2 'external port(s): "1"' | uuidfilt], [0] ]) AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl -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 +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH 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]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 snat 40.0.0.3 192.168.1.6 21-65535 hash]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.4 192.168.1.7 1-3000 random]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat 40.0.0.5 192.168.1.10 1 hash]) +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 lp0 00:00:00:04:05:06 1-3000]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 lp0 1-3000 hash], [1], [], +[ovn-nbctl: invalid mac address 1-3000. +]) +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.6 192.168.1.9 00:00:00:04:05:06 1-3000 hash], [1], [], +[ovn-nbctl: 00:00:00:04:05:06: port name not found +]) + +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 0 random], [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-300 abcd], [1], [], +[ovn-nbctl: invalid port hash abcd. +]) + +AT_CHECK([ovn-nbctl --portrange lr-nat-add lr0 dnat_and_snat 40.0.0.7 192.168.1.10 abcd], [1], [], +[ovn-nbctl: invalid port range abcd. +]) + +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl +TYPE EXTERNAL_IP EXTERNAL_PORT EXTERNAL_PORT_HASH LOGICAL_IP EXTERNAL_MAC LOGICAL_PORT +dnat 40.0.0.4 1-3000 random 192.168.1.7 +dnat 40.0.0.5 1 hash 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 hash 192.168.1.6 ]) AT_CHECK([ovn-nbctl lr-nat-del lr0]) diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index c54e639..3a1b158 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -1088,6 +1088,11 @@ print_lr(const struct nbrec_logical_router *lr, struct ds *s) if (nat->external_port_range[0]) { ds_put_cstr(s, " external port(s): "); ds_put_format(s, "\"%s\"\n", nat->external_port_range); + + if (nat->external_port_hash[0]) { + ds_put_cstr(s, " external port_hash: "); + ds_put_format(s, "\"%s\"\n", nat->external_port_hash); + } } ds_put_cstr(s, " logical ip: "); ds_put_format(s, "\"%s\"\n", nat->logical_ip); @@ -4129,6 +4134,16 @@ out: free(nexthop); } +static inline bool +is_valid_port_hash(const char *port_hash) +{ + if (!strcmp(port_hash, "hash") || !strcmp(port_hash, "random")) { + return true; + } + + return false; +} + static bool is_valid_port_range(const char *port_range) { @@ -4246,6 +4261,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) const char *logical_port = NULL; const char *external_mac = NULL; const char *port_range = NULL; + const char *port_hash = NULL; if (ctx->argc == 6) { if (!is_portrange) { @@ -4259,19 +4275,46 @@ nbctl_lr_nat_add(struct ctl_context *ctx) goto cleanup; } - } 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\"."); - goto cleanup; - } + } else if (ctx->argc == 7) { + if (is_portrange) { + port_range = ctx->argv[5]; + if (!is_valid_port_range(port_range)) { + ctl_error(ctx, "invalid port range %s.", port_range); + goto cleanup; + } - if (ctx->argc == 7 && is_portrange) { - ctl_error(ctx, "lr-nat-add with logical_port " - "must also specify external_mac."); - goto cleanup; + /* No need to validate the hash value, NBDB set will fail, + * If value is not valid */ + port_hash = ctx->argv[6]; + if (!is_valid_port_hash(port_hash)) { + ctl_error(ctx, "invalid port hash %s.", port_hash); + goto cleanup; + } + } else { + if (strcmp(nat_type, "dnat_and_snat")) { + ctl_error(ctx, "logical_port and external_mac are only valid " + "when type is \"dnat_and_snat\"."); + goto cleanup; + } + + logical_port = ctx->argv[5]; + const struct nbrec_logical_switch_port *lsp; + error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); + if (error) { + ctx->error = error; + goto cleanup; + } + + external_mac = ctx->argv[6]; + struct eth_addr ea; + if (!eth_addr_from_string(external_mac, &ea)) { + ctl_error(ctx, "invalid mac address %s.", external_mac); + goto cleanup; + } } + } else if (ctx->argc >= 8) { + logical_port = ctx->argv[5]; const struct nbrec_logical_switch_port *lsp; error = lsp_by_name_or_uuid(ctx, logical_port, true, &lsp); @@ -4286,11 +4329,17 @@ nbctl_lr_nat_add(struct ctl_context *ctx) ctl_error(ctx, "invalid mac address %s.", external_mac); goto cleanup; } - - if (ctx->argc > 7) { - port_range = ctx->argv[7]; - if (!is_valid_port_range(port_range)) { - ctl_error(ctx, "invalid port range %s.", port_range); + port_range = ctx->argv[7]; + if (!is_valid_port_range(port_range)) { + ctl_error(ctx, "invalid port range %s.", port_range); + goto cleanup; + } + if (ctx->argc > 8) { + /* No need to validate the hash value, NBDB set will fail, + * If value is not valid */ + port_hash = ctx->argv[8]; + if (!is_valid_port_hash(port_hash)) { + ctl_error(ctx, "invalid port hash %s.", port_hash); goto cleanup; } } @@ -4299,6 +4348,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx) port_range = NULL; logical_port = NULL; external_mac = NULL; + port_hash = NULL; } bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL; @@ -4387,6 +4437,9 @@ nbctl_lr_nat_add(struct ctl_context *ctx) if (port_range) { nbrec_nat_set_external_port_range(nat, port_range); + if (port_hash) { + nbrec_nat_set_external_port_hash(nat, port_hash); + } } smap_add(&nat_options, "stateless", stateless ? "true":"false"); @@ -4507,13 +4560,15 @@ 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, "%-17.13s%-22.18s%-21.17s%s", - nat->external_port_range, + smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%-" + "22.18s%-21.17s%s",nat->external_port_range, + nat->external_port_hash, nat->logical_ip, nat->external_mac, nat->logical_port); } else { - smap_add_format(&lr_nats, key, "%-17.13s%s", + smap_add_format(&lr_nats, key, "%-17.13s%-22.18s%s", nat->external_port_range, + nat->external_port_hash, nat->logical_ip); } free(key); @@ -4522,9 +4577,9 @@ nbctl_lr_nat_list(struct ctl_context *ctx) const struct smap_node **nodes = smap_sort(&lr_nats); if (nodes) { 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"); + "%-17.13s%-19.15s%-17.13s%-22.18s%-22.18s%-21.17s%s\n", + "TYPE", "EXTERNAL_IP", "EXTERNAL_PORT", "EXTERNAL_PORT_HASH", + "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", @@ -6538,8 +6593,9 @@ static const struct ctl_command_syntax nbctl_commands[] = { /* NAT commands. */ { "lr-nat-add", 4, 7, "ROUTER TYPE EXTERNAL_IP LOGICAL_IP" - "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE]", NULL, - nbctl_lr_nat_add, NULL, "--may-exist,--stateless,--portrange", RW }, + "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE EXTERNAL_PORT_HASH]", + 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 },