On 8/13/26 11:12 PM, Dumitru Ceara wrote:
> 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.

Hi Naveen,

Just an FYI, branching (hard freeze) is scheduled for this Friday
(August 21st).  If you want this series to make it in for 26.09 it would
be preferable to prioritize posting the new version today or tomorrow so
I can get some time to properly review it.

Thanks,
Dumitru


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to