Add extra action called "get_remote_fdb()", which is almost the same as "get_fdb()" but goes into the new OFTABLE_GET_REMOTE_FDB table. Use the new action in the logical flows to determine if the traffic should go towards the remote FDB. The current implementation will give a priority to remove FDB in case there is duplicate MAC between the SB FDB table and the remote FDB table.
In order to make the logical action that assigns to two registers work, add default flows for the FDB and remote FDB side tables. The packet would be dropped otherwise if it would fall through the table without finding any FDB. This doesn't change the behavior for non-EVPN LS, because the packet is either dropped or will go towards the "unknown" multicast group as previously. Reported-at: https://issues.redhat.com/browse/FDP-1387 Signed-off-by: Ales Musil <amu...@redhat.com> --- controller/lflow.c | 1 + controller/physical.c | 14 +++++++++ include/ovn/actions.h | 3 ++ include/ovn/logical-fields.h | 3 ++ lib/actions.c | 53 +++++++++++++++++++++++++------ lib/logical-fields.c | 5 +++ lib/ovn-util.c | 4 +-- northd/northd.c | 61 +++++++++++++++++++++++++++++------- northd/northd.h | 3 ++ tests/ovn.at | 19 +++++++++-- tests/test-ovn.c | 1 + utilities/ovn-trace.c | 6 ++++ 12 files changed, 148 insertions(+), 25 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 04d7748fe..e6f8e26e7 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -885,6 +885,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, .lb_hairpin_reply_ptable = OFTABLE_CHK_LB_HAIRPIN_REPLY, .ct_snat_vip_ptable = OFTABLE_CT_SNAT_HAIRPIN, .fdb_ptable = OFTABLE_GET_FDB, + .remote_fdb_ptable = OFTABLE_GET_REMOTE_FDB, .fdb_lookup_ptable = OFTABLE_LOOKUP_FDB, .in_port_sec_ptable = OFTABLE_CHK_IN_PORT_SEC, .out_port_sec_ptable = OFTABLE_CHK_OUT_PORT_SEC, diff --git a/controller/physical.c b/controller/physical.c index be233cba0..1f9468790 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -3393,5 +3393,19 @@ physical_run(struct physical_ctx *p_ctx, physical_eval_remote_chassis_flows(p_ctx, &ofpacts, flow_table); physical_eval_evpn_flows(p_ctx, &ofpacts, flow_table); + /* Default flow for OFTABLE_GET_FDB table. */ + match_init_catchall(&match); + ofpbuf_clear(&ofpacts); + put_load(0, MFF_LOG_OUTPORT, 0, 32, &ofpacts); + ofctrl_add_flow(flow_table, OFTABLE_GET_FDB, 0, 0, + &match, &ofpacts, hc_uuid); + + /* Default flow for OFTABLE_GET_REMOTE_FDB table. */ + match_init_catchall(&match); + ofpbuf_clear(&ofpacts); + put_load(0, MFF_LOG_REMOTE_OUTPORT, 0, 32, &ofpacts); + ofctrl_add_flow(flow_table, OFTABLE_GET_REMOTE_FDB, 0, 0, + &match, &ofpacts, hc_uuid); + ofpbuf_uninit(&ofpacts); } diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 30e5e4229..0eaef9112 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -121,6 +121,7 @@ struct collector_set_ids; OVNACT(SCTP_ABORT, ovnact_nest) \ OVNACT(PUT_FDB, ovnact_put_fdb) \ OVNACT(GET_FDB, ovnact_get_fdb) \ + OVNACT(GET_REMOTE_FDB, ovnact_get_fdb) \ OVNACT(LOOKUP_FDB, ovnact_lookup_fdb) \ OVNACT(CHECK_IN_PORT_SEC, ovnact_result) \ OVNACT(CHECK_OUT_PORT_SEC, ovnact_result) \ @@ -956,6 +957,8 @@ struct ovnact_encode_params { * 'ct_snat_to_vip' to resubmit. */ uint8_t fdb_ptable; /* OpenFlow table for * 'get_fdb' to resubmit. */ + uint8_t remote_fdb_ptable; /* OpenFlow table for + * 'get_remote_fdb' to resubmit. */ uint8_t fdb_lookup_ptable; /* OpenFlow table for * 'lookup_fdb' to resubmit. */ uint8_t in_port_sec_ptable; /* OpenFlow table for diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h index 8b1c1221e..9a8bf4874 100644 --- a/include/ovn/logical-fields.h +++ b/include/ovn/logical-fields.h @@ -63,6 +63,9 @@ enum ovn_controller_event { #define MFF_LOG_CT_SAVED_STATE MFF_REG4 +#define MFF_LOG_REMOTE_OUTPORT MFF_REG1 /* Logical remote output + * port (32 bits). */ + /* Logical registers that are needed for backwards * compatibility with older northd versions. * XXX: All of them can be removed in 26.09. */ diff --git a/lib/actions.c b/lib/actions.c index fa8aa20c2..e2ececc17 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -4397,19 +4397,22 @@ ovnact_put_fdb_free(struct ovnact_put_fdb *put_fdb OVS_UNUSED) } static void -format_GET_FDB(const struct ovnact_get_fdb *get_fdb, struct ds *s) +format_get_fdb(const struct ovnact_get_fdb *get_fdb, const char *type, + struct ds *s) { expr_field_format(&get_fdb->dst, s); - ds_put_cstr(s, " = get_fdb("); + ds_put_format(s, " = %s(", type); expr_field_format(&get_fdb->mac, s); ds_put_cstr(s, ");"); } static void -encode_GET_FDB(const struct ovnact_get_fdb *get_fdb, +encode_get_fdb(const struct ovnact_get_fdb *get_fdb, const struct ovnact_encode_params *ep, - struct ofpbuf *ofpacts) + bool remote, struct ofpbuf *ofpacts) { + const enum mf_field_id outport_id = + remote ? MFF_LOG_REMOTE_OUTPORT : MFF_LOG_OUTPORT; struct mf_subfield dst = expr_resolve_field(&get_fdb->dst); ovs_assert(dst.field); @@ -4417,25 +4420,53 @@ encode_GET_FDB(const struct ovnact_get_fdb *get_fdb, { expr_resolve_field(&get_fdb->mac), MFF_ETH_DST }, }; encode_setup_args(args, ARRAY_SIZE(args), ofpacts); - put_load(0, MFF_LOG_OUTPORT, 0, 32, ofpacts); - emit_resubmit(ofpacts, ep->fdb_ptable); + put_load(0, outport_id, 0, 32, ofpacts); + emit_resubmit(ofpacts, remote ? ep->remote_fdb_ptable : ep->fdb_ptable); encode_restore_args(args, ARRAY_SIZE(args), ofpacts); - if (dst.field->id != MFF_LOG_OUTPORT) { + if (dst.field->id != outport_id) { struct ofpact_reg_move *orm = ofpact_put_REG_MOVE(ofpacts); orm->dst = dst; - orm->src.field = mf_from_id(MFF_LOG_OUTPORT); + orm->src.field = mf_from_id(outport_id); orm->src.ofs = 0; orm->src.n_bits = 32; } } +static void +format_GET_FDB(const struct ovnact_get_fdb *get_fdb, struct ds *s) +{ + format_get_fdb(get_fdb, "get_fdb", s); +} + +static void +encode_GET_FDB(const struct ovnact_get_fdb *get_fdb, + const struct ovnact_encode_params *ep, + struct ofpbuf *ofpacts) +{ + encode_get_fdb(get_fdb, ep, false, ofpacts); +} + +static void +format_GET_REMOTE_FDB(const struct ovnact_get_fdb *get_fdb, struct ds *s) +{ + format_get_fdb(get_fdb, "get_remote_fdb", s); +} + +static void +encode_GET_REMOTE_FDB(const struct ovnact_get_fdb *get_fdb, + const struct ovnact_encode_params *ep, + struct ofpbuf *ofpacts) +{ + encode_get_fdb(get_fdb, ep, true, ofpacts); +} + static void parse_get_fdb(struct action_context *ctx, struct expr_field *dst, struct ovnact_get_fdb *get_fdb) { - lexer_get(ctx->lexer); /* Skip get_bfd. */ + lexer_get(ctx->lexer); /* Skip get_bfd/get_remote_fdb. */ lexer_get(ctx->lexer); /* Skip '('. */ /* Validate that the destination is a 32-bit, modifiable field if it @@ -5777,6 +5808,10 @@ parse_set_action(struct action_context *ctx) && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { parse_get_fdb( ctx, &lhs, ovnact_put_GET_FDB(ctx->ovnacts)); + } else if (!strcmp(ctx->lexer->token.s, "get_remote_fdb") + && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { + parse_get_fdb( + ctx, &lhs, ovnact_put_GET_REMOTE_FDB(ctx->ovnacts)); } else if (!strcmp(ctx->lexer->token.s, "lookup_fdb") && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) { parse_lookup_fdb( diff --git a/lib/logical-fields.c b/lib/logical-fields.c index e479a78c1..726c0542d 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -72,6 +72,11 @@ ovn_init_symtab(struct shash *symtab) expr_symtab_add_string(symtab, "inport", MFF_LOG_INPORT, NULL); expr_symtab_add_string(symtab, "outport", MFF_LOG_OUTPORT, NULL); + /* The port isn't reserved along the pipeline it's just defined as symbol + * to support matching on string and moving between string registers. */ + expr_symtab_add_string(symtab, "remote_outport", + MFF_LOG_REMOTE_OUTPORT, NULL); + /* Logical registers: * 128-bit xxregs * 64-bit xregs diff --git a/lib/ovn-util.c b/lib/ovn-util.c index e8a32c96a..152de2bc6 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -901,8 +901,8 @@ 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 "1158333617 10744" -#define OVN_INTERNAL_MINOR_VER 9 +#define OVN_NORTHD_PIPELINE_CSUM "2405300854 10800" +#define OVN_INTERNAL_MINOR_VER 10 /* Returns the OVN version. The caller must free the returned value. */ char * diff --git a/northd/northd.c b/northd/northd.c index 4ab1800ba..b45cac3cf 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -254,12 +254,14 @@ static const char *reg_ct_state[] = { * | R0 | REGBIT_{CONNTRACK/DHCP/DNS} | | | * | | REGBIT_{HAIRPIN/HAIRPIN_REPLY} | | | * | | REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | | | - * | | REGBIT_ACL_{LABEL/STATELESS} | X | | - * +----+----------------------------------------------+ X | | - * | R1 | REG_CT_TP_DST (0..15) | R | | - * | | REG_CT_PROTO (16..23) | E | | - * | | (>= IN_CT_EXTRACT && <= IN_LB_AFF_LEARN) | G | | - * +----+----------------------------------------------+ 0 | | + * | | REGBIT_ACL_{LABEL/STATELESS} | | | + * +----+----------------------------------------------+ | | + * | R1 | REG_CT_TP_DST (0..15) | | | + * | | REG_CT_PROTO (16..23) | | | + * | | (>= IN_CT_EXTRACT && <= IN_LB_AFF_LEARN) | X | | + * | | remote_outport | X | | + * | | (>= L2_LKUP && <= L2_UNKNOWN) | R | | + * +----+----------------------------------------------+ E | | * | R2 | REG_LB_PORT | G | | * | | (>= IN_PRE_STATEFUL && <= IN_LB_AFF_LEARN) | 0 | | * | | REG_ACL_ID | | | @@ -823,10 +825,10 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) "%u", age_threshold); } - int64_t vni = ovn_smap_get_llong(&od->nbs->other_config, - "dynamic-routing-vni", -1); - if (ovn_is_valid_vni(vni)) { - smap_add_format(&ids, "dynamic-routing-vni", "%"PRIi64, vni); + if (od->has_evpn_vni) { + const char *vni = smap_get(&od->nbs->other_config, + "dynamic-routing-vni"); + smap_add(&ids, "dynamic-routing-vni", vni); } } @@ -983,6 +985,12 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, false)) { od->lb_with_stateless_mode = true; } + + int64_t vni = ovn_smap_get_llong(&od->nbs->other_config, + "dynamic-routing-vni", -1); + if (ovn_is_valid_vni(vni)) { + od->has_evpn_vni = true; + } } const struct nbrec_logical_router *nbr; @@ -6233,12 +6241,17 @@ build_lswitch_learn_fdb_od( struct lflow_ref *lflow_ref) { ovs_assert(od->nbs); + const char *lkp_action = od->has_evpn_vni + ? "outport = get_fdb(eth.dst); " + "remote_outport = get_remote_fdb(eth.dst); next;" + : "outport = get_fdb(eth.dst); next;"; + ovn_lflow_add(lflows, od, S_SWITCH_IN_LOOKUP_FDB, 0, "1", "next;", lflow_ref); ovn_lflow_add(lflows, od, S_SWITCH_IN_PUT_FDB, 0, "1", "next;", lflow_ref); ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 0, "1", - "outport = get_fdb(eth.dst); next;", lflow_ref); + lkp_action, lflow_ref); ovn_lflow_add(lflows, od, S_SWITCH_OUT_LOOKUP_FDB, 0, "1", "next;", lflow_ref); @@ -9768,6 +9781,26 @@ is_vlan_transparent(const struct ovn_datapath *od) return smap_get_bool(&od->nbs->other_config, "vlan-passthru", false); } +static void +build_lswitch_lflows_evpn_l2_unknown(struct ovn_datapath *od, + struct lflow_table *lflows, + struct lflow_ref *lflow_ref) +{ + if (od->has_unknown) { + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50, + "outport == \"none\" && remote_outport == \"none\"", + "outport = \""MC_UNKNOWN "\"; output;", lflow_ref); + } else { + ovn_lflow_add_drop_with_desc( + lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50, "outport == \"none\" && " + "remote_outport == \"none\"", "No L2 destination", lflow_ref); + } + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 25, + "remote_outport == \"none\"", "output;", lflow_ref); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 0, "1", + "outport = remote_outport; output;", lflow_ref); +} + static void build_lswitch_lflows_l2_unknown(struct ovn_datapath *od, struct lflow_table *lflows, @@ -18021,7 +18054,11 @@ build_lswitch_and_lrouter_iterate_by_ls(struct ovn_datapath *od, ovn_lflow_add(lsi->lflows, od, S_SWITCH_IN_CT_EXTRACT, 0, "1", "next;", NULL); build_lswitch_lb_affinity_default_flows(od, lsi->lflows, NULL); - build_lswitch_lflows_l2_unknown(od, lsi->lflows, NULL); + if (od->has_evpn_vni) { + build_lswitch_lflows_evpn_l2_unknown(od, lsi->lflows, NULL); + } else { + build_lswitch_lflows_l2_unknown(od, lsi->lflows, NULL); + } build_mcast_flood_lswitch(od, lsi->lflows, &lsi->actions, NULL); } diff --git a/northd/northd.h b/northd/northd.h index ac00c4ec4..fbd76704d 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -399,6 +399,9 @@ struct ovn_datapath { * This is applicable only to routers with "remote" ports. */ bool is_transit_router; + /* Indicates that the LS has valid vni associated with it. */ + bool has_evpn_vni; + /* OVN northd only needs to know about logical router gateway ports for * NAT/LB on a distributed router. The "distributed gateway ports" are * populated only when there is a gateway chassis or ha chassis group diff --git a/tests/ovn.at b/tests/ovn.at index 2a834dc41..1dfbd5434 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -2087,6 +2087,13 @@ reg15 = get_fdb(eth.dst); outport = get_fdb(ip4.dst); Cannot use 32-bit field ip4.dst[[0..31]] where 48-bit field is required. +# get_remote_fdb +reg1 = get_remote_fdb(eth.dst); + encodes as set_field:0->reg1,resubmit(,OFTABLE_GET_REMOTE_FDB),move:NXM_NX_REG1[[]]->NXM_NX_XXREG0[[64..95]] + +reg1 = get_remote_fdb(eth.src); + encodes as push:NXM_OF_ETH_DST[[]],push:NXM_OF_ETH_SRC[[]],pop:NXM_OF_ETH_DST[[]],set_field:0->reg1,resubmit(,OFTABLE_GET_REMOTE_FDB),pop:NXM_OF_ETH_DST[[]],move:NXM_NX_REG1[[]]->NXM_NX_XXREG0[[64..95]] + # lookup_fdb reg0[[0]] = lookup_fdb(inport, eth.src); encodes as set_field:0/0x100->reg10,resubmit(,OFTABLE_LOOKUP_FDB),move:NXM_NX_REG10[[8]]->NXM_NX_XXREG0[[96]] @@ -32987,10 +32994,12 @@ as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_GET_FDB > hv2_offlows_table71.t AT_CAPTURE_FILE([hv1_offlows_table71.txt]) AT_CAPTURE_FILE([hv2_offlows_table71.txt]) AT_CHECK_UNQUOTED([cat hv1_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:13 actions=load:0x$port_key->NXM_NX_REG15[[]] ]) AT_CHECK_UNQUOTED([cat hv2_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:13 actions=load:0x$port_key->NXM_NX_REG15[[]] ]) @@ -33024,6 +33033,7 @@ AT_CAPTURE_FILE([hv3_offlows_table71.txt]) AT_CAPTURE_FILE([hv3_offlows_table72.txt]) AT_CHECK_UNQUOTED([cat hv3_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:13 actions=load:0x$port_key->NXM_NX_REG15[[]] ]) @@ -33056,16 +33066,19 @@ AT_CAPTURE_FILE([hv1_offlows_table71.txt]) AT_CAPTURE_FILE([hv2_offlows_table71.txt]) AT_CAPTURE_FILE([hv3_offlows_table71.txt]) AT_CHECK_UNQUOTED([cat hv1_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:13 actions=load:0x$port_key->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:14 actions=load:0x$port_key->NXM_NX_REG15[[]] ]) AT_CHECK_UNQUOTED([cat hv2_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:13 actions=load:0x$port_key->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:14 actions=load:0x$port_key->NXM_NX_REG15[[]] ]) AT_CHECK_UNQUOTED([cat hv3_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:13 actions=load:0x$port_key->NXM_NX_REG15[[]] priority=100,metadata=0x$dp_key,dl_dst=50:54:00:00:00:14 actions=load:0x$port_key->NXM_NX_REG15[[]] ]) @@ -33258,10 +33271,12 @@ as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_GET_FDB > hv2_offlows_table71.t AT_CAPTURE_FILE([hv1_offlows_table71.txt]) AT_CAPTURE_FILE([hv2_offlows_table71.txt]) -AT_CHECK([cat hv1_offlows_table71.txt | grep -v NXST], [1], [dnl +AT_CHECK([cat hv1_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] ]) -AT_CHECK([cat hv2_offlows_table71.txt | grep -v NXST], [1], [dnl +AT_CHECK([cat hv2_offlows_table71.txt | grep -v NXST | cut -d ' ' -f8- | sort], [0], [dnl +priority=0 actions=load:0->NXM_NX_REG15[[]] ]) as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_LOOKUP_FDB > hv1_offlows_table72.txt diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 1580f44c4..fae7e7bd5 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1380,6 +1380,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) .lb_hairpin_reply_ptable = OFTABLE_CHK_LB_HAIRPIN_REPLY, .ct_snat_vip_ptable = OFTABLE_CT_SNAT_HAIRPIN, .fdb_ptable = OFTABLE_GET_FDB, + .remote_fdb_ptable = OFTABLE_GET_REMOTE_FDB, .fdb_lookup_ptable = OFTABLE_LOOKUP_FDB, .common_nat_ct_zone = MFF_LOG_DNAT_ZONE, .in_port_sec_ptable = OFTABLE_CHK_IN_PORT_SEC, diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index a63a3be19..caeec9619 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -3536,6 +3536,12 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, execute_get_fdb(ovnact_get_GET_FDB(a), dp, uflow); break; + case OVNACT_GET_REMOTE_FDB: + ovntrace_node_append(super, OVNTRACE_NODE_OUTPUT, + "/* The remote FDB table is different" + " per each chassis. */"); + break; + case OVNACT_LOOKUP_FDB: execute_lookup_fdb(ovnact_get_LOOKUP_FDB(a), dp, uflow, super); break; -- 2.50.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
