Git sha: c9b4b88f4453acf982bc6fc8fc4dba6f7f363247
Author: Eelco Chaudron <[email protected]>
Subject: netdev-offload: Temporarily move thread-related APIs to dpif-netdev.

This patch temporarily moves offload thread-related APIs from 
netdev-offload to dpif-netdev as an intermediate refactoring step before 
they are moved to dpif-offload-dpdk. The APIs include thread ID 
management, thread initialization, and UFID-to-thread mapping functions.

> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 6575cb9e8..272294daa 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -409,11 +409,18 @@ struct dp_offload_thread {
>  static struct dp_offload_thread *dp_offload_threads;
>  static void *dp_netdev_flow_offload_main(void *arg);
>  
> +/* XXX: Temporarily forward declarations, will be removed during cleanup. */
> +static unsigned int dpdk_offload_ufid_to_thread_id(const ovs_u128 ufid);
> +static unsigned int dpdk_offload_thread_init(void);
> +void dpdk_offload_thread_set_thread_nb(unsigned int thread_nb);
> +unsigned int dpdk_offload_thread_nb(void);
> +unsigned int dpdk_offload_thread_id(void);

[ ... ]

> +/* XXX: Temporarily duplicates definition in dpif-offload-dpdk.c. */
> +#define DEFAULT_OFFLOAD_THREAD_NB 1
> +static unsigned int offload_thread_nb = DEFAULT_OFFLOAD_THREAD_NB;
> +
> +DECLARE_EXTERN_PER_THREAD_DATA(unsigned int, dpdk_offload_thread_id);
> +DEFINE_EXTERN_PER_THREAD_DATA(dpdk_offload_thread_id, OVSTHREAD_ID_UNSET);

Does this code introduce multiple definitions of DEFAULT_OFFLOAD_THREAD_NB 
and offload_thread_nb? The comment mentions a duplicate definition exists 
in dpif-offload-dpdk.c.

> +unsigned int
> +dpdk_offload_thread_id(void)
> +{
> +    unsigned int id = *dpdk_offload_thread_id_get();
> +
> +    if (OVS_UNLIKELY(id == OVSTHREAD_ID_UNSET)) {
> +        id = dpdk_offload_thread_init();
> +    }
> +
> +    return id;
> +}

[ ... ]

> +static unsigned int
> +dpdk_offload_thread_init(void)
> +{
> +    static atomic_count next_id = ATOMIC_COUNT_INIT(0);
> +    bool thread_is_hw_offload;
> +    bool thread_is_rcu;
> +
> +    thread_is_hw_offload = !strncmp(get_subprogram_name(),
> +                                    "dpdk_offload", strlen("dpdk_offload"));
> +    thread_is_rcu = !strncmp(get_subprogram_name(), "urcu", strlen("urcu"));

Will this thread name check work correctly after changing the thread name 
from "hw_offload" to "dpdk_offload" in dp_netdev_offload_init()? The 
original netdev_offload.c code checked for "hw_offload".

> diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c
> index 155f631b3..6014db38c 100644

[ ... ]

> -/* XXX: Temporary functions below, which will be removed once fully
> - *      refactored. */
> -unsigned int dpif_offload_dpdk_get_thread_nb(void);
> -unsigned int dpif_offload_dpdk_get_thread_nb(void)
> -{
> -    return offload_thread_nb;
> -}

[ ... ]

> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index 174afee33..0852e2536 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
> @@ -58,18 +58,6 @@
>  
>  VLOG_DEFINE_THIS_MODULE(netdev_offload);
>  
> -/* XXX: Temporarily duplicates definition in dpif-offload-dpdk.c. */
> -#define MAX_OFFLOAD_THREAD_NB 10
> -#define DEFAULT_OFFLOAD_THREAD_NB 1
> -static unsigned int offload_thread_nb = DEFAULT_OFFLOAD_THREAD_NB;
> -
> -unsigned int dpif_offload_dpdk_get_thread_nb(void); /* XXX: Temporarily
> -                                                     * external declaration
> -                                                     * until fully 
> refactored.
> -                                                     */
> -
> -DEFINE_EXTERN_PER_THREAD_DATA(netdev_offload_thread_id, OVSTHREAD_ID_UNSET);

Does removing the MAX_OFFLOAD_THREAD_NB definition potentially break bounds 
checking elsewhere in the code?

> @@ -639,11 +569,6 @@ netdev_set_flow_api_enabled(const struct smap 
> *ovs_other_config OVS_UNUSED)
>          static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>  
>          if (ovsthread_once_start(&once)) {
> -
> -#ifdef DPDK_NETDEV
> -            offload_thread_nb = dpif_offload_dpdk_get_thread_nb();
> -#endif
> -
>              netdev_ports_flow_init();
>              ovsthread_once_done(&once);
>          }

How does the thread number initialization work now that this DPDK_NETDEV 
conditional block has been removed?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to