On 8/12/26 4:30 PM, Naveen Yerramneni wrote: > > >> On 12 Aug 2026, at 3:35 PM, Dumitru Ceara <[email protected]> wrote: >> >> !-------------------------------------------------------------------| >> CAUTION: External Email >> >> |-------------------------------------------------------------------! >> >> Hi Naveen, > > Hi Dumitru, >
Hi Naveen, >> >> On 8/11/26 5:10 PM, Naveen Yerramneni wrote: >>> >>> >>>> On 11 Aug 2026, at 8:27 PM, Naveen Yerramneni >>>> <[email protected]> wrote: >>>> >>>> >>>> >>>>> On 10 Aug 2026, at 7:02 PM, Dumitru Ceara <[email protected]> wrote: >>>>> >>>>> !-------------------------------------------------------------------| >>>>> CAUTION: External Email >>>>> >>>>> |-------------------------------------------------------------------! >>>>> >>>>> Hi Naveen, >>>> >>>> Hi Dumitru, >>>> >>>>> >>>>> Thanks for v5, please see some review comments below. But before we go >>>>> to v6 I'd like to first agree on the right path forward about the new >>>>> flag this patch is adding, more details below inline. >>>> >>>> Thanks for the review! >>>> >>>>> >>>>> On 8/4/26 4:31 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 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. >>>>>> >>>>>> - Introduce a logical flag flags.inport_in_mc_unknown (bit 26) >>>>>> that marks packets entering on an MC_UNKNOWN-member port (i.e. a >>>>>> port with "unknown" in addresses and receive_multicast not >>>>>> disabled, the only ports that can both originate and receive >>>>>> unknown-unicast floods). ovn-controller sets it in the >>>>>> physical-to-logical stage (OFTABLE_PHY_TO_LOG resubmits into a >>>>>> new side table OFTABLE_INPORT_IN_MC_UNKNOWN_LOOKUP), which is >>>>>> populated for the local members of the per-datapath _MC_unknown >>>>>> multicast group in SB. The flag is carried in MFF_LOG_FLAGS >>>>>> into the egress pipeline. >>>>>> >>>>>> - 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_inport()) for unicast IP packets that entered on >>>>>> an MC_UNKNOWN-member port (flags.inport_in_mc_unknown == 1). >>>>>> 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. >>>>>> * 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. >>>>>> >>>>>> 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 >>>>> >>>>> Same nit about your signed-off-by that should be last. >>>> >>>> Ack. >>>> >>>>> >>>>>> --- >>>>>> TODO.rst | 10 ++ >>>>>> controller/lflow.h | 1 + >>>>>> controller/physical.c | 39 +++++ >>>>>> include/ovn/logical-fields.h | 4 + >>>>>> lib/logical-fields.c | 5 + >>>>>> northd/northd.c | 151 +++++++++++++--- >>>>>> ovn-nb.xml | 13 ++ >>>>>> tests/ovn-macros.at | 1 + >>>>>> tests/ovn-northd.at | 326 +++++++++++++++++++++++++++++------ >>>>>> tests/ovn.at | 113 ++++++++++++ >>>>>> 10 files changed, 585 insertions(+), 78 deletions(-) >>>>>> >>>>>> 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/controller/lflow.h b/controller/lflow.h >>>>>> index fa206392f..27104486f 100644 >>>>>> --- a/controller/lflow.h >>>>>> +++ b/controller/lflow.h >>>>>> @@ -106,6 +106,7 @@ struct uuid; >>>>>> #define OFTABLE_LEARN_REMOTE_FDB 112 >>>>>> #define OFTABLE_EVPN_ARP_LOOKUP 113 >>>>>> #define OFTABLE_NF_ORIG_INPORT_LEARN 114 >>>>>> +#define OFTABLE_INPORT_IN_MC_UNKNOWN_LOOKUP 115 >>>>>> >>>>>> /* Verify that table regions do not overlap. */ >>>>>> BUILD_ASSERT_DECL(OFTABLE_LOG_INGRESS_PIPELINE + LOG_PIPELINE_INGRESS_LEN >>>>>> diff --git a/controller/physical.c b/controller/physical.c >>>>>> index 452256e7a..3cd7b2195 100644 >>>>>> --- a/controller/physical.c >>>>>> +++ b/controller/physical.c >>>>>> @@ -1605,6 +1605,9 @@ load_logical_ingress_metadata(const struct >>>>>> sbrec_port_binding *binding, >>>>>> } >>>>>> put_load(encap_id, MFF_LOG_ENCAP_ID, 16, 16, ofpacts_p); >>>>>> } >>>>>> + >>>>>> + /* Resubmit to OFTABLE_INPORT_IN_MC_UNKNOWN_LOOKUP table. */ >>>>>> + put_resubmit(OFTABLE_INPORT_IN_MC_UNKNOWN_LOOKUP, ofpacts_p); >>>>> >>>>> So I've been thinking some more about this. On v4 (I think) I asked >>>>> whether it's not an option to just execute nf_learn_orig_inport() on all >>>>> IP packets in the NF logical stages. >>>>> >>>>> In the end it's just an optimization to run it only on packets received >>>>> from ports configured with "unknown" addresses. It wouldn't hurt if we >>>>> just ran it on all IP packets, right? >>>>> >>>>> That would remove the need for the new inport_in_mc_unknown flag all >>>>> together right? >>>>> >>>>> And it should just work fine for well configured clusters. It would >>>>> also solve the performance related concerns I have when looking at this >>>>> patch (I know I suggested the side table but): >>>>> - the resubmit will happen for all packets ingressing on all logical >>>>> switches >>>>> - the check in "consider_mc_group()" in physical.c is kind of ugly >>>>> because it singles out "unknown" multicast groups, it may also be >>>>> relatively costly as we need to manage these new flows as well >>>>> - the very specific flag name itself (inport_in_mc_unknown) is already a >>>>> sign that we're just working around a specific problem, ideally what >>>>> happens in the physical->logical stage should be generic enough, this is >>>>> not so much the case if we accept your patch. >>>> >>>> >>>> Yes, this is an optimization to learn entries only from ports configured >>>> with "unknown" addresses, so we add only the required entries to >>>> OFTABLE_NF_ORIG_INPORT_LEARN table. >>>> >>>> I also considered learning on the NF node before redirect. I did not use >>>> that approach because it would add more flows (all redirected flows) to >>>> OFTABLE_NF_ORIG_INPORT_LEARN table on the NF node. We would likely >>>> also need a new stage at the end of, or after, ingress to detect and drop >>>> loopback >>>> packets on the NF node before egress pipeline, since egress processing >>>> runs on the >>>> destination node. >>>> >>>> >>> >>> Hi Dumitru, >>> >>> If you prefer to always learn the inport, I will update the code and send >>> v6. >> >> At this point it seems like the simplest/safest approach to me so I'd >> prefer always learning the inport if the packet is going to a NF. >> >> If in the future we realize we need to restrict this further (e.g., due >> to too many flows being learned) we can revisit it and think of a better >> way. As said above, one of the reasons I'm reluctant to accept the v5 >> approach is because I'm not sure what impact it may have at scale on >> non-NF deployments. > > Sure. I will send v6 with the changes. > Thanks. >> >>> Also, I am thinking to set the learned flows idle timeout to 10 secs >>> instead of 30secs >>> to match default idle timeout of OVS data path offloaded flows. >>> >> >> I'm not against it but I'm wondering what you're trying to achieve with >> that. > > Idle flows will get evicted a bit early. It may not make significant > difference. > Setting below OVS data path offloaded idle timeout value may cause > frequent up calls hence thought of setting it to 10 secs. > I see, that might be fine. [...] >>>>>> @@ -19907,6 +19989,25 @@ build_network_function(const struct >>>>>> ovn_datapath *od, >>>>>> } >>>>>> } >>>>>> } >>>>>> + >>>>>> + if (has_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 when the packet enters >>>>>> + * on an MC_unknown-member port, so a post-NF loopback copy can >>>>>> + * be detected and dropped when the destination port has NF >>>>>> + * redirection enabled. */ >>>>> >>>>> I'm confused about why doing this in "overlay only" switches? If we go >>>>> for my suggestion of unconditionally using the learn action then we can >>>>> just add it as part of the priority-99 flows installed in the NF_IN/OUT >>>>> stages right? >>>> >>>> This is still needed for overlay subnets. In VLAN-backed cases, post-NF >>>> return >>>> stays on the same node path where redirect-path learn already ran. In >>>> overlay, redirect may happen on the destination node, while post-NF flood >>>> may return toward the source node. Without the default-path learn, we >>>> cannot reliably catch loopback packets. >>>> >> >> So does this mean we'll always hit the learn flows for these kinds of >> switches (when they have NF applied) even if the packet is not going to >> be sent out to a network function? > > Yes. > As long as that's only for overlay switches with NF applied I guess that's ok. Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
