[dpdk-stable] [PATCH 1/3] doc: fix typo in ipc doc

[dpdk-stable] [PATCH 1/3] doc: fix typo in ipc doc

[Anatoly Burakov]

The word "syncrhonous" appears twice. Fix it.

Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/prog_guide/multi_proc_support.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/guides/prog_guide/multi_proc_support.rst b/doc/guides/prog_guide/multi_proc_support.rst
index 6196d3f21..1e78a1f02 100644
--- a/doc/guides/prog_guide/multi_proc_support.rst
+++ b/doc/guides/prog_guide/multi_proc_support.rst
@@ -263,9 +263,9 @@ To send a request, a message descriptor ``rte_mp_msg`` must be populated.
 Additionally, a ``timespec`` value must be specified as a timeout, after which
 IPC will stop waiting and return.
 
-For synchronous synchronous requests, the ``rte_mp_reply`` descriptor must also
-be created. This is where the responses will be stored. The list of fields that
-will be populated by IPC are as follows:
+For synchronous requests, the ``rte_mp_reply`` descriptor must also be created.
+This is where the responses will be stored. The list of fields that will be
+populated by IPC are as follows:
 
 * ``nb_sent`` - number indicating how many requests were sent (i.e. how many
   peer processes were active at the time of the request).
-- 
2.17.1

[dpdk-stable] [PATCH 2/3] ipc: add warnings about not using ipc with memory API's

[Anatoly Burakov]

IPC and memory-related API's should not be mixed because memory
relies on IPC internally. Add explicit warnings to IPC API and
to the documentation about this.

Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/prog_guide/env_abstraction_layer.rst | 8 ++++++++
 doc/guides/prog_guide/multi_proc_support.rst    | 5 +++++
 lib/librte_eal/common/include/rte_eal.h         | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
index c27f730c7..f15bcd976 100644
--- a/doc/guides/prog_guide/env_abstraction_layer.rst
+++ b/doc/guides/prog_guide/env_abstraction_layer.rst
@@ -147,6 +147,14 @@ A default validator callback is provided by EAL, which can be enabled with a
 ``--socket-limit`` command-line option, for a simple way to limit maximum amount
 of memory that can be used by DPDK application.
 
+.. warning::
+    Memory subsystem uses DPDK IPC internally, so memory allocations/callbacks
+    and IPC must not be mixed: it is not safe to allocate/free memory inside
+    memory-related or IPC callbacks, and it is not safe to use IPC inside
+    memory-related callbacks. See chapter
+    :ref:`Multi-process Support <Multi-process_Support>` for more details about
+    DPDK IPC.
+
 + Legacy memory mode
 
 This mode is enabled by specifying ``--legacy-mem`` command-line switch to the
diff --git a/doc/guides/prog_guide/multi_proc_support.rst b/doc/guides/prog_guide/multi_proc_support.rst
index 1e78a1f02..665474a4c 100644
--- a/doc/guides/prog_guide/multi_proc_support.rst
+++ b/doc/guides/prog_guide/multi_proc_support.rst
@@ -318,6 +318,11 @@ supported. However, since sending messages (not requests) does not involve an
 IPC thread, sending messages while processing another message or request is
 supported.
 
