[ovs-dev] [PATCH v6 2/4 ovn] OVN: Vlan backed DVR N-S, redirect-type option

Numan Siddique nusiddiq at redhat.com
Sat Aug 24 18:53:06 UTC 2019


On Sat, Aug 17, 2019 at 6:07 AM Ankur Sharma <ankur.sharma at nutanix.com>
wrote:

> Background:
> With c0974331b7a19a87ab8f1f2cec8fbe366af92fa2, we have added
> support for E-W workflow for vlan backed DVRs.
>
> This series enables N-S workflow for vlan backed DVRs.
>
> Key difference between E-W and N-S traffic flow is that
> N-S flow requires a gateway chassis. A gateway chassis
> will be respondible for following:
> a. Doing Network Address Translation (NAT).
> b. Becoming entry and exit point for North->South
>    and South->North traffic respectively.
>
> OVN by default always uses overlay encapsulation to redirect
> the packet to gateway chassis. This series will enable
> the redirection to gateway chassis in the absence of encapsulation.
>
> This patch:
> a. Add a new key-value in options of a router port.
> b. This new config key will be used by ovn-controller
>    to determine if a redirected packet will go out of
>    tunnel port or localnet port.
> c. key is "redirect-type" and it takes "overlay" and
>    "vlan" as values.
>

Thanks for the patch.

I would suggest change the value "vlan" to "localnet" or "bridged" as  we
can also have flat provider networks.



> d. Added ovn-nbctl command to set and get redirect-type
>    option on a router port.
> e. This new configuration is added because vlan or overlay
>    based forwarding is considered to be a logical switch property,
>    hence for a router configuration has to be done at the router port
>    level.
> f. Restored the function ovsdb_datum_to_smap, which helps in ensuring
>    that we do not overwrite existing options, while adding a new
>    key-value pair to it. This function exists in 2.8.5, i am not
>    able to figure out so far, which release/why it was removed.
>    I do not see a harm in adding it back.
>

This is odd. The "ovs" subtree is used for the compilation of ovn and is
treated as
read-only. You first need to submit a patch to the ovs repo. Once it is
merged,
we need to update the ovs subtree. Having said that, I don't think we
really need
the function ovsdb_datum_to_smap() function.


Signed-off-by: Ankur Sharma <ankur.sharma at nutanix.com>

