On 8/4/26 4:31 AM, Naveen Yerramneni wrote: > This commit introduces two NF-related logical actions, nf_learn_orig_inport() > and nf_lookup_orig_inport(), used to store and retrieve a packet's original > ingress port. These actions will be used by northd to identify and drop > post-NF loopback copies in logical pipeline stages. > > - "nf_learn_orig_inport(ipv6 = true|false)" installs a learned flow in > a new OFTABLE_NF_ORIG_INPORT_LEARN table. The learned flow is keyed > on the logical datapath and the IP source and destination addresses > and matches future packets whose logical output port equals the > logical input port of the packet that created it, i.e. packets that > are about to be sent back out of the port they entered on. Matching > packets get MLF_POST_NF_LOOP_BACK set. The learned flow has a 30s idle > timeout. > > - "R = nf_lookup_orig_inport()" resubmits to the learn table and stores > the result of the lookup (MLF_POST_NF_LOOP_BACK) into the 1-bit field R. >
Hi Naveen, Thanks for the new version! > Signed-off-by: Naveen Yerramneni <[email protected]> > Acked-by: Aditya Mehakare <[email protected]> > CC: Sragdhara Datta Chaudhuri <[email protected]> > Assisted-by: Claude Opus 4.7, Cursor Actually, being pedantic, your Signed-off-by should be the last tag in the commit message, it indicates that you "agree to the Developer's Certificate of Origin" (as mentioned in our docs in submitting-patches.rst) for the whole patch, including the AI-assisted changes. > --- > controller/lflow.h | 1 + > include/ovn/actions.h | 14 +++ > include/ovn/logical-fields.h | 4 + > lib/actions.c | 190 +++++++++++++++++++++++++++++++++++ > lib/ovn-util.c | 2 +- > ovn-sb.xml | 57 +++++++++++ > tests/ovn-macros.at | 1 + > tests/ovn.at | 19 ++++ > utilities/ovn-trace.c | 18 ++++ > 9 files changed, 305 insertions(+), 1 deletion(-) > > diff --git a/controller/lflow.h b/controller/lflow.h > index c211fe195..fa206392f 100644 > --- a/controller/lflow.h > +++ b/controller/lflow.h > @@ -105,6 +105,7 @@ struct uuid; > #define OFTABLE_GET_REMOTE_FDB 111 > #define OFTABLE_LEARN_REMOTE_FDB 112 > #define OFTABLE_EVPN_ARP_LOOKUP 113 > +#define OFTABLE_NF_ORIG_INPORT_LEARN 114 Nit: alignment. > > /* Verify that table regions do not overlap. */ > BUILD_ASSERT_DECL(OFTABLE_LOG_INGRESS_PIPELINE + LOG_PIPELINE_INGRESS_LEN > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 2b81f3f05..6a394b16b 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -140,6 +140,8 @@ struct collector_set_ids; > OVNACT(CT_STATE_SAVE, ovnact_result) \ > OVNACT(MIRROR, ovnact_mirror) \ > OVNACT(CHK_EVPN_ARP, ovnact_chk_evpn_arp) \ > + OVNACT(NF_LEARN_ORIG_INPORT, ovnact_nf_learn) \ > + OVNACT(NF_LOOKUP_ORIG_INPORT, ovnact_nf_lookup) \ Nit: indentation of \. > > /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ > enum OVS_PACKED_ENUM ovnact_type { > @@ -513,6 +515,18 @@ struct ovnact_lookup_fdb { > struct expr_field dst; /* 1-bit destination field. */ > }; > > +/* OVNACT_NF_LEARN_ORIG_INPORT. */ > +struct ovnact_nf_learn { > + struct ovnact ovnact; > + bool ipv6; > +}; > + > +/* OVNACT_NF_LOOKUP_ORIG_INPORT. */ > +struct ovnact_nf_lookup { > + struct ovnact ovnact; > + struct expr_field dst; /* 1-bit destination field. */ > +}; > + > /* OVNACT_SAMPLE */ > struct ovnact_sample { > struct ovnact ovnact; > diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h > index 9694d105f..e95dee316 100644 > --- a/include/ovn/logical-fields.h > +++ b/include/ovn/logical-fields.h > @@ -140,6 +140,7 @@ enum mff_log_flags_bits { > MLF_PKT_SAMPLED_BIT = 23, > MLF_RECIRC_BIT = 24, > MLF_EVPN_LOOKUP_BIT = 25, > + MLF_POST_NF_LOOP_BACK_BIT = 26, > MLF_NETWORK_ID_START_BIT = 28, > MLF_NETWORK_ID_END_BIT = 31, > }; > @@ -219,6 +220,9 @@ enum mff_log_flags { > /* Indicate that the lookup in the EVPN ARP table was successful. */ > MLF_EVPN_LOOKUP = (1 << MLF_EVPN_LOOKUP_BIT), > > + /* Set on a post-NF packet going back to the original ingress port. */ > + MLF_POST_NF_LOOP_BACK = (1 << MLF_POST_NF_LOOP_BACK_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/actions.c b/lib/actions.c > index 93d7aefd2..0cdfc5efe 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -4717,6 +4717,189 @@ ovnact_lookup_fdb_free(struct ovnact_lookup_fdb > *get_fdb OVS_UNUSED) > { > } > > +static void > +format_NF_LEARN_ORIG_INPORT(const struct ovnact_nf_learn *nf_learn, > + struct ds *s) Nit: alignment. > +{ > + ds_put_format(s, "nf_learn_orig_inport(ipv6 = %s);", > + nf_learn->ipv6 ? "true" : "false"); > +} > + > +/* Idle timeout, in seconds, of the flow installed by > + * nf_learn_orig_inport(). */ > +#define NF_LEARN_ORIG_INPORT_IDLE_TIMEOUT_S 30 > + > +/* Adds a NXAST_LEARN spec matching the value of field 'id' (as it is in the > + * packet that triggers the learn) on the same field of future packets. */ > +static void > +nf_learn_put_match_field(struct ofpbuf *ofpacts, enum mf_field_id id) > +{ > + struct ofpact_learn_spec *ol_spec = > + ofpbuf_put_zeros(ofpacts, sizeof *ol_spec); > + ol_spec->dst.field = mf_from_id(id); > + ol_spec->dst.ofs = 0; > + ol_spec->dst.n_bits = ol_spec->dst.field->n_bits; > + ol_spec->n_bits = ol_spec->dst.n_bits; > + ol_spec->dst_type = NX_LEARN_DST_MATCH; > + ol_spec->src_type = NX_LEARN_SRC_FIELD; > + ol_spec->src.field = mf_from_id(id); > +} > + > +static void > +encode_NF_LEARN_ORIG_INPORT(const struct ovnact_nf_learn *nf_learn, > + const struct ovnact_encode_params *ep, > + struct ofpbuf *ofpacts) Nit: alignment. > +{ > + size_t ol_offset = ofpacts->size; > + struct ofpact_learn *ol = ofpact_put_LEARN(ofpacts); > + struct match match = MATCH_CATCHALL_INITIALIZER; > + struct ofpact_learn_spec *ol_spec; > + unsigned int imm_bytes; > + uint8_t *src_imm; > + > + ol->flags = NX_LEARN_F_DELETE_LEARNED; > + ol->idle_timeout = NF_LEARN_ORIG_INPORT_IDLE_TIMEOUT_S; > + ol->hard_timeout = OFP_FLOW_PERMANENT; > + ol->priority = OFP_DEFAULT_PRIORITY; > + ol->table_id = OFTABLE_NF_ORIG_INPORT_LEARN; > + ol->cookie = htonll(ep->lflow_uuid.parts[0]); > + > + /* Match on the logical datapath. */ > + nf_learn_put_match_field(ofpacts, MFF_METADATA); > + > + /* Match on the same ETH type as the packet that created the flow. */ > + ol_spec = ofpbuf_put_zeros(ofpacts, sizeof *ol_spec); > + ol_spec->dst.field = mf_from_id(MFF_ETH_TYPE); > + ol_spec->dst.ofs = 0; > + ol_spec->dst.n_bits = ol_spec->dst.field->n_bits; > + ol_spec->n_bits = ol_spec->dst.n_bits; > + ol_spec->dst_type = NX_LEARN_DST_MATCH; > + ol_spec->src_type = NX_LEARN_SRC_IMMEDIATE; > + union mf_value imm_eth_type = { > + .be16 = nf_learn->ipv6 ? htons(ETH_TYPE_IPV6) : htons(ETH_TYPE_IP) > + }; > + mf_write_subfield_value(&ol_spec->dst, &imm_eth_type, &match); > + /* Push value last, as this may reallocate 'ol_spec'. */ > + imm_bytes = DIV_ROUND_UP(ol_spec->dst.n_bits, 8); > + src_imm = ofpbuf_put_zeros(ofpacts, OFPACT_ALIGN(imm_bytes)); > + memcpy(src_imm, &imm_eth_type, imm_bytes); > + > + /* Match on the IP source and destination addresses. */ > + nf_learn_put_match_field(ofpacts, > + nf_learn->ipv6 ? MFF_IPV6_SRC : MFF_IPV4_SRC); > + nf_learn_put_match_field(ofpacts, > + nf_learn->ipv6 ? MFF_IPV6_DST : MFF_IPV4_DST); > + > + /* Match future packets whose logical output port equals the logical > + * input port of the packet that created this flow. */ > + ol_spec = ofpbuf_put_zeros(ofpacts, sizeof *ol_spec); > + ol_spec->dst.field = mf_from_id(MFF_LOG_OUTPORT); > + ol_spec->dst.ofs = 0; > + ol_spec->dst.n_bits = ol_spec->dst.field->n_bits; > + ol_spec->n_bits = ol_spec->dst.n_bits; > + ol_spec->dst_type = NX_LEARN_DST_MATCH; > + ol_spec->src_type = NX_LEARN_SRC_FIELD; > + ol_spec->src.field = mf_from_id(MFF_LOG_INPORT); > + > + /* Set MLF_POST_NF_LOOP_BACK on matching packets. */ > + ol_spec = ofpbuf_put_zeros(ofpacts, sizeof *ol_spec); > + ol_spec->dst.field = mf_from_id(MFF_LOG_FLAGS); > + ol_spec->dst.ofs = MLF_POST_NF_LOOP_BACK_BIT; > + ol_spec->dst.n_bits = 1; > + ol_spec->n_bits = ol_spec->dst.n_bits; > + ol_spec->dst_type = NX_LEARN_DST_LOAD; > + ol_spec->src_type = NX_LEARN_SRC_IMMEDIATE; > + union mf_value imm_hit = { .u8 = 1 }; > + mf_write_subfield_value(&ol_spec->dst, &imm_hit, &match); > + /* Push value last, as this may reallocate 'ol_spec'. */ > + imm_bytes = DIV_ROUND_UP(ol_spec->dst.n_bits, 8); > + src_imm = ofpbuf_put_zeros(ofpacts, OFPACT_ALIGN(imm_bytes)); > + memcpy(src_imm, &imm_hit, imm_bytes); > + > + ol = ofpbuf_at_assert(ofpacts, ol_offset, sizeof *ol); > + ofpact_finish_LEARN(ofpacts, &ol); > +} > + > +static void > +parse_nf_learn_orig_inport(struct action_context *ctx, > + struct ovnact_nf_learn *nf_learn) Nit: alignment. > +{ > + lexer_force_match(ctx->lexer, LEX_T_LPAREN); > + if (!lexer_match_id(ctx->lexer, "ipv6")) { > + lexer_syntax_error(ctx->lexer, "invalid parameter"); > + return; > + } > + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { > + return; > + } > + if (lexer_match_id(ctx->lexer, "true")) { > + nf_learn->ipv6 = true; > + } else if (lexer_match_id(ctx->lexer, "false")) { > + nf_learn->ipv6 = false; > + } else { > + lexer_syntax_error(ctx->lexer, "expecting true or false"); > + return; > + } > + lexer_force_match(ctx->lexer, LEX_T_RPAREN); > +} > + > +static void > +ovnact_nf_learn_free(struct ovnact_nf_learn *nf_learn OVS_UNUSED) > +{ > +} > + > +static void > +format_NF_LOOKUP_ORIG_INPORT(const struct ovnact_nf_lookup *nf_lookup, > + struct ds *s) Nit: alignment. > +{ > + expr_field_format(&nf_lookup->dst, s); > + ds_put_cstr(s, " = nf_lookup_orig_inport();"); > +} > + > +static void > +encode_NF_LOOKUP_ORIG_INPORT( > + const struct ovnact_nf_lookup *nf_lookup, > + const struct ovnact_encode_params *ep OVS_UNUSED, > + struct ofpbuf *ofpacts) > +{ > + struct mf_subfield dst = expr_resolve_field(&nf_lookup->dst); > + ovs_assert(dst.field); > + > + put_load(0, MFF_LOG_FLAGS, MLF_POST_NF_LOOP_BACK_BIT, 1, ofpacts); > + emit_resubmit(ofpacts, OFTABLE_NF_ORIG_INPORT_LEARN); > + > + struct ofpact_reg_move *orm = ofpact_put_REG_MOVE(ofpacts); > + orm->dst = dst; > + orm->src.field = mf_from_id(MFF_LOG_FLAGS); > + orm->src.ofs = MLF_POST_NF_LOOP_BACK_BIT; > + orm->src.n_bits = 1; > +} > + > +static void > +parse_nf_lookup_orig_inport(struct action_context *ctx, > + struct expr_field *dst, > + struct ovnact_nf_lookup *nf_lookup) Nit: alignment. > +{ > + lexer_get(ctx->lexer); /* Skip nf_lookup_orig_inport. */ > + lexer_force_match(ctx->lexer, LEX_T_LPAREN); > + > + /* Validate that the destination is a 1-bit, modifiable field. */ > + char *error = expr_type_check(dst, 1, true, ctx->scope); > + if (error) { > + lexer_error(ctx->lexer, "%s", error); > + free(error); > + return; > + } > + nf_lookup->dst = *dst; > + > + lexer_force_match(ctx->lexer, LEX_T_RPAREN); > +} > + > +static void > +ovnact_nf_lookup_free(struct ovnact_nf_lookup *nf_lookup OVS_UNUSED) > +{ > +} > + > static void > parse_check_in_port_sec(struct action_context *ctx, > const struct expr_field *dst, > @@ -5976,6 +6159,10 @@ parse_set_action(struct action_context *ctx) > && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { > parse_lookup_fdb( > ctx, &lhs, ovnact_put_LOOKUP_FDB(ctx->ovnacts)); > + } else if (!strcmp(ctx->lexer->token.s, "nf_lookup_orig_inport") > + && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { > + parse_nf_lookup_orig_inport( > + ctx, &lhs, ovnact_put_NF_LOOKUP_ORIG_INPORT(ctx->ovnacts)); > } else if (!strcmp(ctx->lexer->token.s, "check_in_port_sec") > && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { > parse_check_in_port_sec( > @@ -6147,6 +6334,9 @@ parse_action(struct action_context *ctx) > ovnact_put_FLOOD_REMOTE(ctx->ovnacts); > } else if (lexer_match_id(ctx->lexer, "mirror")) { > parse_MIRROR_action(ctx); > + } else if (lexer_match_id(ctx->lexer, "nf_learn_orig_inport")) { > + parse_nf_learn_orig_inport( > + ctx, ovnact_put_NF_LEARN_ORIG_INPORT(ctx->ovnacts)); > } else { > lexer_syntax_error(ctx->lexer, "expecting action"); > } > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index d9b78fbb6..35e085d99 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -1007,7 +1007,7 @@ ip_address_and_port_from_lb_key(const char *key, char > **ip_address, > * > * NOTE: If OVN_NORTHD_PIPELINE_CSUM is updated make sure to double check > * whether an update of OVN_INTERNAL_MINOR_VER is required. */ > -#define OVN_NORTHD_PIPELINE_CSUM "118971668 11318" > +#define OVN_NORTHD_PIPELINE_CSUM "849109292 11426" > #define OVN_INTERNAL_MINOR_VER 15 This needs to be bumped too, see submitting-patches.rst. > > /* Returns the OVN version. The caller must free the returned value. */ > diff --git a/ovn-sb.xml b/ovn-sb.xml > index e403eb360..a6c8b6c76 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -1832,6 +1832,63 @@ > </p> > </dd> > > + <dt> > + <code>nf_learn_orig_inport(ipv6 = <var>B</var>);</code> > + </dt> > + > + <dd> > + <p> > + Records the logical input port that the current IP packet > + entered the logical switch on by learning a flow. The learned > + flow matches a later packet of the same flow (same logical > + datapath and IP source and destination addresses) when its > + logical output port equals that input port, that is, when the > + packet is about to be sent back out of the port it originally > + arrived on. > + </p> > + > + <p> > + <var>B</var> must be <code>true</code> to learn an IPv6 flow or > + <code>false</code> to learn an IPv4 flow. The learned flow has a > + 30 second idle timeout. This action is used together with > + <code>nf_lookup_orig_inport()</code> to drop the duplicate > + copies that a network function (NF) redirection can produce when > + the destination MAC address is still unknown, which would > + otherwise cause MAC address flaps or L2 loops. > + </p> > + > + <p> > + <b>Example:</b> > + <code>nf_learn_orig_inport(ipv6 = false);</code> > + </p> > + </dd> > + > + <dt> > + <code><var>R</var> = nf_lookup_orig_inport();</code> > + </dt> > + > + <dd> > + <p> > + <b>Result</b>: stored to a 1-bit subfield <var>R</var>. > + </p> > + > + <p> > + Looks up the flow learned by > + <code>nf_learn_orig_inport()</code>. Stores <code>1</code> in > + <var>R</var> if the current packet matches a learned flow, that > is, > + if it is an IP packet that, after network function redirection, > is > + about to be sent back out of the logical input port it originally > + arrived on. Otherwise stores <code>0</code>. > + </p> > + > + <p> > + <b>Example:</b> > + <code> > + reg0[0] = nf_lookup_orig_inport(); > + </code> > + </p> > + </dd> > + > <dt><code>nd_ns { <var>action</var>; </code>...<code> };</code></dt> > <dd> > <p> > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index ab14b65c6..2a1faf677 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -1655,5 +1655,6 @@ m4_define([OFTABLE_CT_ORIG_PROTO_LOAD], [110]) > m4_define([OFTABLE_GET_REMOTE_FDB], [111]) > m4_define([OFTABLE_LEARN_REMOTE_FDB], [112]) > m4_define([OFTABLE_EVPN_ARP_LOOKUP], [113]) > +m4_define([OFTABLE_NF_ORIG_INPORT_LEARN], [114]) > > m4_define([OFTABLE_SAVE_INPORT_HEX], [m4_eval(OFTABLE_SAVE_INPORT, 16)]) > diff --git a/tests/ovn.at b/tests/ovn.at > index a88ea9650..47bf5f4ec 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -2129,6 +2129,25 @@ reg1[[1]] = lookup_fdb(outport, ip4.src); > reg1[[1]] = lookup_fdb(ip4.src, eth.src); > Cannot use numeric field ip4.src where string field is required. > > +# nf_learn_orig_inport / nf_lookup_orig_inport > +nf_learn_orig_inport(ipv6 = false); > + encodes as > learn(table=OFTABLE_NF_ORIG_INPORT_LEARN,idle_timeout=30,delete_learned,cookie=0xaaaaaaaa,OXM_OF_METADATA[[]],eth_type=0x800,NXM_OF_IP_SRC[[]],NXM_OF_IP_DST[[]],NXM_NX_REG15[[]]=NXM_NX_REG14[[0..-1]],load:0x1->NXM_NX_REG10[[26]]) > + > +nf_learn_orig_inport(ipv6 = true); > + encodes as > learn(table=OFTABLE_NF_ORIG_INPORT_LEARN,idle_timeout=30,delete_learned,cookie=0xaaaaaaaa,OXM_OF_METADATA[[]],eth_type=0x86dd,NXM_NX_IPV6_SRC[[]],NXM_NX_IPV6_DST[[]],NXM_NX_REG15[[]]=NXM_NX_REG14[[0..-1]],load:0x1->NXM_NX_REG10[[26]]) > + > +nf_learn_orig_inport(); > + Syntax error at `)' invalid parameter. > + > +nf_learn_orig_inport(ipv6 = maybe); > + Syntax error at `maybe' expecting true or false. > + > +reg0[[0]] = nf_lookup_orig_inport(); > + encodes as > set_field:0/0x4000000->reg10,resubmit(,OFTABLE_NF_ORIG_INPORT_LEARN),move:NXM_NX_REG10[[26]]->NXM_NX_XXREG0[[96]] > + > +reg0 = nf_lookup_orig_inport(); > + Cannot use 32-bit field reg0[[0..31]] where 1-bit field is required. > + > # check_in_port_sec > reg0[[0]] = check_in_port_sec(); > encodes as > set_field:0/0x1000->reg10,resubmit(,OFTABLE_CHK_IN_PORT_SEC),move:NXM_NX_REG10[[12]]->NXM_NX_XXREG0[[96]] > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > index 0dcee89ac..535cbe9b5 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -3216,6 +3216,17 @@ execute_ct_save_state(const struct ovnact_result *dl, > struct flow *uflow, > ds_destroy(&s); > } > > +static void > +execute_nf_lookup_orig_inport(const struct ovnact_nf_lookup *nf_lookup, > + struct flow *uflow, struct ovs_list *super) Nit: alignment. > +{ > + struct mf_subfield dst = expr_resolve_field(&nf_lookup->dst); > + union mf_subvalue sv = { .u8_val = 0 }; > + mf_write_subfield_flow(&dst, &sv, uflow); > + ovntrace_node_append(super, OVNTRACE_NODE_ACTION, > + "/* Assumed nf_lookup_orig_inport miss. */"); > +} > + > static void > execute_ct_orig_tp_dst(const struct ovnact_result *res, struct flow *uflow, > struct ovs_list *super) > @@ -3641,6 +3652,13 @@ trace_actions(const struct ovnact *ovnacts, size_t > ovnacts_len, > execute_chk_evpn_arp(ovnact_get_CHK_EVPN_ARP(a), dp, uflow, > super); > break; > + case OVNACT_NF_LEARN_ORIG_INPORT: > + /* Nothing to do for tracing. */ > + break; > + case OVNACT_NF_LOOKUP_ORIG_INPORT: > + execute_nf_lookup_orig_inport( > + ovnact_get_NF_LOOKUP_ORIG_INPORT(a), uflow, super); > + break; > } > } > ofpbuf_uninit(&stack); Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
