diff --git a/northd/northd.c b/northd/northd.c index 983c464ebf..f218211ee3 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -11567,21 +11567,48 @@ 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 { + er = CONTAINER_OF(ovs_list_front(&eg->route_list), + struct ecmp_route_list_node, list_node); + ds_put_format(&actions, "%"PRIu16"; next;", er->id); } - 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 +13570,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..a696d39475 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, ...); + diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index dcc3dbbc31..cd14f66684 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -6799,20 +6799,23 @@ 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]] = 1; 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;) + table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 1), action=(reg0 = 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;) table=??(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0), action=(next;) ])