From 84631245554c015617e7e7871d0602d1778ab14f Mon Sep 17 00:00:00 2001 From: Rosemarie O'Riorden Date: Fri, 13 Sep 2024 12:44:07 -0400 Subject: [PATCH] northd: fixed test output When preparing to build ECMP and static route flows, Reported-at: https://issues.redhat.com/browse/FDP-786 Signed-off-by: Rosemarie O'Riorden --- northd/northd.c | 48 +++++++++++++++++++++++++++++++++-------- northd/ovn-northd.8.xml | 3 ++- tests/ovn-northd.at | 6 ++++-- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 983c464ebf..83c824b2f2 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -11567,21 +11567,46 @@ build_ecmp_route_flow(struct lflow_table *lflows, struct ovn_datapath *od, struct ds actions = DS_EMPTY_INITIALIZER; ds_put_format(&actions, "ip.ttl--; flags.loopback = 1; %s = %"PRIu16 - "; %s = select(", REG_ECMP_GROUP_ID, eg->id, + "; %s = ", REG_ECMP_GROUP_ID, eg->id, //here, CONTAINER_OF casts list REG_ECMP_MEMBER_ID); - bool is_first = true; - LIST_FOR_EACH (er, list_node, &eg->route_list) { - if (is_first) { - is_first = false; - } else { - ds_put_cstr(&actions, ", "); + if (!ovs_list_is_singleton(&eg->route_list)) { + bool is_first = true; + ds_put_cstr(&actions, "select("); + LIST_FOR_EACH (er, list_node, &eg->route_list) { + if (is_first) { + is_first = false; + } else { + ds_put_cstr(&actions, ", "); + } + ds_put_format(&actions, "%"PRIu16, er->id); } - ds_put_format(&actions, "%"PRIu16, er->id); + ds_put_cstr(&actions, ");"); + } else { + ds_put_cstr(&actions, "N; next;"); } - ds_put_cstr(&actions, ");"); + /* + ds_clear(&actions); + if (rp->n_valid_nexthops > 1) { + ds_put_format(&actions, "%s = %"PRIu16 + "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, + REG_ECMP_MEMBER_ID); + + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { + if (i > 0) { + ds_put_cstr(&actions, ", "); + } + ds_put_format(&actions, "%"PRIuSIZE, i + 1); + } + ds_put_cstr(&actions, ");"); + } else { + ds_put_format(&actions, "%s = %"PRIu16 + "; %s = 1; next;", REG_ECMP_GROUP_ID, + ecmp_group_id, REG_ECMP_MEMBER_ID); + } + */ ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, priority, ds_cstr(&route_match), ds_cstr(&actions), lflow_ref); @@ -13543,6 +13568,11 @@ build_static_route_flows_for_lrouter( if (group) { ecmp_groups_add_route(group, route); } + } else if (route->ecmp_symmetric_reply) { + /* Traffic for symmetric reply routes has to be conntracked + * even if there is only one next-hop, in case another + * next-hop is added later. */ + ecmp_groups_add(&ecmp_groups, route); } else { unique_routes_add(&unique_routes, route); } diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index ede38882a4..a4de55052e 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -4328,7 +4328,8 @@ next; ip.ttl--; flags.loopback = 1; reg8[0..15] = GID; -select(reg8[16..31], MID1, MID2, ...); +select(reg8[16..31], MID1, MID2, ...); #fix order +#add that if there's only one route in ECMP group actions will be different diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index dcc3dbbc31..6d0d0697fe 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -6799,17 +6799,19 @@ check ovn-nbctl lsp-set-type public-lr0 router check ovn-nbctl lsp-set-addresses public-lr0 router check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public +# ECMP flows will be added even if there is only one next-hop. check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.0.10 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=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; reg1 = 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=194 , 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; next;) table=??(lr_in_ip_routing ), priority=74 , match=(ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;) - table=??(lr_in_ip_routing ), priority=97 , match=(reg7 == 0 && ip4.dst == 1.0.0.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;) -]) + table=??(lr_in_ip_routing ), priority=97 , match=(reg7 == 0 && ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; reg8[16..31] = N; next;) +]) AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | ovn_strip_lflows], [0], [dnl table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1), action=(drop;)