On 5/19/23 20:18, Vladislav Odintsov wrote:
> 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 <[email protected]>
> ---
> 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.
> + */
I changed this to:
"Packets received from VTEP ports must go directly to L2LKP table."
and applied it to main and backported to all stable branches down to 22.06.
Regards,
Dumitru
> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
