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
