> On 19 Aug 2026, at 4:36 PM, Dumitru Ceara <[email protected]> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> 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,

Hi Dumitru,

> 
> 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.

Sorry for the delay. I addressed your comments and sent v6.

During testing, we observed learned flows in 
OFTABLE_NF_ORIG_INPORT_LEARN are usually idle unless a post-NF
loopback packet occurs, because outport is part of the match criteria.
As a result, learned flows can age out even when the connection is
still active, causing additional upcalls. To reduce this, I increased the
learned-flow idle timeout to 60 seconds.

Regarding high learned flows count: I do not yet have enough data to 
characterise
the system behaviour under high-flow conditions (for example, very high 
connection rates).

I can think of one corner case where packet drops may happen unintentionally:
If an underlay ToR misbehaves and sends a packet back on the same inport,
two learned flows with different outports get created for the same IP tuple.
This is not expected in normal deployments, but I wanted to mention it.

> 
> Thanks,
> Dumitru

Thanks,
Naveen

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

Reply via email to