[dpdk-dev,v4,1/5] eal: add internal flag indicating init has completed

Message ID a9cae982af2307c7fa59406723bca84004de5ff6.1520000413.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Anatoly Burakov March 2, 2018, 3:14 p.m. UTC
  Currently, primary process initialization is finalized by setting
the RTE_MAGIC value in the shared config. However, it is not
possible to check whether secondary process initialization has
completed. Add such a value to internal config.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v4: make init_complete volatile
    
    This patch is dependent upon earlier IPC fixes patchset [1].
    
    [1] http://dpdk.org/dev/patchwork/bundle/aburakov/IPC_Fixes/
    
    v4: make init_complete volatile
    
    v3: no changes
    
    v2: no changes
    
    This patch is dependent upon earlier IPC fixes patchset [1].
    
    [1] http://dpdk.org/dev/patchwork/bundle/aburakov/IPC_Fixes/

 lib/librte_eal/common/eal_common_options.c | 1 +
 lib/librte_eal/common/eal_internal_cfg.h   | 2 ++
 lib/librte_eal/linuxapp/eal/eal.c          | 2 ++
 3 files changed, 5 insertions(+)
  

Comments

Anatoly Burakov March 7, 2018, 4:56 p.m. UTC | #1
This is an assortment of loosely related improvements to IPC, mostly
related to handling corner cases and avoiding race conditions.

Main addition is an attempt to avoid undefined behavior when receiving
messages while secondary process is initializing. It is assumed that
once callback is registered, it is safe to receive messages.

If the callback wasn't registered, then there are two choices - either
we haven't reached the stage where we register this callback (init is
not finished), or user has forgotten to register callback for this
message. The latter can only be known once initialization is complete,
so until init is complete, treat this process as not-existing if there
is no registered callback for the message. This will handle both
scenarios.

v5: - added cover-letter :)
    - drop the "don't send messages to processes which haven't finished
      initializing" model added in previous version. instead, allow
      everyone to receive all messages, but check if initialization is
      completed, and check if there is a callback registered for this
      message. if there is no callback, assume we just didn't get around
      to it yet, so just send a special message to the requestor that
      it should treat this process as if it wasn't there.
v4: - make init_complete volatile
    - changed from "don't process messages until init complete" to
      "don't send messages to processes which haven't finished
      initializing", as the former would have resulted in timeouts if
      init took too long to complete
    - fixed resource leaks
    - added patch to simplify IPC timeouts handling
v3: - move init_complete until after receiving message
v2: - added patch to prevent IPC from sending messages while primary
      is initializing
    - added patch to generate filter from eal_mp_socket_path() instead
      of hardcoding the value

Anatoly Burakov (6):
  eal: add internal flag indicating init has completed
  eal: abstract away IPC socket path generation
  eal: don't hardcode socket filter value in IPC
  eal: lock IPC directory on init and send
  eal: simplify IPC sync request timeout code
  eal: ignore messages until init is complete

 lib/librte_eal/common/eal_common_options.c |   1 +
 lib/librte_eal/common/eal_common_proc.c    | 382 +++++++++++++++++------------
 lib/librte_eal/common/eal_internal_cfg.h   |   2 +
 lib/librte_eal/linuxapp/eal/eal.c          |   2 +
 4 files changed, 228 insertions(+), 159 deletions(-)
  
Anatoly Burakov March 13, 2018, 5:42 p.m. UTC | #2
This is an assortment of loosely related improvements to IPC, mostly
related to handling corner cases and avoiding race conditions.

Main addition is an attempt to avoid undefined behavior when receiving
messages while secondary process is initializing. It is assumed that
once callback is registered, it is safe to receive messages.

If the callback wasn't registered, then there are two choices - either
we haven't reached the stage where we register this callback (init is
not finished), or user has forgotten to register callback for this
message. The latter can only be known once initialization is complete,
so until init is complete, treat this process as not-existing if there
is no registered callback for the message. This will handle both
scenarios.

v6: - clang compile fix
    - clarify commit message for patch 6
    - simplify patch 6
v5: - added cover-letter :)
    - drop the "don't send messages to processes which haven't finished
      initializing" model added in previous version. instead, allow
      everyone to receive all messages, but check if initialization is
      completed, and check if there is a callback registered for this
      message. if there is no callback, assume we just didn't get around
      to it yet, so just send a special message to the requestor that
      it should treat this process as if it wasn't there.
v4: - make init_complete volatile
    - changed from "don't process messages until init complete" to
      "don't send messages to processes which haven't finished
      initializing", as the former would have resulted in timeouts if
      init took too long to complete
    - fixed resource leaks
    - added patch to simplify IPC timeouts handling
v3: - move init_complete until after receiving message
v2: - added patch to prevent IPC from sending messages while primary
      is initializing
    - added patch to generate filter from eal_mp_socket_path() instead
      of hardcoding the value

Anatoly Burakov (6):
  eal: add internal flag indicating init has completed
  eal: abstract away IPC socket path generation
  eal: don't hardcode socket filter value in IPC
  eal: lock IPC directory on init and send
  eal: simplify IPC sync request timeout code
  eal: ignore messages until init is complete

 lib/librte_eal/common/eal_common_options.c |   1 +
 lib/librte_eal/common/eal_common_proc.c    | 178 ++++++++++++++++++++---------
 lib/librte_eal/common/eal_internal_cfg.h   |   2 +
 lib/librte_eal/linuxapp/eal/eal.c          |   2 +
 4 files changed, 129 insertions(+), 54 deletions(-)
  
Thomas Monjalon March 21, 2018, 5:43 p.m. UTC | #3
> Anatoly Burakov (6):
>   eal: add internal flag indicating init has completed
>   eal: abstract away IPC socket path generation
>   eal: don't hardcode socket filter value in IPC
>   eal: lock IPC directory on init and send
>   eal: simplify IPC sync request timeout code
>   eal: ignore messages until init is complete

Applied, thanks
  

Patch

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 9f2f8d2..0be80cb 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -194,6 +194,7 @@  eal_reset_internal_config(struct internal_config *internal_cfg)
 	internal_cfg->vmware_tsc_map = 0;
 	internal_cfg->create_uio_dev = 0;
 	internal_cfg->user_mbuf_pool_ops_name = NULL;
+	internal_cfg->init_complete = 0;
 }
 
 static int
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 1169fcc..a0082d1 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -56,6 +56,8 @@  struct internal_config {
 			/**< user defined mbuf pool ops name */
 	unsigned num_hugepage_sizes;      /**< how many sizes on this system */
 	struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
+	volatile unsigned int init_complete;
+	/**< indicates whether EAL has completed initialization */
 };
 extern struct internal_config internal_config; /**< Global EAL configuration. */
 
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 38306bf..2ecd07b 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -669,6 +669,8 @@  rte_eal_mcfg_complete(void)
 	/* ALL shared mem_config related INIT DONE */
 	if (rte_config.process_type == RTE_PROC_PRIMARY)
 		rte_config.mem_config->magic = RTE_MAGIC;
+
+	internal_config.init_complete = 1;
 }
 
 /*