> On 23 Jul 2026, at 10:17 PM, Naveen Yerramneni > <[email protected]> wrote: > > > >> 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.
Hi Dumitru, I have addressed your comments and sent v5. Thanks, Naveen > >> >>>> >>>>>> >>>>>>> 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
