This patch fixes a situation, where logical flow with incorrect syntax could be generated. If a logical switch has two attached logical router ports and one of them has configured gateway chassis, then incorrect flow can have the match like: `reg0[14] == 1 && (is_chassis_resident("cr-lrp2") || ` or `is_chassis_resident("cr-lrp1"))`
The flow's match was reworked to have at maximum one 'is_chassis_resident()' part. For each cr-lport a new lflow is created. There should not be many cr-lports within one datapath (normally there is just one), so the lflows count shouldn't increase dramatically. Now the match looks like: `reg0[14] == 1 && is_chassis_resident("cr-lrp2")` As an additional enhancement, the code became easier and tests were also simplified. Documentation and relevant testcases were updated. Fixes: 4e90bcf55c2e ("controller, northd, vtep: support routed networks with HW VTEP") Signed-off-by: Vladislav Odintsov <odiv...@gmail.com> --- northd/northd.c | 35 ++++++++++++++--------------------- northd/ovn-northd.8.xml | 13 +++++++------ tests/ovn.at | 17 +++-------------- 3 files changed, 24 insertions(+), 41 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 07b127cdf..d6c26735d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -7819,37 +7819,30 @@ static void build_vtep_hairpin(struct ovn_datapath *od, struct hmap *lflows) { /* Ingress Pre-ARP flows for VTEP hairpining traffic. Priority 1000: - * Packets that received from non-VTEP ports should continue processing. */ - + * Packets that received from VTEP ports must go directly to L2LKP table. + */ char *action = xasprintf("next(pipeline=ingress, table=%d);", ovn_stage_get_table(S_SWITCH_IN_L2_LKUP)); - /* send all traffic from VTEP directly to L2LKP table. */ ovn_lflow_add(lflows, od, S_SWITCH_IN_HAIRPIN, 1000, REGBIT_FROM_RAMP" == 1", action); free(action); - struct ds match = DS_EMPTY_INITIALIZER; - size_t n_ports = od->n_router_ports; - bool dp_has_l3dgw_ports = false; - for (int i = 0; i < n_ports; i++) { - if (is_l3dgw_port(od->router_ports[i]->peer)) { - ds_put_format(&match, "%sis_chassis_resident(%s)%s", - i == 0 ? REGBIT_FROM_RAMP" == 1 && (" : "", - od->router_ports[i]->peer->cr_port->json_key, - i < n_ports - 1 ? " || " : ")"); - dp_has_l3dgw_ports = true; - } - } - /* Ingress pre-arp flow for traffic from VTEP (ramp) switch. * Priority 2000: Packets, that were received from VTEP (ramp) switch and * router ports of current datapath are l3dgw ports and they reside on * current chassis, should be passed to next table for ARP/ND hairpin - * processing. - */ - if (dp_has_l3dgw_ports) { - ovn_lflow_add(lflows, od, S_SWITCH_IN_HAIRPIN, 2000, ds_cstr(&match), - "next;"); + * processing. */ + struct ds match = DS_EMPTY_INITIALIZER; + for (int i = 0; i < od->n_router_ports; i++) { + struct ovn_port *op = od->router_ports[i]->peer; + if (is_l3dgw_port(op)) { + ds_clear(&match); + ds_put_format(&match, + REGBIT_FROM_RAMP" == 1 && is_chassis_resident(%s)", + op->cr_port->json_key); + ovn_lflow_add(lflows, od, S_SWITCH_IN_HAIRPIN, 2000, + ds_cstr(&match), "next;"); + } } ds_destroy(&match); } diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 540fe03bd..a8ef00a28 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1144,16 +1144,17 @@ <li> <p> For each distributed gateway router port <var>RP</var> attached to - the logical switch, a priority-2000 flow is added with the match - <code>reg0[14] == 1 && is_chassis_resident(<var>RP</var>) - </code> and action <code>next;</code> to pass the traffic to the - next table to respond to the ARP requests for the router port IPs. + the logical switch and has chassis redirect port <var>cr-RP</var>, a + priority-2000 flow is added with the match + <pre> +<code>reg0[14] == 1 && is_chassis_resident(<var>cr-RP</var>)</code> + </pre> + and action <code>next;</code>. </p> <p> <code>reg0[14]</code> register bit is set in the ingress L2 port - security check table for traffic received from HW VTEP (ramp) - ports. + security check table for traffic received from HW VTEP (ramp) ports. </p> </li> diff --git a/tests/ovn.at b/tests/ovn.at index 9e6e8a14a..53349530b 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -4432,24 +4432,13 @@ response=${sha}${lrpmac}08060001080006040002${lrpmac}${tpa}${sha}${spa} echo $response >> 3.expected # First ensure basic flow contents are as we expect. -AT_CHECK([ovn-sbctl lflow-list lsw0 | grep 'reg0[\[14\]]' | sort | sed 's/table=../table=??/g' | sed 's/is_chassis_resident([[^)]]*)/is_chassis_resident("??")/g'], [0], [dnl +AT_CHECK([ovn-sbctl lflow-list lsw0 | grep 'reg0[\[14\]]' | sort | sed 's/table=../table=??/g'], [0], [dnl table=??(ls_in_check_port_sec), priority=70 , match=(inport == "lp-vtep"), action=(reg0[[14]] = 1; next(pipeline=ingress, table=??);) table=??(ls_in_hairpin ), priority=1000 , match=(reg0[[14]] == 1), action=(next(pipeline=ingress, table=??);) - table=??(ls_in_hairpin ), priority=2000 , match=(reg0[[14]] == 1 && (is_chassis_resident("??") || is_chassis_resident("??"))), action=(next;) + table=??(ls_in_hairpin ), priority=2000 , match=(reg0[[14]] == 1 && is_chassis_resident("cr-lrp1")), action=(next;) + table=??(ls_in_hairpin ), priority=2000 , match=(reg0[[14]] == 1 && is_chassis_resident("cr-lrp2")), action=(next;) ]) -# We've ensured that the expected hairpin flows are present -# and that the expected number of "is_chassis_resident" fields are in -# the flow. Now we need to ensure the contents are correct. -# Unfortunately, the order of the "is_chassis_resident" fields is -# unpredictable. Therefore we sort them so the order is predictable. -actual_chassis=$(ovn-sbctl lflow-list lsw0 | grep 'ls_in_hairpin' | grep 'priority=2000' | grep -o 'is_chassis_resident([[^)]]*)' | sort) - -expected_chassis='is_chassis_resident("cr-lrp1") -is_chassis_resident("cr-lrp2")' - -check test "$expected_chassis" = "$actual_chassis" - # dump information with counters echo "------ OVN dump ------" ovn-nbctl show -- 2.36.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev