> Logical router adv in ovn-ic the sb learned routes from
> dynamic routing. Also, it doesn't learn route in ovn-ic
> if prefix is already learned in sb learned route table.
Hi Lucas,
I think the patch is fine, just few nit inline.
Regards,
Lorenzo
>
> Signed-off-by: Lucas Vargas Dias <lucas.vd...@luizalabs.com>
> ---
> ic/ovn-ic.c | 102 +++++++++++++++++++++++++++++++++----
> tests/ovn-ic.at | 130 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 223 insertions(+), 9 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index d1a59d372..5bf576eac 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -70,6 +70,7 @@ struct ic_context {
> struct ovsdb_idl_index *nbrec_port_by_name;
> struct ovsdb_idl_index *sbrec_chassis_by_name;
> struct ovsdb_idl_index *sbrec_port_binding_by_name;
> + struct ovsdb_idl_index *sbrec_datapath_binding_by_nb_uuid;
> struct ovsdb_idl_index *sbrec_service_monitor_by_remote_type;
> struct ovsdb_idl_index *sbrec_service_monitor_by_ic_learned;
> struct ovsdb_idl_index
> *sbrec_service_monitor_by_remote_type_logical_port;
> @@ -513,6 +514,21 @@ find_sb_pb_by_name(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> return pb;
> }
>
> +static const struct sbrec_datapath_binding *
> +find_dp_by_uuid(struct ovsdb_idl_index *sbrec_datapath_binding,
> + const struct uuid *nb_uuid)
> +{
> + const struct sbrec_datapath_binding *key =
> + sbrec_datapath_binding_index_init_row(sbrec_datapath_binding);
> + sbrec_datapath_binding_set_nb_uuid(key, nb_uuid, 1);
> +
> + const struct sbrec_datapath_binding *dp =
> + sbrec_datapath_binding_index_find(sbrec_datapath_binding, key);
> + sbrec_datapath_binding_index_destroy_row(key);
> +
> + return dp;
> +}
> +
> static const struct sbrec_port_binding *
> find_peer_port(struct ic_context *ctx,
> const struct sbrec_port_binding *sb_pb)
> @@ -1191,7 +1207,7 @@ add_to_routes_ad(struct hmap *routes_ad, const struct
> in6_addr prefix,
> const struct nbrec_load_balancer *nb_lb,
> const char *route_tag)
> {
> - ovs_assert(nb_route || nb_lrp || nb_lb);
> + ovs_assert(nb_route || nb_lrp || nb_lb || nb_lr);
>
> if (route_table == NULL) {
> route_table = "";
> @@ -1223,9 +1239,12 @@ add_to_routes_ad(struct hmap *routes_ad, const struct
> in6_addr prefix,
> } else if (nb_lb) {
> VLOG_WARN_RL(&rl, msg_fmt, origin, "loadbalancer",
> UUID_ARGS(&nb_lb->header_.uuid));
> - } else {
> + } else if (nb_lrp){
> VLOG_WARN_RL(&rl, msg_fmt, origin, "lrp",
> UUID_ARGS(&nb_lrp->header_.uuid));
> + } else {
> + VLOG_WARN_RL(&rl, msg_fmt, origin, "route",
> + UUID_ARGS(&nb_lr->header_.uuid));
> }
> }
> }
> @@ -1301,8 +1320,16 @@ add_network_to_routes_ad(struct hmap *routes_ad, const
> char *network,
>
> if (!route_need_advertise(NULL, &prefix, plen, nb_options,
> nb_lr, ts_lrp)) {
> - VLOG_DBG("Route ad: skip network %s of lrp %s.",
> - network, nb_lrp->name);
> + if (VLOG_IS_DBG_ENABLED()) {
> + struct ds msg = DS_EMPTY_INITIALIZER;
> + ds_put_format(&msg, "Route ad: skip network %s", network);
> + if (nb_lrp) {
> + ds_put_format(&msg, " of lrp %s", nb_lrp->name);
> + }
> + ds_put_format(&msg, ".");
> + VLOG_DBG("%s", ds_cstr(&msg));
> + ds_destroy(&msg);
> + }
> return;
> }
>
> @@ -1316,8 +1343,12 @@ add_network_to_routes_ad(struct hmap *routes_ad, const
> char *network,
> struct ds msg = DS_EMPTY_INITIALIZER;
>
> ds_put_format(&msg, "Adding direct network route to <main> routing "
> - "table: %s of lrp %s, nexthop ", network,
> nb_lrp->name);
> + "table: %s", network);
>
> + if (nb_lrp) {
> + ds_put_format(&msg, " of lrp %s,", nb_lrp->name);
> + }
> + ds_put_format(&msg, " nexthop ");
> if (IN6_IS_ADDR_V4MAPPED(&nexthop)) {
> ds_put_format(&msg, IP_FMT,
> IP_ARGS(in6_addr_get_mapped_ipv4(&nexthop)));
> @@ -1498,7 +1529,8 @@ route_matches_local_lb(const struct nbrec_load_balancer
> *nb_lb,
> }
>
> static bool
> -route_need_learn(const struct nbrec_logical_router *lr,
> +route_need_learn(struct ic_context *ctx,
> + const struct nbrec_logical_router *lr,
> const struct icsbrec_route *isb_route,
> struct in6_addr *prefix, unsigned int plen,
> const struct smap *nb_options,
> @@ -1564,6 +1596,23 @@ route_need_learn(const struct nbrec_logical_router *lr,
> }
> }
>
> + const struct sbrec_datapath_binding *dp =
> + find_dp_by_uuid(ctx->sbrec_datapath_binding_by_nb_uuid,
> + &lr->header_.uuid);
> + if (!dp) {
> + return true;
> + }
> + const struct sbrec_learned_route_table *sbrec_learned_route_table =
> +
> sbrec_learned_route_table_get(ctx->ovnsb_idl);
> + const struct sbrec_learned_route *sb_route;
Maybe we can try to use an index here to speed-up the lookup? Not sure if it
is worth doing so. What do you think?
> + SBREC_LEARNED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route,
> + sbrec_learned_route_table) {
> + if (uuid_equals(&sb_route->datapath->header_.uuid, &dp->header_.uuid)
> + && !strcmp(isb_route->ip_prefix, sb_route->ip_prefix)) {
> + return false;
> + }
> + }
> +
> return true;
> }
>
> @@ -1686,7 +1735,7 @@ sync_learned_routes(struct ic_context *ctx,
> const struct nbrec_logical_router_port *lrp;
> VECTOR_FOR_EACH (&ic_lr->isb_pbs, isb_pb) {
> lrp_name = get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port);
> - lrp = get_lrp_by_lrp_name(ctx, lrp_name);
> + lrp = lrp_name ? get_lrp_by_lrp_name(ctx, lrp_name) : NULL;
> if (lrp) {
> ts_route_table = smap_get_def(&lrp->options, "route_table", "");
> route_filter_tag = smap_get_def(&lrp->options,
> @@ -1745,7 +1794,7 @@ sync_learned_routes(struct ic_context *ctx,
> isb_route->nexthop);
> continue;
> }
> - if (!route_need_learn(ic_lr->lr, isb_route, &prefix, plen,
> + if (!route_need_learn(ctx, ic_lr->lr, isb_route, &prefix, plen,
> &nb_global->options, lrp, &nexthop)) {
> continue;
> }
> @@ -1816,7 +1865,8 @@ ad_route_sync_external_ids(const struct ic_route_info
> *route_adv,
> smap_get_uuid(&isb_route->external_ids, "lr-id", &isb_ext_lr_id);
> nb_id = route_adv->nb_lb ? route_adv->nb_lb->header_.uuid :
> route_adv->nb_route ? route_adv->nb_route->header_.uuid :
> - route_adv->nb_lrp->header_.uuid;
> + route_adv->nb_lrp ? route_adv->nb_lrp->header_.uuid :
> + route_adv->nb_lr->header_.uuid;
>
> lr_id = route_adv->nb_lr->header_.uuid;
> if (!uuid_equals(&isb_ext_id, &nb_id)) {
> @@ -2008,6 +2058,26 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> }
> }
> }
> +
> + const struct sbrec_datapath_binding *dp =
> + find_dp_by_uuid(ctx->sbrec_datapath_binding_by_nb_uuid,
> + &lr->header_.uuid);
> + if (!dp) {
> + return;
> + }
> + const struct sbrec_learned_route_table *sbrec_learned_route_table =
> +
> sbrec_learned_route_table_get(ctx->ovnsb_idl);
> + const struct sbrec_learned_route *sb_route;
> + SBREC_LEARNED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route,
> + sbrec_learned_route_table) {
> + if (uuid_equals(&sb_route->datapath->header_.uuid,
> + &dp->header_.uuid)) {
> + add_network_to_routes_ad(routes_ad, sb_route->ip_prefix, NULL,
> + ts_port_addrs,
> + &nb_global->options,
> + lr, route_tag, ts_lrp);
> + }
> + }
> }
>
> static void
> @@ -3067,6 +3137,15 @@ main(int argc, char *argv[])
> ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> &sbrec_service_monitor_col_options);
>
> + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_learned_route);
> + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> + &sbrec_learned_route_col_nexthop);
> + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> + &sbrec_learned_route_col_ip_prefix);
> + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> + &sbrec_learned_route_col_logical_port);
> + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> + &sbrec_learned_route_col_datapath);
> /* Create IDL indexes */
> struct ovsdb_idl_index *nbrec_ls_by_name
> = ovsdb_idl_index_create1(ovnnb_idl_loop.idl,
> @@ -3080,6 +3159,9 @@ main(int argc, char *argv[])
> struct ovsdb_idl_index *sbrec_port_binding_by_name
> = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> &sbrec_port_binding_col_logical_port);
> + struct ovsdb_idl_index *sbrec_datapath_binding_by_nb_uuid
> + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> + &sbrec_datapath_binding_col_nb_uuid);
> struct ovsdb_idl_index *sbrec_chassis_by_name
> = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> &sbrec_chassis_col_name);
> @@ -3190,6 +3272,8 @@ main(int argc, char *argv[])
> .nbrec_lrp_by_name = nbrec_lrp_by_name,
> .nbrec_port_by_name = nbrec_port_by_name,
> .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
> + .sbrec_datapath_binding_by_nb_uuid =
> + sbrec_datapath_binding_by_nb_uuid,
> .sbrec_chassis_by_name = sbrec_chassis_by_name,
> .sbrec_service_monitor_by_remote_type =
> sbrec_service_monitor_by_remote_type,
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index 368b6701b..cf280ea7b 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -4047,3 +4047,133 @@ OVN_CLEANUP_IC([az1], [az2], [az3])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-ic -- Check ovn-ic adv and learn from SB Learned Route])
> +
> +ovn_init_ic_db
> +
> +for i in 1 2; do
> + ovn_start az$i
> + ovn_as az$i
> +
> + # Enable route learning at AZ level
> + check ovn-nbctl set nb_global . options:ic-route-learn=true
> + # Enable route advertising at AZ level
> + check ovn-nbctl set nb_global . options:ic-route-adv=true
> +done
> +
> +# Create new transit switches and LRs. Test topology is next:
> +#
> +#
I guess you can drop a new-line here.
> +# logical router (lr11) - transit switch (ts11) - logical router (lr12)
> +#
> +#
same here.
> +
> +# Create lr11, lr12 and ts11 and connect them
> +for i in 1 2; do
> + ovn_as az$i
> +
> + lr=lr1$i
> + check ovn-nbctl lr-add $lr
> +
> + for j in 1; do
you can drop the for here.
> + ts=ts1$j
> + check ovn-ic-nbctl --wait=sb --may-exist ts-add $ts
> +
> + lrp=lrp-$lr-$ts
> + lsp=lsp-$ts-$lr
> + # Create LRP and connect to TS
> + check ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:a$j:0$i
> 169.254.10$j.$i/24
> + check ovn-nbctl lsp-add $ts $lsp \
> + -- lsp-set-addresses $lsp router \
> + -- lsp-set-type $lsp router \
> + -- lsp-set-options $lsp router-port=$lrp
> + done
> +done
> +
> +# Create directly-connected route in lr11
> +check ovn_as az2 ovn-nbctl lrp-add lr12 lrp-lr12 aa:aa:aa:aa:bb:01
> "192.168.0.1/24"
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.0.0/24 169.254.101.2
> +])
> +
> +ovn_as az2
> +check ovn-nbctl --wait=sb set Logical_Router lr12
> option:dynamic-routing=true \
> + option:dynamic-routing-redistribute="connected,static"
> +check ovn_as az2 ovn-nbctl --wait=sb lrp-add lr12 lr12-dr1 00:00:00:00:ff:01
> 10.0.0.1/24
> +dr1=$(fetch_column port_binding _uuid logical_port=lr12-dr1)
> +datapath=$(fetch_column datapath_binding _uuid external_ids:name=lr12)
> +
> +check_uuid ovn-sbctl create Learned_Route \
> + datapath=$datapath \
> + logical_port=$dr1 \
> + ip_prefix=192.168.1.0/24 \
> + nexthop=10.0.0.20
> +
> +# Check Learned_Route adv in ovn-ic
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.0.0/24 169.254.101.2
> +192.168.1.0/24 169.254.101.2
> +])
> +
> +ovn_as az2
> +learned=$(fetch_column Learned_Route _uuid ip_prefix=192.168.1.0/24)
> +check ovn-sbctl destroy Learned_Route $learned
> +
> +# Check Learned_Route removed is not adv and learned.
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.0.0/24 169.254.101.2
> +])
> +
drop new-line here.
> +
> +# Duplicate routes, SB learn route by two paths.
> +ovn_as az2
> +check_uuid ovn-sbctl create Learned_Route \
> + datapath=$datapath \
> + logical_port=$dr1 \
> + ip_prefix=192.168.1.0/24 \
> + nexthop=10.0.0.20
> +
> +check ovn-nbctl --wait=sb lrp-add lr12 lr12-dr2 00:00:00:00:ff:02
> 169.254.254.1/24
> +dr2=$(fetch_column port_binding _uuid logical_port=lr12-dr2)
> +
> +check_uuid ovn-sbctl create Learned_Route \
> + datapath=$datapath \
> + logical_port=$dr2 \
> + ip_prefix=192.168.1.0/24 \
> + nexthop=169.254.254.3
> +
> +
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.0.0/24 169.254.101.2
> +192.168.1.0/24 169.254.101.2
> +])
> +
> +# Insert the same route Learned Route in the lr11 and don't learn by ovn-ic.
> +ovn_as az1
> +datapath_lr11=$(fetch_column datapath_binding _uuid external_ids:name=lr11)
> +check ovn-nbctl --wait=sb lrp-add lr11 lr11-dr1 00:00:00:00:ff:03
> 169.254.254.2/24
> +sw1_dr1=$(fetch_column port_binding _uuid logical_port=lr11-dr1)
> +check_uuid ovn-sbctl create Learned_Route \
> + datapath=$datapath_lr11 \
> + logical_port=$sw1_dr1 \
> + ip_prefix=192.168.1.0/24 \
> + nexthop=169.254.254.3
> +
> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.0.0/24 169.254.101.2
> +])
I guess you can check we are using the 'local' route here.
> +
> +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl lr-route-list lr12 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +])
> +
> +OVN_CLEANUP_IC([az1], [az2])
> +
> +AT_CLEANUP
> +])
> --
> 2.34.1
>
>
> --
>
>
>
>
> _'Esta mensagem é direcionada apenas para os endereços constantes no
> cabeçalho inicial. Se você não está listado nos endereços constantes no
> cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa
> mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão
> imediatamente anuladas e proibidas'._
>
>
> * **'Apesar do Magazine Luiza tomar
> todas as precauções razoáveis para assegurar que nenhum vírus esteja
> presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por
> quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.*
>
>
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
