On 12/2/22 18:31, Vladislav Odintsov wrote:
> 1. Remove excess nbrec_logical_router variable.
> 2. Remove excess call to add_static_to_routes_ad().
> 3. Remove double route_table check in ic_route_fin().
Nit: s/route_table/nexthop/
> 4. Move variable declarations out of loop.
>
> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com>
> ---
> ic/ovn-ic.c | 31 ++++++++++---------------------
> 1 file changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 3e02b4c98..59468545d 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -895,8 +895,7 @@ ic_route_find(struct hmap *routes, const struct in6_addr
> *prefix,
> r->plen == plen &&
> ipv6_addr_equals(&r->nexthop, nexthop) &&
> !strcmp(r->origin, origin) &&
> - !strcmp(r->route_table ? r->route_table : "", route_table) &&
> - ipv6_addr_equals(&r->nexthop, nexthop)) {
> + !strcmp(r->route_table ? r->route_table : "", route_table)) {
> return r;
> }
> }
> @@ -1109,17 +1108,8 @@ add_static_to_routes_ad(
> struct hmap *routes_ad,
> const struct nbrec_logical_router_static_route *nb_route,
> const struct lport_addresses *nexthop_addresses,
> - const struct smap *nb_options, const char *route_table)
> + const struct smap *nb_options)
> {
> - if (strcmp(route_table, nb_route->route_table)) {
> - if (VLOG_IS_DBG_ENABLED()) {
> - VLOG_DBG("Skip advertising route %s -> %s as its route table %s
> !="
> - " %s of TS port", nb_route->ip_prefix,
> nb_route->nexthop,
> - nb_route->route_table, route_table);
> - }
> - return;
> - }
> -
> struct in6_addr prefix, nexthop;
> unsigned int plen;
> if (!parse_route(nb_route->ip_prefix, nb_route->nexthop,
> @@ -1541,13 +1531,13 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> {
> const struct nbrec_logical_router *lr = ic_lr->lr;
>
> + const struct nbrec_logical_router_static_route *nb_route;
I'm not sure I agree with this one. Why not keep it inside the for loop
below. We don't use 'nb_route' afterwards AFAICT.
> + struct uuid id;
> +
> /* Check static routes of the LR */
> for (int i = 0; i < lr->n_static_routes; i++) {
> - const struct nbrec_logical_router_static_route *nb_route
> - = lr->static_routes[i];
> - struct uuid isb_uuid;
> - if (smap_get_uuid(&nb_route->external_ids, "ic-learned-route",
> - &isb_uuid)) {
> + nb_route = lr->static_routes[i];
> + if (smap_get_uuid(&nb_route->external_ids, "ic-learned-route", &id))
> {
> /* It is a learned route */
> if (!add_to_routes_learned(&ic_lr->routes_learned, nb_route)) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> @@ -1557,10 +1547,10 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> nbrec_logical_router_update_static_routes_delvalue(lr,
> nb_route);
> }
> - } else {
> + } else if (!strcmp(ts_route_table, nb_route->route_table)) {
> /* It may be a route to be advertised */
> add_static_to_routes_ad(routes_ad, nb_route, ts_port_addrs,
> - &nb_global->options, ts_route_table);
> + &nb_global->options);
> }
> }
>
> @@ -1593,7 +1583,6 @@ advertise_lr_routes(struct ic_context *ctx,
> const struct icsbrec_port_binding *isb_pb;
> const char *lrp_name, *ts_name, *route_table;
> struct lport_addresses ts_port_addrs;
> - const struct nbrec_logical_router *lr = ic_lr->lr;
> const struct icnbrec_transit_switch *key;
>
> struct hmap routes_ad = HMAP_INITIALIZER(&routes_ad);
> @@ -1611,7 +1600,7 @@ advertise_lr_routes(struct ic_context *ctx,
> VLOG_INFO_RL(&rl, "Route sync ignores port %s on ts %s for
> router"
> " %s because the addresses are invalid.",
> isb_pb->logical_port, isb_pb->transit_switch,
> - lr->name);
> + ic_lr->lr->name);
> continue;
> }
> lrp_name = get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port);
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
