On 1/20/25 4:00 PM, Felix Huettner wrote:
> On Tue, Jan 14, 2025 at 12:22:57PM +0100, Dumitru Ceara wrote:
>> Hi Felix,
>>
>> Overall this looks OK; I do have some comments/suggestions though.
>
> Hi Dumitru,
>
> thanks a lot for the review. All minor things will be adressed in v3,
> the others have comments below.
>
>>
>> On 12/18/24 11:24 AM, Felix Huettner via dev wrote:
>>> here we expand the previous routes-sync engine node to not only
>>
>> Nit: Here
>>
>>> advertise routes to the southbound table, but also learn received routes
>>> from this table.
>>>
>>> These routes are then passed to the same logic that connected and static
>>> routes are using for flow generation.
>>> However we prioritize these routes lower than connected or static routes
>>> as information in cluster (for the same prefix length) should always be
>>> more correct then learned routes.
>>> This is also consistent with the behaviour of phyiscal routers.
>>
>> Nit: admin distance is often configurable in physical routers - you're
>> right however that the default is likely to give priority to connected
>> and then static routes.
>
> +1
>
>>
>>>
>>> Signed-off-by: Felix Huettner <[email protected]>
>>> ---
>>> NEWS | 4 +
>>> lib/stopwatch-names.h | 1 +
>>> northd/automake.mk | 2 +
>>> northd/en-learned-route-sync.c | 221 ++++++++++++++++++++++++++++++
>>> northd/en-learned-route-sync.h | 31 +++++
>>> northd/en-lflow.c | 5 +-
>>> northd/inc-proc-northd.c | 11 +-
>>> northd/northd.c | 240 ++++++++++++++++++++-------------
>>> northd/northd.h | 32 ++++-
>>> tests/ovn-northd.at | 160 +++++++++++++++++-----
>>> 10 files changed, 574 insertions(+), 133 deletions(-)
>>> create mode 100644 northd/en-learned-route-sync.c
>>> create mode 100644 northd/en-learned-route-sync.h
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 33cb2ca89..c64453007 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -18,6 +18,10 @@ Post v24.09.0
>>> The routes can furthe be filtered by setting
>>> `dynamic-routing-connected`
>>> and `dynamic-routing-static` on the LR or LRP. The LRP settings
>>> overwrite
>>> the LR settings for all routes using this interface as an exit.
>>> + - Allow Logical Routers to dynamically learn routes from outside the
>>> fabric.
>>> + Routes entered into the "Route" table in the southbound database will
>>> be
>>> + learned by the respective LR. They are included in the route table
>>> with
>>> + a lower priority than static routes.
>>>
>>> OVN v24.09.0 - 13 Sep 2024
>>> --------------------------
>>> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
>>> index dc4129ee5..c12dd28d0 100644
>>> --- a/lib/stopwatch-names.h
>>> +++ b/lib/stopwatch-names.h
>>> @@ -35,5 +35,6 @@
>>> #define LR_STATEFUL_RUN_STOPWATCH_NAME "lr_stateful"
>>> #define LS_STATEFUL_RUN_STOPWATCH_NAME "ls_stateful"
>>> #define ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME "advertised_route_sync"
>>> +#define LEARNED_ROUTE_SYNC_RUN_STOPWATCH_NAME "learned_route_sync"
>>>
>>> #endif
>>> diff --git a/northd/automake.mk b/northd/automake.mk
>>> index a2797237a..6f4689d37 100644
>>> --- a/northd/automake.mk
>>> +++ b/northd/automake.mk
>>> @@ -36,6 +36,8 @@ northd_ovn_northd_SOURCES = \
>>> northd/en-sampling-app.h \
>>> northd/en-advertised-route-sync.c \
>>> northd/en-advertised-route-sync.h \
>>> + northd/en-learned-route-sync.c \
>>> + northd/en-learned-route-sync.h \
>>> northd/inc-proc-northd.c \
>>> northd/inc-proc-northd.h \
>>> northd/ipam.c \
>>> diff --git a/northd/en-learned-route-sync.c b/northd/en-learned-route-sync.c
>>> new file mode 100644
>>> index 000000000..962ccd10e
>>> --- /dev/null
>>> +++ b/northd/en-learned-route-sync.c
>>> @@ -0,0 +1,221 @@
>>> +/*
>>
>> Missing copyright.
>>
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at:
>>> + *
>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + */
>>> +
>>> +#include <config.h>
>>> +#include <stdbool.h>
>>> +
>>> +#include "openvswitch/vlog.h"
>>> +#include "stopwatch.h"
>>> +#include "northd.h"
>>> +
>>> +#include "en-learned-route-sync.h"
>>> +#include "lib/stopwatch-names.h"
>>> +#include "openvswitch/hmap.h"
>>> +#include "ovn-util.h"
>>> +
>>> +VLOG_DEFINE_THIS_MODULE(en_learned_route_sync);
>>> +
>>> +static void
>>> +routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
>>> + const struct sbrec_learned_route_table
>>> + *sbrec_learned_route_table,
>>
>> This looks a bit weird to me. In other cases we indent a bit
>> differently, e.g.:
>>
>> static void
>> routes_table_sync(
>> struct ovsdb_idl_txn *ovnsb_txn,
>> const struct sbrec_learned_route_table *sbrec_learned_route_table,
>> const struct hmap *parsed_routes,
>> const struct hmap *lr_ports,
>> const struct ovn_datapaths *lr_datapaths,
>> struct hmap *parsed_routes_out);
>>
>>> + const struct hmap *parsed_routes,
>>> + const struct hmap *lr_ports,
>>> + const struct ovn_datapaths *lr_datapaths,
>>> + struct hmap *parsed_routes_out);
>>> +
>>> +bool
>>> +learned_route_sync_northd_change_handler(struct engine_node *node,
>>> + void *data_ OVS_UNUSED)
>>> +{
>>> + struct northd_data *northd_data = engine_get_input_data("northd",
>>> node);
>>> + if (!northd_has_tracked_data(&northd_data->trk_data)) {
>>> + return false;
>>> + }
>>> +
>>> + /* This node uses the below data from the en_northd engine node.
>>> + * See (lr_stateful_get_input_data())
>>> + * 1. northd_data->lr_datapaths
>>> + * 2. northd_data->lr_ports
>>> + * This data gets updated when a logical router or logical router
>>> port
>>> + * is created or deleted.
>>> + * Northd engine node presently falls back to full recompute when
>>> + * this happens and so does this node.
>>> + * Note: When we add I-P to the created/deleted logical routers or
>>> + * logical router ports, we need to revisit this handler.
>>> + */
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static void
>>> +routes_sync_init(struct learned_route_sync_data *data)
>>> +{
>>> + hmap_init(&data->parsed_routes);
>>> +}
>>> +
>>> +static void
>>> +routes_sync_destroy(struct learned_route_sync_data *data)
>>> +{
>>> + struct parsed_route *r;
>>> + HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) {
>>> + parsed_route_free(r);
>>> + }
>>
>> I'd extract this part into a routes_sync_clear(data) function and call
>> it here and in en_learned_route_sync_run().
>>
>>> + hmap_destroy(&data->parsed_routes);
>>> +}
>>> +
>>> +void
>>> +*en_learned_route_sync_init(struct engine_node *node OVS_UNUSED,
>>> + struct engine_arg *arg OVS_UNUSED)
>>
>> Nit: the return type is "void *" not "void"; this should be:
>>
>> void *
>> en_learned_route_sync_init(struct engine_node *node OVS_UNUSED,
>> struct engine_arg *arg OVS_UNUSED)
>> {
>> struct learned_route_sync_data *data = xzalloc(sizeof *data);
>> routes_sync_init(data);
>> return data;
>> }
>>
>>> +{
>>> + struct learned_route_sync_data *data = xzalloc(sizeof *data);
>>> + routes_sync_init(data);
>>> + return data;
>>> +}
>>> +
>>> +void
>>> +en_learned_route_sync_cleanup(void *data)
>>> +{
>>> + routes_sync_destroy(data);
>>> +}
>>> +
>>> +void
>>> +en_learned_route_sync_run(struct engine_node *node, void *data)
>>> +{
>>> + routes_sync_destroy(data);
>>> + routes_sync_init(data);
>>> +
>>> + struct learned_route_sync_data *routes_sync_data = data;
>>> + struct routes_data *routes_data
>>> + = engine_get_input_data("routes", node);
>>> + const struct engine_context *eng_ctx = engine_get_context();
>>> + const struct sbrec_learned_route_table *sbrec_learned_route_table =
>>> + EN_OVSDB_GET(engine_get_input("SB_learned_route", node));
>>> + struct northd_data *northd_data = engine_get_input_data("northd",
>>> node);
>>> +
>>> + stopwatch_start(LEARNED_ROUTE_SYNC_RUN_STOPWATCH_NAME, time_msec());
>>
>> Missing stopwatch_create().
>>
>>> +
>>> + routes_table_sync(eng_ctx->ovnsb_idl_txn, sbrec_learned_route_table,
>>> + &routes_data->parsed_routes,
>>> + &northd_data->lr_ports,
>>> + &northd_data->lr_datapaths,
>>> + &routes_sync_data->parsed_routes);
>>> +
>>> + stopwatch_stop(LEARNED_ROUTE_SYNC_RUN_STOPWATCH_NAME, time_msec());
>>> + engine_set_node_state(node, EN_UPDATED);
>>> +}
>>> +
>>> +
>>> +static void
>>> +parse_route_from_sbrec_route(struct hmap *parsed_routes_out,
>>> + const struct hmap *lr_ports,
>>> + const struct hmap *lr_datapaths,
>>> + const struct sbrec_learned_route *route)
>>> +{
>>> + const struct ovn_datapath *od = ovn_datapath_from_sbrec(
>>> + NULL, lr_datapaths, route->datapath);
>>> +
>>
>> We should probably be more defensive here. "od" can be NULL and also
>> ovn_datapath_is_stale(od) can be false. In both cases I guess we should
>> probably ignore the SB route.
>
> Thanks a lot, yea fixed that.
>
>>
>>> + /* Verify that the next hop is an IP address with an all-ones mask. */
>>> + struct in6_addr *nexthop = xmalloc(sizeof(*nexthop));
>>
>> No parenthesis needed for sizeof.
>>
>>> + unsigned int plen;
>>> + if (!ip46_parse_cidr(route->nexthop, nexthop, &plen)) {
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>> + VLOG_WARN_RL(&rl, "bad 'nexthop' %s in learned route "
>>> + UUID_FMT, route->nexthop,
>>> + UUID_ARGS(&route->header_.uuid));
>>> + free(nexthop);
>>> + return;
>>> + }
>>> + if ((IN6_IS_ADDR_V4MAPPED(nexthop) && plen != 32) ||
>>> + (!IN6_IS_ADDR_V4MAPPED(nexthop) && plen != 128)) {
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>> + VLOG_WARN_RL(&rl, "bad next hop mask %s in learned route "
>>> + UUID_FMT, route->nexthop,
>>> + UUID_ARGS(&route->header_.uuid));
>>> + free(nexthop);
>>> + return;
>>> + }
>>> +
>>> + /* Parse ip_prefix */
>>> + struct in6_addr prefix;
>>> + if (!ip46_parse_cidr(route->ip_prefix, &prefix, &plen)) {
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>> + VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in learned route "
>>> + UUID_FMT, route->ip_prefix,
>>> + UUID_ARGS(&route->header_.uuid));
>>> + free(nexthop);
>>> + return;
>>> + }
>>> +
>>> + /* Verify that ip_prefix and nexthop have same address familiy. */
>>
>> We have this now:
>>
>> commit 559924291c7fad3e5c6a67f4c2127f5e73b8c575
>> Author: Felix Huettner <[email protected]>
>> Date: Wed Dec 4 16:10:56 2024 +0100
>>
>> northd: Handle routing for other address families.
>>
>> So why wouldn't we allow learning routes over different address families?
>
> This is now allowed. I just missed to remove this filter here :)
>
>>
>>> + if (IN6_IS_ADDR_V4MAPPED(&prefix) != IN6_IS_ADDR_V4MAPPED(nexthop)) {
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>> + VLOG_WARN_RL(&rl, "Address family doesn't match between
>>> 'ip_prefix'"
>>> + " %s and 'nexthop' %s in learned route "UUID_FMT,
>>> + route->ip_prefix, route->nexthop,
>>> + UUID_ARGS(&route->header_.uuid));
>>> + free(nexthop);
>>> + return;
>>> + }
>>> +
>>> + /* Verify that ip_prefix and nexthop are on the same network. */
>>> + const char *lrp_addr_s = NULL;
>>> + struct ovn_port *out_port = NULL;
>>> + if (!find_route_outport(lr_ports, route->logical_port->logical_port,
>>> + route->ip_prefix, route->nexthop,
>>> + IN6_IS_ADDR_V4MAPPED(&prefix),
>>> + false,
>>> + &out_port, &lrp_addr_s)) {
>>> + free(nexthop);
>>> + return;
>>> + }
>>> +
>>> + parsed_route_add(od, nexthop, &prefix, plen, false, lrp_addr_s,
>>> + out_port, 0, false, false, NULL,
>>> + ROUTE_SOURCE_LEARNED, &route->header_,
>>> parsed_routes_out);
>>> +}
>>> +
>>> +static void
>>> +routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
>>> + const struct sbrec_learned_route_table
>>> + *sbrec_learned_route_table,
>>> + const struct hmap *parsed_routes,
>>> + const struct hmap *lr_ports,
>>> + const struct ovn_datapaths *lr_datapaths,
>>> + struct hmap *parsed_routes_out)
>>
>> Similar comment about indentation as for the prototype declaration.
>>
>>> +{
>>> + if (!ovnsb_txn) {
>>> + return;
>>> + }
>>
>> No need to check the ovnsb_txn, it can never be NULL inside an engine_run().
>>
>>> +
>>> + struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes);
>>> +
>>> + const struct parsed_route *route;
>>> +
>>> + const struct sbrec_learned_route *sb_route;
>>> + SBREC_LEARNED_ROUTE_TABLE_FOR_EACH (sb_route,
>>> sbrec_learned_route_table) {
>>> + parse_route_from_sbrec_route(parsed_routes_out, lr_ports,
>>> + &lr_datapaths->datapaths,
>>> + sb_route);
>>> +
>>> + }
>>> +
>>> + HMAP_FOR_EACH (route, key_node, parsed_routes) {
>>> + hmap_insert(parsed_routes_out,
>>> &parsed_route_clone(route)->key_node,
>>> + parsed_route_hash(route));
>>> + }
>>> +
>>> + hmap_destroy(&sync_routes);
>>> +}
>>> +
>>> diff --git a/northd/en-learned-route-sync.h b/northd/en-learned-route-sync.h
>>> new file mode 100644
>>> index 000000000..55a7e9f73
>>> --- /dev/null
>>> +++ b/northd/en-learned-route-sync.h
>>> @@ -0,0 +1,31 @@
>>> +/*
>>
>> Missing copyright.
>>
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at:
>>> + *
>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + */
>>> +#ifndef EN_LEARNED_ROUTE_SYNC_H
>>> +#define EN_LEARNED_ROUTE_SYNC_H 1
>>> +
>>> +#include "lib/inc-proc-eng.h"
>>> +#include "openvswitch/hmap.h"
>>> +
>>> +struct learned_route_sync_data {
>>> + struct hmap parsed_routes;
>>> +};
>>> +
>>> +bool learned_route_sync_northd_change_handler(struct engine_node *node,
>>> + void *data);
>>
>> Nit" indentation.
>>
>>> +void *en_learned_route_sync_init(struct engine_node *, struct engine_arg
>>> *);
>>> +void en_learned_route_sync_cleanup(void *data);
>>> +void en_learned_route_sync_run(struct engine_node *, void *data);
>>> +
>>> +
>>> +#endif /* EN_LEARNED_ROUTE_SYNC_H */
>>> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
>>> index fa1f0236d..62224eb63 100644
>>> --- a/northd/en-lflow.c
>>> +++ b/northd/en-lflow.c
>>> @@ -26,6 +26,7 @@
>>> #include "en-northd.h"
>>> #include "en-meters.h"
>>> #include "en-sampling-app.h"
>>> +#include "en-learned-route-sync.h"
>>> #include "lflow-mgr.h"
>>>
>>> #include "lib/inc-proc-eng.h"
>>> @@ -46,6 +47,8 @@ lflow_get_input_data(struct engine_node *node,
>>> engine_get_input_data("bfd_sync", node);
>>> struct routes_data *routes_data =
>>> engine_get_input_data("routes", node);
>>> + struct learned_route_sync_data *learned_route_sync_data =
>>> + engine_get_input_data("learned_route_sync", node);
>>> struct route_policies_data *route_policies_data =
>>> engine_get_input_data("route_policies", node);
>>> struct port_group_data *pg_data =
>>> @@ -82,7 +85,7 @@ lflow_get_input_data(struct engine_node *node,
>>> lflow_input->lb_datapaths_map = &northd_data->lb_datapaths_map;
>>> lflow_input->svc_monitor_map = &northd_data->svc_monitor_map;
>>> lflow_input->bfd_ports = &bfd_sync_data->bfd_ports;
>>> - lflow_input->parsed_routes = &routes_data->parsed_routes;
>>> + lflow_input->parsed_routes = &learned_route_sync_data->parsed_routes;
>>> lflow_input->route_tables = &routes_data->route_tables;
>>> lflow_input->route_policies = &route_policies_data->route_policies;
>>>
>>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
>>> index 77a7d637c..ed9e27de9 100644
>>> --- a/northd/inc-proc-northd.c
>>> +++ b/northd/inc-proc-northd.c
>>> @@ -42,6 +42,7 @@
>>> #include "en-sync-sb.h"
>>> #include "en-sync-from-sb.h"
>>> #include "en-advertised-route-sync.h"
>>> +#include "en-learned-route-sync.h"
>>> #include "unixctl.h"
>>> #include "util.h"
>>>
>>> @@ -104,7 +105,8 @@ static unixctl_cb_func chassis_features_list;
>>> SB_NODE(static_mac_binding, "static_mac_binding") \
>>> SB_NODE(chassis_template_var, "chassis_template_var") \
>>> SB_NODE(logical_dp_group, "logical_dp_group") \
>>> - SB_NODE(advertised_route, "advertised_route")
>>> + SB_NODE(advertised_route, "advertised_route") \
>>> + SB_NODE(learned_route, "learned_route")
>>>
>>> enum sb_engine_node {
>>> #define SB_NODE(NAME, NAME_STR) SB_##NAME,
>>> @@ -164,6 +166,7 @@ static ENGINE_NODE(routes, "routes");
>>> static ENGINE_NODE(bfd, "bfd");
>>> static ENGINE_NODE(bfd_sync, "bfd_sync");
>>> static ENGINE_NODE(advertised_route_sync, "advertised_route_sync");
>>> +static ENGINE_NODE(learned_route_sync, "learned_route_sync");
>>
>> Would it be possible to add incremental processing unit tests for this
>> node too?
>>
>>>
>>> void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>>> struct ovsdb_idl_loop *sb)
>>> @@ -270,6 +273,11 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>>> engine_add_input(&en_advertised_route_sync, &en_sb_advertised_route,
>>> engine_noop_handler);
>>>
>>> + engine_add_input(&en_learned_route_sync, &en_routes, NULL);
>>> + engine_add_input(&en_learned_route_sync, &en_sb_learned_route, NULL);
>>> + engine_add_input(&en_learned_route_sync, &en_northd,
>>> + learned_route_sync_northd_change_handler);
>>> +
>>> engine_add_input(&en_sync_meters, &en_nb_acl, NULL);
>>> engine_add_input(&en_sync_meters, &en_nb_meter, NULL);
>>> engine_add_input(&en_sync_meters, &en_sb_meter, NULL);
>>> @@ -283,6 +291,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>>> engine_add_input(&en_lflow, &en_bfd_sync, NULL);
>>> engine_add_input(&en_lflow, &en_route_policies, NULL);
>>> engine_add_input(&en_lflow, &en_routes, NULL);
>>> + engine_add_input(&en_lflow, &en_learned_route_sync, NULL);
>>
>> Because we also do:
>>
>>> + engine_add_input(&en_learned_route_sync, &en_sb_learned_route, NULL);
>>
>> That means that each and every time a dynamic route changes we trigger a
>> full recompute of the en_lflow node. That's very expensive. Would it
>> be possible to add an incremental processing event handler for
>> en_learned_route_sync changes? Maybe a compromise would be to write a
>> simpler handler that would regenerate all the logical flows for all
>> dynamic routes whenever a learned route is added/updated?
>
> i fully agree with that.
> Would it be ok for you if i first push out a v3 with all the other changes so
> they can be reviewed and then take a look at this? Or would you want to
> have that as part of this patch?
>
> I am just concerned that i take a while to build it and this then takes
> time away from working on the other review comments.
>
Let's add a TODO for it (an "XXX" comment and/or an item in TODO.rst)
and handle it as a follow up. I don't want to block the rest of the
series on this.
>>
>>
>>> engine_add_input(&en_lflow, &en_global_config,
>>> node_global_config_handler);
>>>
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index 0b495a2b6..b003d4791 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -302,11 +302,14 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2));
>>> /*
>>> * Route offsets implement logic to prioritize traffic for routes with
>>> * same ip_prefix values:
>>> - * - connected route overrides static one;
>>> - * - static route overrides src-ip route. */
>>> -#define ROUTE_PRIO_OFFSET_MULTIPLIER 5
>>> -#define ROUTE_PRIO_OFFSET_STATIC 2
>>> -#define ROUTE_PRIO_OFFSET_CONNECTED 4
>>> + * 1. (highest priority) connected routes
>>> + * 2. static routes
>>> + * 3. routes learned from the outside via ovn-controller (e.g. bgp)
>>> + * 4. (lowest priority) src-ip routes */
>>> +#define ROUTE_PRIO_OFFSET_MULTIPLIER 8
>>> +#define ROUTE_PRIO_OFFSET_LEARNED 2
>>> +#define ROUTE_PRIO_OFFSET_STATIC 4
>>> +#define ROUTE_PRIO_OFFSET_CONNECTED 6
>>>
>>> /* Returns the type of the datapath to which a flow with the given 'stage'
>>> may
>>> * be added. */
>>> @@ -11120,7 +11123,7 @@ build_route_table_lflow(struct ovn_datapath *od,
>>> struct lflow_table *lflows,
>>> }
>>>
>>> static uint32_t
>>> -route_hash(struct parsed_route *route)
>>> +route_hash(const struct parsed_route *route)
>>> {
>>> return hash_bytes(&route->prefix, sizeof route->prefix,
>>> (uint32_t)route->plen);
>>> @@ -11171,7 +11174,7 @@ parsed_route_lookup(struct hmap *routes, size_t
>>> hash,
>>> continue;
>>> }
>>>
>>> - if (pr->route != new_pr->route) {
>>> + if (pr->source_hint != new_pr->source_hint) {
>>> continue;
>>> }
>>>
>>> @@ -11198,7 +11201,37 @@ parsed_route_lookup(struct hmap *routes, size_t
>>> hash,
>>> return NULL;
>>> }
>>>
>>> -static void
>>> +struct parsed_route * parsed_route_clone(const struct parsed_route *pr) {
>>
>> Nit: spacing is a bit weird. We prefer:
>>
>> struct parsed_route *parsed_route_clone()
>>
>>> + struct parsed_route *new_pr = xzalloc(sizeof *new_pr);
>>> + new_pr->prefix = pr->prefix;
>>> + new_pr->plen = pr->plen;
>>> + if (pr->nexthop) {
>>> + new_pr->nexthop = xmemdup(pr->nexthop, sizeof(*pr->nexthop));
>>
>> From our coding style:
>>
>> "The ``sizeof`` operator is unique among C operators in that it accepts
>> two very different kinds of operands: an expression or a type. In
>> general, prefer to specify an expression, e.g.
>> ``int *x = xmalloc(sizeof *x);``. When the operand of ``sizeof`` is an
>> expression, there is no need to parenthesize that operand, and please
>> don't."
>>
>>> + }
>>
>> Too bad OVS doesn't have a nullable_xmemdup().
>>
>>> + new_pr->route_table_id = pr->route_table_id;
>>> + new_pr->is_src_route = pr->is_src_route;
>>> + new_pr->hash = route_hash(pr);
>>> + new_pr->ecmp_symmetric_reply = pr->ecmp_symmetric_reply;
>>> + new_pr->is_discard_route = pr->is_discard_route;
>>> + new_pr->od = pr->od;
>>> + new_pr->stale = pr->stale;
>>> + new_pr->source = pr->source;
>>> + new_pr->source_hint = pr->source_hint;
>>> + if (pr->lrp_addr_s) {
>>> + new_pr->lrp_addr_s = xstrdup(pr->lrp_addr_s);
>>> + }
>>
>> If you use nullable_xstrdup() you don't need the explicit null check.
>>
>>> + if (pr->out_port) {
>>> + new_pr->out_port = pr->out_port;
>>> + }
>>
>> No need for the NULL check.
>>
>>> + sset_clone(&new_pr->ecmp_selection_fields, &pr->ecmp_selection_fields);
>>> + return new_pr;
>>> +}
>>
>> Do we really need a full fledged clone though? Can't we reference count
>> parsed_routes instead? So have parsed_route_clone(pr) increment the ref
>> count of "pr" and parsed_route_free(pr) decrement the ref count of "pr"
>> and cleanup "pr" if the refcount reaches 0?
>
> For now yes we need the full clone. The thing is that a parsed_route can
> be part of two hmaps. One created by the "route" engine node which
> includes connected and static routes. The other one is from the
> "learned_route_sync" engine node. That includes all routes of the
> "route" engine node in addition to all entries from sb Learned_Route.
> If the route is not fully cloned it can not be part of both hmaps.
Ah, you're right.
>
> I would not want to have the "learned_route_sync" engine node just
> append on the hmap of the "route" node. The output of the "route" node
> is used also by other nodes so this would cause some strangeness.
>
> An alternative might be to let later parts of the code that iterate over
> all routes just iterate over separate lists from "route" and
> "learned_route_sync", but that also feels quite ugly.
>
> Or is there a nicer way you could think of.
>
Maybe there isn't. They're different objects. Let's leave it as is for
now. We an always improve this if needed.
>>
>>> +
>>> +size_t parsed_route_hash(const struct parsed_route *pr) {
>>> + return uuid_hash(&pr->od->key);
>>> +}
>>> +
>>> +void
>>> parsed_route_free(struct parsed_route *pr) {
>>
>> Nit: we're exporting this _free() function outside the module now. We
>> should probably make it behave like a proper *_free() function and
>> gracefully handle NULL arguments.
>>
>> if (!pr) {
>> return;
>> }
>>
>> From our coding style guidelines:
>>
>> "Functions that destroy an instance of a dynamically-allocated type
>> should accept and ignore a null pointer argument. Code that calls such a
>> function (including the C standard library function ``free()``) should
>> omit a null-pointer check. We find that this usually makes code easier
>> to read."
>
> Thanks a lot, fixed.
>
>>
>>> free(pr->lrp_addr_s);
>>> free(pr->nexthop);
>>> @@ -11206,7 +11239,7 @@ parsed_route_free(struct parsed_route *pr) {
>>> free(pr);
>>> }
>>>
>>> -static void
>>> +void
>>> parsed_route_add(const struct ovn_datapath *od,
>>> struct in6_addr *nexthop,
>>> const struct in6_addr *prefix,
>>> @@ -11214,12 +11247,12 @@ parsed_route_add(const struct ovn_datapath *od,
>>> bool is_discard_route,
>>> const char *lrp_addr_s,
>>> const struct ovn_port *out_port,
>>> - const struct nbrec_logical_router_static_route *route,
>>> uint32_t route_table_id,
>>> bool is_src_route,
>>> bool ecmp_symmetric_reply,
>>> const struct sset *ecmp_selection_fields,
>>> enum route_source source,
>>> + const struct ovsdb_idl_row *source_hint,
>>> struct hmap *routes)
>>> {
>>>
>>> @@ -11238,14 +11271,14 @@ parsed_route_add(const struct ovn_datapath *od,
>>> }
>>> new_pr->out_port = out_port;
>>> new_pr->source = source;
>>> - new_pr->route = route;
>>> if (ecmp_selection_fields) {
>>> sset_clone(&new_pr->ecmp_selection_fields, ecmp_selection_fields);
>>> } else {
>>> sset_init(&new_pr->ecmp_selection_fields);
>>> }
>>> + new_pr->source_hint = source_hint;
>>>
>>> - size_t hash = uuid_hash(&od->key);
>>> + size_t hash = parsed_route_hash(new_pr);
>>> struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr);
>>> if (!pr) {
>>> hmap_insert(routes, &new_pr->key_node, hash);
>>> @@ -11376,9 +11409,9 @@ parsed_routes_add_static(const struct ovn_datapath
>>> *od,
>>> }
>>>
>>> parsed_route_add(od, nexthop, &prefix, plen, is_discard_route,
>>> lrp_addr_s,
>>> - out_port, route, route_table_id, is_src_route,
>>> + out_port, route_table_id, is_src_route,
>>> ecmp_symmetric_reply, &ecmp_selection_fields, source,
>>> - routes);
>>> + &route->header_, routes);
>>> sset_destroy(&ecmp_selection_fields);
>>> }
>>>
>>> @@ -11394,9 +11427,9 @@ parsed_routes_add_connected(const struct
>>> ovn_datapath *od,
>>>
>>> parsed_route_add(od, NULL, &prefix, addr->plen,
>>> false, addr->addr_s, op,
>>> - NULL, 0, false,
>>> + 0, false,
>>> false, NULL, ROUTE_SOURCE_CONNECTED,
>>> - routes);
>>> + &op->nbrp->header_, routes);
>>> }
>>>
>>> for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
>>> @@ -11406,9 +11439,9 @@ parsed_routes_add_connected(const struct
>>> ovn_datapath *od,
>>>
>>> parsed_route_add(od, NULL, &prefix, addr->plen,
>>> false, addr->addr_s, op,
>>> - NULL, 0, false,
>>> + 0, false,
>>> false, NULL, ROUTE_SOURCE_CONNECTED,
>>> - routes);
>>> + &op->nbrp->header_, routes);
>>> }
>>> }
>>>
>>> @@ -11610,13 +11643,30 @@ build_route_prefix_s(const struct in6_addr
>>> *prefix, unsigned int plen)
>>> return prefix_s;
>>> }
>>>
>>> +static int
>>> +route_source_to_offset(enum route_source source)
>>
>> I know this is just code that you had to move around but I think
>> negative offsets are weird and will never be the case. Logical flow
>> (ovn_lflow) priority is uint16_t. Let's make this function return
>> "uint16_t" too.
>>
>>> +{
>>> + switch (source) {
>>> + case ROUTE_SOURCE_CONNECTED:
>>
>> Please align 'case' with 'switch' while we're at it.
>>
>>> + return ROUTE_PRIO_OFFSET_CONNECTED;
>>> + case ROUTE_SOURCE_STATIC:
>>> + return ROUTE_PRIO_OFFSET_STATIC;
>>> + case ROUTE_SOURCE_LEARNED:
>>> + return ROUTE_PRIO_OFFSET_LEARNED;
>>> + default:
>>> + OVS_NOT_REACHED();
>>> + }
>>> +}
>>> +
>>> static void
>>> build_route_match(const struct ovn_port *op_inport, uint32_t rtb_id,
>>> const char *network_s, int plen, bool is_src_route,
>>> - bool is_ipv4, struct ds *match, uint16_t *priority, int
>>> ofs,
>>> - bool has_protocol_match)
>>> + bool is_ipv4, struct ds *match, uint16_t *priority,
>>> + enum route_source source, bool has_protocol_match)
>>> {
>>> const char *dir;
>>> + int ofs = route_source_to_offset(source);
>>> +
>>> /* The priority here is calculated to implement longest-prefix-match
>>> * routing. */
>>> if (is_src_route) {
>>> @@ -11629,7 +11679,8 @@ build_route_match(const struct ovn_port *op_inport,
>>> uint32_t rtb_id,
>>> if (op_inport) {
>>> ds_put_format(match, "inport == %s && ", op_inport->json_key);
>>> }
>>> - if (rtb_id || ofs == ROUTE_PRIO_OFFSET_STATIC) {
>>> + if (rtb_id || source == ROUTE_SOURCE_STATIC ||
>>> + source == ROUTE_SOURCE_LEARNED) {
>>> ds_put_format(match, "%s == %d && ", REG_ROUTE_TABLE_ID, rtb_id);
>>> }
>>>
>>> @@ -11642,6 +11693,45 @@ build_route_match(const struct ovn_port
>>> *op_inport, uint32_t rtb_id,
>>> network_s, plen);
>>> }
>>>
>>> +bool
>>> +find_route_outport(const struct hmap *lr_ports, const char *output_port,
>>> + const char *ip_prefix, const char *nexthop, bool
>>> is_ipv4,
>>> + bool force_out_port,
>>> + struct ovn_port **out_port, const char **lrp_addr_s)
>>> +{
>>> + *out_port = ovn_port_find(lr_ports, output_port);
>>> + if (!*out_port) {
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>> + VLOG_WARN_RL(&rl, "Bad out port %s for static route %s",
>>> + output_port, ip_prefix);
>>> + return false;
>>> + }
>>> + if (nexthop[0]) {
>>> + *lrp_addr_s = find_lrp_member_ip(*out_port, nexthop);
>>> + }
>>> + if (!*lrp_addr_s) {
>>> + if (!force_out_port) {
>>> + return false;
>>> + }
>>> + /* There are no IP networks configured on the router's port via
>>> + * which 'route->nexthop' is theoretically reachable. But since
>>> + * 'out_port' has been specified, we honor it by trying to reach
>>> + * 'route->nexthop' via the first IP address of 'out_port'.
>>> + * (There are cases, e.g in GCE, where each VM gets a /32 IP
>>> + * address and the default gateway is still reachable from it.) */
>>> + if (is_ipv4) {
>>> + if ((*out_port)->lrp_networks.n_ipv4_addrs) {
>>> + *lrp_addr_s =
>>> (*out_port)->lrp_networks.ipv4_addrs[0].addr_s;
>>> + }
>>> + } else {
>>> + if ((*out_port)->lrp_networks.n_ipv6_addrs) {
>>> + *lrp_addr_s =
>>> (*out_port)->lrp_networks.ipv6_addrs[0].addr_s;
>>> + }
>>> + }
>>> + }
>>> + return true;
>>> +}
>>> +
>>> /* Output: p_lrp_addr_s and p_out_port. */
>>> static bool
>>> find_static_route_outport(const struct ovn_datapath *od,
>>> @@ -11652,33 +11742,10 @@ find_static_route_outport(const struct
>>> ovn_datapath *od,
>>> const char *lrp_addr_s = NULL;
>>> struct ovn_port *out_port = NULL;
>>> if (route->output_port) {
>>> - out_port = ovn_port_find(lr_ports, route->output_port);
>>> - if (!out_port) {
>>> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>> - VLOG_WARN_RL(&rl, "Bad out port %s for static route %s",
>>> - route->output_port, route->ip_prefix);
>>> + if (!find_route_outport(lr_ports, route->output_port,
>>> route->ip_prefix,
>>> + route->nexthop, is_ipv4, true, &out_port, &lrp_addr_s)) {
>>> return false;
>>> }
>>> - if (route->nexthop[0]) {
>>> - lrp_addr_s = find_lrp_member_ip(out_port, route->nexthop);
>>> - }
>>> - if (!lrp_addr_s) {
>>> - /* There are no IP networks configured on the router's port via
>>> - * which 'route->nexthop' is theoretically reachable. But
>>> since
>>> - * 'out_port' has been specified, we honor it by trying to
>>> reach
>>> - * 'route->nexthop' via the first IP address of 'out_port'.
>>> - * (There are cases, e.g in GCE, where each VM gets a /32 IP
>>> - * address and the default gateway is still reachable from
>>> it.) */
>>> - if (is_ipv4) {
>>> - if (out_port->lrp_networks.n_ipv4_addrs) {
>>> - lrp_addr_s =
>>> out_port->lrp_networks.ipv4_addrs[0].addr_s;
>>> - }
>>> - } else {
>>> - if (out_port->lrp_networks.n_ipv6_addrs) {
>>> - lrp_addr_s =
>>> out_port->lrp_networks.ipv6_addrs[0].addr_s;
>>> - }
>>> - }
>>> - }
>>> } else {
>>> /* output_port is not specified, find the
>>> * router port matching the next hop. */
>>> @@ -11717,7 +11784,6 @@ add_ecmp_symmetric_reply_flows(struct lflow_table
>>> *lflows,
>>> struct ds *route_match,
>>> struct lflow_ref *lflow_ref)
>>> {
>>> - const struct nbrec_logical_router_static_route *st_route =
>>> route->route;
>>> struct ds match = DS_EMPTY_INITIALIZER;
>>> struct ds actions = DS_EMPTY_INITIALIZER;
>>> struct ds ecmp_reply = DS_EMPTY_INITIALIZER;
>>> @@ -11734,12 +11800,12 @@ add_ecmp_symmetric_reply_flows(struct lflow_table
>>> *lflows,
>>> free(cidr);
>>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DEFRAG, 100,
>>> ds_cstr(&match), "ct_next;",
>>> - &st_route->header_, lflow_ref);
>>> + route->source_hint, lflow_ref);
>>>
>>> /* And packets that go out over an ECMP route need conntrack */
>>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DEFRAG, 100,
>>> ds_cstr(route_match), "ct_next;",
>>> - &st_route->header_, lflow_ref);
>>> + route->source_hint, lflow_ref);
>>>
>>> /* Save src eth and inport in ct_label for packets that arrive over
>>> * an ECMP route.
>>> @@ -11755,7 +11821,7 @@ add_ecmp_symmetric_reply_flows(struct lflow_table
>>> *lflows,
>>> out_port->sb->tunnel_key);
>>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ECMP_STATEFUL, 100,
>>> ds_cstr(&match), ds_cstr(&actions),
>>> - &st_route->header_,
>>> + route->source_hint,
>>> lflow_ref);
>>>
>>> /* Bypass ECMP selection if we already have ct_label information
>>> @@ -11776,13 +11842,13 @@ add_ecmp_symmetric_reply_flows(struct lflow_table
>>> *lflows,
>>> port_ip, out_port->json_key);
>>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING, 10300,
>>> ds_cstr(&match), ds_cstr(&actions),
>>> - &st_route->header_,
>>> + route->source_hint,
>>> lflow_ref);
>>>
>>> /* Egress reply traffic for symmetric ECMP routes skips router
>>> policies. */
>>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, 65535,
>>> ds_cstr(&ecmp_reply), "next;",
>>> - &st_route->header_,
>>> + route->source_hint,
>>> lflow_ref);
>>>
>>> /* Use REG_ECMP_ETH_FULL to pass the eth field from ct_label to
>>> eth.dst to
>>> @@ -11799,7 +11865,7 @@ add_ecmp_symmetric_reply_flows(struct lflow_table
>>> *lflows,
>>> " pop(" REG_ECMP_ETH_FULL "); next;";
>>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ARP_RESOLVE,
>>> 200, ds_cstr(&ecmp_reply),
>>> - action, &st_route->header_,
>>> + action, route->source_hint,
>>> lflow_ref);
>>>
>>> ds_destroy(&match);
>>> @@ -11807,19 +11873,6 @@ add_ecmp_symmetric_reply_flows(struct lflow_table
>>> *lflows,
>>> ds_destroy(&ecmp_reply);
>>> }
>>>
>>> -static int
>>> -route_source_to_offset(enum route_source source)
>>> -{
>>> - switch (source) {
>>> - case ROUTE_SOURCE_CONNECTED:
>>> - return ROUTE_PRIO_OFFSET_CONNECTED;
>>> - case ROUTE_SOURCE_STATIC:
>>> - return ROUTE_PRIO_OFFSET_STATIC;
>>> - default:
>>> - OVS_NOT_REACHED();
>>> - }
>>> -}
>>> -
>>> static void
>>> build_ecmp_route_flow(struct lflow_table *lflows, struct ovn_datapath *od,
>>> struct ecmp_groups_node *eg, struct lflow_ref
>>> *lflow_ref,
>>> @@ -11832,10 +11885,9 @@ build_ecmp_route_flow(struct lflow_table *lflows,
>>> struct ovn_datapath *od,
>>> struct ds route_match = DS_EMPTY_INITIALIZER;
>>>
>>> char *prefix_s = build_route_prefix_s(&eg->prefix, eg->plen);
>>> - int ofs = route_source_to_offset(eg->source);
>>> build_route_match(NULL, eg->route_table_id, prefix_s, eg->plen,
>>> eg->is_src_route, is_ipv4_prefix, &route_match,
>>> - &priority, ofs,
>>> + &priority, eg->source,
>>> protocol != NULL);
>>> free(prefix_s);
>>>
>>> @@ -11898,18 +11950,17 @@ build_ecmp_route_flow(struct lflow_table *lflows,
>>> struct ovn_datapath *od,
>>> struct ds match = DS_EMPTY_INITIALIZER;
>>> struct sset visited_ports = SSET_INITIALIZER(&visited_ports);
>>> LIST_FOR_EACH (er, list_node, &eg->route_list) {
>>> - const struct parsed_route *route_ = er->route;
>>> - const struct nbrec_logical_router_static_route *route =
>>> route_->route;
>>> - bool is_ipv4_nexthop = IN6_IS_ADDR_V4MAPPED(route_->nexthop);
>>> + const struct parsed_route *route = er->route;
>>> + bool is_ipv4_nexthop = IN6_IS_ADDR_V4MAPPED(route->nexthop);
>>> /* Symmetric ECMP reply is only usable on gateway routers.
>>> * It is NOT usable on distributed routers with a gateway port.
>>> */
>>> if (smap_get(&od->nbr->options, "chassis") &&
>>> - route_->ecmp_symmetric_reply && sset_add(&visited_ports,
>>> -
>>> route_->out_port->key)) {
>>> - add_ecmp_symmetric_reply_flows(lflows, od, route_->lrp_addr_s,
>>> - route_->out_port,
>>> - route_, &route_match,
>>> + route->ecmp_symmetric_reply && sset_add(&visited_ports,
>>> +
>>> route->out_port->key)) {
>>> + add_ecmp_symmetric_reply_flows(lflows, od, route->lrp_addr_s,
>>> + route->out_port,
>>> + route, &route_match,
>>> lflow_ref);
>>> }
>>> ds_clear(&match);
>>> @@ -11919,7 +11970,7 @@ build_ecmp_route_flow(struct lflow_table *lflows,
>>> struct ovn_datapath *od,
>>> ds_clear(&actions);
>>> ds_put_format(&actions, "%s = ",
>>> is_ipv4_nexthop ? REG_NEXT_HOP_IPV4 :
>>> REG_NEXT_HOP_IPV6);
>>> - ipv6_format_mapped(route_->nexthop, &actions);
>>> + ipv6_format_mapped(route->nexthop, &actions);
>>> ds_put_format(&actions, "; "
>>> "%s = %s; "
>>> "eth.src = %s; "
>>> @@ -11927,13 +11978,13 @@ build_ecmp_route_flow(struct lflow_table *lflows,
>>> struct ovn_datapath *od,
>>> REGBIT_NEXTHOP_IS_IPV4" = %d; "
>>> "next;",
>>> is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6,
>>> - route_->lrp_addr_s,
>>> - route_->out_port->lrp_networks.ea_s,
>>> - route_->out_port->json_key,
>>> + route->lrp_addr_s,
>>> + route->out_port->lrp_networks.ea_s,
>>> + route->out_port->json_key,
>>> is_ipv4_nexthop);
>>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP,
>>> 100,
>>> ds_cstr(&match), ds_cstr(&actions),
>>> - &route->header_, lflow_ref);
>>> + route->source_hint, lflow_ref);
>>> }
>>> sset_destroy(&visited_ports);
>>> ds_destroy(&match);
>>> @@ -11955,8 +12006,6 @@ add_route(struct lflow_table *lflows, struct
>>> ovn_datapath *od,
>>> uint16_t priority;
>>> const struct ovn_port *op_inport = NULL;
>>>
>>> - int ofs = route_source_to_offset(source);
>>> -
>>> /* IPv6 link-local addresses must be scoped to the local router port.
>>> */
>>> if (!is_ipv4_prefix) {
>>> struct in6_addr network;
>>> @@ -11966,7 +12015,7 @@ add_route(struct lflow_table *lflows, struct
>>> ovn_datapath *od,
>>> }
>>> }
>>> build_route_match(op_inport, rtb_id, network_s, plen, is_src_route,
>>> - is_ipv4_prefix, &match, &priority, ofs, false);
>>> + is_ipv4_prefix, &match, &priority, source, false);
>>>
>>> struct ds common_actions = DS_EMPTY_INITIALIZER;
>>> struct ds actions = DS_EMPTY_INITIALIZER;
>>> @@ -12015,23 +12064,22 @@ add_route(struct lflow_table *lflows, struct
>>> ovn_datapath *od,
>>>
>>> static void
>>> build_route_flow(struct lflow_table *lflows, struct ovn_datapath *od,
>>> - const struct parsed_route *route_,
>>> + const struct parsed_route *route,
>>> const struct sset *bfd_ports,
>>> struct lflow_ref *lflow_ref)
>>> {
>>> - const struct nbrec_logical_router_static_route *route = route_->route;
>>> - bool is_ipv4_prefix = IN6_IS_ADDR_V4MAPPED(&route_->prefix);
>>> - bool is_ipv4_nexthop = route_->nexthop
>>> - ? IN6_IS_ADDR_V4MAPPED(route_->nexthop)
>>> + bool is_ipv4_prefix = IN6_IS_ADDR_V4MAPPED(&route->prefix);
>>> + bool is_ipv4_nexthop = route->nexthop
>>> + ? IN6_IS_ADDR_V4MAPPED(route->nexthop)
>>> : is_ipv4_prefix;
>>>
>>> - char *prefix_s = build_route_prefix_s(&route_->prefix, route_->plen);
>>> - add_route(lflows, route_->is_discard_route ? od : route_->out_port->od,
>>> - route_->out_port, route_->lrp_addr_s, prefix_s,
>>> - route_->plen, route_->nexthop, route_->is_src_route,
>>> - route_->route_table_id, bfd_ports,
>>> - route ? &route->header_ : &route_->out_port->nbrp->header_,
>>> - route_->is_discard_route, route_->source, lflow_ref,
>>> + char *prefix_s = build_route_prefix_s(&route->prefix, route->plen);
>>> + add_route(lflows, route->is_discard_route ? od : route->out_port->od,
>>> + route->out_port, route->lrp_addr_s, prefix_s,
>>> + route->plen, route->nexthop, route->is_src_route,
>>> + route->route_table_id, bfd_ports,
>>> + route->source_hint,
>>> + route->is_discard_route, route->source, lflow_ref,
>>> is_ipv4_prefix, is_ipv4_nexthop);
>>>
>>> free(prefix_s);
>>> diff --git a/northd/northd.h b/northd/northd.h
>>> index 2be34e249..385a46ade 100644
>>> --- a/northd/northd.h
>>> +++ b/northd/northd.h
>>> @@ -185,6 +185,10 @@ struct routes_data {
>>> struct hmap bfd_active_connections;
>>> };
>>>
>>> +struct routes_sync_data {
>>> + struct hmap parsed_routes;
>>> +};
>>> +
>>
>> Unused leftover?
>>
>>> struct route_policies_data {
>>> struct hmap route_policies;
>>> struct hmap bfd_active_connections;
>>> @@ -701,6 +705,8 @@ enum route_source {
>>> ROUTE_SOURCE_CONNECTED,
>>> /* The route is derived from a northbound static route entry. */
>>> ROUTE_SOURCE_STATIC,
>>> + /* the route is learned by an ovn-controller */
>>
>> Comments should be sentences. Also, I think I'd rephrase it as:
>>
>> The route is dynamically learned.
>>
>>> + ROUTE_SOURCE_LEARNED,
>>> };
>>>
>>> struct parsed_route {
>>> @@ -711,17 +717,41 @@ struct parsed_route {
>>> bool is_src_route;
>>> uint32_t route_table_id;
>>> uint32_t hash;
>>> - const struct nbrec_logical_router_static_route *route;
>>> bool ecmp_symmetric_reply;
>>> bool is_discard_route;
>>> const struct ovn_datapath *od;
>>> bool stale;
>>> struct sset ecmp_selection_fields;
>>> enum route_source source;
>>> + const struct ovsdb_idl_row *source_hint;
>>> char *lrp_addr_s;
>>> const struct ovn_port *out_port;
>>> };
>>>
>>> +struct parsed_route * parsed_route_clone(const struct parsed_route *pr);
>>
>> Spacing: " * ".
>>
>> Nit: argument name ("pr") can be omitted.
>>
>>> +size_t parsed_route_hash(const struct parsed_route *pr);
>>
>> Nit: argument name ("pr") can be omitted.
>>
>>> +void parsed_route_free(struct parsed_route *pr);
>>
>> Nit: argument name ("pr") can be omitted.
>>
>>> +void parsed_route_add(const struct ovn_datapath *od,
>>> + struct in6_addr *nexthop,
>>> + const struct in6_addr *prefix,
>>> + unsigned int plen,
>>> + bool is_discard_route,
>>> + const char *lrp_addr_s,
>>> + const struct ovn_port *out_port,
>>> + uint32_t route_table_id,
>>> + bool is_src_route,
>>> + bool ecmp_symmetric_reply,
>>> + const struct sset *ecmp_selection_fields,
>>> + enum route_source source,
>>> + const struct ovsdb_idl_row *source_hint,
>>> + struct hmap *routes);
>>> +
>>> +bool
>>> +find_route_outport(const struct hmap *lr_ports, const char *output_port,
>>> + const char *ip_prefix, const char *nexthop, bool
>>> is_ipv4,
>>> + bool force_out_port,
>>> + struct ovn_port **out_port, const char **lrp_addr_s);
>>> +
>>> void ovnnb_db_run(struct northd_input *input_data,
>>> struct northd_data *data,
>>> struct ovsdb_idl_txn *ovnnb_txn,
>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>> index 370f1d38d..b1c88fd8e 100644
>>> --- a/tests/ovn-northd.at
>>> +++ b/tests/ovn-northd.at
>>> @@ -6823,9 +6823,9 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows |
>>> ovn_strip_lflows], [0], [dnl
>>> table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> table=??(lr_in_ip_routing ), priority=10300,
>>> match=(ct_mark.ecmp_reply_port == 1 && reg7 == 0 && ip4.dst == 1.0.0.1/32),
>>> action=(ip.ttl--; flags.loopback = 1; eth.src = 00:00:20:20:12:13; reg5 =
>>> 192.168.0.1; outport = "lr0-public"; next;)
>>> table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> - table=??(lr_in_ip_routing ), priority=124 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=162 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=260 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> ])
>>>
>>> AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | ovn_strip_lflows],
>>> [0], [dnl
>>> @@ -6841,9 +6841,9 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows |
>>> ovn_strip_lflows], [0], [dnl
>>> table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> table=??(lr_in_ip_routing ), priority=10300,
>>> match=(ct_mark.ecmp_reply_port == 1 && reg7 == 0 && ip4.dst == 1.0.0.1/32),
>>> action=(ip.ttl--; flags.loopback = 1; eth.src = 00:00:20:20:12:13; reg5 =
>>> 192.168.0.1; outport = "lr0-public"; next;)
>>> table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> - table=??(lr_in_ip_routing ), priority=124 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=162 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(1, 2);)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=260 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(1, 2);)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> ])
>>> AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed
>>> 's/192\.168\.0\..0/192.168.0.??/' | ovn_strip_lflows], [0], [dnl
>>> table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1),
>>> action=(drop;)
>>> @@ -6870,9 +6870,9 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows |
>>> ovn_strip_lflows], [0], [dnl
>>> table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> table=??(lr_in_ip_routing ), priority=10300,
>>> match=(ct_mark.ecmp_reply_port == 1 && reg7 == 0 && ip4.dst == 1.0.0.1/32),
>>> action=(ip.ttl--; flags.loopback = 1; eth.src = 00:00:20:20:12:13; reg5 =
>>> 192.168.0.1; outport = "lr0-public"; next;)
>>> table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> - table=??(lr_in_ip_routing ), priority=124 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=162 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(1, 2);)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=260 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(1, 2);)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> ])
>>> AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed
>>> 's/192\.168\.0\..0/192.168.0.??/' | ovn_strip_lflows], [0], [dnl
>>> table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1),
>>> action=(drop;)
>>> @@ -6888,14 +6888,14 @@ check ovn-nbctl --wait=sb lr-route-add lr0
>>> 1.0.0.0/24 192.168.0.10
>>> ovn-sbctl dump-flows lr0 > lr0flows
>>>
>>> AT_CHECK([grep -e "lr_in_ip_routing.*192.168.0.10" lr0flows |
>>> ovn_strip_lflows], [0], [dnl
>>> - table=??(lr_in_ip_routing ), priority=122 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 1.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> ])
>>>
>>> check ovn-nbctl --wait=sb lr-route-add lr0 2.0.0.0/24 lr0-public
>>>
>>> ovn-sbctl dump-flows lr0 > lr0flows
>>> AT_CHECK([grep -e "lr_in_ip_routing.*2.0.0.0" lr0flows |
>>> ovn_strip_lflows], [0], [dnl
>>> - table=??(lr_in_ip_routing ), priority=122 , match=(reg7 == 0 &&
>>> ip4.dst == 2.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> ip4.dst; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 2.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> ip4.dst; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> ])
>>>
>>> check ovn-nbctl lr-route-add lr0 3.3.0.0/16 192.168.0.11
>>> @@ -6910,7 +6910,7 @@ check ovn-nbctl set logical_router_static_route
>>> $route2_uuid selection_fields="i
>>> check ovn-nbctl --wait=sb sync
>>> ovn-sbctl dump-flows lr0 > lr0flows
>>> AT_CHECK([grep -e "(lr_in_ip_routing ).*3.3.0.0" lr0flows | sed
>>> 's/table=../table=??/' | sort], [0], [dnl
>>> - table=??(lr_in_ip_routing ), priority=82 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src");)
>>> + table=??(lr_in_ip_routing ), priority=132 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src");)
>>> ])
>>>
>>> check ovn-nbctl set logical_router_static_route $route1_uuid
>>> selection_fields="ip_src,ip_dst,tp_src,tp_dst"
>>> @@ -6919,10 +6919,10 @@ check ovn-nbctl set logical_router_static_route
>>> $route2_uuid selection_fields="i
>>> check ovn-nbctl --wait=sb sync
>>> ovn-sbctl dump-flows lr0 > lr0flows
>>> AT_CHECK([grep -e "(lr_in_ip_routing ).*3.3.0.0" lr0flows | sed
>>> 's/table=../table=??/' | sort], [0], [dnl
>>> - table=??(lr_in_ip_routing ), priority=82 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src");)
>>> - table=??(lr_in_ip_routing ), priority=83 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16 && sctp), action=(ip.ttl--; flags.loopback = 1;
>>> reg8[[0..15]] = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src,sctp_dst,sctp_src");)
>>> - table=??(lr_in_ip_routing ), priority=83 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16 && tcp), action=(ip.ttl--; flags.loopback = 1;
>>> reg8[[0..15]] = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src,tcp_dst,tcp_src");)
>>> - table=??(lr_in_ip_routing ), priority=83 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16 && udp), action=(ip.ttl--; flags.loopback = 1;
>>> reg8[[0..15]] = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src,udp_dst,udp_src");)
>>> + table=??(lr_in_ip_routing ), priority=132 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]]
>>> = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src");)
>>> + table=??(lr_in_ip_routing ), priority=133 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16 && sctp), action=(ip.ttl--; flags.loopback = 1;
>>> reg8[[0..15]] = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src,sctp_dst,sctp_src");)
>>> + table=??(lr_in_ip_routing ), priority=133 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16 && tcp), action=(ip.ttl--; flags.loopback = 1;
>>> reg8[[0..15]] = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src,tcp_dst,tcp_src");)
>>> + table=??(lr_in_ip_routing ), priority=133 , match=(reg7 == 0 &&
>>> ip4.dst == 3.3.0.0/16 && udp), action=(ip.ttl--; flags.loopback = 1;
>>> reg8[[0..15]] = 1; reg8[[16..31]] = select(values=(1, 2);
>>> hash_fields="ip_dst,ip_proto,ip_src,udp_dst,udp_src");)
>>> ])
>>>
>>> AT_CLEANUP
>>> @@ -6960,14 +6960,14 @@ ovn-sbctl dump-flows lr0 > lr0flows
>>> AT_CHECK([grep -e "lr_in_ip_routing " lr0flows | ovn_strip_lflows], [0],
>>> [dnl
>>> table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> - table=??(lr_in_ip_routing ), priority=122 , match=(reg7 == 0 &&
>>> ip4.dst == 10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=122 , match=(reg7 == 0 &&
>>> ip4.dst == 11.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> 2001:db8::10; xxreg1 = 2001:db8::1; eth.src = 00:00:20:20:12:14; outport =
>>> "lr0-private"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> - table=??(lr_in_ip_routing ), priority=124 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=322 , match=(reg7 == 0 &&
>>> ip6.dst == 2001:db8:1::/64), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.20; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=322 , match=(reg7 == 0 &&
>>> ip6.dst == 2001:db8:2::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> 2001:db8::20; xxreg1 = 2001:db8::1; eth.src = 00:00:20:20:12:14; outport =
>>> "lr0-private"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport ==
>>> "lr0-private" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] =
>>> 0; xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1214; eth.src =
>>> 00:00:20:20:12:14; outport = "lr0-private"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(ip6.dst ==
>>> 2001:db8::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = ip6.dst;
>>> xxreg1 = 2001:db8::1; eth.src = 00:00:20:20:12:14; outport = "lr0-private";
>>> flags.loopback = 1; reg9[[9]] = 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 11.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> 2001:db8::10; xxreg1 = 2001:db8::1; eth.src = 00:00:20:20:12:14; outport =
>>> "lr0-private"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=516 , match=(reg7 == 0 &&
>>> ip6.dst == 2001:db8:1::/64), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.20; reg5 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport =
>>> "lr0-public"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=516 , match=(reg7 == 0 &&
>>> ip6.dst == 2001:db8:2::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> 2001:db8::20; xxreg1 = 2001:db8::1; eth.src = 00:00:20:20:12:14; outport =
>>> "lr0-private"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-private" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] =
>>> 0; xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1214; eth.src =
>>> 00:00:20:20:12:14; outport = "lr0-private"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src =
>>> 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; reg9[[9]] =
>>> 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(ip6.dst ==
>>> 2001:db8::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = ip6.dst;
>>> xxreg1 = 2001:db8::1; eth.src = 00:00:20:20:12:14; outport = "lr0-private";
>>> flags.loopback = 1; reg9[[9]] = 0; next;)
>>> ])
>>>
>>> AT_CHECK([grep -e "lr_in_arp_resolve" lr0flows | ovn_strip_lflows], [0],
>>> [dnl
>>> @@ -7406,16 +7406,16 @@ AT_CHECK([grep "lr_in_ip_routing_pre" lr0flows |
>>> ovn_strip_lflows], [0], [dnl
>>> grep -e "(lr_in_ip_routing ).*outport" lr0flows
>>>
>>> AT_CHECK([grep -e "(lr_in_ip_routing ).*outport" lr0flows |
>>> ovn_strip_lflows], [0], [dnl
>>> - table=??(lr_in_ip_routing ), priority=122 , match=(reg7 == 1 &&
>>> ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.1.10; reg5 = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport =
>>> "lrp1"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=124 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = "lrp0";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=124 , match=(ip4.dst ==
>>> 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport = "lrp1";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=124 , match=(ip4.dst ==
>>> 192.168.2.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.2.1; eth.src = 00:00:00:00:02:01; outport = "lrp2";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=162 , match=(reg7 == 2 &&
>>> ip4.dst == 1.1.1.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.20; reg5 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport =
>>> "lrp0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=2 , match=(reg7 == 0 &&
>>> ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport =
>>> "lrp0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=2 , match=(reg7 == 2 &&
>>> ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport =
>>> "lrp0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport == "lrp0"
>>> && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> ip6.dst; xxreg1 = fe80::200:ff:fe00:1; eth.src = 00:00:00:00:00:01; outport
>>> = "lrp0"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport == "lrp1"
>>> && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> ip6.dst; xxreg1 = fe80::200:ff:fe00:101; eth.src = 00:00:00:00:01:01;
>>> outport = "lrp1"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> - table=??(lr_in_ip_routing ), priority=324 , match=(inport == "lrp2"
>>> && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> ip6.dst; xxreg1 = fe80::200:ff:fe00:201; eth.src = 00:00:00:00:02:01;
>>> outport = "lrp2"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 1 &&
>>> ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.1.10; reg5 = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport =
>>> "lrp1"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = "lrp0";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport = "lrp1";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 192.168.2.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5
>>> = 192.168.2.1; eth.src = 00:00:00:00:02:01; outport = "lrp2";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=260 , match=(reg7 == 2 &&
>>> ip4.dst == 1.1.1.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.20; reg5 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport =
>>> "lrp0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=4 , match=(reg7 == 0 &&
>>> ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport =
>>> "lrp0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=4 , match=(reg7 == 2 &&
>>> ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 192.168.0.10; reg5 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport =
>>> "lrp0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport == "lrp0"
>>> && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> ip6.dst; xxreg1 = fe80::200:ff:fe00:1; eth.src = 00:00:00:00:00:01; outport
>>> = "lrp0"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport == "lrp1"
>>> && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> ip6.dst; xxreg1 = fe80::200:ff:fe00:101; eth.src = 00:00:00:00:01:01;
>>> outport = "lrp1"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport == "lrp2"
>>> && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 =
>>> ip6.dst; xxreg1 = fe80::200:ff:fe00:201; eth.src = 00:00:00:00:02:01;
>>> outport = "lrp2"; flags.loopback = 1; reg9[[9]] = 0; next;)
>>> ])
>>>
>>> AT_CLEANUP
>>> @@ -14500,3 +14500,95 @@ AT_CHECK([ovn-sbctl --columns ip_prefix --bare
>>> find Advertised_Route datapath=$d
>>> AT_CLEANUP
>>> ])
>>>
>>> +OVN_FOR_EACH_NORTHD_NO_HV([
>>> +AT_SETUP([dynamic-routing - learning routes from sb])
>>> +AT_KEYWORDS([dynamic-routing])
>>> +ovn_start
>>> +
>>> +# we start with announcing routes on a lr with 2 lrps and 2 static routes
>>> +check ovn-nbctl lr-add lr0
>>> +check ovn-nbctl --wait=sb set Logical_Router lr0
>>> option:dynamic-routing=true \
>>> + option:dynamic-routing-connected=true \
>>> + option:dynamic-routing-static=true
>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
>>> +sw0=$(ovn-sbctl --bare --columns _uuid list port_binding lr0-sw0)
>>
>> Please use fetch_column instead.
>>
>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 10.0.1.1/24
>>> +sw1=$(ovn-sbctl --bare --columns _uuid list port_binding lr0-sw1)
>>
>> Please use fetch_column instead.
>>
>>> +check ovn-nbctl --wait=sb lr-route-add lr0 192.168.0.0/24 10.0.0.10
>>> +check ovn-nbctl --wait=sb lr-route-add lr0 192.168.1.0/24 10.0.1.10
>>> +check_row_count Advertised_Route 4
>>> +datapath=$(ovn-sbctl --bare --columns _uuid list datapath_binding lr0)
>>
>> Please use fetch_column instead.
>>
>>> +
>>> +# validate the routes
>>> +ovn-sbctl dump-flows lr0 > lr0flows
>>> +AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0],
>>> [dnl
>>> + table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.0.10; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport =
>>> "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.1.10; reg5 = 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport =
>>> "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw0" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff01; eth.src =
>>> 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw1" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff02; eth.src =
>>> 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> +])
>>> +
>>> +# learn a route to 172.16.0.0/24 via 10.0.0.11 learned on lr0-sw0
>>> +ovn-sbctl create Learned_Route datapath=$datapath logical_port=$sw0
>>> ip_prefix=172.16.0.0/24 nexthop=10.0.0.11
>>> +check ovn-nbctl --wait=sb sync
>>> +check_row_count Advertised_Route 4
>>> +check_row_count Learned_Route 1
>>> +ovn-sbctl dump-flows lr0 > lr0flows
>>> +AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0],
>>> [dnl
>>> + table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=194 , match=(reg7 == 0 &&
>>> ip4.dst == 172.16.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.0.11; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport =
>>> "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.0.10; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport =
>>> "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.1.10; reg5 = 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport =
>>> "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw0" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff01; eth.src =
>>> 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw1" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff02; eth.src =
>>> 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> +])
>>> +
>>> +# learn a route to 172.16.1.0/24 via 100.100.100.100 learned on lr0-sw1
>>> +# this is not reachable so will not produce a lflow
>>> +ovn-sbctl create Learned_Route datapath=$datapath logical_port=$sw1
>>> ip_prefix=172.16.1.0/24 nexthop=100.100.100.100
>>> +check ovn-nbctl --wait=sb sync
>>> +check_row_count Advertised_Route 4
>>> +check_row_count Learned_Route 2
>>> +ovn-sbctl dump-flows lr0 > lr0flows
>>> +AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0],
>>> [dnl
>>> + table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=194 , match=(reg7 == 0 &&
>>> ip4.dst == 172.16.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.0.11; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport =
>>> "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.0.10; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport =
>>> "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.1.10; reg5 = 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport =
>>> "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw0" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff01; eth.src =
>>> 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw1" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff02; eth.src =
>>> 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> +])
>>> +
>>> +# if we now add 100.100.100.10/24 as an additional network to lr0-sw1 we
>>> will
>>> +# get another connected route and the previous received route will be
>>> active
>>> +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw1
>>> networks="10.0.1.1/24 100.100.100.10/24"
>>> +check_row_count Advertised_Route 5
>>> +check_row_count Learned_Route 2
>>> +ovn-sbctl dump-flows lr0 > lr0flows
>>> +AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0],
>>> [dnl
>>> + table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
>>> action=(drop;)
>>> + table=??(lr_in_ip_routing ), priority=194 , match=(reg7 == 0 &&
>>> ip4.dst == 172.16.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.0.11; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport =
>>> "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=194 , match=(reg7 == 0 &&
>>> ip4.dst == 172.16.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 100.100.100.100; reg5 = 100.100.100.10; eth.src = 00:00:00:00:ff:02;
>>> outport = "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.0.10; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport =
>>> "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 &&
>>> ip4.dst == 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
>>> 10.0.1.10; reg5 = 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport =
>>> "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 10.0.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
>>> 10.0.1.1; eth.src = 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback
>>> = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
>>> 100.100.100.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst;
>>> reg5 = 100.100.100.10; eth.src = 00:00:00:00:ff:02; outport = "lr0-sw1";
>>> flags.loopback = 1; reg9[[9]] = 1; next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw0" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff01; eth.src =
>>> 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> + table=??(lr_in_ip_routing ), priority=518 , match=(inport ==
>>> "lr0-sw1" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0;
>>> xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff02; eth.src =
>>> 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 0;
>>> next;)
>>> +])
>>> +
>>> +AT_CLEANUP
>>> +])
>>> +
>>
>> Should we update the test to cover IPv6 too?
>>
>> Thanks,
>> Dumitru
>>
>
> Thanks for the review
> Felix
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