---
>  northd/ovn-northd.c   |  6 +++++
>  ovn-nb.xml            | 43 ++++++++++++++++++++++++++++++++
>  ovs/lib/ovsdb-data.c  | 11 +++++++++
>  ovs/lib/ovsdb-data.h  |  2 ++
>  tests/ovn-nbctl.at    | 25 +++++++++++++++++++
>  tests/ovn-northd.at   | 31 +++++++++++++++++++++++
>  utilities/ovn-nbctl.c | 68
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 186 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index e861344..89ca8df 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2445,6 +2445,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>          if (op->derived) {
>              const char *redirect_chassis = smap_get(&op->nbrp->options,
>                                                      "redirect-chassis");
> +            const char *redirect_type = smap_get(&op->nbrp->options,
> +                                                 "redirect-type");
> +
>              int n_gw_options_set = 0;
>              if (op->nbrp->ha_chassis_group) {
>                  n_gw_options_set++;
> @@ -2536,6 +2539,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>                  sbrec_port_binding_set_gateway_chassis(op->sb, NULL, 0);
>              }
>              smap_add(&new, "distributed-port", op->nbrp->name);
> +            if (redirect_type) {
> +                smap_add(&new, "redirect-type", redirect_type);
> +            }
>          } else {
>              if (op->peer) {
>                  smap_add(&new, "peer", op->peer->key);
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index e166190..8bb6221 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1948,6 +1948,49 @@
>            issues.
>          </p>
>        </column>
> +
> +      <column name="options" key="redirect-type">
> +        <p>
> +          This options dictates if a packet redirected to
> +          <code>gateway chassis</code> will be overlay encapsulated
> +          or go as a regular vlan packet.
> +        </p>
> +
> +        <p>
> +          Option takes following values
> +        </p>
> +
> +        <ul>
> +          <li>
> +            OVERLAY
> +          </li>
> +
> +          <li>
> +            VLAN
> +          </li>
> +        </ul>
> +
> +        <p>
> +          OVERLAY option will ensure that redirected packet goes out as
> +          encapsulation via the tunnel port.
> +        </p>
> +
> +        <p>
> +          VLAN option will ensure that redirected packet goes out as vlan
> +          tagged via the localnet port.
> +        </p>
> +
> +        <p>
> +          OVERLAY is the default redirection type.
> +        </p>
> +
> +        <p>
> +          Option is applicable only to gateway chassis attached logical
> +          router ports.
> +        </p>
> +
> +      </column>
> +
>      </group>
>
>      <group title="Attachment">
> diff --git a/ovs/lib/ovsdb-data.c b/ovs/lib/ovsdb-data.c
> index b0fb20d..c7fcb8a 100644
> --- a/ovs/lib/ovsdb-data.c
> +++ b/ovs/lib/ovsdb-data.c
> @@ -1691,6 +1691,17 @@ ovsdb_datum_from_smap(struct ovsdb_datum *datum,
> const struct smap *smap)
>      ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
>  }
>
> +/* Initializes smap from a string-to-string datum map. */
> +void
> +ovsdb_datum_to_smap(struct smap *smap, const struct ovsdb_datum *datum)
> +{
> +    size_t i = 0;
> +    for (; i < datum->n; i++) {
> +        smap_add(smap, datum->keys[i].string,  datum->values[i].string);
> +    }
> +    ovs_assert(i == smap_count(smap));
> +}
> +
>  struct ovsdb_error * OVS_WARN_UNUSED_RESULT
>  ovsdb_datum_convert(struct ovsdb_datum *dst,
>                      const struct ovsdb_type *dst_type,
> diff --git a/ovs/lib/ovsdb-data.h b/ovs/lib/ovsdb-data.h
> index c5a80ee..bf2cd8a 100644
> --- a/ovs/lib/ovsdb-data.h
> +++ b/ovs/lib/ovsdb-data.h
> @@ -191,6 +191,8 @@ void ovsdb_datum_to_bare(const struct ovsdb_datum *,
>                           const struct ovsdb_type *, struct ds *);
>
>  void ovsdb_datum_from_smap(struct ovsdb_datum *, const struct smap *);
> +void ovsdb_datum_to_smap(struct smap *smap, const struct ovsdb_datum
> *datum);
> +
>
>  struct ovsdb_error *ovsdb_datum_convert(struct ovsdb_datum *dst,
>                                          const struct ovsdb_type *dst_type,
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index cf06966..39b0bff 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -1220,6 +1220,31 @@ lrp0-chassis1     1
>
>  dnl ---------------------------------------------------------------------
>
> +OVN_NBCTL_TEST([ovn_nbctl_redirect_type], [logical router port redirect
> type], [
> +AT_CHECK([ovn-nbctl lr-add lr0])
> +AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02:03 192.168.1.1/24])
> +AT_CHECK([ovn-nbctl lrp-get-redirect-type lrp0], [0], [dnl
> +overlay
> +])
> +AT_CHECK([ovn-nbctl lrp-set-redirect-type lp0 vlan], [1], [],
> +[ovn-nbctl: lp0: port name not found
> +])
> +AT_CHECK([ovn-nbctl lrp-set-redirect-type lrp0 vlan], [0], [])
> +AT_CHECK([ovn-nbctl lrp-get-redirect-type lrp0], [0], [dnl
> +vlan
> +])
> +AT_CHECK([ovn-nbctl lrp-set-redirect-type lrp0 overlay], [0], [])
> +AT_CHECK([ovn-nbctl lrp-get-redirect-type lrp0], [0], [dnl
> +overlay
> +])
> +AT_CHECK([ovn-nbctl lrp-set-redirect-type lrp0 abcd], [1], [],
> +[ovn-nbctl: Invalid redirect type: abcd
> +])
> +
> +])
> +
> +dnl ---------------------------------------------------------------------
> +
>  OVN_NBCTL_TEST([ovn_nbctl_lrp_enable], [logical router port enable and
> disable], [
>  AT_CHECK([ovn-nbctl lr-add lr0])
>  AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02:03 192.168.1.1/24])
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 0dea04e..8718130 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -936,3 +936,34 @@ OVS_WAIT_UNTIL([
>      test 0 = $?])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- check Redirect Chassis propagation from NB to SB])
> +AT_SKIP_IF([test $HAVE_PYTHON = no])
> +ovn_start
> +
> +ovn-sbctl chassis-add gw1 geneve 127.0.0.1
> +
> +ovn-nbctl lr-add R1
> +ovn-nbctl lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24
> +
> +ovn-nbctl ls-add S1
> +ovn-nbctl lsp-add S1 S1-R1
> +ovn-nbctl lsp-set-type S1-R1 router
> +ovn-nbctl lsp-set-addresses S1-R1 router
> +ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1
> +
> +ovn-nbctl lrp-set-gateway-chassis R1-S1 gw1
> +
> +uuid=`ovn-sbctl --columns=_uuid --bare find Port_Binding
> logical_port=cr-R1-S1`
> +echo "CR-LRP UUID is: " $uuid
> +
> +ovn-nbctl lrp-set-redirect-type R1-S1 vlan
> +AT_CHECK([ovn-sbctl get Port_Binding ${uuid} options:redirect-type], [0],
> [vlan
> +])
> +
> +ovn-nbctl lrp-set-redirect-type R1-S1 overlay
> +AT_CHECK([ovn-sbctl get Port_Binding ${uuid} options:redirect-type], [0],
> [overlay
> +])
> +
> +
> +AT_CLEANUP
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index b8b440e..5444fc7 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -667,6 +667,14 @@ Logical router port commands:\n\
>                              ('enabled' or 'disabled')\n\
>    lrp-get-enabled PORT      get administrative state PORT\n\
>                              ('enabled' or 'disabled')\n\
> +  lrp-set-redirect-type PORT TYPE\n\
> +                            set whether redirected packet to gateway
> chassis\n\
> +                            of PORT will be encapsulated or not\n\
> +                            ('overlay' or 'vlan')\n\
> +  lrp-get-redirect-type PORT\n\
> +                            get whether redirected packet to gateway
> chassis\n\
> +                            of PORT will be encapsulated or not\n\
> +                            ('overlay' or 'vlan')\n\
>  \n\
>  Route commands:\n\
>    [--policy=POLICY] lr-route-add ROUTER PREFIX NEXTHOP [PORT]\n\
> @@ -4597,6 +4605,62 @@ nbctl_lrp_get_enabled(struct ctl_context *ctx)
>                    !lrp->enabled ||
>                    *lrp->enabled ? "enabled" : "disabled");
>  }
> +
> +/* Set the logical router port redirect type. */
> +static void
> +nbctl_lrp_set_redirect_type(struct ctl_context *ctx)
> +{
> +    const char *id = ctx->argv[1];
> +    const char *type = ctx->argv[2];
> +    const struct nbrec_logical_router_port *lrp = NULL;
> +    struct smap lrp_options;
> +
> +    char *error = lrp_by_name_or_uuid(ctx, id, true, &lrp);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    if (strcasecmp(type, "vlan") && strcasecmp(type, "overlay")) {
> +        error = xasprintf("Invalid redirect type: %s", type);
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    smap_init(&lrp_options);
> +
> +    ovsdb_datum_to_smap(&lrp_options,
> +                        nbrec_logical_router_port_get_options
> +                        (lrp, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING));
> +
>

I don't think you need to use  ovsdb_datum_to_smap() to add/append the
option "redirect-type"
to the lrp options.

You can do something like

struct smap lrp_options;
smap_clone(&lrp_options, &op->nbsp->options);
smap_replace(&lrp_options, "redirect-type", type);

Thanks
Numan


+    if (smap_get(&lrp_options, "redirect-type")) {
> +        smap_replace(&lrp_options, "redirect-type", type);
> +    } else {
> +        smap_add(&lrp_options, "redirect-type", type);
> +    }
> +
> +    nbrec_logical_router_port_set_options(lrp, &lrp_options);
> +
> +    smap_destroy(&lrp_options);
> +}
> +
> +static void
> +nbctl_lrp_get_redirect_type(struct ctl_context *ctx)
> +{
> +    const char *id = ctx->argv[1];
> +    const struct nbrec_logical_router_port *lrp = NULL;
> +
> +    char *error = lrp_by_name_or_uuid(ctx, id, true, &lrp);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    const char *redirect_type = smap_get(&lrp->options, "redirect-type");
> +    ds_put_format(&ctx->output, "%s\n",
> +                  !redirect_type ? "overlay": redirect_type);
> +}
> +
>
>  struct ipv4_route {
>      int priority;
> @@ -5604,6 +5668,10 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
>        NULL, "", RW },
>      { "lrp-get-enabled", 1, 1, "PORT", NULL, nbctl_lrp_get_enabled,
>        NULL, "", RO },
> +    { "lrp-set-redirect-type", 2, 2, "PORT TYPE", NULL,
> +      nbctl_lrp_set_redirect_type, NULL, "", RW },
> +    { "lrp-get-redirect-type", 1, 1, "PORT", NULL,
> nbctl_lrp_get_redirect_type,
> +      NULL, "", RO },
>
>      /* logical router route commands. */
>      { "lr-route-add", 3, 4, "ROUTER PREFIX NEXTHOP [PORT]", NULL,
> --
> 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