Currently we parse the 'other_config' column in Openvswitch table in bridge.c. We extract the values (just 'pmd-cpu-mask' for now) and we pass them down to the datapath, via different layers.
If we want to pass other values to dpif-netdev.c (like we recently discussed) we would have to touch ofproto.c, ofproto-dpif.c and dpif.c. This patch sends the entire other_config column to dpif-netdev, so that dpif-netdev can extract the values it's interested in. No functional change. Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com> --- I don't like that dpif-netdev receives the whole other_config column, because it contains other values which are completely unrelated, but unfortunately there's no better place in the database for datapath specific configuration. --- lib/dpif-netdev.c | 9 +++++---- lib/dpif-netlink.c | 2 +- lib/dpif-provider.h | 8 +++----- lib/dpif.c | 12 ++++++------ lib/dpif.h | 2 +- ofproto/ofproto-dpif.c | 19 ++++++++++++++++--- ofproto/ofproto-provider.h | 11 ++++++++--- ofproto/ofproto.c | 13 +++++++++---- ofproto/ofproto.h | 3 ++- vswitchd/bridge.c | 18 +++++++++++++++++- 10 files changed, 68 insertions(+), 29 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 719a51823..0be5db514 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -2724,12 +2724,13 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops) } } -/* Changes the number or the affinity of pmd threads. The changes are actually - * applied in dpif_netdev_run(). */ +/* Applies datapath configuration from the database. Some of the changes are + * actually applied in dpif_netdev_run(). */ static int -dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask) +dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config) { struct dp_netdev *dp = get_dp_netdev(dpif); + const char *cmask = smap_get(other_config, "pmd-cpu-mask"); if (!nullable_string_is_equal(dp->pmd_cmask, cmask)) { free(dp->pmd_cmask); @@ -4844,7 +4845,7 @@ const struct dpif_class dpif_netdev_class = { dpif_netdev_operate, NULL, /* recv_set */ NULL, /* handlers_set */ - dpif_netdev_pmd_set, + dpif_netdev_set_config, dpif_netdev_queue_to_priority, NULL, /* recv */ NULL, /* recv_wait */ diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index c8b0e37f9..9762a87be 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -2387,7 +2387,7 @@ const struct dpif_class dpif_netlink_class = { dpif_netlink_operate, dpif_netlink_recv_set, dpif_netlink_handlers_set, - NULL, /* poll_thread_set */ + NULL, /* set_config */ dpif_netlink_queue_to_priority, dpif_netlink_recv, dpif_netlink_recv_wait, diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index d3b2bb91d..a0dc1ef35 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -326,11 +326,9 @@ struct dpif_class { * */ int (*handlers_set)(struct dpif *dpif, uint32_t n_handlers); - /* If 'dpif' creates its own I/O polling threads, refreshes poll threads - * configuration. 'cmask' configures the cpu mask for setting the polling - * threads' cpu affinity. The implementation might postpone applying the - * changes until run() is called. */ - int (*poll_threads_set)(struct dpif *dpif, const char *cmask); + /* Pass custom configuration options to the datapath. The implementation + * might postpone applying the changes until run() is called. */ + int (*set_config)(struct dpif *dpif, const struct smap *other_config); /* Translates OpenFlow queue ID 'queue_id' (in host byte order) into a * priority value used for setting packet priority. */ diff --git a/lib/dpif.c b/lib/dpif.c index 374f013ab..57aa3c6c4 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1440,17 +1440,17 @@ dpif_print_packet(struct dpif *dpif, struct dpif_upcall *upcall) } } -/* If 'dpif' creates its own I/O polling threads, refreshes poll threads - * configuration. */ +/* Pass custom configuration to the datapath implementation. Some of the + * changes can be postponed until dpif_run() is called. */ int -dpif_poll_threads_set(struct dpif *dpif, const char *cmask) +dpif_set_config(struct dpif *dpif, const struct smap *cfg) { int error = 0; - if (dpif->dpif_class->poll_threads_set) { - error = dpif->dpif_class->poll_threads_set(dpif, cmask); + if (dpif->dpif_class->set_config) { + error = dpif->dpif_class->set_config(dpif, cfg); if (error) { - log_operation(dpif, "poll_threads_set", error); + log_operation(dpif, "set_config", error); } } diff --git a/lib/dpif.h b/lib/dpif.h index aa4fb8b84..d5b4b5827 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -845,7 +845,7 @@ void dpif_register_upcall_cb(struct dpif *, upcall_callback *, void *aux); int dpif_recv_set(struct dpif *, bool enable); int dpif_handlers_set(struct dpif *, uint32_t n_handlers); -int dpif_poll_threads_set(struct dpif *, const char *cmask); +int dpif_set_config(struct dpif *, const struct smap *cfg); int dpif_port_set_config(struct dpif *, odp_port_t, const struct smap *cfg); int dpif_recv(struct dpif *, uint32_t handler_id, struct dpif_upcall *, struct ofpbuf *); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index e1112eb89..e51577abc 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -331,9 +331,6 @@ type_run(const char *type) return 0; } - /* This must be called before dpif_run() */ - dpif_poll_threads_set(backer->dpif, pmd_cpu_mask); - if (dpif_run(backer->dpif)) { backer->need_revalidate = REV_RECONFIGURE; } @@ -4516,6 +4513,21 @@ get_datapath_version(const struct ofproto *ofproto_) } static void +type_set_config(const char *type, const struct smap *other_config) +{ + struct dpif_backer *backer; + + backer = shash_find_data(&all_dpif_backers, type); + if (!backer) { + /* This is not necessarily a problem, since backers are only + * created on demand. */ + return; + } + + dpif_set_config(backer->dpif, other_config); +} + +static void ct_flush(const struct ofproto *ofproto_, const uint16_t *zone) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); @@ -5284,5 +5296,6 @@ const struct ofproto_class ofproto_dpif_class = { NULL, /* group_modify */ group_get_stats, /* group_get_stats */ get_datapath_version, /* get_datapath_version */ + type_set_config, ct_flush, /* ct_flush */ }; diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 3739ebce7..7a797e7e7 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -59,6 +59,7 @@ struct bfd_cfg; struct meter; struct ofoperation; struct ofproto_packet_out; +struct smap; extern struct ovs_mutex ofproto_mutex; @@ -508,9 +509,6 @@ extern unsigned ofproto_max_idle; * ofproto-dpif implementation. */ extern size_t n_handlers, n_revalidators; -/* Cpu mask for pmd threads. */ -extern char *pmd_cpu_mask; - static inline struct rule *rule_from_cls_rule(const struct cls_rule *); void ofproto_rule_expire(struct rule *rule, uint8_t reason) @@ -1812,6 +1810,13 @@ struct ofproto_class { */ const char *(*get_datapath_version)(const struct ofproto *); + /* Pass custom configuration options to the 'type' datapath. + * + * This function should be NULL if an implementation does not support it. + */ + void (*type_set_config)(const char *type, + const struct smap *other_config); + /* ## ------------------- ## */ /* ## Connection tracking ## */ /* ## ------------------- ## */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 0b5e0fa8e..cd2f8a1a2 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -297,7 +297,6 @@ unsigned ofproto_flow_limit = OFPROTO_FLOW_LIMIT_DEFAULT; unsigned ofproto_max_idle = OFPROTO_MAX_IDLE_DEFAULT; size_t n_handlers, n_revalidators; -char *pmd_cpu_mask; /* Map from datapath name to struct ofproto, for use by unixctl commands. */ static struct hmap all_ofprotos = HMAP_INITIALIZER(&all_ofprotos); @@ -740,10 +739,16 @@ ofproto_port_set_mcast_snooping(struct ofproto *ofproto, void *aux, } void -ofproto_set_cpu_mask(const char *cmask) +ofproto_type_set_config(const char *datapath_type, const struct smap *cfg) { - free(pmd_cpu_mask); - pmd_cpu_mask = nullable_xstrdup(cmask); + const struct ofproto_class *class; + + datapath_type = ofproto_normalize_type(datapath_type); + class = ofproto_class_find__(datapath_type); + + if (class->type_set_config) { + class->type_set_config(datapath_type, cfg); + } } void diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 9df1a87e6..97e63e60e 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -322,7 +322,8 @@ int ofproto_set_mcast_snooping(struct ofproto *ofproto, int ofproto_port_set_mcast_snooping(struct ofproto *ofproto, void *aux, const struct ofproto_mcast_snooping_port_settings *s); void ofproto_set_threads(int n_handlers, int n_revalidators); -void ofproto_set_cpu_mask(const char *cmask); +void ofproto_type_set_config(const char *type, + const struct smap *other_config); void ofproto_set_dp_desc(struct ofproto *, const char *dp_desc); int ofproto_set_snoops(struct ofproto *, const struct sset *snoops); int ofproto_set_netflow(struct ofproto *, diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 4122b5c79..21c3c7915 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -570,6 +570,21 @@ collect_in_band_managers(const struct ovsrec_open_vswitch *ovs_cfg, } static void +config_ofproto_types(const struct smap *other_config) +{ + struct sset types; + const char *type; + + /* Pass custom configuration to datapath types. */ + sset_init(&types); + ofproto_enumerate_types(&types); + SSET_FOR_EACH (type, &types) { + ofproto_type_set_config(type, other_config); + } + sset_destroy(&types); +} + +static void bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) { struct sockaddr_in *managers; @@ -583,7 +598,6 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) OFPROTO_FLOW_LIMIT_DEFAULT)); ofproto_set_max_idle(smap_get_int(&ovs_cfg->other_config, "max-idle", OFPROTO_MAX_IDLE_DEFAULT)); - ofproto_set_cpu_mask(smap_get(&ovs_cfg->other_config, "pmd-cpu-mask")); ofproto_set_threads( smap_get_int(&ovs_cfg->other_config, "n-handler-threads", 0), @@ -692,6 +706,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) } free(managers); + config_ofproto_types(&ovs_cfg->other_config); + /* The ofproto-dpif provider does some final reconfiguration in its * ->type_run() function. We have to call it before notifying the database * client that reconfiguration is complete, otherwise there is a very -- 2.11.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev