On 29 Sep 2026, at 10:34, David Marchand wrote:
> The dpdk_list serves different usecases:
> - maintaining all DPDK ports link status (dpdk_watchdog),
> - looking up a DPDK port id for checking which netdevice uses a DPDK
> port (if any),
> - looking up a vhost_id for checking which netdevice uses a vHost port
> (if any),
> - running some command on all DPDK and vHost backed ports,
>
> Have a separate list (and mutex) for vHost ports objects.
>
> Thanks to this, specialised usecases don't need to care about the
> device type.
Thanks David, mainly some questions around the lock placement.
The rest seems ok.
//Eelco
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
[...]
> -/* Contains all 'struct dpdk_dev's. */
> -static struct ovs_list dpdk_list OVS_GUARDED_BY(dpdk_mutex)
> - = OVS_LIST_INITIALIZER(&dpdk_list);
> +/* Contains all DPDK ports. */
Should this be "Contains all ethernet ports." to
distinguish from the vhost list below?
> +static struct ovs_list dpdk_eth_list OVS_GUARDED_BY(dpdk_eth_mutex)
> + = OVS_LIST_INITIALIZER(&dpdk_eth_list);
[...]
> - ovs_mutex_lock(&dpdk_mutex);
> + ovs_mutex_lock(&dpdk_eth_mutex);
Would it be possible to move this lock closer to the list
insertion in netdev_dpdk_eth_construct()? The field
initialization above does not appear to need the list
mutex.
I have the same comment for
netdev_dpdk_vhost(_client)_construct() but then it would
mean the list push needs to move out of
vhost_common_construct(). Or if you want to keep the
locking as is, we could also pass the list to add to into
common_construct().
Maybe also take a look at the lock usage in the
destructors.
> common_construct(common, SOCKET0);
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