+Since the memory sybsystem uses IPC internally, memory allocations and IPC must
+not be mixed: it is not safe to use IPC inside a memory-related callback, nor is
+it safe to allocate/free memory inside IPC callbacks. Attempting to do so may
+lead to a deadlock.
+
 Asynchronous request callbacks may be triggered either from IPC thread or from
 interrupt thread, depending on whether the request has timed out. It is
 therefore suggested to avoid waiting for interrupt-based events (such as alarms)
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 0603eaf25..7db022532 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -225,6 +225,8 @@ struct rte_mp_reply {
  *
  * As we create  socket channel for primary/secondary communication, use
  * this function typedef to register action for coming messages.
+ *
+ * @note No memory allocations should take place inside the callback.
  */
 typedef int (*rte_mp_t)(const struct rte_mp_msg *msg, const void *peer);
 
@@ -234,6 +236,8 @@ typedef int (*rte_mp_t)(const struct rte_mp_msg *msg, const void *peer);
  * As we create socket channel for primary/secondary communication, use
  * this function typedef to register action for coming responses to asynchronous
  * requests.
+ *
+ * @note No memory allocations should take place inside the callback.
  */
 typedef int (*rte_mp_async_reply_t)(const struct rte_mp_msg *request,
 		const struct rte_mp_reply *reply);
@@ -308,6 +312,9 @@ rte_mp_sendmsg(struct rte_mp_msg *msg);
  *
  * @note The caller is responsible to free reply->replies.
  *
+ * @note This API must not be used inside memory-related or IPC callbacks, and
+ *   no memory allocations should take place inside such callback.
+ *
  * @param req
  *   The req argument contains the customized request message.
  *
-- 
2.17.1

[dpdk-stable] [PATCH 3/3] ipc: add warnings about correct API usage

[Anatoly Burakov]

When handling synchronous or asynchronous requests, the reply
must be sent explicitly even if the result of the operation is
an error, to avoid the other side timing out. Make note of this
in documentation explicitly.

Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/prog_guide/multi_proc_support.rst |  7 +++++++
 lib/librte_eal/common/include/rte_eal.h      | 15 +++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/doc/guides/prog_guide/multi_proc_support.rst b/doc/guides/prog_guide/multi_proc_support.rst
index 665474a4c..b9758d3fb 100644
--- a/doc/guides/prog_guide/multi_proc_support.rst
+++ b/doc/guides/prog_guide/multi_proc_support.rst
@@ -309,6 +309,13 @@ If a response is required, a new ``rte_mp_msg`` message descriptor must be
 constructed and sent via ``rte_mp_reply()`` function, along with ``peer``
 pointer. The resulting response will then be delivered to the correct requestor.
 
+.. warning::
+    Simply returning a value when processing a request callback will not send a
+    response to the request - it must always be explicitly sent even in case
+    of errors. Implementation of error signalling rests with the application,
+    there is no built-in way to indicate success or error for a request. Failing
+    to do so will cause the requestor to time out while waiting on a response.
+
 Misc considerations
 ~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 7db022532..82ee50fd5 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -226,6 +226,11 @@ struct rte_mp_reply {
  * As we create  socket channel for primary/secondary communication, use
  * this function typedef to register action for coming messages.
  *
+ * @note When handling IPC request callbacks, the reply must be sent even in
+ *   cases of error handling. Simply retuning success or failure will *not* send
+ *   a response to the requestor. Implementation of error signalling mechanism
+ *   is up to the application.
+ *
  * @note No memory allocations should take place inside the callback.
  */
 typedef int (*rte_mp_t)(const struct rte_mp_msg *msg, const void *peer);
@@ -237,6 +242,11 @@ typedef int (*rte_mp_t)(const struct rte_mp_msg *msg, const void *peer);
  * this function typedef to register action for coming responses to asynchronous
  * requests.
  *
+ * @note When handling IPC request callbacks, the reply must be sent even in
+ *   cases of error handling. Simply retuning success or failure will *not* send
+ *   a response to the requestor. Implementation of error signalling mechanism
+ *   is up to the application.
+ *
  * @note No memory allocations should take place inside the callback.
  */
 typedef int (*rte_mp_async_reply_t)(const struct rte_mp_msg *request,
@@ -368,6 +378,11 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
  * This function will send a reply message in response to a request message
  * received previously.
  *
+ * @note When handling IPC request callbacks, the reply must be sent even in
+ *   cases of error handling. Simply retuning success or failure will *not* send
+ *   a response to the requestor. Implementation of error signalling mechanism
+ *   is up to the application.
+ *
  * @param msg
  *   The msg argument contains the customized message.
  *
-- 
2.17.1

Re: [dpdk-stable] [PATCH 3/3] ipc: add warnings about correct API usage

[Thomas Monjalon]

03/05/2019 13:50, Anatoly Burakov:
> When handling synchronous or asynchronous requests, the reply
> must be sent explicitly even if the result of the operation is
> an error, to avoid the other side timing out. Make note of this
> in documentation explicitly.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -226,6 +226,11 @@ struct rte_mp_reply {
>   * As we create  socket channel for primary/secondary communication, use
>   * this function typedef to register action for coming messages.
>   *
> + * @note When handling IPC request callbacks, the reply must be sent even in
> + *   cases of error handling. Simply retuning success or failure will *not* send

retuning -> returning
Will fix on apply.

Re: [dpdk-stable] [dpdk-dev] [PATCH 1/3] doc: fix typo in ipc doc

[Rami Rosen]

Anatoly Burakov ‏<anatoly.burakov@intel.com>:

> The word "syncrhonous" appears twice. Fix it.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>

Reviewed-by: Rami Rosen <ramirose@gmail.com>

Re: [dpdk-stable] [PATCH 1/3] doc: fix typo in ipc doc

[Thomas Monjalon]

03/05/2019 13:50, Anatoly Burakov:
> The word "syncrhonous" appears twice. Fix it.
[...]
> --- a/doc/guides/prog_guide/multi_proc_support.rst
> +++ b/doc/guides/prog_guide/multi_proc_support.rst
> -For synchronous synchronous requests, the ``rte_mp_reply`` descriptor must also
> -be created. This is where the responses will be stored. The list of fields that
> -will be populated by IPC are as follows:
> +For synchronous requests, the ``rte_mp_reply`` descriptor must also be created.
> +This is where the responses will be stored. The list of fields that will be
> +populated by IPC are as follows:

For this reason, it is better to return to new line at the end of a sentence,
or after a comma. So the future patch will change only one line.
I will split this one.

Re: [dpdk-stable] [PATCH 1/3] doc: fix typo in ipc doc

[Mcnamara, John]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Friday, May 3, 2019 12:51 PM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/3] doc: fix typo in ipc doc
> 
> The word "syncrhonous" appears twice. Fix it.

Acked-by: John McNamara <john.mcnamara@intel.com>

Re: [dpdk-stable] [dpdk-dev] [PATCH 1/3] doc: fix typo in ipc doc

[Thomas Monjalon]

09/05/2019 17:25, Mcnamara, John:
> From: Burakov, Anatoly
> > The word "syncrhonous" appears twice. Fix it.

"synchronous" ;)

Fixes: e22266669e86 ("doc: add IPC guide")

> Acked-by: John McNamara <john.mcnamara@intel.com>

Series applied, thanks