> On 23 Jul 2026, at 10:11 PM, Dumitru Ceara <[email protected]> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> On 7/23/26 6:24 PM, Naveen Yerramneni wrote:
>> 
>> 
>>> On 23 Jul 2026, at 9:12 PM, Dumitru Ceara <[email protected]> wrote:
>>> 
>>> !-------------------------------------------------------------------|
>>> CAUTION: External Email
>>> 
>>> |-------------------------------------------------------------------!
>>> 
>>> On 7/23/26 5:30 PM, Naveen Yerramneni wrote:
>>>> 
>>>> 
>>>>> On 23 Jul 2026, at 3:35 PM, Dumitru Ceara <[email protected]> wrote:
>>>>> 
>>>>> !-------------------------------------------------------------------|
>>>>> CAUTION: External Email
>>>>> 
>>>>> |-------------------------------------------------------------------!
>>>>> 
>>>>> 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?
>>>> 
>>>> 
>>>> There is a "receive_multicast" option in Logical_Switch_Port
>>>> options that controls whether unknown-unicast and multicast
>>>> packets are forwarded to a port.  Today this option is not
>>>> propagated to the SB Port_Binding.
>>>> 
>>>> This is the reason I thought of adding mc_unknown_ingress
>>>> option in SB Port_Binding table.
>>>> 
>>> 
>>> I see.  What about the alternative approach of generating the action
>>> that sets the flag only for ports that are actually part of the
>>> mc_unknown group?
>>> 
>>> I.e., in consider_mc_group() in physical.c.
>>> 
>>> Wouldn't that work?
>> 
>> Instead of directly setting the flag by logical flows you prefer
>> calling a logical action which set this flag ?
>> 
>> To call this action in the logical pipeline, should we piggy-back
>> on some of the existing flows if possible ?
>> 
> 
> I meant something else actually.  The goal is to set the bit for packets
> received on ports that are part of the MC_Unknown SB multicast group.
> 
> In ovn-controller we know exactly what those ports are.  So I was
> thinking that in physical.c we could take advantage of that and set the
> bit there.
> 
> If we go the logical flows way the change wouldn't be too different than
> what you were proposing in this patch and I didn't really like that
> approach because it's complicated to understand the combinations of
> conditions under which the bit is set.
> 
> So maybe we can instead have a side openflow table and in
> load_logical_ingress_metadata() resubmit to that side table.  The side
> table will contain rules that match on datapath_key + logical_inport
> (metadata + reg14) and set the bit to 1 if that port is part of the
> mc_unknown multicast group.
> 
> Side tables are defined in lflow.h, e.g.:
> 
> #define OFTABLE_CT_ORIG_PROTO_LOAD        110
> #define OFTABLE_GET_REMOTE_FDB            111
> #define OFTABLE_LEARN_REMOTE_FDB          112
> #define OFTABLE_EVPN_ARP_LOOKUP           113
> 
> In physical.c, consider_mc_group() could populate the new side table
> when MC_Unknown groups change.
> 
> Wouldn't that work and avoid the need for more SB options?

Thanks for explanation! 

I think this should be ok, will look into this.

> 
>>> 
>>>>> 
>>>>>> 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.
>>>> 
>>>> Ack.
>>>> 
>>>>> 
>>>>>> 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.
>>>> 
>>>> Ack.
>>>> 
>>>>> 
>>>>> 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.
>>>> 
>>>> Ack.
>>>> 
>>>> Thanks,
>>>> Naveen
>>>> 
>>> 
>>> Regards,
>>> Dumitru


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

Reply via email to