commit: 3c48bd8602249f9ac86e4e59be1d847da84230eb
Author: Eelco Chaudron <[email protected]>
dpif-offload: Move hw-offload configuration to dpif-offload.
This patch moves hardware offload configuration from netdev-offload to
dpif-offload, initially as global functions/variables, with later
patches planned to integrate them into the offload provider framework.
> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
> index 672df8a38..ed40f2657 100644
> --- a/lib/dpif-offload.c
> +++ b/lib/dpif-offload.c
[ ... ]
> +static atomic_bool dpif_offload_global_enabled = false;
> +static atomic_bool dpif_offload_rebalance_policy = false;
[ ... ]
> @@ -481,4 +501,23 @@ dpif_offload_set_global_cfg(const struct smap *other_cfg)
> }
> }
>
> + /* Handle other global configuration settings.
> + *
> + * According to the manual the 'hw-offload' parameter requires a restart
> + * when changed. In practice this is only needed on disable, as it will
> + * not actually disable hw-offload when requested. */
This comment mentions that hw-offload cannot be disabled once enabled
but does the code actually enforce this? Can dpif_offload_set_global_cfg
be called multiple times with hw-offload changing from true to false?
> + if (smap_get_bool(other_cfg, "hw-offload", false)) {
> + static struct ovsthread_once once_enable =
> OVSTHREAD_ONCE_INITIALIZER;
> +
> + if (ovsthread_once_start(&once_enable)) {
> + atomic_store_relaxed(&dpif_offload_global_enabled, true);
> +
> + if (smap_get_bool(other_cfg, "offload-rebalance", false)) {
> + atomic_store_relaxed(&dpif_offload_rebalance_policy, true);
> + }
> +
> + ovsthread_once_done(&once_enable);
> + }
> + }
The offload-rebalance setting is only evaluated once during the first
enable. What happens if someone sets hw-offload=true initially without
offload-rebalance, then later restarts with both settings enabled? Will
the rebalance policy remain false?
> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index 8a9d36555..01fdadbc3 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
[ ... ]
> -static bool netdev_flow_api_enabled = false;
[ ... ]
> @@ -855,11 +855,10 @@ netdev_ports_flow_init(void)
> void
> netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
> {
> - if (smap_get_bool(ovs_other_config, "hw-offload", false)) {
> + if (dpif_offload_is_offload_enabled()) {
> static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>
> if (ovsthread_once_start(&once)) {
> - netdev_flow_api_enabled = true;
Does this create a dependency ordering issue? The function
netdev_set_flow_api_enabled checks dpif_offload_is_offload_enabled,
but dpif_offload_set_global_cfg needs to be called first to set the
enabled flag. Is the calling sequence guaranteed to be correct?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
