From: Srikar Tati <[email protected]> An EVPN FDB entry with more than one path (multi-homing) uses an OpenFlow select group with selection_method=dp_hash. dp_hash needs a recirculation, which the tc datapath cannot offload, so the group stays in software on a NIC with hardware offload.
Add the "dynamic-routing-fdb-ecmp-selection-fields" option to Logical Switches. It takes the same fields as the "selection_fields" column of a static route. northd validates the option and copies it to the Datapath_Binding of an EVPN switch. ovn-controller then creates the FDB select groups with selection_method=hash on these fields. tp_src and tp_dst become the TCP, UDP and SCTP ports, because one group serves every protocol and ovs-vswitchd hashes only the fields that are present in a packet. evpn_fdb now also depends on SB Datapath_Binding, so that a change of the option updates the groups. Assisted-by: Claude Opus 5.5, Claude Code Signed-off-by: Srikar Tati <[email protected]> --- NEWS | 4 +++ controller/evpn-fdb.c | 33 +++++++++++++++++++++ controller/evpn-fdb.h | 3 ++ controller/ovn-controller.c | 46 +++++++++++++++++++++++++++++ controller/physical.c | 7 ++++- lib/ovn-util.c | 31 +++++++++++++++++++ lib/ovn-util.h | 2 ++ northd/en-datapath-logical-switch.c | 19 ++++++++++++ ovn-nb.xml | 25 ++++++++++++++++ tests/ovn-inc-proc-graph-dump.at | 2 ++ tests/ovn-northd.at | 42 ++++++++++++++++++++++++++ tests/system-ovn.at | 29 ++++++++++++++++++ 12 files changed, 242 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 5a822cc21..cfb331a45 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,10 @@ Post v26.09.0 Logical Routers. It selects the fields that ECMP groups of learned routes hash on, the same as the "selection_fields" column of a static route. + * Add the "other_config:dynamic-routing-fdb-ecmp-selection-fields" + option to Logical Switches. It selects the fields that the + OpenFlow groups of multi-path EVPN FDB entries hash on, instead of + dp_hash. - Removed OVN's ovs-bugtool plugin and helper scripts. - Removed ovn-sim utility scripts. - Removed ovn-docker utility scripts. diff --git a/controller/evpn-fdb.c b/controller/evpn-fdb.c index 76ab816f3..acb97ab8f 100644 --- a/controller/evpn-fdb.c +++ b/controller/evpn-fdb.c @@ -24,6 +24,7 @@ #include "openvswitch/dynamic-string.h" #include "openvswitch/vlog.h" #include "ovn-sb-idl.h" +#include "ovn-util.h" #include "packets.h" #include "unixctl.h" @@ -125,6 +126,27 @@ evpn_fdb_resolve_paths(const struct evpn_fdb_ctx_in *f_ctx_in, vector_qsort(paths, evpn_fdb_path_cmp); } +/* Returns the fields of the ECMP select group of the FDB entries of 'edp' + * for selection_method=hash, or NULL for selection_method=dp_hash. */ +static char * +evpn_fdb_hash_fields(const struct evpn_datapath *edp) +{ + const char *fields_s = + smap_get(&edp->ldp->datapath->external_ids, + "dynamic-routing-fdb-ecmp-selection-fields"); + if (!fields_s) { + return NULL; + } + + struct sset fields = SSET_INITIALIZER(&fields); + char *hash_fields = NULL; + if (ovn_ecmp_selection_fields_parse(fields_s, &fields)) { + hash_fields = ovn_ecmp_selection_fields_to_hash_fields(&fields); + } + sset_destroy(&fields); + return hash_fields; +} + /* Returns true if the paths in 'fdb' match the contents of 'paths'. * Caller must ensure 'paths' is non-empty (memcmp with NULL is UB). */ static bool @@ -192,6 +214,15 @@ evpn_fdb_run(const struct evpn_fdb_ctx_in *f_ctx_in, updated = true; } + char *hash_fields = evpn_fdb_hash_fields(edp); + if (!nullable_string_is_equal(fdb->hash_fields, hash_fields)) { + free(fdb->hash_fields); + fdb->hash_fields = hash_fields; + updated = true; + } else { + free(hash_fields); + } + if (updated) { hmapx_add(f_ctx_out->updated_fdbs, fdb); } @@ -208,6 +239,7 @@ evpn_fdb_run(const struct evpn_fdb_ctx_in *f_ctx_in, uuidset_insert(f_ctx_out->removed_fdbs, &fdb->flow_uuid); hmap_remove(f_ctx_out->fdbs, &fdb->hmap_node); vector_destroy(&fdb->paths); + free(fdb->hash_fields); free(fdb); } @@ -220,6 +252,7 @@ evpn_fdbs_destroy(struct hmap *fdbs) struct evpn_fdb *fdb; HMAP_FOR_EACH_POP (fdb, hmap_node, fdbs) { vector_destroy(&fdb->paths); + free(fdb->hash_fields); free(fdb); } hmap_destroy(fdbs); diff --git a/controller/evpn-fdb.h b/controller/evpn-fdb.h index 65e1fc4fd..6ed3a343e 100644 --- a/controller/evpn-fdb.h +++ b/controller/evpn-fdb.h @@ -65,6 +65,9 @@ struct evpn_fdb { /* Contains 'struct evpn_fdb_path', one per ECMP path. * For single-path entries len == 1. */ struct vector paths; + /* Fields of the ECMP select group for selection_method=hash, NULL for + * selection_method=dp_hash. */ + char *hash_fields; }; void evpn_fdb_run(const struct evpn_fdb_ctx_in *, struct evpn_fdb_ctx_out *); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index d57ff316d..84842c2c7 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -6967,6 +6967,47 @@ evpn_fdb_vtep_binding_handler(struct engine_node *node, void *data OVS_UNUSED) return EN_UNHANDLED; } +static enum engine_input_handler_result +evpn_fdb_datapath_binding_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + const struct sbrec_datapath_binding_table *dp_table = + EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); + const struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + const struct sbrec_datapath_binding *dp; + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { + if (sbrec_datapath_binding_is_new(dp) || + sbrec_datapath_binding_is_deleted(dp)) { + /* The removal and addition is handled via the + * en_neighbor_exchange I-P node. */ + continue; + } + + struct local_datapath *ld = + get_local_datapath(&rt_data->local_datapaths, dp->tunnel_key); + if (!ld || !ld->is_switch) { + continue; + } + + int64_t vni = ovn_smap_get_llong(&dp->external_ids, + "dynamic-routing-vni", -1); + if (!ovn_is_valid_vni(vni)) { + continue; + } + + /* The ECMP selection fields of the FDB groups are stored in + * external_ids. */ + if (sbrec_datapath_binding_is_updated( + dp, SBREC_DATAPATH_BINDING_COL_EXTERNAL_IDS)) { + return EN_UNHANDLED; + } + } + + return EN_HANDLED_UNCHANGED; +} + static void * en_evpn_arp_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) @@ -7621,6 +7662,11 @@ inc_proc_ovn_controller_init( engine_add_input(&en_evpn_fdb, &en_evpn_vtep_binding, evpn_fdb_vtep_binding_handler); engine_add_input(&en_evpn_fdb, &en_nexthop_exchange, NULL); + /* The runtime_data are needed only for local datapaths, any update of + * local datapath will be reflected via en_neighbor_exchange. */ + engine_add_input(&en_evpn_fdb, &en_runtime_data, engine_noop_handler); + engine_add_input(&en_evpn_fdb, &en_sb_datapath_binding, + evpn_fdb_datapath_binding_handler); engine_add_input(&en_evpn_arp, &en_neighbor_exchange, NULL); engine_add_input(&en_evpn_arp, &en_evpn_vtep_binding, diff --git a/controller/physical.c b/controller/physical.c index 8c5d9b940..93022d94f 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -3652,7 +3652,12 @@ physical_consider_evpn_fdb(const struct evpn_fdb *fdb, const struct mf_field *mf = mf_from_id(MFF_LOG_REMOTE_OUTPORT); ds_clear(group_ds); - ds_put_cstr(group_ds, "type=select,selection_method=dp_hash"); + if (fdb->hash_fields) { + ds_put_format(group_ds, "type=select,selection_method=hash," + "fields(%s)", fdb->hash_fields); + } else { + ds_put_cstr(group_ds, "type=select,selection_method=dp_hash"); + } for (size_t i = 0; i < vector_len(&fdb->paths); i++) { struct evpn_fdb_path path = diff --git a/lib/ovn-util.c b/lib/ovn-util.c index 4ec648318..9ded1c042 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -1685,6 +1685,37 @@ ovn_ecmp_selection_fields_parse(const char *fields_s, return ok; } +/* Returns the fields of an OpenFlow select group with selection_method=hash + * for the ECMP 'selection_fields', as a malloc'ed string that the caller + * must free, or NULL if 'selection_fields' is empty. One group serves + * every protocol, so tp_src and tp_dst become the ports of TCP, UDP and + * SCTP; ovs-vswitchd hashes only the fields that are present in a + * packet. */ +char * +ovn_ecmp_selection_fields_to_hash_fields(const struct sset *selection_fields) +{ + if (sset_is_empty(selection_fields)) { + return NULL; + } + + struct sset of_fields = SSET_INITIALIZER(&of_fields); + const char *field; + SSET_FOR_EACH (field, selection_fields) { + if (!strcmp(field, "tp_src") || !strcmp(field, "tp_dst")) { + const char *dir = field + strlen("tp_"); + sset_add_and_free(&of_fields, xasprintf("tcp_%s", dir)); + sset_add_and_free(&of_fields, xasprintf("udp_%s", dir)); + sset_add_and_free(&of_fields, xasprintf("sctp_%s", dir)); + } else { + sset_add(&of_fields, field); + } + } + + char *hash_fields = sset_join(&of_fields, ",", ""); + sset_destroy(&of_fields); + return hash_fields; +} + struct sset * lrp_network_sset(const char **networks, int n_networks) { diff --git a/lib/ovn-util.h b/lib/ovn-util.h index fc88bf6d6..f17df69d8 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -739,6 +739,8 @@ void ovn_ecmp_selection_fields_add(struct sset *selection_fields, const char *field); bool ovn_ecmp_selection_fields_parse(const char *fields_s, struct sset *selection_fields); +char *ovn_ecmp_selection_fields_to_hash_fields( + const struct sset *selection_fields); const struct sbrec_port_binding *lport_lookup_by_name( struct ovsdb_idl_index *sbrec_port_binding_by_name, diff --git a/northd/en-datapath-logical-switch.c b/northd/en-datapath-logical-switch.c index 874a608a7..9ce66cbd5 100644 --- a/northd/en-datapath-logical-switch.c +++ b/northd/en-datapath-logical-switch.c @@ -173,6 +173,25 @@ gather_external_ids(const struct nbrec_logical_switch *nbs, smap_add(external_ids, "dynamic-routing-redistribute", redistribute); } + + const char *fdb_ecmp_fields = + smap_get(&nbs->other_config, + "dynamic-routing-fdb-ecmp-selection-fields"); + if (fdb_ecmp_fields) { + struct sset fields = SSET_INITIALIZER(&fields); + if (ovn_ecmp_selection_fields_parse(fdb_ecmp_fields, &fields)) { + smap_add(external_ids, + "dynamic-routing-fdb-ecmp-selection-fields", + fdb_ecmp_fields); + } else { + static struct vlog_rate_limit rl = + VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_WARN_RL(&rl, "bad 'dynamic-routing-fdb-ecmp-selection-" + "fields' %s on switch %s, using the default " + "selection method", fdb_ecmp_fields, nbs->name); + } + sset_destroy(&fields); + } } /* For backwards-compatibility, also store the NB UUID in diff --git a/ovn-nb.xml b/ovn-nb.xml index 0a3154858..b67eca7fc 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -1014,6 +1014,31 @@ </p> </column> + <column name="other_config" + key="dynamic-routing-fdb-ecmp-selection-fields" + type='{"type": "string"}'> + <p> + Only relevant if <ref column="other_config" key="dynamic-routing-vni" + table="Logical_switch"/> is set to valid VNI. + </p> + + <p> + A comma separated list of the fields that select the remote VTEP + for an EVPN FDB entry with more than one path (multi-homing), for + example <code>ip_proto,ip_src,ip_dst,tp_src,tp_dst</code>. The + supported fields are the same as for the + <ref column="selection_fields" table="Logical_Router_Static_Route"/> + column of a static route. <code>tp_src</code> and + <code>tp_dst</code> select the TCP, UDP and SCTP ports. When set, + <code>ovn-controller</code> creates the OpenFlow select group of + the entry with the <code>hash</code> selection method on these + fields instead of <code>dp_hash</code>. This lets a NIC offload + the group, because <code>hash</code> does not need a + recirculation. If the list contains a field that is not + supported, OVN uses <code>dp_hash</code>. + </p> + </column> + <column name="other_config" key="dynamic-routing-redistribute" type='{"type": "string"}'> <p> diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index 8348c3467..e1ef5b612 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -422,6 +422,8 @@ digraph "Incremental-Processing-Engine" { neighbor_exchange -> evpn_fdb [[label=""]]; evpn_vtep_binding -> evpn_fdb [[label="evpn_fdb_vtep_binding_handler"]]; nexthop_exchange -> evpn_fdb [[label=""]]; + runtime_data -> evpn_fdb [[label="engine_noop_handler"]]; + SB_datapath_binding -> evpn_fdb [[label="evpn_fdb_datapath_binding_handler"]]; evpn_arp [[style=filled, shape=box, fillcolor=white, label="evpn_arp"]]; neighbor_exchange -> evpn_arp [[label=""]]; evpn_vtep_binding -> evpn_arp [[label="evpn_arp_vtep_binding_handler"]]; diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 6e99e9aa4..f1ef4ddc9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -22306,6 +22306,48 @@ OVN_CLEANUP_NORTHD AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([dynamic-routing - EVPN FDB ecmp selection fields]) +AT_KEYWORDS([dynamic-routing]) +ovn_start + +check ovn-nbctl --wait=sb ls-add ls-evpn \ + -- set logical_switch ls-evpn \ + other_config:dynamic-routing-fdb-ecmp-selection-fields="ip_src,ip_dst" +dp=$(fetch_column datapath_binding _uuid external_ids:name=ls-evpn) + +# The option is only synced for an EVPN switch. +AT_CHECK([ovn-sbctl get datapath_binding $dp \ + external_ids:dynamic-routing-fdb-ecmp-selection-fields], [1], [ignore], + [ignore]) + +check ovn-nbctl --wait=sb set logical_switch ls-evpn \ + other_config:dynamic-routing-vni=10 +AT_CHECK([ovn-sbctl get datapath_binding $dp \ + external_ids:dynamic-routing-fdb-ecmp-selection-fields], [0], [dnl +"ip_src,ip_dst" +]) + +check ovn-nbctl --wait=sb set logical_switch ls-evpn \ + other_config:dynamic-routing-fdb-ecmp-selection-fields="ip_src,tp_src" +AT_CHECK([ovn-sbctl get datapath_binding $dp \ + external_ids:dynamic-routing-fdb-ecmp-selection-fields], [0], [dnl +"ip_src,tp_src" +]) + +# A field that is not supported is not synced. +check ovn-nbctl --wait=sb set logical_switch ls-evpn \ + other_config:dynamic-routing-fdb-ecmp-selection-fields="ip_src,foo" +AT_CHECK([ovn-sbctl get datapath_binding $dp \ + external_ids:dynamic-routing-fdb-ecmp-selection-fields], [1], [ignore], + [ignore]) +check grep -q "bad 'dynamic-routing-fdb-ecmp-selection-fields' ip_src,foo" \ + northd/ovn-northd.log + +OVN_CLEANUP_NORTHD +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD_NO_HV([ AT_SETUP([LS incremental processing - other_config fields]) AT_KEYWORDS([ovn, incremental]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 13e62bf9b..388df9651 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -18386,6 +18386,35 @@ AT_CHECK_UNQUOTED( type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=load:2147483649->reg1[[0..31]],bucket=bucket_id=1,weight:200,actions=load:2147483650->reg1[[0..31]]: GID ]) +# With ECMP selection fields the group uses the hash selection method, +# which a NIC can offload. +check ovn-nbctl --wait=hv set logical_switch ls-evpn \ + other_config:dynamic-routing-fdb-ecmp-selection-fields="ip_src,ip_dst,tp_src,tp_dst" +AT_CHECK_UNQUOTED( + [ovn-appctl group-table-list | \ + sed 's/:[[[:space:]]]*[[0-9]]*$/: GID/'], [0], [dnl +type=select,selection_method=hash,fields(ip_dst,ip_proto,ip_src,sctp_dst,sctp_src,tcp_dst,tcp_src,udp_dst,udp_src),bucket=bucket_id=0,weight:100,actions=load:2147483649->reg1[[0..31]],bucket=bucket_id=1,weight:200,actions=load:2147483650->reg1[[0..31]]: GID +]) +OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow15 dump-groups br-int | \ + grep -q 'selection_method=hash']) + +# A field that is not supported keeps dp_hash. +check ovn-nbctl --wait=hv set logical_switch ls-evpn \ + other_config:dynamic-routing-fdb-ecmp-selection-fields="ip_src,foo" +AT_CHECK_UNQUOTED( + [ovn-appctl group-table-list | \ + sed 's/:[[[:space:]]]*[[0-9]]*$/: GID/'], [0], [dnl +type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=load:2147483649->reg1[[0..31]],bucket=bucket_id=1,weight:200,actions=load:2147483650->reg1[[0..31]]: GID +]) + +check ovn-nbctl --wait=hv remove logical_switch ls-evpn \ + other_config dynamic-routing-fdb-ecmp-selection-fields +AT_CHECK_UNQUOTED( + [ovn-appctl group-table-list | \ + sed 's/:[[[:space:]]]*[[0-9]]*$/: GID/'], [0], [dnl +type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=load:2147483649->reg1[[0..31]],bucket=bucket_id=1,weight:200,actions=load:2147483650->reg1[[0..31]]: GID +]) + # Verify learning suppression flows: two ECMP drop flows added. AT_CHECK_UNQUOTED( [ovs-ofctl dump-flows br-int table=OFTABLE_LEARN_REMOTE_FDB | grep priority | \ -- 2.54.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
