[ovs-dev] [PATCH v5 1/2 ovn] External IP based NAT: Add Columns and CLI
Numan Siddique
numans at ovn.org
Tue Sep 1 08:39:16 UTC 2020
Hi Ankur,
Sorry for the late review. This patch LGTM except for one comment. I have
few comments in patch 2.
Thanks
Numan
On Thu, Aug 20, 2020 at 8:05 AM Ankur Sharma <svc.mail.git at nutanix.com>
wrote:
> From: Ankur Sharma <ankur.sharma at nutanix.com>
>
> This patch adds following columns to NAT table.
>
> a. allowed_ext_ips:
> Represents Address Set of External IPs for which
> a NAT rule is applicable.
>
> b. exempted_ext_ips:
> Represents Address Set of External IPs for which
> a NAT rule is NOT applicable.
>
> Additionally, patch adds nbctl cli to set these column values.
> ovn-nbctl [--is-exempted] lr-nat-update-ext-ip
>
> Signed-off-by: Ankur Sharma <ankur.sharma at nutanix.com>
> ---
> ovn-nb.ovsschema | 14 +++++-
> ovn-nb.xml | 48 +++++++++++++++++++++
> tests/ovn-nbctl.at | 44 ++++++++++++++++++-
> utilities/ovn-nbctl.c | 116
> +++++++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 218 insertions(+), 4 deletions(-)
>
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 0c939b7..c2f64b5 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> - "version": "5.25.0",
> - "cksum": "1354137211 26116",
> + "version": "5.25.1",
>
The version should be "5.26.0"
Thanks
Numan
> + "cksum": "1930791063 26575",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -403,6 +403,16 @@
> "snat",
>
> "dnat_and_snat"
> ]]}}},
> + "allowed_ext_ips": {"type": {
> + "key": {"type": "uuid", "refTable": "Address_Set",
> + "refType": "strong"},
> + "min": 0,
> + "max": 1}},
> + "exempted_ext_ips": {"type": {
> + "key": {"type": "uuid", "refTable": "Address_Set",
> + "refType": "strong"},
> + "min": 0,
> + "max": 1}},
> "options": {"type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}},
> "external_ids": {
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 9f3621d..ac452a4 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -2716,6 +2716,54 @@
> </p>
> </column>
>
> + <column name="allowed_ext_ips">
> + It represents Address Set of external ips that NAT rule is
> applicable to.
> + For SNAT type NAT rules, this refers to destination addresses.
> + For DNAT type NAT rules, this refers to source addresses.
> +
> + <p>
> + This configuration overrides the default NAT behavior of applying
> a
> + rule solely based on internal IP. Without this configuration, NAT
> + happens without considering the external IP (i.e dest/source for
> + snat/dnat type rule). With this configuration NAT rule is applied
> + ONLY if external ip is in the input Address Set.
> + </p>
> + </column>
> +
> + <column name="exempted_ext_ips">
> + It represents Address Set of external ips that NAT rule is NOT
> + applicable to.
> + For SNAT type NAT rules, this refers to destination addresses.
> + For DNAT type NAT rules, this refers to source addresses.
> +
> + <p>
> + This configuration overrides the default NAT behavior of applying
> a
> + rule solely based on internal IP. Without this configuration, NAT
> + happens without considering the external IP (i.e dest/source for
> + snat/dnat type rule). With this configuration NAT rule is NOT
> applied
> + if external ip is in the input Address Set.
> + </p>
> +
> + <p>
> + If there are NAT rules in a logical router with overlapping IP
> prefixes
> + (including /32), then usage of <var>exempted_ext_ips</var> should
> be
> + avoided in following scenario.
> + a. SNAT rule (let us say RULE1) with logical_ip PREFIX/MASK
> + (let us say 50.0.0.0/24).
> + b. SNAT rule (let us say RULE2) with logical_ip PREFIX/MASK+1
> + (let us say 50.0.0.0/25).
> + c. Now, if exempted_ext_ips is associated with RULE2, then a
> logical
> + ip which matches both 50.0.0.0/24 and 50.0.0.0/25 may get the
> RULE2
> + applied to it instead of RULE1.
> + </p>
> +
> + <p>
> + <var>allowed_ext_ips</var> and <var>exempted_ext_ips</var> are
> mutually
> + exclusive to each other. If both Address Sets are set for a rule,
> + then the NAT rule is not considered.
> + </p>
> + </column>
> +
> <column name="options" key="stateless">
> Indicates if a dnat_and_snat rule should lead to connection
> tracking state or not.
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 619051d..baf7a87 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -685,7 +685,49 @@ snat 40.0.0.3 21-65535
> 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])])
> +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])
> +
> +AT_CHECK([ovn-nbctl lr-nat-del lr0])
> +
> +ovn-nbctl create Address_Set name=allowed_range addresses=\"1.1.1.1\"
> +ovn-nbctl create Address_Set name=disallowed_range addresses=\"2.2.2.2\"
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 40.0.0.3 192.168.1.6])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 snat 192.168.1.6
> allowed_range])
> +AT_CHECK([ovn-nbctl --is-exempted lr-nat-update-ext-ip lr0 snat
> 192.168.1.6 disallowed_range])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 snat 192.168.1.6
> allowed_range_tmp], [1], [],
> +[ovn-nbctl: allowed_range_tmp: Address Set name not found
> +])
> +AT_CHECK([ovn-nbctl --is-exempted lr-nat-update-ext-ip lr0 snat
> 192.168.1.6 disallowed_range_tmp], [1], [],
> +[ovn-nbctl: disallowed_range_tmp: Address Set name not found
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 40.0.0.4 192.168.1.7])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 dnat 40.0.0.4 allowed_range])
> +AT_CHECK([ovn-nbctl --is-exempted lr-nat-update-ext-ip lr0 dnat 40.0.0.4
> disallowed_range])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 dnat 40.0.0.4
> allowed_range_tmp], [1], [],
> +[ovn-nbctl: allowed_range_tmp: Address Set name not found
> +])
> +AT_CHECK([ovn-nbctl --is-exempted lr-nat-update-ext-ip lr0 dnat 40.0.0.4
> disallowed_range_tmp], [1], [],
> +[ovn-nbctl: disallowed_range_tmp: Address Set name not found
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 40.0.0.5 192.168.1.8])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 dnat_and_snat 40.0.0.5
> allowed_range])
> +AT_CHECK([ovn-nbctl --is-exempted lr-nat-update-ext-ip lr0 dnat 40.0.0.4
> disallowed_range])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 dnat 40.0.0.4
> allowed_range_tmp], [1], [],
> +[ovn-nbctl: allowed_range_tmp: Address Set name not found
> +])
> +AT_CHECK([ovn-nbctl --is-exempted lr-nat-update-ext-ip lr0 dnat 40.0.0.4
> disallowed_range_tmp], [1], [],
> +[ovn-nbctl: disallowed_range_tmp: Address Set name not found
> +])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 snat 192.168.1.6], [1], [],
> +[ovn-nbctl: 'lr-nat-update-ext-ip' command requires at least 4 arguments
> +])
> +AT_CHECK([ovn-nbctl lr-nat-update-ext-ip lr0 snat 192.168.16
> allowed_range], [1], [],
> +[ovn-nbctl: 192.168.16: Invalid IP address or CIDR
> +])
> +
> +AT_CHECK([ovn-nbctl lr-nat-del lr0])])
>
> dnl ---------------------------------------------------------------------
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index d7bb4b4..809044a 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -839,6 +839,46 @@ lr_by_name_or_uuid(struct ctl_context *ctx, const
> char *id,
> return NULL;
> }
>
> +/* Find an Address Set given its id. */
> +static char * OVS_WARN_UNUSED_RESULT
> +address_set_by_name_or_uuid(struct ctl_context *ctx,
> + const char *id, bool must_exist,
> + const struct nbrec_address_set **addr_set_p)
> +{
> + const struct nbrec_address_set *addr_set = NULL;
> + bool is_uuid = false;
> + struct uuid addr_set_uuid;
> +
> + *addr_set_p = NULL;
> + if (uuid_from_string(&addr_set_uuid, id)) {
> + is_uuid = true;
> + addr_set = nbrec_address_set_get_for_uuid(ctx->idl,
> &addr_set_uuid);
> + }
> +
> + if (!addr_set) {
> + const struct nbrec_address_set *iter;
> +
> + NBREC_ADDRESS_SET_FOR_EACH(iter, ctx->idl) {
> + if (strcmp(iter->name, id)) {
> + continue;
> + }
> + if (addr_set) {
> + return xasprintf("Multiple Address Sets named '%s'. "
> + "Use a UUID.", id);
> + }
> + addr_set = iter;
> + }
> + }
> +
> + if (!addr_set && must_exist) {
> + return xasprintf("%s: Address Set %s not found",
> + id, is_uuid ? "UUID" : "name");
> + }
> +
> + *addr_set_p = addr_set;
> + return NULL;
> +}
> +
> static char * OVS_WARN_UNUSED_RESULT
> ls_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool
> must_exist,
> const struct nbrec_logical_switch **ls_p)
> @@ -4486,6 +4526,79 @@ nbctl_lr_nat_list(struct ctl_context *ctx)
> smap_destroy(&lr_nats);
> }
>
> +static void
> +nbctl_lr_nat_set_ext_ips(struct ctl_context *ctx)
> +{
> + const struct nbrec_logical_router *lr = NULL;
> + const struct nbrec_address_set *addr_set = NULL;
> + bool is_exempted = shash_find(&ctx->options, "--is-exempted");
> + bool nat_found = false;
> +
> + if (ctx->argc < 5) {
> + ctl_error(ctx, "Incomplete input, Required arguments are: "
> + "ROUTER TYPE IP ADDRESS_SET");
> + return;
> + }
> +
> + char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
> + if (error) {
> + ctx->error = error;
> + return;
> + }
> +
> + const char *nat_type = ctx->argv[2];
> + if (strcmp(nat_type, "dnat") && strcmp(nat_type, "snat")
> + && strcmp(nat_type, "dnat_and_snat")) {
> + ctl_error(ctx, "%s: type must be one of \"dnat\", \"snat\" and "
> + "\"dnat_and_snat\".", nat_type);
> + return;
> + }
> +
> + error = address_set_by_name_or_uuid(ctx, ctx->argv[4], true,
> &addr_set);
> + if (error) {
> + ctx->error = error;
> + return;
> + }
> +
> + char *nat_ip = normalize_prefix_str(ctx->argv[3]);
> + if (!nat_ip) {
> + ctl_error(ctx, "%s: Invalid IP address or CIDR", ctx->argv[3]);
> + return;
> + }
> +
> + int is_snat = !strcmp("snat", nat_type);
> +
> + /* Update the matching NAT. */
> + for (size_t i = 0; i < lr->n_nat; i++) {
> + struct nbrec_nat *nat = lr->nat[i];
> + char *old_ip = normalize_prefix_str(is_snat
> + ? nat->logical_ip
> + : nat->external_ip);
> +
> + if (!old_ip) {
> + continue;
> + }
> +
> + if (!strcmp(nat_type, nat->type) && !strcmp(nat_ip, old_ip)) {
> + nat_found = true;
> + nbrec_logical_router_verify_nat(lr);
> + if (is_exempted) {
> + nbrec_nat_set_exempted_ext_ips(nat, addr_set);
> + } else {
> + nbrec_nat_set_allowed_ext_ips(nat, addr_set);
> + }
> + return;
> + }
> + }
> +
> + if (!nat_found) {
> + ctl_error(ctx, "%s: Could not locate nat rule for: %s.",
> + nat_type, nat_ip);
> + }
> +
> + free(nat_ip);
> +}
> +
>
> static char * OVS_WARN_UNUSED_RESULT
> lrp_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool
> must_exist,
> @@ -6397,7 +6510,8 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
> { "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 },
> -
> + { "lr-nat-update-ext-ip", 4, 4, "ROUTER TYPE IP ADDRESS_SET", NULL,
> + nbctl_lr_nat_set_ext_ips, NULL, "--is-exempted", RW},
> /* load balancer commands. */
> { "lb-add", 3, 4, "LB VIP[:PORT] IP[:PORT]... [PROTOCOL]", NULL,
> nbctl_lb_add, NULL, "--may-exist,--add-duplicate", RW },
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
More information about the dev
mailing list