[ovs-dev] [PATCH ovn v8 1/5] ic: maintain route origin - connected/static
Numan Siddique
numans at ovn.org
Wed Nov 17 02:13:09 UTC 2021
On Sat, Nov 13, 2021 at 4:44 AM Vladislav Odintsov <odivlad at gmail.com> wrote:
>
> This commits adds ability to save route's origin while IC learning.
> Directly connected routes are saved in IC SB DB with "connected"
> origin column value.
> Static routes have "static" value in origin column.
>
> This logic would be used in next patch to compute priority
> for lr_in_ip_routing stage lflows.
>
> Signed-off-by: Vladislav Odintsov <odivlad at gmail.com>
Hi Vladislav,
The patch LGTM.
I think you missed documenting the option "origin" for the NB
Logical_Router_Static_Route
option. Was this intentional? From what I understand, ovn-ic will
set this option.
So for a setup where ovn-ic is not used, can a user/CMS configure this option ?
I think it would be good to document this in ovn-nb.xml.
With the documentation added:
Acked-by: Numan Siddique <numans at ovn.org>
Numan
> ---
> ic/ovn-ic.c | 34 +++++++++++++++++++--------
> lib/ovn-util.h | 3 +++
> ovn-ic-sb.ovsschema | 7 ++++--
> ovn-ic-sb.xml | 10 ++++++++
> tests/ovn-ic.at | 57 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 99 insertions(+), 12 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 303e93a4f..70abae108 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -854,6 +854,7 @@ struct ic_route_info {
> struct in6_addr prefix;
> unsigned int plen;
> struct in6_addr nexthop;
> + const char *origin;
>
> /* Either nb_route or nb_lrp is set and the other one must be NULL.
> * - For a route that is learned from IC-SB, or a static route that is
> @@ -867,22 +868,25 @@ struct ic_route_info {
>
> static uint32_t
> ic_route_hash(const struct in6_addr *prefix, unsigned int plen,
> - const struct in6_addr *nexthop)
> + const struct in6_addr *nexthop, const char *origin)
> {
> uint32_t basis = hash_bytes(prefix, sizeof *prefix, (uint32_t)plen);
> + basis = hash_string(origin, basis);
> return hash_bytes(nexthop, sizeof *nexthop, basis);
> }
>
> static struct ic_route_info *
> ic_route_find(struct hmap *routes, const struct in6_addr *prefix,
> - unsigned int plen, const struct in6_addr *nexthop)
> + unsigned int plen, const struct in6_addr *nexthop,
> + const char *origin)
> {
> struct ic_route_info *r;
> - uint32_t hash = ic_route_hash(prefix, plen, nexthop);
> + uint32_t hash = ic_route_hash(prefix, plen, nexthop, origin);
> HMAP_FOR_EACH_WITH_HASH (r, node, hash, routes) {
> if (ipv6_addr_equals(&r->prefix, prefix) &&
> r->plen == plen &&
> - ipv6_addr_equals(&r->nexthop, nexthop)) {
> + ipv6_addr_equals(&r->nexthop, nexthop) &&
> + !strcmp(r->origin, origin)) {
> return r;
> }
> }
> @@ -926,13 +930,15 @@ add_to_routes_learned(struct hmap *routes_learned,
> &prefix, &plen, &nexthop)) {
> return false;
> }
> + const char *origin = smap_get_def(&nb_route->options, "origin", "");
> struct ic_route_info *ic_route = xzalloc(sizeof *ic_route);
> ic_route->prefix = prefix;
> ic_route->plen = plen;
> ic_route->nexthop = nexthop;
> ic_route->nb_route = nb_route;
> + ic_route->origin = origin;
> hmap_insert(routes_learned, &ic_route->node,
> - ic_route_hash(&prefix, plen, &nexthop));
> + ic_route_hash(&prefix, plen, &nexthop, origin));
> return true;
> }
>
> @@ -1093,8 +1099,9 @@ add_to_routes_ad(struct hmap *routes_ad,
> ic_route->plen = plen;
> ic_route->nexthop = nexthop;
> ic_route->nb_route = nb_route;
> + ic_route->origin = ROUTE_ORIGIN_STATIC;
> hmap_insert(routes_ad, &ic_route->node,
> - ic_route_hash(&prefix, plen, &nexthop));
> + ic_route_hash(&prefix, plen, &nexthop, ROUTE_ORIGIN_STATIC));
> }
>
> static void
> @@ -1143,8 +1150,10 @@ add_network_to_routes_ad(struct hmap *routes_ad, const char *network,
> ic_route->plen = plen;
> ic_route->nexthop = nexthop;
> ic_route->nb_lrp = nb_lrp;
> + ic_route->origin = ROUTE_ORIGIN_CONNECTED;
> hmap_insert(routes_ad, &ic_route->node,
> - ic_route_hash(&prefix, plen, &nexthop));
> + ic_route_hash(&prefix, plen, &nexthop,
> + ROUTE_ORIGIN_CONNECTED));
> }
>
> static bool
> @@ -1206,7 +1215,8 @@ sync_learned_route(struct ic_context *ctx,
> continue;
> }
> struct ic_route_info *route_learned
> - = ic_route_find(&ic_lr->routes_learned, &prefix, plen, &nexthop);
> + = ic_route_find(&ic_lr->routes_learned, &prefix, plen, &nexthop,
> + isb_route->origin);
> if (route_learned) {
> /* Sync external-ids */
> struct uuid ext_id;
> @@ -1233,6 +1243,8 @@ sync_learned_route(struct ic_context *ctx,
> UUID_ARGS(&isb_route->header_.uuid));
> nbrec_logical_router_static_route_update_external_ids_setkey(
> nb_route, "ic-learned-route", uuid_s);
> + nbrec_logical_router_static_route_update_options_setkey(
> + nb_route, "origin", isb_route->origin);
> free(uuid_s);
> nbrec_logical_router_update_static_routes_addvalue(
> ic_lr->lr, nb_route);
> @@ -1297,8 +1309,9 @@ advertise_route(struct ic_context *ctx,
> icsbrec_route_delete(isb_route);
> continue;
> }
> - struct ic_route_info *route_adv =
> - ic_route_find(routes_ad, &prefix, plen, &nexthop);
> + struct ic_route_info *route_adv = ic_route_find(routes_ad, &prefix,
> + plen, &nexthop,
> + isb_route->origin);
> if (!route_adv) {
> /* Delete the extra route from IC-SB. */
> VLOG_DBG("Delete route %s -> %s from IC-SB, which is not found"
> @@ -1338,6 +1351,7 @@ advertise_route(struct ic_context *ctx,
> }
> icsbrec_route_set_ip_prefix(isb_route, prefix_s);
> icsbrec_route_set_nexthop(isb_route, nexthop_s);
> + icsbrec_route_set_origin(isb_route, route_adv->origin);
> free(prefix_s);
> free(nexthop_s);
>
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 2fa92e069..a923c3b65 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -25,6 +25,9 @@
> #define ovn_print_version(MIN_OFP, MAX_OFP) \
> ovs_print_version(MIN_OFP, MAX_OFP)
>
> +#define ROUTE_ORIGIN_CONNECTED "connected"
> +#define ROUTE_ORIGIN_STATIC "static"
> +
> struct nbrec_logical_router_port;
> struct sbrec_logical_flow;
> struct svec;
> diff --git a/ovn-ic-sb.ovsschema b/ovn-ic-sb.ovsschema
> index 5364b21b4..42ce85d7d 100644
> --- a/ovn-ic-sb.ovsschema
> +++ b/ovn-ic-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_IC_Southbound",
> - "version": "1.0.0",
> - "cksum": "108951192 6585",
> + "version": "1.1.0",
> + "cksum": "423535838 6733",
> "tables": {
> "IC_SB_Global": {
> "columns": {
> @@ -94,6 +94,9 @@
> "refTable": "Availability_Zone"}}},
> "ip_prefix": {"type": "string"},
> "nexthop": {"type": "string"},
> + "origin": {"type": {"key": {
> + "type": "string",
> + "enum": ["set", ["connected", "static"]]}}},
> "external_ids": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> diff --git a/ovn-ic-sb.xml b/ovn-ic-sb.xml
> index 3582cff47..d8338e4d3 100644
> --- a/ovn-ic-sb.xml
> +++ b/ovn-ic-sb.xml
> @@ -313,6 +313,16 @@
> <column name="nexthop">
> Nexthop IP address for this route.
> </column>
> +
> + <column name="origin">
> + Can be one of <code>connected</code> or <code>static</code>. Routes to
> + directly-connected subnets - LRP's CIDRs are inserted to OVN IC SB DB
> + with <code>connected</code> value in <ref column="origin"/>. Static
> + routes are inserted to OVN IC SB DB with <code>static</code> value.
> + Next when route is learned to another AZ NB DB by ovn-ic, route origin
> + is synced to <ref table="Logical_Router_Static_Route" column="options"
> + key="origin"/>.
> + </column>
> </group>
>
> <group title="Common Columns">
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index 9086974a3..7e8498b2f 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -423,6 +423,63 @@ AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 |
> # Test routes from lr12 didn't leak as learned to lr21
> AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr21], [0], [])
>
> +# cleanup
> +ovn-ic-nbctl --if-exists ts-del ts1
> +ovn_as az1 ovn-nbctl lr-del lr11
> +ovn_as az1 ovn-nbctl lr-del lr21
> +ovn_as az2 ovn-nbctl lr-del lr12
> +ovn_as az2 ovn-nbctl lr-del lr22
> +
> +# check routes origin advertisement and learning
> +
> +# setup topology with connected, static and source routes
> +ovn-ic-nbctl ts-add ts1
> +for i in 1 2; do
> + ovn_as az$i
> +
> + # Enable route learning at AZ level
> + ovn-nbctl set nb_global . options:ic-route-learn=true
> + # Enable route advertising at AZ level
> + ovn-nbctl set nb_global . options:ic-route-adv=true
> +
> + # Create LRP and connect to TS
> + ovn-nbctl lr-add lr$i
> + ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i 169.254.100.$i/24
> + ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
> + -- lsp-set-addresses lsp-ts1-lr$i router \
> + -- lsp-set-type lsp-ts1-lr$i router \
> + -- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
> +
> + ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i 192.168.$i.1/24
> +
> + # Create static routes
> + ovn-nbctl lr-route-add lr$i 10.11.$i.0/24 169.254.0.1
> +
> + # Create a src-ip route, which shouldn't be synced
> + ovn-nbctl --policy=src-ip lr-route-add lr$i 10.22.$i.0/24 169.254.0.2
> +done
> +
> +for i in 1 2; do
> + OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep learned])
> +done
> +
> +# check that advertised routes in ic-sb have correct origin
> +ovn-ic-sbctl list route
> +wait_row_count ic-sb:Route 1 ip_prefix=10.11.1.0/24 origin=static
> +wait_row_count ic-sb:Route 1 ip_prefix=192.168.1.1/24 origin=connected
> +wait_row_count ic-sb:Route 1 ip_prefix=10.11.2.0/24 origin=static
> +wait_row_count ic-sb:Route 1 ip_prefix=192.168.2.1/24 origin=connected
> +
> +# check that learned routes in ic-sb have correct origin
> +
> +ovn_as az1
> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=10.11.2.0/24 options:origin=static
> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=192.168.2.1/24 options:origin=connected
> +
> +ovn_as az2
> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=10.11.1.0/24 options:origin=static
> +wait_row_count nb:Logical_Router_Static_Route 1 ip_prefix=192.168.1.1/24 options:origin=connected
> +
> OVN_CLEANUP_IC([az1], [az2])
>
> AT_CLEANUP
> --
> 2.30.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list