On Mon, Jan 20, 2025 at 04:27:15PM +0100, Dumitru Ceara wrote:
> 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.
Thanks a lot, i will include that in v3.
>
> >>
> >>
> >>> 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.
Sounds good.
>
> >>
> >>> +
> >>> +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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
