> On 21 Aug 2026, at 4:43 PM, Dumitru Ceara <[email protected]> wrote: > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > On 8/20/26 6:09 PM, Naveen Yerramneni wrote: >> A Network Function (NF) in inline mode redirects matched traffic >> through a service VM. When the redirected traffic is IP >> unknown-unicast (destination MAC not yet in the FDB), the packet >> coming back from the NF is re-flooded by the switch, because the >> destination MAC is still unknown. That re-flood produces a copy on >> the same port the packet originally arrived on, causing MAC flaps and >> potential L2 loops. >> > > Hi Naveen, > > Thanks for the new revision! > >> Following are the example packet flows. >> >> Example 1, VLAN switch (MAC flap): >> >> Topology >> VM1, VM2 and the NF are on the same logical switch (LS), which is >> VLAN-backed (localnet port on every node). VM1 is on N1, VM2 on >> N2 (its port has "unknown" in addresses and a to-lport ACL that >> redirects to the inline NF on N3). >> >> Flow >> 1. VM1 sends pkt to dst MAC X (not in FDB). >> 2. The LS floods the pkt; on N1 the copy exits the localnet port >> and the TOR floods it to N2. >> 3. On N2 the pkt ingresses on localnet; the LS floods it and the >> copy reaches VM2 (unknown-addr). >> 4. The ACL redirects the pkt to the NF on N3; the NF returns it >> to N2. >> 5. X is still not in the FDB, so the LS floods again; on N2 one >> copy exits the localnet port. >> >> Result >> The TOR now sees VM1's source MAC on N2's port, but it had just >> learned VM1's MAC on N1's port, so the MAC flaps on the TOR. >> >> Example 2, VLAN switch with two protected VMs (loop): >> >> Topology >> As above, plus VM3 (on N3, same LS) also has "unknown" in >> addresses and is also NF-protected. >> >> Flow >> 1-5 as above; in parallel, the original flood also reaches N3 via >> the TOR, where it is redirected to N3's NF, returns, and the >> LS re-floods it out N3's localnet port. >> 6. N3's re-flood reaches N2 via the TOR; the LS floods it on N2, >> the copy hits VM2 (unknown-addr), is redirected to the NF, and >> re-floods out N2's localnet port. >> 7. That re-flood reaches N3 via the TOR; the LS floods it on N3, >> the copy hits VM3 (also unknown-addr), is redirected to the >> NF, re-floods out localnet, ... >> >> Result >> The pkt keeps bouncing between N2 and N3 via the TOR, i.e. an >> L2 loop, on top of continuous MAC flaps. The loop persists as >> long as X stays unknown. >> >> Example 3, overlay switch (copy reflected to inport): >> >> Topology >> VM1, VM2 and the NF are on the same LS and all on N1. VM1 and >> VM2 both have "unknown" in addresses; VM2's to-lport ACL >> redirects to the inline NF. >> >> Flow >> 1. VM1 sends pkt to dst MAC X (not in FDB). >> 2. The LS floods to MC_UNKNOWN members on N1; one copy goes to >> VM2. >> 3. VM2's ACL redirects its copy to the NF; the NF returns it to >> N1. >> 4. X is still not in the FDB, so the LS floods to MC_UNKNOWN >> again; one copy is headed back out VM1's own port. >> >> Result >> VM1 receives a copy of the packet it just sent (reflected to the >> inport). >> >> Fix: >> >> Use the nf_learn_orig_inport() / nf_lookup_orig_inport() actions >> from the previous commit to remember the original ingress port and >> drop the copy if it is about to be sent back out of that port. >> >> - In the NF redirect stage (ls_in_nf for a from-lport ACL, >> ls_out_nf for a to-lport ACL), existing priority-99 redirect >> flows are updated to learn the original ingress port >> (nf_learn_orig_inport()) before redirecting unicast IP packets >> to the NF. >> >> - Priority 50 (overlay switches only, in ls_in_nf): learn on >> the inport's ingress node when the ACL did not redirect >> the packet (REGBIT_NF_ENABLED == 0), so the learn stays >> co-located with the lookup even when the redirecting port and >> the inport live on different nodes. VLAN-backed switches >> do not need this: the post-NF packet is always sent back to >> the inport's node. >> >> - On the post-NF return path, run >> REGBIT_NF_LOOKUP_HIT = nf_lookup_orig_inport() on both NF ports. >> A priority-115 flow in ls_out_pre_acl runs the lookup on >> input_port and skips already-traversed egress stages. A >> priority-2 flow in ls_out_nf runs the lookup on output_port for >> packets redirected from the ingress pipeline (for example, a >> from-lport request that is re-flooded after NF processing and >> re-enters egress). >> >> - A new priority-110 flow in ls_out_check_port_sec drops packets >> with REGBIT_NF_LOOKUP_HIT == 1, i.e. the copies about to be >> sent back out of the port they originally arrived on. >> >> All new flows are gated on the switch having an inline NF group on >> an ACL. >> >> Acked-by: Aditya Mehakare <[email protected]> >> Fixes: 8e2d6fa14804 ("northd, tests: Network Function insertion logical flow >> programming.") >> CC: Sragdhara Datta Chaudhuri <[email protected]> >> Assisted-by: Claude Opus 4.7, Cursor >> Signed-off-by: Naveen Yerramneni <[email protected]> >> --- >> Documentation/ref/ovn-logical-flows.7.rst | 61 ++++-- >> TODO.rst | 10 + >> northd/northd.c | 121 +++++++++--- >> ovn-nb.xml | 13 ++ >> tests/ovn-northd.at | 229 ++++++++++++++++++++-- >> tests/ovn.at | 105 ++++++++++ >> 6 files changed, 486 insertions(+), 53 deletions(-) >> >> diff --git a/Documentation/ref/ovn-logical-flows.7.rst >> b/Documentation/ref/ovn-logical-flows.7.rst >> index 0ec69e39e..6718c7fa8 100644 >> --- a/Documentation/ref/ovn-logical-flows.7.rst >> +++ b/Documentation/ref/ovn-logical-flows.7.rst >> @@ -946,11 +946,15 @@ refer to either the parent or child ports as >> applicable to this logical switch. >> table. >> >> - In inline: For each active network function with *id* that is referenced >> in a >> - network function group, a priority-99 flow matches ``reg8[21] == 1 && >> - reg8[22] == 1 && reg0[22..29] == id`` and sets ``outport=P; output;`` >> where >> - *P* is the ``inport`` of that network function. This redirects request >> - packets for flows matching ``from-lport`` ACLs with network_function_group >> - to the specific network function selected by the Pre Network Function >> stage. >> + network function group, priority-99 flows match ``ip4 && reg8[21] == 1 && >> + reg8[22] == 1 && reg0[22..29] == id`` and ``ip6 && reg8[21] == 1 && >> + reg8[22] == 1 && reg0[22..29] == id`` and set >> + ``nf_learn_orig_inport(ipv6 = false|true); outport=P; output;`` >> + where *P* is the ``inport`` of that network function. This redirects >> request >> + packets for flows matching ``from-lport`` ACLs with >> + network_function_group to the specific network function selected by the >> Pre >> + Network Function stage. It also learns the original inport before >> + redirection. >> >> - In vtap mode: For each active network function with *id*, a priority-99 >> forward flow matches ``reg8[21] == 1 && reg8[22] == 1 && reg0[22..29] == >> id`` >> @@ -965,6 +969,12 @@ refer to either the parent or child ports as applicable >> to this logical switch. >> This redirects response and related packets for ``to-lport`` ACLs to the >> same network function that handled the request. >> >> +- In inline (overlay logical switches only), two priority-50 flows match >> + ``ip4 && reg8[21] == 0`` and ``ip6 && reg8[21] == 0`` and set >> + ``nf_learn_orig_inport(ipv6 = false|true); next;``. This learns on the >> + packet's ingress node even when redirection happens on a different node, >> + so the learn stays co-located with the lookup. >> + >> - In vtap mode: A priority-99 reverse flow matches ``reg8[21] == 1 && >> reg8[22] >> == 0 && ct_label.nf_id == id`` and sets ``clone { outport = P; output; }; >> next;`` to mirror response/related packets to the same NF. >> @@ -1645,13 +1655,17 @@ peer of a logical router port. This flow is added to >> skip the connection >> tracking of packets which will be entering logical router datapath from >> logical >> switch datapath for routing. >> >> -This table also has a priority-110 flow for each network_function >> ``inport`` *P* >> -that matches ``inport == P``. The action is to skip all the egress tables >> up to >> -the :ref:`Network Function <ls-out-13>` table and advance the packet >> directly to >> -the table after that. This is for the case where packet redirection happens >> in >> -egress :ref:`Network Function <ls-out-13>` table. The same packet when it >> comes >> -out of the other port of network function, they should not be processed >> again by >> -the same egress stages, specially they should skip the conntrack processing. >> +This table also has a priority-115 flow for each network_function ``inport`` >> +*P* >> +that matches ``inport == P`` and applies >> +``reg8[24] = nf_lookup_orig_inport(); next(pipeline=egress, table=T);`` >> where >> +*T* is the table immediately following :ref:`Network Function <ls-out-13>`. >> +This is for the case where packet redirection happens in egress >> +:ref:`Network Function <ls-out-13>` table. When the packet comes back from >> the >> +other NF port, it should not be processed again by the same egress stages. >> +, specially they should skip the conntrack processing. This flow also >> performs > > Something is a bit broken here. I rephrased this to: > > This is for the case where packet redirection happens in egress > :ref:`Network Function <ls-out-13>` table. When the packet comes back from > the > other NF port, it should not be processed again by the same egress stages, > specifically conntrack processing should be skipped. This flow also performs > ``nf_lookup_orig_inport()`` before continuing so post-NF loopback copies can > be detected and dropped. > >> +``nf_lookup_orig_inport()`` before continuing so post-NF loopback copies can >> +be detected and dropped. >> >> .. _ls-out-3: >> >> @@ -1886,13 +1900,15 @@ in ``ct_label.nf_id`` during request processing. >> table. >> >> - In inline: For each active network function with *id* that is referenced >> in a >> - network function group, a priority-99 flow matches ``reg8[21] == 1 && >> - reg8[22] == 1 && reg0[22..29] == id`` and sets ``outport=P; reg8[23] = 1; >> - next(pipeline=ingress, table=T)`` where *P* is the ``outport`` of >> - that network function and *T* is the ingress table :ref:`Destination >> Lookup >> - <ls-in-33>`. This redirects request packets matching ``to-lport`` ACLs >> with >> - network_function_group to the specific network function selected by the >> Pre >> - Network Function stage. >> + network function group, priority-99 flows match ``ip4 && reg8[21] == 1 && >> + reg8[22] == 1 && reg0[22..29] == id`` and ``ip6 && reg8[21] == 1 && >> + reg8[22] == 1 && reg0[22..29] == id`` and set >> + ``nf_learn_orig_inport(ipv6 = false|true); outport=P; reg8[23] = 1; >> + next(pipeline=ingress, table=T);`` where *P* is the NF outport and *T* is >> + the ingress :ref:`Destination Lookup <ls-in-33>` table. This redirects >> + request packets matching ``to-lport`` ACLs with network_function_group to >> the >> + specific network function selected by the Pre Network Function stage, and >> + learns the original inport before redirection. >> >> - In vtap mode: For each active network function with *id*, a priority-99 >> forward flow matches ``reg8[21] == 1 && reg8[22] == 1 && reg0[22..29] == >> id`` >> @@ -1922,6 +1938,10 @@ in ``ct_label.nf_id`` during request processing. >> port) and advances to the next table so packets to the NF are not mirrored >> again. >> >> +- In inline: A priority-2 flow matches ``inport == P`` where *P* is the NF >> + ``outport`` and sets ``reg8[24] = nf_lookup_orig_inport(); next;``. This >> + captures post-NF packets that re-enter egress from ingress recirculation. >> + >> - In vtap mode: In egress Pre ACL table, a priority-110 flow matches ``ip && >> outport == P`` with action ``ct_clear; next;`` for the vtap NF port so >> packets toward the NF are not committed to conntrack. >> @@ -1944,6 +1964,9 @@ This is similar to the port security logic in table >> :ref:`Ingress Port Security >> check <ls-in-0>` except that action ``check_out_port_sec`` is used to check >> the >> port security rules. This table adds the below logical flows. >> >> +- A priority-110 flow matching ``reg8[24] == 1`` with action ``drop;``. >> This >> + drops post-NF loopback copies detected by ``nf_lookup_orig_inport()``. >> + >> - A priority 100 flow which matches on the multicast traffic and applies the >> action ``REGBIT_PORT_SEC_DROP" = 0; next;"`` to skip the out port security >> checks. >> diff --git a/TODO.rst b/TODO.rst >> index beca38daf..9e7807b58 100644 >> --- a/TODO.rst >> +++ b/TODO.rst >> @@ -177,6 +177,16 @@ OVN To-do List >> * Geneve tunnel is used for supporting this feature for VLAN network. >> Extend the support over VxLAN tunnel as well. >> >> + * Inline-NF loop prevention has limitations in topologies where >> + a post-NF flood copy may reach the same logical inport from a >> + different node than the packet's original ingress node. In that >> + case, the lookup runs on a different node than where the inport >> + was learned, so loopback copies are not identified. >> + Known example: OVN-IC L2 stretch with the NF and IC gateway on >> + different chassis. With ``unknown``-addressed ports, the post-NF >> + packet can be flooded across AZs and come back to the source AZ via >> + another node, so the lookup does not hit the original learn entry. >> + >> * CI >> >> * ovn-kubernetes: Only a subset of the ovn-kubernetes features is currently >> diff --git a/northd/northd.c b/northd/northd.c >> index 9f33b50cf..4e40bc8b4 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -176,6 +176,9 @@ static bool vxlan_mode; >> #define REGBIT_NF_ENABLED "reg8[21]" >> #define REGBIT_NF_ORIG_DIR "reg8[22]" >> #define REGBIT_NF_EGRESS_LOOPBACK "reg8[23]" >> +/* Set on a post-NF packet flowing back out its original ingress port; >> + * such packets are dropped. */ >> +#define REGBIT_NF_LOOKUP_HIT "reg8[24]" >> /* Register to store the network function group id */ >> #define REG_NF_GROUP_ID "reg0[22..29]" >> /* REG_NF_ID overrides REG_NF_GROUP_ID in the pre_network_function stage. */ >> @@ -315,6 +318,8 @@ static const char *reg_ct_state[] = { >> * | | REGBIT_NF_{ENABLED/ORIG_DIR/ | G | >> | >> * | | EGRESS_LOOPBACK} | 4 | >> | >> * | | (>= ACL_EVAL* && <= NF*) | | >> | >> + * | | REGBIT_NF_LOOKUP_HIT | | >> | >> + * | | (>= OUT_PRE_ACL && <= OUT_CHECK_PORT_SEC)| | >> | >> * +----+----------------------------------------------+ >> +-----------------------------------+ >> * | R9 | OBS_POINT_ID_EST | | >> | >> * | | (>= ACL_EVAL* && <= ACL_ACTION*) | | >> | >> @@ -19776,35 +19781,64 @@ consider_network_function_inline(struct >> lflow_table *lflows, >> >> /* Add forward flows for redirection: >> * Flows to handle request packets for new or existing connections. >> + * These flows learn the original inport before redirection. >> * >> * from-lport ACL in_nf priority 99: >> * in_acl_eval has already categorized it and populated nf_enabled, >> * direction and nfg_id registers. in_pre_nf sets the active network >> - * function id in nf_id register. Here this rule sets the outport to the >> - * NF port and does output action to skip the rest of the ingress >> pipeline. >> + * function id in nf_id register. Here this rule learns original inport, >> + * sets the outport to the NF port and does output action to skip the >> + * rest of the ingress pipeline. >> * >> * to-lport ACL out_nf priority 99: >> * out_acl_eval, and out_pre_nf set the nf related registers. Then the >> - * out_nf stage sets the outport to NF port and submits the >> - * packet back to ingress pipeline l2_lkup table. The l2_lkup would skip >> - * mac based lookup as the NF_EGRESS_LOOPBACK is set. >> + * out_nf stage learns original inport, sets the outport to NF port and >> + * submits the packet back to ingress pipeline l2_lkup table. The >> l2_lkup >> + * would skip mac based lookup as the NF_EGRESS_LOOPBACK is set. >> */ >> + ds_put_format(&match, REGBIT_NF_ENABLED" == 1 && " >> + REGBIT_NF_ORIG_DIR" == 1 && " >> + REG_NF_ID " == %"PRIu8, >> + (uint8_t) nf->id); >> + size_t fwd_match_len = match.length; >> + >> + /* IPv4 redirect flow: learn original inport before redirect. */ >> + ds_put_format(&match, " && ip4"); >> + ds_clear(&action); >> if (ingress) { >> - ds_put_format(&action, "outport = %s; output;", >> + ds_put_format(&action, "nf_learn_orig_inport(ipv6 = false); " >> + "outport = %s; output;", >> redirect_port->json_key); >> } else { >> - ds_put_format(&action, "outport = %s; " >> + ds_put_format(&action, "nf_learn_orig_inport(ipv6 = false); " >> + "outport = %s; " >> REGBIT_NF_EGRESS_LOOPBACK" = 1; " >> "next(pipeline=ingress, table=%d);", >> redirect_port->json_key, >> ovn_stage_get_table(S_SWITCH_IN_L2_LKUP)); >> } >> - ds_put_format(&match, REGBIT_NF_ENABLED" == 1 && " >> - REGBIT_NF_ORIG_DIR" == 1 && " >> - REG_NF_ID " == %"PRIu8, >> - (uint8_t) nf->id); >> ovn_lflow_add(lflows, od, fwd_stage, 99, ds_cstr(&match), >> ds_cstr(&action), lflow_ref); >> + ds_truncate(&match, fwd_match_len); >> + >> + /* IPv6 redirect flow: learn original inport before redirect. */ >> + ds_put_format(&match, " && ip6"); >> + ds_clear(&action); >> + if (ingress) { >> + ds_put_format(&action, "nf_learn_orig_inport(ipv6 = true); " >> + "outport = %s; output;", >> + redirect_port->json_key); >> + } else { >> + ds_put_format(&action, "nf_learn_orig_inport(ipv6 = true); " >> + "outport = %s; " >> + REGBIT_NF_EGRESS_LOOPBACK" = 1; " >> + "next(pipeline=ingress, table=%d);", >> + redirect_port->json_key, >> + ovn_stage_get_table(S_SWITCH_IN_L2_LKUP)); >> + } >> + ovn_lflow_add(lflows, od, fwd_stage, 99, ds_cstr(&match), >> + ds_cstr(&action), lflow_ref); >> + >> ds_clear(&match); >> ds_clear(&action); >> >> @@ -19876,19 +19910,31 @@ consider_network_function_inline(struct >> lflow_table *lflows, >> ds_clear(&match); >> ds_clear(&action); >> >> - /* For packets redirected from egress pipleline to the NF, when they >> come >> - * out from the other NF port, we don't want to process them again >> through >> - * egress stages they already went through, especially not again through >> - * conntrack as these packets are already accounted for there. Hence we >> - * need to skip the initial pipeline stages for such packets and >> directly >> - * start from the NF table. The packets that fall under this category >> are >> - * the response packets from NF for from-lport ACLs and request packets >> - * received from NF for to-lport ACLs. */ >> + /* Post-NF Processing: Resumes pipeline after ls_out_nf to bypass >> + * previously completed egress stages (e.g., conntrack). Uses priority >> + * 115 in out_pre_acl to take precedence over priority-110 conntrack >> + * skip flows, and calls nf_lookup_orig_inport() to identify loopback >> + * packets returning on their original inport. */ >> ds_put_format(&match, "inport == %s", input_port->json_key); >> - ds_put_format(&action, "next(pipeline=egress, table=%d);", >> - (ovn_stage_get_table(S_SWITCH_OUT_NF) + 1)); >> - ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, ds_cstr(&match), >> + ds_put_format(&action, >> + REGBIT_NF_LOOKUP_HIT " = nf_lookup_orig_inport(); " >> + "next(pipeline=egress, table=%d);", >> + ovn_stage_get_table(S_SWITCH_OUT_NF) + 1); >> + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 115, ds_cstr(&match), >> ds_cstr(&action), lflow_ref); >> + ds_clear(&match); >> + ds_clear(&action); >> + >> + /* Priority 2 in out_nf (output_port): post-NF packet re-entering the >> + * egress pipeline; run the lookup here so ls_out_check_port_sec drops >> + * a loopback copy. */ >> + ds_put_format(&match, "inport == %s", output_port->json_key); >> + ds_put_format(&action, >> + REGBIT_NF_LOOKUP_HIT " = nf_lookup_orig_inport(); next;"); >> + ovn_lflow_add(lflows, od, S_SWITCH_OUT_NF, 2, ds_cstr(&match), >> + ds_cstr(&action), lflow_ref); >> + ds_clear(&match); >> + ds_clear(&action); >> >> /* Priority 120 flows in out_stateful: >> * If packet was received on a tunnel interface and being forwarded to a >> @@ -20107,6 +20153,7 @@ build_network_function(const struct ovn_datapath *od, >> { >> unsigned long *nfg_ingress_bitmap = >> bitmap_allocate(MAX_OVN_NF_GROUP_IDS); >> unsigned long *nfg_egress_bitmap = bitmap_allocate(MAX_OVN_NF_GROUP_IDS); >> + bool has_inline_nfg = false; >> >> /* This flow matches packets injected from out_nf stage - >> * after it sets the outport - back to in_l2_lkup stage. This rule must >> be >> @@ -20175,6 +20222,10 @@ build_network_function(const struct ovn_datapath >> *od, >> continue; >> } >> nfg_bitmap = bitmap_set1(nfg_bitmap, nfg_id); >> + has_inline_nfg = >> + has_inline_nfg || >> + !network_function_group_is_vtap_mode( >> + acl->network_function_group); > > This is quite hard to read. Same for the PG part below. > > I made the following incremental change: > > diff --git a/northd/northd.c b/northd/northd.c > index 4e40bc8b4a..88e3ece884 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -20216,16 +20216,16 @@ build_network_function(const struct ovn_datapath > *od, > ? true : false; > unsigned long *nfg_bitmap = ingress ? nfg_ingress_bitmap > : nfg_egress_bitmap; > - unsigned int nfg_id = acl->network_function_group->id; > - if (bitmap_is_set(nfg_bitmap, nfg_id)) { > + const struct nbrec_network_function_group *nfg = > + acl->network_function_group; > + if (bitmap_is_set(nfg_bitmap, nfg->id)) { > /* Skip if same NF is used again. */ > continue; > } > - nfg_bitmap = bitmap_set1(nfg_bitmap, nfg_id); > - has_inline_nfg = > - has_inline_nfg || > - !network_function_group_is_vtap_mode( > - acl->network_function_group); > + nfg_bitmap = bitmap_set1(nfg_bitmap, nfg->id); > + if (!has_inline_nfg && > !network_function_group_is_vtap_mode(nfg)) { > + has_inline_nfg = true; > + } > consider_network_function(lflows, od, acl->network_function_group, > ingress, lflow_ref); > } > @@ -20245,16 +20245,18 @@ build_network_function(const struct ovn_datapath > *od, > unsigned long *nfg_bitmap = ingress > ? nfg_ingress_bitmap > : nfg_egress_bitmap; > - unsigned int nfg_id = acl->network_function_group->id; > - if (bitmap_is_set(nfg_bitmap, nfg_id)) { > + const struct nbrec_network_function_group *nfg = > + acl->network_function_group; > + if (bitmap_is_set(nfg_bitmap, nfg->id)) { > /* Skip if same NF is used again. */ > continue; > } > - nfg_bitmap = bitmap_set1(nfg_bitmap, nfg_id); > - has_inline_nfg = > - has_inline_nfg || > - !network_function_group_is_vtap_mode( > - acl->network_function_group); > + nfg_bitmap = bitmap_set1(nfg_bitmap, nfg->id); > + > + if (!has_inline_nfg && > + !network_function_group_is_vtap_mode(nfg)) { > + has_inline_nfg = true; > + } > consider_network_function(lflows, od, > acl->network_function_group, > ingress, lflow_ref); > >> consider_network_function(lflows, od, >> acl->network_function_group, >> ingress, lflow_ref); >> } >> @@ -20200,6 +20251,10 @@ build_network_function(const struct ovn_datapath >> *od, >> continue; >> } >> nfg_bitmap = bitmap_set1(nfg_bitmap, nfg_id); >> + has_inline_nfg = >> + has_inline_nfg || >> + !network_function_group_is_vtap_mode( >> + acl->network_function_group); >> consider_network_function(lflows, od, >> acl->network_function_group, >> ingress, lflow_ref); >> @@ -20207,6 +20262,28 @@ build_network_function(const struct ovn_datapath >> *od, >> } >> } >> } >> + >> + if (has_inline_nfg) { >> + /* Drop the loopback copy flagged by nf_lookup_orig_inport(). */ >> + ovn_lflow_add(lflows, od, S_SWITCH_OUT_CHECK_PORT_SEC, 110, >> + REGBIT_NF_LOOKUP_HIT " == 1", debug_drop_action(), >> + lflow_ref); >> + >> + /* Overlay only: always learn the inport so a post-NF loopback copy >> + * can be detected and dropped when redirection happens on a >> different >> + * node than where the packet originally entered the LS. */ >> + if (!ls_has_localnet_port(od)) { >> + ovn_lflow_add(lflows, od, S_SWITCH_IN_NF, 50, >> + "ip4 && " REGBIT_NF_ENABLED" == 0", >> + "nf_learn_orig_inport(ipv6 = false); next;", >> + lflow_ref); >> + ovn_lflow_add(lflows, od, S_SWITCH_IN_NF, 50, >> + "ip6 && " REGBIT_NF_ENABLED" == 0", >> + "nf_learn_orig_inport(ipv6 = true); next;", >> + lflow_ref); >> + } >> + } >> + >> bitmap_free(nfg_ingress_bitmap); >> bitmap_free(nfg_egress_bitmap); >> } >> diff --git a/ovn-nb.xml b/ovn-nb.xml >> index 8a9d19fa9..7c1d5004a 100644 >> --- a/ovn-nb.xml >> +++ b/ovn-nb.xml >> @@ -6760,6 +6760,19 @@ or >> as down, the fallback method is applied. >> </p> >> >> + <p> >> + <b>Known limitation (inline mode)</b>: in topologies where a >> + post-NF flood copy may reach the same logical inport from a >> + different node than the packet's original ingress node. In that > > The sentence that ends here reads weird. I rephrased it to: > > <b>Known limitation (inline mode)</b>: in topologies where a > post-NF flood copy may reach the same logical inport from a > different node than the packet's original ingress node, > the lookup runs on a different node than where the inport > was learned, so loopback copies are not identified. > > >> + case, the lookup runs on a different node than where the inport >> + was learned, so loopback copies are not identified. >> + Known example: OVN-IC L2 stretch with the NF and IC gateway on > > s/Known/For > >> + different chassis. With <code>unknown</code>-addressed ports, the >> + post-NF packet can be flooded across AZs and come back to the >> + source AZ via another node, so the lookup does not hit the >> + original learn entry. >> + </p> >> + >> <column name="name"> >> Name of the <ref table="Network_Function_Group"/>. Name should be >> unique. >> </column> >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index 2b9e95d7f..f814f4707 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -21758,12 +21758,16 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "sw0-nf-p1"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "sw0-nf-p2"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> - table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101), action=(outport = "sw0-nf-p1"; >> output;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "sw0-nf-p1"; output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "sw0-nf-p1"; output;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw0-nf-p1"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw0-nf-p2"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw0-nf-p2"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 101), action=(outport = "sw0-nf-p2"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> ]) >> >> @@ -21772,7 +21776,7 @@ AT_CHECK([grep "ls_in_l2_lkup" sw0flows | >> ovn_strip_lflows | grep 'priority=100' >> ]) >> >> AT_CHECK([grep "ls_out_pre_acl" sw0flows | ovn_strip_lflows | grep >> 'sw0-nf-p1'], [0], [dnl >> - table=??(ls_out_pre_acl ), priority=110 , match=(inport == >> "sw0-nf-p1"), action=(next(pipeline=egress, table=??);) >> + table=??(ls_out_pre_acl ), priority=115 , match=(inport == >> "sw0-nf-p1"), action=(reg8[[24]] = nf_lookup_orig_inport(); >> next(pipeline=egress, table=??);) >> ]) >> >> AT_CHECK( >> @@ -21868,8 +21872,11 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "sw0-nf-p3"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "sw0-nf-p4"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 102), action=(outport = "sw0-nf-p3"; >> output;) >> - table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101), action=(outport = "sw0-nf-p1"; >> output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "sw0-nf-p1"; output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "sw0-nf-p1"; output;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw0-nf-p1"), action=(next;) >> @@ -21877,8 +21884,11 @@ AT_CHECK( >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw0-nf-p3"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw0-nf-p4"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw0-nf-p2"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw0-nf-p4"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 101), action=(outport = "sw0-nf-p2"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> - table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102), action=(outport = "sw0-nf-p4"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "sw0-nf-p4"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "sw0-nf-p4"; reg8[[23]] >> = 1; next(pipeline=ingress, table=??);) >> ]) >> >> # ICMP packets to sw0-p1 should be redirected to sw0-nf-p4. >> @@ -21950,8 +21960,11 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "sw1-nf-p3"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "sw1-nf-p4"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 102), action=(outport = "sw1-nf-p3"; >> output;) >> - table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101), action=(outport = "sw1-nf-p1"; >> output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "sw1-nf-p1"; output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "sw1-nf-p1"; output;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw1-nf-p1"), action=(next;) >> @@ -21959,8 +21972,11 @@ AT_CHECK( >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw1-nf-p3"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "sw1-nf-p4"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw1-nf-p2"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw1-nf-p4"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 101), action=(outport = "sw1-nf-p2"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> - table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102), action=(outport = "sw1-nf-p4"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "sw1-nf-p4"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "sw1-nf-p4"; reg8[[23]] >> = 1; next(pipeline=ingress, table=??);) >> ]) >> >> # ICMP packets from sw1-p1 should be redirected to sw1-nf-p1. >> @@ -21993,6 +22009,176 @@ OVN_CLEANUP_NORTHD >> AT_CLEANUP >> ]) >> >> +OVN_FOR_EACH_NORTHD_NO_HV([ >> +AT_SETUP([Network function -- inline post-NF re-flood loop prevention]) > > It's weird that the two tests (one here and one in ovn.at) have the same name. > I added the "-- lflows" suffix to this one here. > >> + >> +dnl When an LS has ports with "unknown" addresses, a packet redirected >> +dnl to an inline NF can be re-flooded back out its original inport after NF >> +dnl processing. This is prevented by learning the original inport on the >> way >> +dnl in, then on the post-NF return path looking it up and dropping any >> +dnl copy headed back out that same port. This test verifies the >> LEARN/LOOKUP/DROP >> +dnl flows northd installs for loop prevention. >> + >> +ovn_start >> + >> +check ovn-nbctl ls-add sw0 >> + >> +dnl NF port-pair. >> +check ovn-nbctl lsp-add sw0 sw0-nf-p1 >> +check ovn-nbctl lsp-add sw0 sw0-nf-p2 >> +check ovn-nbctl set logical_switch_port sw0-nf-p1 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw0-nf-p2 >> +check ovn-nbctl set logical_switch_port sw0-nf-p2 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw0-nf-p1 >> + >> +dnl A VM port and a localnet port, both with "unknown" in addresses. >> +check ovn-nbctl lsp-add sw0 sw0-vm1 -- \ >> + lsp-set-addresses sw0-vm1 "00:00:00:00:00:01 10.0.0.10" unknown >> +check ovn-nbctl lsp-add sw0 sw0-ln -- \ >> + lsp-set-type sw0-ln localnet -- \ >> + lsp-set-options sw0-ln network_name=phys -- \ >> + lsp-set-addresses sw0-ln unknown >> + >> +dnl A second VM port without "unknown". >> +check ovn-nbctl lsp-add sw0 sw0-vm2 -- \ >> + lsp-set-addresses sw0-vm2 "00:00:00:00:00:02 10.0.0.11" >> + >> +check ovn-nbctl nf-add nf0 101 sw0-nf-p1 sw0-nf-p2 >> +check ovn-nbctl nfg-add nfg0 201 inline nf0 >> +check ovn-nbctl acl-add sw0 from-lport 1234 'ip' allow-related nfg0 >> + >> +dnl Bind NF ports so nf_get_active() considers the NF healthy. >> +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 >> +chassis_uuid=$(fetch_column Chassis _uuid name=gw1) >> +check ovn-sbctl set port_binding sw0-nf-p1 up=true chassis=$chassis_uuid >> +check ovn-sbctl set port_binding sw0-nf-p2 up=true chassis=$chassis_uuid >> +check ovn-nbctl --wait=sb sync >> + >> +ovn-sbctl dump-flows sw0 > sw0flows >> +AT_CAPTURE_FILE([sw0flows]) >> + >> +dnl LEARN (redirect path): priority-99 LEARN+redirect flows in ls_in_nf >> +dnl for IPv4 and IPv6. >> +AT_CHECK([grep 'ls_in_network_function' sw0flows | ovn_strip_lflows | \ >> + grep nf_learn | grep 'priority=99' | sort], [0], [dnl >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "sw0-nf-p1"; output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 101 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "sw0-nf-p1"; output;) >> +]) >> + >> +dnl LEARN (default path): the priority-50 LEARN is only emitted on overlay >> +dnl switches. sw0 is VLAN-backed (has a localnet port), so it must NOT >> +dnl have any priority-50 nf_learn flow. >> +AT_CHECK([grep 'ls_in_network_function' sw0flows | ovn_strip_lflows | \ >> + grep nf_learn | grep 'priority=50' | wc -l], [0], [0 >> +]) >> + >> +dnl LOOKUP: nf_lookup_orig_inport() on both NF ports. input_port runs >> +dnl in ls_out_pre_acl, output_port in ls_out_nf. >> +AT_CHECK([grep 'ls_out_pre_acl' sw0flows | ovn_strip_lflows | \ >> + grep 'sw0-nf-p1'], [0], [dnl >> + table=??(ls_out_pre_acl ), priority=115 , match=(inport == >> "sw0-nf-p1"), action=(reg8[[24]] = nf_lookup_orig_inport(); >> next(pipeline=egress, table=??);) >> +]) >> +AT_CHECK([grep 'ls_out_network_function' sw0flows | ovn_strip_lflows | \ >> + grep nf_lookup], [0], [dnl >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw0-nf-p2"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> +]) >> + >> +dnl DROP: ls_out_check_port_sec drops on REGBIT_NF_LOOKUP_HIT == 1. >> +AT_CHECK([grep 'ls_out_check_port_sec' sw0flows | ovn_strip_lflows | \ >> + grep 'reg8\[[24\]] == 1'], [0], [dnl >> + table=??(ls_out_check_port_sec), priority=110 , match=(reg8[[24]] == 1), >> action=(drop;) >> +]) >> + >> +AS_BOX([An overlay switch gets the default-path LEARN]) >> + >> +dnl On an overlay switch the NF redirect may happen on a different node >> +dnl than the inport. To keep the LEARN co-located with the LOOKUP, >> +dnl northd emits a priority-50 default-path LEARN in ls_in_nf so the >> +dnl inport is recorded on the same node the packet enters on. >> +check ovn-nbctl ls-add sw1 >> +check ovn-nbctl lsp-add sw1 sw1-nf-p1 >> +check ovn-nbctl lsp-add sw1 sw1-nf-p2 >> +check ovn-nbctl set logical_switch_port sw1-nf-p1 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw1-nf-p2 >> +check ovn-nbctl set logical_switch_port sw1-nf-p2 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw1-nf-p1 >> +check ovn-nbctl nf-add nf2 103 sw1-nf-p1 sw1-nf-p2 >> +check ovn-nbctl nfg-add nfg2 203 inline nf2 >> +check ovn-nbctl acl-add sw1 from-lport 1234 'ip' allow-related nfg2 >> + >> +check ovn-sbctl set port_binding sw1-nf-p1 up=true chassis=$chassis_uuid >> +check ovn-sbctl set port_binding sw1-nf-p2 up=true chassis=$chassis_uuid >> +check ovn-nbctl --wait=sb sync >> + >> +ovn-sbctl dump-flows sw1 > sw1flows >> +AT_CAPTURE_FILE([sw1flows]) >> + >> +AT_CHECK([grep 'ls_in_network_function' sw1flows | ovn_strip_lflows | \ >> + grep nf_learn | grep 'priority=50' | sort], [0], [dnl >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> +]) >> + >> +AS_BOX([A second inline NFG on a to-lport ACL emits its own LEARN+LOOKUP]) >> + >> +dnl A second NF pair attached via a to-lport ACL adds: >> +dnl - priority-100 LEARN+redirect flows in ls_out_nf for that NFG, and >> +dnl - LOOKUP flows on both of that NF's ports. >> +check ovn-nbctl lsp-add sw0 sw0-nf-p3 >> +check ovn-nbctl lsp-add sw0 sw0-nf-p4 >> +check ovn-nbctl set logical_switch_port sw0-nf-p3 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw0-nf-p4 >> +check ovn-nbctl set logical_switch_port sw0-nf-p4 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw0-nf-p3 >> +check ovn-nbctl nf-add nf1 102 sw0-nf-p3 sw0-nf-p4 >> +check ovn-nbctl nfg-add nfg1 202 inline nf1 >> +check ovn-nbctl acl-add sw0 to-lport 1234 'ip' allow-related nfg1 >> + >> +check ovn-sbctl set port_binding sw0-nf-p3 up=true chassis=$chassis_uuid >> +check ovn-sbctl set port_binding sw0-nf-p4 up=true chassis=$chassis_uuid >> +check ovn-nbctl --wait=sb sync >> + >> +ovn-sbctl dump-flows sw0 > sw0flows >> + >> +AT_CHECK([grep 'ls_out_pre_acl' sw0flows | ovn_strip_lflows | \ >> + grep -E 'sw0-nf-p[[13]]' | sort], [0], [dnl >> + table=??(ls_out_pre_acl ), priority=115 , match=(inport == >> "sw0-nf-p1"), action=(reg8[[24]] = nf_lookup_orig_inport(); >> next(pipeline=egress, table=??);) >> + table=??(ls_out_pre_acl ), priority=115 , match=(inport == >> "sw0-nf-p3"), action=(reg8[[24]] = nf_lookup_orig_inport(); >> next(pipeline=egress, table=??);) >> +]) >> +AT_CHECK([grep 'ls_out_network_function' sw0flows | ovn_strip_lflows | \ >> + grep nf_lookup | sort], [0], [dnl >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw0-nf-p2"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "sw0-nf-p4"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> +]) >> + >> +AT_CHECK([grep 'ls_out_network_function' sw0flows | ovn_strip_lflows | \ >> + grep nf_learn | sort], [0], [dnl >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "sw0-nf-p4"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 102 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "sw0-nf-p4"; reg8[[23]] >> = 1; next(pipeline=ingress, table=??);) >> +]) >> + >> +AS_BOX([Without any inline-mode NFG attached, no loop-prevention flows]) >> + >> +check ovn-nbctl clear logical_switch sw0 acls >> +check as northd ovn-appctl -t ovn-northd inc-engine/recompute >> +check ovn-nbctl --wait=sb sync >> +ovn-sbctl dump-flows sw0 > sw0flows >> + >> +AT_CHECK([grep -E 'nf_(learn|lookup)_orig_inport' sw0flows | wc -l], [0], [0 >> +]) >> +AT_CHECK([grep -E 'reg8\[[24\]]' sw0flows | wc -l], [0], [0 >> +]) >> + >> +OVN_CLEANUP_NORTHD >> +AT_CLEANUP >> +]) >> + >> OVN_FOR_EACH_NORTHD([ >> AT_SETUP([Network function health check]) >> AT_KEYWORDS([ovn]) >> @@ -22093,15 +22279,20 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "child-1"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "child-2"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 1), action=(outport = "child-1"; >> output;) >> - table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1), action=(outport = "child-1"; >> output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "child-1"; output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "child-1"; output;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "child-1"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "child-2"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "child-2"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 1), action=(outport = "child-2"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> - table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1), action=(outport = "child-2"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "child-2"; reg8[[23]] >> = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "child-2"; reg8[[23]] = >> 1; next(pipeline=ingress, table=??);) >> ]) >> >> # Set the service monitor for nf0 to online and nf1 to online >> @@ -22134,15 +22325,20 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "child-1"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "child-2"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 1), action=(outport = "child-1"; >> output;) >> - table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1), action=(outport = "child-1"; >> output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "child-1"; output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "child-1"; output;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "child-1"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "child-2"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "child-2"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 1), action=(outport = "child-2"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> - table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1), action=(outport = "child-2"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "child-2"; reg8[[23]] >> = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 1 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "child-2"; reg8[[23]] = >> 1; next(pipeline=ingress, table=??);) >> ]) >> >> # Set the service monitor for nf0 to offline and nf1 to online >> @@ -22175,15 +22371,20 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "child-3"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(inport == >> "child-4"), action=(reg5[[16..31]] = ct_label.tun_if_id; next;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 2), action=(outport = "child-3"; >> output;) >> - table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 2), action=(outport = "child-3"; >> output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 2 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "child-3"; output;) >> + table=??(ls_in_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 2 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "child-3"; output;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "child-3"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(outport == >> "child-4"), action=(next;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_out_network_function), priority=2 , match=(inport == >> "child-4"), action=(reg8[[24]] = nf_lookup_orig_inport(); next;) >> table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 0 && ct_label.nf_id == 2), action=(outport = "child-4"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> - table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 2), action=(outport = "child-4"; >> reg8[[23]] = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 2 && ip4), >> action=(nf_learn_orig_inport(ipv6 = false); outport = "child-4"; reg8[[23]] >> = 1; next(pipeline=ingress, table=??);) >> + table=??(ls_out_network_function), priority=99 , match=(reg8[[21]] == 1 >> && reg8[[22]] == 1 && reg0[[22..29]] == 2 && ip6), >> action=(nf_learn_orig_inport(ipv6 = true); outport = "child-4"; reg8[[23]] = >> 1; next(pipeline=ingress, table=??);) >> ]) >> >> # Set the service monitor for nf0 to offline and nf1 to offline >> @@ -22212,6 +22413,8 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=0 , match=(1), action=(next;) >> table=??(ls_in_network_function), priority=1 , match=(reg8[[21]] == 1), >> action=(drop;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> @@ -22243,6 +22446,8 @@ AT_CHECK( >> table=??(ls_in_network_function), priority=0 , match=(1), action=(next;) >> table=??(ls_in_network_function), priority=1 , match=(reg8[[21]] == 1), >> action=(drop;) >> table=??(ls_in_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip4 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = false); next;) >> + table=??(ls_in_network_function), priority=50 , match=(ip6 && >> reg8[[21]] == 0), action=(nf_learn_orig_inport(ipv6 = true); next;) >> table=??(ls_out_network_function), priority=0 , match=(1), >> action=(next;) >> table=??(ls_out_network_function), priority=1 , match=(reg8[[21]] == >> 1), action=(drop;) >> table=??(ls_out_network_function), priority=100 , match=(reg8[[21]] == 1 >> && eth.mcast), action=(next;) >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 52fb75286..ea20a7d8d 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -45165,6 +45165,111 @@ OVN_CLEANUP_NORTHD >> AT_CLEANUP >> ]) >> >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([Network function -- inline post-NF re-flood loop prevention]) >> +AT_KEYWORDS([ovn]) >> +ovn_start >> + >> +# When an LS has ports with "unknown" addresses, a packet redirected to >> +# an inline NF can be re-flooded back out its original inport after NF >> +# processing. End-to-end check: sw0-p1 sends to an unknown MAC and the >> +# post-NF re-flood copy heading back out sw0-p1 is dropped, while sw0-p2 >> +# still receives it. >> + >> +check ovn-nbctl ls-add sw0 >> +check ovn-nbctl lsp-add sw0 sw0-p1 -- \ >> + lsp-set-addresses sw0-p1 "f0:00:00:00:00:01 192.168.0.11" "unknown" >> +check ovn-nbctl lsp-add sw0 sw0-p2 -- \ >> + lsp-set-addresses sw0-p2 "f0:00:00:00:00:02 192.168.0.12" "unknown" >> +for i in 1 2; do >> + check ovn-nbctl lsp-add sw0 sw0-nf-p$i -- \ >> + lsp-set-addresses sw0-nf-p$i "f0:00:00:00:01:0$i" >> +done >> +check ovn-nbctl set logical_switch_port sw0-nf-p1 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw0-nf-p2 >> +check ovn-nbctl set logical_switch_port sw0-nf-p2 \ >> + options:receive_multicast=false options:lsp_learn_mac=false \ >> + options:is-nf=true options:nf-linked-port=sw0-nf-p1 >> +check ovn-nbctl nf-add nf0 1 sw0-nf-p1 sw0-nf-p2 >> +check ovn-nbctl nfg-add nfg0 1 inline nf0 >> +check ovn-nbctl acl-add sw0 to-lport 1002 'outport == "sw0-p2" && ip4' \ >> + allow-related nfg0 >> + >> +# VLAN-backed: add a localnet port. >> +check ovn-nbctl lsp-add-localnet-port sw0 ln0 phys >> +check ovn-nbctl set logical_switch_port ln0 tag_request=100 >> + >> +net_add n >> +sim_add hv1 >> +as hv1 >> +ovs-vsctl add-br br-phys > > Missing 'check'. > >> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys >> +ovn_attach n br-phys 192.168.1.1 >> + >> +ovs-vsctl add-port br-int vif1 -- \ >> + set interface vif1 external-ids:iface-id=sw0-p1 \ >> + options:tx_pcap=hv1/vif1-tx.pcap options:rxq_pcap=hv1/vif1-rx.pcap >> +ovs-vsctl add-port br-int vif2 -- \ >> + set interface vif2 external-ids:iface-id=sw0-p2 \ >> + options:tx_pcap=hv1/vif2-tx.pcap options:rxq_pcap=hv1/vif2-rx.pcap >> +for i in 1 2; do >> + ovs-vsctl add-port br-int vif-nf$i -- \ >> + set interface vif-nf$i external-ids:iface-id=sw0-nf-p$i \ >> + options:tx_pcap=hv1/vif-nf$i-tx.pcap \ >> + options:rxq_pcap=hv1/vif-nf$i-rx.pcap > > Same missing 'check' for all of the above. > >> +done >> + >> +OVN_POPULATE_ARP >> +wait_for_ports_up >> +check ovn-nbctl --wait=hv sync >> + >> +# Match the test packet in the pcaps (src IP 192.168.0.11 followed by dst IP >> +# 192.168.0.12). >> +icmp_pkt_filter=c0a8000bc0a8000c > > The following reads a tiny bit better: > > src_ip=$(ip_to_hex 192 168 0 11) > dst_ip=$(ip_to_hex 192 168 0 12) > icmp_pkt_filter="${src_ip}${dst_ip}" > >> + >> +# sw0-p1 sends to an unknown destination MAC. The flooded copy destined to >> +# sw0-p2 is redirected to the NF output port (sw0-nf-p2); >> +packet="inport==\"sw0-p1\" && eth.src==f0:00:00:00:00:01 && >> + eth.dst==00:00:00:00:00:99 && ip.ttl==64 && ip4.src==192.168.0.11 && >> + ip4.dst==192.168.0.12 && icmp4.type==8 && icmp4.code==0" >> +check as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" >> +OVS_WAIT_UNTIL([ >> + pkts=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif-nf2-tx.pcap | >> grep ${icmp_pkt_filter} -c` >> + test $pkts -ge 1 >> +]) >> + >> +# The NF returns the packet on sw0-nf-p1 with an unknown dst MAC, triggering >> +# a re-flood. Loop prevention drops the copy returning to sw0-p1 (original >> +# inport), while sw0-p2 receives its copy. >> +packet="inport==\"sw0-nf-p1\" && eth.src==f0:00:00:00:00:01 && >> + eth.dst==00:00:00:00:00:99 && ip.ttl==64 && ip4.src==192.168.0.11 && >> + ip4.dst==192.168.0.12 && icmp4.type==8 && icmp4.code==0" >> +check as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" >> +OVS_WAIT_UNTIL([ >> + pkts=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap | >> grep ${icmp_pkt_filter} -c` >> + test $pkts -ge 1 >> +]) >> + >> +# The original inport must not receive the looped-back copy. >> +AT_CHECK([ >> + pkts=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap | >> grep ${icmp_pkt_filter} -c` >> + test 0 -eq $pkts >> +]) >> + >> +# The loop-prevention drop flow that has REGBIT_NF_LOOKUP_HIT >> (reg8=0x1000000) >> +# bit set must have hit at least once. >> +OVS_WAIT_UNTIL([ >> + hits=`as hv1 ovs-ofctl dump-flows br-int | grep "reg8=0x1000000" | \ >> + grep -v n_packets=0 | wc -l` >> + test $hits -ge 1 >> +]) >> + >> +OVN_CLEANUP([hv1 >> +ignored_tables=OFTABLE_GET_FDB,OFTABLE_LOOKUP_FDB]) >> +AT_CLEANUP >> +]) >> + >> OVN_FOR_EACH_NORTHD([ >> AT_SETUP([Unicast ARP when proxy ARP is configured]) >> CHECK_SCAPY > > I took care of the issues flagged above and applied this to main.
Thanks a lot Dumitru for addressing the issues. > > Regards, > Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
