On 7/23/26 11:27 AM, Naveen Yerramneni wrote: > > >> On 20 Jul 2026, at 7:34 PM, Dumitru Ceara <[email protected]> wrote: >> >> !-------------------------------------------------------------------| >> CAUTION: External Email >> >> |-------------------------------------------------------------------! >> >> On 7/14/26 10:21 AM, 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. >>> >>> 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 source port): >>> >>> 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 >>> source port). >>> >>> Fix: >>> >>> Use the nf_learn_orig_src_port() / nf_lookup_orig_src_port() 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. >>> >>> - Add a logical flag flags.inport_in_mc_unknown (bit 26) that >>> marks packets entering on an MC_UNKNOWN-member port (i.e. one >>> with "unknown" in addresses and receive_multicast not disabled, >>> the only ports that can both originate and receive >>> unknown-unicast floods). northd sets it in ls_in_lookup_fdb, >>> piggybacking on the existing FDB-learn or flags.localnet flow >>> where possible, and emits a dedicated priority-50 flow for the >>> remaining MC_UNKNOWN members (ex: ports where >>> lsp_learn_fdb=false). >>> >>> - In the NF redirect stage (ls_in_nf for a from-lport ACL, >>> ls_out_nf for a to-lport ACL), on a switch with an inline NF group >>> on an ACL, learn the original ingress port >>> (nf_learn_orig_src_port()) for unicast IP packets that entered on >>> an MC_UNKNOWN-member port (flags.inport_in_mc_unknown set). The >>> flag is set in ls_in_lookup_fdb and carried in MFF_LOG_FLAGS into >>> the egress pipeline, so it is still available in ls_out_nf. Two >>> flows per IP version: >>> * Priority 100: learn, then redirect to the NF, when the ACL >>> selected the packet for redirect. The existing NF-port and >>> multicast skip flows move up to priority 110 so multicast is >>> skipped before this flow, which therefore needs no !eth.mcast >>> match. >>> * Priority 50 (overlay switches only, in ls_in_nf): learn, then >>> continue to the next stage, when the ACL did not redirect the >>> packet (REGBIT_NF_ENABLED == 0). On an overlay switch the >>> redirecting port and the source port can be on different nodes, >>> so the priority-100 learn would land on a different node than >>> the lookup; learning here, on the source port's own ingress >>> node, keeps them co-located. VLAN-backed switches do not need >>> this: the re-flood returns to the redirecting node, where the >>> priority-100 learn already ran. >>> >>> - On the post-NF return path run >>> REGBIT_NF_LOOKUP_HIT = nf_lookup_orig_src_port() on both NF >>> ports. The existing ls_out_pre_acl skip-stages flow does it for >>> the input_port (priority bumped 110->115 to win over the >>> priority-110 conntrack skip flow); a new priority-2 flow in >>> ls_out_nf does it for the output_port (packets redirected from >>> the ingress pipeline, e.g. a from-lport request re-flooded after >>> the NF, doing a fresh egress after conntrack). >>> >>> - 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. >>> >>> On VLAN-backed switches the re-flood returns to the redirecting node >>> and egresses the shared localnet port, so the learn and the lookup >>> always run on that node: dropping the copy headed back out that port >>> removes both the MAC flap and the underlay L2 loop. >>> >>> On overlay switches the copy headed back to the source port is always >>> dropped, as in example 3: the learn runs on the source port's ingress >>> and the lookup on its egress, both on the source port's node. >>> >>> All new flows are gated on the switch having an inline NF group on >>> an ACL. >>> >>> Signed-off-by: Naveen Yerramneni <[email protected]> >>> 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 >>> --- >> >> Hi Naveen, >> >>> NEWS | 3 + >>> include/ovn/logical-fields.h | 9 + >>> lib/logical-fields.c | 5 + >>> northd/northd.c | 257 ++++++++++++++++++----- >>> northd/northd.h | 19 ++ >>> ovn-sb.xml | 8 + >>> tests/ovn-northd.at | 393 +++++++++++++++++++++++++++++------ >>> tests/ovn.at | 114 +++++++++- >>> 8 files changed, 690 insertions(+), 118 deletions(-) >>> >>> diff --git a/NEWS b/NEWS >>> index 384e30820..9a7f03f19 100644 >>> --- a/NEWS >>> +++ b/NEWS >>> @@ -68,6 +68,9 @@ Post v26.03.0 >>> (type 11) and Parameter Problem (type 12) - generated by an external >>> router are un-NATed correctly. This makes Path MTU discovery and >>> traceroute work through stateless NAT. >>> + - Fixed MAC flaps and L2 loops caused by inline Network_Function >>> + redirection of unknown-unicast IP traffic, where the copy returning >>> + from the NF could be re-flooded out of the original ingress port. >>> >> >> This is arguably a bug fix, it doesn't really need a NEWS item. OTOH, >> if we ever need to backport this we need to ensure that northd doesn't >> use the new actions if not all ovn-controller are running an updated >> version. I presume your goal is to just fix this behavior on versions >>> = 26.09.0. >> >>> OVN v26.03.0 - xxx xx xxxx >>> -------------------------- >>> diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h >>> index b2f1af64a..e3dddd0c4 100644 >>> --- a/include/ovn/logical-fields.h >>> +++ b/include/ovn/logical-fields.h >>> @@ -141,6 +141,7 @@ enum mff_log_flags_bits { >>> MLF_RECIRC_BIT = 24, >>> MLF_EVPN_LOOKUP_BIT = 25, >>> MLF_NF_LOOKUP_HIT_BIT = 26, >>> + MLF_INPORT_IN_MC_UNKNOWN_BIT = 27, >>> MLF_NETWORK_ID_START_BIT = 28, >>> MLF_NETWORK_ID_END_BIT = 31, >>> }; >>> @@ -226,6 +227,14 @@ enum mff_log_flags { >>> * MAC flaps / L2 loops after network function redirection. */ >>> MLF_NF_LOOKUP_HIT = (1 << MLF_NF_LOOKUP_HIT_BIT), >>> >>> + /* Indicate that the packet entered the logical switch on a port that >>> + * is a member of MC_UNKNOWN (i.e. the port has "unknown" in its >>> + * addresses and "receive_multicast" is not disabled, so it can both >>> + * receive and originate unknown-unicast floods). Used by the inline >>> + * network function loop prevention to learn the original source port >>> + * before redirection. */ >>> + MLF_INPORT_IN_MC_UNKNOWN = (1 << MLF_INPORT_IN_MC_UNKNOWN_BIT), >>> + >>> /* Assign network ID to packet to choose correct network for snat when >>> * lb_force_snat_ip=router_ip. */ >>> MLF_NETWORK_ID = (OVN_MAX_NETWORK_ID << MLF_NETWORK_ID_START_BIT), >>> diff --git a/lib/logical-fields.c b/lib/logical-fields.c >>> index 35d9a2e22..8295d71f4 100644 >>> --- a/lib/logical-fields.c >>> +++ b/lib/logical-fields.c >>> @@ -188,6 +188,11 @@ ovn_init_symtab(struct shash *symtab) >>> snprintf(flags_str, sizeof flags_str, "flags[%d]", MLF_PKT_SAMPLED_BIT); >>> expr_symtab_add_subfield(symtab, "flags.pkt_sampled", NULL, flags_str); >>> >>> + snprintf(flags_str, sizeof flags_str, "flags[%d]", >>> + MLF_INPORT_IN_MC_UNKNOWN_BIT); >>> + expr_symtab_add_subfield(symtab, "flags.inport_in_mc_unknown", NULL, >>> + flags_str); >>> + >>> /* Connection tracking state. */ >>> expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, >>> false, >>> WR_CT_COMMIT); >>> diff --git a/northd/northd.c b/northd/northd.c >>> index 3a4afa063..65ed1edaf 100644 >>> --- a/northd/northd.c >>> +++ b/northd/northd.c >>> @@ -176,6 +176,10 @@ 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 when a packet returning from an inline network function is about to >>> + * be sent back out of the port it originally arrived on; such loopback >>> + * copies 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 +319,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*) | | >>> | >>> @@ -6386,6 +6392,19 @@ build_lswitch_port_sec_op(struct ovn_port *op, >>> struct lflow_table *lflows, >>> } >>> } >>> >>> +/* True if 'op' emits the FDB-learn (LOOKUP_FDB / PUT_FDB) flows. */ >>> +static bool >>> +lsp_emits_fdb_learn_lflow(const struct ovn_port *op) >>> +{ >>> + if (op->lsp_has_port_sec || !op->has_unknown) { >>> + return false; >>> + } >>> + return lsp_is_remote(op->nbsp) >>> + || (!strcmp(op->nbsp->type, "") && lsp_can_learn_mac(op->nbsp)) >>> + || lsp_is_switch(op->nbsp) >>> + || (lsp_is_localnet(op->nbsp) && localnet_can_learn_mac(op->nbsp)); >>> +} >>> + >>> static void >>> build_lswitch_learn_fdb_op( >>> struct ovn_port *op, struct lflow_table *lflows, >>> @@ -6393,36 +6412,35 @@ build_lswitch_learn_fdb_op( >>> { >>> ovs_assert(op->nbsp); >>> >>> - if (op->lsp_has_port_sec || !op->has_unknown) { >>> + if (!lsp_emits_fdb_learn_lflow(op)) { >>> return; >>> } >>> >>> bool remote = lsp_is_remote(op->nbsp); >>> >>> - if (remote || (!strcmp(op->nbsp->type, "") && >>> lsp_can_learn_mac(op->nbsp)) >>> - || lsp_is_switch(op->nbsp) >>> - || (lsp_is_localnet(op->nbsp) && >>> localnet_can_learn_mac(op->nbsp))) { >>> - ds_clear(match); >>> - ds_clear(actions); >>> - ds_put_format(match, "inport == %s", op->json_key); >>> - if (lsp_is_localnet(op->nbsp)) { >>> - ds_put_cstr(actions, "flags.localnet = 1; "); >>> - } >>> - ds_put_format(actions, REGBIT_LKUP_FDB >>> - " = lookup_fdb(inport, eth.src); next;"); >>> - ovn_lflow_add(lflows, op->od, remote ? S_SWITCH_OUT_LOOKUP_FDB >>> - : S_SWITCH_IN_LOOKUP_FDB, >>> - 100, ds_cstr(match), ds_cstr(actions), op->lflow_ref, >>> - WITH_IO_PORT(op->key), >>> WITH_HINT(&op->nbsp->header_)); >>> - >>> - ds_put_cstr(match, " && "REGBIT_LKUP_FDB" == 0"); >>> - ds_clear(actions); >>> - ds_put_cstr(actions, "put_fdb(inport, eth.src); next;"); >>> - ovn_lflow_add(lflows, op->od, remote ? S_SWITCH_OUT_PUT_FDB >>> - : S_SWITCH_IN_PUT_FDB, >>> - 100, ds_cstr(match), ds_cstr(actions), op->lflow_ref, >>> - WITH_IO_PORT(op->key), >>> WITH_HINT(&op->nbsp->header_)); >>> + ds_clear(match); >>> + ds_clear(actions); >>> + ds_put_format(match, "inport == %s", op->json_key); >>> + if (lsp_is_localnet(op->nbsp)) { >>> + ds_put_cstr(actions, "flags.localnet = 1; "); >>> } >>> + if (!remote && lsp_is_mc_unknown_ingress_member(op)) { >> >> lsp_is_mc_unknown_ingress_member() already checks that the port is not >> remote. >> >> Also why skip remote ports? >> >> But the larger question is: we're messing up the code quite a bit with >> this "is traffic coming from a LSP with <unknown> addresses set but that >> isn't a remote port" condition everywhere. What's the downside to using >> the learn action all the time? >> >> If we _really_ need this condition, why not just implement it through >> physical.c flows like we do for MLF_RX_FROM_TUNNEL_BIT for example? >> >> All these new logical flow additions in various places and with various >> combinations of conditions are very hard to maintain on the long term. > > Hi Dumitru, > > Thanks for the review. > > Apologies for the late reply. >
Hi Naveen, > I think we can remove the remote port check. > OK. > I was trying to avoid learning excessive entries in the > OFTABLE_NF_ORIG_INPORT_LEARN table, which made the patch a bit > more complex. > > As per your suggestion, we can set MLF_INPORT_IN_MC_UNKNOWN in the > physical pipeline. To keep this driven by northd, we can have > northd set options:mc_unknown_ingress on the SB Port_Binding for Why would we need that? We already have SB.Port_Binding.mac = "unknown" for those ports. I don't think we need a new Port_Binding option. Right? > eligible LSPs, and load MLF_INPORT_IN_MC_UNKNOWN in the > OFTABLE_PHY_TO_LOG stage when that option is set. NF stages keep > the existing "flags.inport_in_mc_unknown == 1" guard on > learn/lookup. > > If this approach looks fine, I'll address the rest of your comments > and send v5. > Sounds good. > On a separate note, while thinking through the L2 stretch case with > overlay subnets over OVN IC (spine/leaf, VMs on leaf LSes connected > via a Transit_Switch with type=remote ports), I noticed a gap when > there are ports with "unknown" addresses on both AZs and the NF is > not co-located with the IC gateway that decapsulates the packet: > the post-NF flood copy can still leak back out towards the source > side and cause a MAC flap on the Transit_Switch FDB. > > For now I'd like to call out the L2 stretch case as a known > limitation and request to consider this patch as the immediate fix > for non-stretch deployments. > OK, we have the TODO.rst file for such known gaps. We should also document it in the NB man page I guess. > For a longer-term solution I'm still exploring options. One > direction I'm considering is to extend the Geneve header so that > post-NF packets carry the original source inport, and use it on > the receiving chassis to drop copies whose outport matches the > carried inport. This keeps the "same context of how the packet > was redirected" available on the return path without needing a > learn/lookup table. Let me know your thoughts, or if there are > better alternatives. One thing to be careful with when using such approaches is upgrades. We need to make sure both source and destination ovn chassis properly handle the new geneve value. Also, this will have further implications on the overlay MTU, so we need to properly document this kind of change, at least. In any case, let's discuss it further in the next development cycle as this cannot reasonably make it in time for 26.09. > > Thanks, > Naveen > Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
