On 2/2/26 5:07 PM, Eelco Chaudron wrote:
> The dpif-netdev code used a static variable 'first_set_config' to
> perform one-time initialization when dpif_netdev_set_config() was first
> called. Because this variable was global rather than per-datapath, the
> initialization would not be re-triggered if a datapath was destroyed and
> recreated.
>
> Replace the static variable with a per-datapath ovsthread_once
> structure, ensuring that each dp_netdev instance properly performs its
> one-time initialization regardless of other datapaths' lifecycle.
>
> Fixes: de3bbdc479a9 ("dpif-netdev: Add PMD load based sleeping.")
> Fixes: 7bd1867b6d6a ("dpif-offload-dpdk: Abstract rte_flow implementation
> from dpif-netdev.")
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
> lib/dpif-netdev.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index c9c43ddfb6..45ac2701e0 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -327,6 +327,7 @@ struct dp_netdev {
>
> struct seq *reconfigure_seq;
> uint64_t last_reconfigure_seq;
> + struct ovsthread_once once_set_config;
>
> /* Cpu mask for pin of pmd threads. */
> char *pmd_cmask;
> @@ -1804,6 +1805,7 @@ create_dp_netdev(const char *name, const struct
> dpif_class *class,
>
> dp->reconfigure_seq = seq_create();
> dp->last_reconfigure_seq = seq_read(dp->reconfigure_seq);
> + dp->once_set_config = (struct ovsthread_once) OVSTHREAD_ONCE_INITIALIZER;
>
> /* Init meter resources. */
> cmap_init(&dp->meters);
> @@ -1943,6 +1945,7 @@ dp_netdev_free(struct dp_netdev *dp)
>
>
> seq_destroy(dp->reconfigure_seq);
> + ovsthread_once_destroy(&dp->once_set_config);
>
> seq_destroy(dp->port_seq);
> hmap_destroy(&dp->ports);
> @@ -4394,7 +4397,6 @@ dpif_netdev_set_config(struct dpif *dpif, const struct
> smap *other_config)
> uint32_t rebalance_load, rebalance_improve;
> bool log_autolb = false;
> enum sched_assignment_type pmd_rxq_assign_type;
> - static bool first_set_config = true;
>
> tx_flush_interval = smap_get_int(other_config, "tx-flush-interval",
> DEFAULT_TX_FLUSH_INTERVAL);
> @@ -4543,16 +4545,18 @@ dpif_netdev_set_config(struct dpif *dpif, const
> struct smap *other_config)
> set_pmd_auto_lb(dp, autolb_state, log_autolb);
>
> bool sleep_changed = set_all_pmd_max_sleeps(dp, other_config);
> - if (first_set_config || sleep_changed) {
> - log_all_pmd_sleeps(dp);
> - }
>
> - if (first_set_config) {
> + if (ovsthread_once_start(&dp->once_set_config)) {
> +
nit: No need to add an extra line here.
> + log_all_pmd_sleeps(dp);
> dpif_offload_datapath_register_flow_unreference_cb(
> dpif, offload_flow_reference_unreference_cb);
> +
> + ovsthread_once_done(&dp->once_set_config);
> + } else if (sleep_changed) {
> + log_all_pmd_sleeps(dp);
> }
>
> - first_set_config = false;
> return 0;
> }
>
I suppose, it's only an issue if you want to run both dummy and netdev datapaths
at the same time, which wouldn't be a great experience either way. But makes
sense
to fix.
Acked-by: Ilya Maximets <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
