[PATCH] eal: prevent socket closure before MP sync

[PATCH] eal: prevent socket closure before MP sync

[Yang Ming]

The secordary process should not close socket file for MP
channel before performing MP request synchronization.
This prevents error logs when the secondary process exits
without any operation on the crypto device while the primary
process starts the device.

Case situation:
eal_bus_cleanup has been added in rte_eal_cleanup. But for the
secondary process, rte_eal_cleanup firstly performs
rte_mp_channel_cleanup, which closes socket file for the MP
channel, making mp_fd invalid. Subsequently, eal_bus_cleanup
triggers vdev_cleanup, which calls mp_request_sync to send a
message via the MP channel. Since mp_fd is invalid, error logs
occur.

Error logs occur as below when the secordary process exit:
EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad
file descriptor
EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:
ipsec_mb_mp_msg
USER1: Create MR request to primary process failed.

Function call trace:
1. rte_eal_cleanup->rte_mp_channel_cleanup->close_socket_fd
2. rte_eal_cleanup->eal_bus_cleanup->vdev_cleanup->
rte_vdev_driver->ipsec_mb_remove->ipsec_mb_qp_release->
ipsec_mb_secondary_qp_op->rte_mp_request_sync->mp_request_sync->
send_msg->sendmsg(mp_fd, &msgh, 0);

Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
Cc: kevin.laatz@intel.com
Cc: stable@dpdk.org

Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
---
 lib/eal/linux/eal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index b1e63e37fc..73ea47b12d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1318,11 +1318,11 @@ rte_eal_cleanup(void)
 		rte_memseg_walk(mark_freeable, NULL);
 
 	rte_service_finalize();
+	eal_bus_cleanup();
 #ifdef VFIO_PRESENT
 	vfio_mp_sync_cleanup();
 #endif
 	rte_mp_channel_cleanup();
-	eal_bus_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
 	eal_mp_dev_hotplug_cleanup();
-- 
2.34.1

Re: [PATCH] eal: prevent socket closure before MP sync

[Stephen Hemminger]

On Fri, 14 Mar 2025 18:36:38 +0800
Yang Ming <ming.1.yang@nokia-sbell.com> wrote:

> The secordary process should not close socket file for MP
> channel before performing MP request synchronization.
> This prevents error logs when the secondary process exits
> without any operation on the crypto device while the primary
> process starts the device.
> 
> Case situation:
> eal_bus_cleanup has been added in rte_eal_cleanup. But for the
> secondary process, rte_eal_cleanup firstly performs
> rte_mp_channel_cleanup, which closes socket file for the MP
> channel, making mp_fd invalid. Subsequently, eal_bus_cleanup
> triggers vdev_cleanup, which calls mp_request_sync to send a
> message via the MP channel. Since mp_fd is invalid, error logs
> occur.
> 
> Error logs occur as below when the secordary process exit:
> EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad
> file descriptor
> EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:
> ipsec_mb_mp_msg
> USER1: Create MR request to primary process failed.
> 
> Function call trace:
> 1. rte_eal_cleanup->rte_mp_channel_cleanup->close_socket_fd
> 2. rte_eal_cleanup->eal_bus_cleanup->vdev_cleanup->
> rte_vdev_driver->ipsec_mb_remove->ipsec_mb_qp_release->
> ipsec_mb_secondary_qp_op->rte_mp_request_sync->mp_request_sync->
> send_msg->sendmsg(mp_fd, &msgh, 0);
> 
> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
> Cc: kevin.laatz@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>

Looks good, could there be a test?

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [External] Re: [PATCH] eal: prevent socket closure before MP sync

[Yang Ming]

On 2025/3/17 21:56, Stephen Hemminger wrote:
> Caution: This is an external email. Please be very careful when clicking links or opening attachments. See http://nok.it/nsb for additional information.
>
> On Fri, 14 Mar 2025 18:36:38 +0800
> Yang Ming <ming.1.yang@nokia-sbell.com> wrote:
>
>> The secordary process should not close socket file for MP
>> channel before performing MP request synchronization.
>> This prevents error logs when the secondary process exits
>> without any operation on the crypto device while the primary
>> process starts the device.
>>
>> Case situation:
>> eal_bus_cleanup has been added in rte_eal_cleanup. But for the
>> secondary process, rte_eal_cleanup firstly performs
>> rte_mp_channel_cleanup, which closes socket file for the MP
>> channel, making mp_fd invalid. Subsequently, eal_bus_cleanup
>> triggers vdev_cleanup, which calls mp_request_sync to send a
>> message via the MP channel. Since mp_fd is invalid, error logs
>> occur.
>>
>> Error logs occur as below when the secordary process exit:
>> EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad
>> file descriptor
>> EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:
>> ipsec_mb_mp_msg
>> USER1: Create MR request to primary process failed.
>>
>> Function call trace:
>> 1. rte_eal_cleanup->rte_mp_channel_cleanup->close_socket_fd
>> 2. rte_eal_cleanup->eal_bus_cleanup->vdev_cleanup->
>> rte_vdev_driver->ipsec_mb_remove->ipsec_mb_qp_release->
>> ipsec_mb_secondary_qp_op->rte_mp_request_sync->mp_request_sync->
>> send_msg->sendmsg(mp_fd, &msgh, 0);
>>
>> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
>> Cc: kevin.laatz@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
> Looks good, could there be a test?
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>
Hi Thanks Stephen,

We have some findings for this change in our product test line. I will 
do more investigation for this patch.

Brs,

Yang Ming

Re: [External] Re: [PATCH] eal: prevent socket closure before MP sync

[Yang Ming]

On 2025/3/27 17:28, Yang Ming wrote:
>
> On 2025/3/17 21:56, Stephen Hemminger wrote:
>> Caution: This is an external email. Please be very careful when 
>> clicking links or opening attachments. See http://nok.it/nsb for 
>> additional information.
>>
>> On Fri, 14 Mar 2025 18:36:38 +0800
>> Yang Ming <ming.1.yang@nokia-sbell.com> wrote:
>>
>>> The secordary process should not close socket file for MP
>>> channel before performing MP request synchronization.
>>> This prevents error logs when the secondary process exits
>>> without any operation on the crypto device while the primary
>>> process starts the device.
>>>
>>> Case situation:
>>> eal_bus_cleanup has been added in rte_eal_cleanup. But for the
>>> secondary process, rte_eal_cleanup firstly performs
>>> rte_mp_channel_cleanup, which closes socket file for the MP
>>> channel, making mp_fd invalid. Subsequently, eal_bus_cleanup
>>> triggers vdev_cleanup, which calls mp_request_sync to send a
>>> message via the MP channel. Since mp_fd is invalid, error logs
>>> occur.
>>>
>>> Error logs occur as below when the secordary process exit:
>>> EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad
>>> file descriptor
>>> EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:
>>> ipsec_mb_mp_msg
>>> USER1: Create MR request to primary process failed.
>>>
>>> Function call trace:
>>> 1. rte_eal_cleanup->rte_mp_channel_cleanup->close_socket_fd
>>> 2. rte_eal_cleanup->eal_bus_cleanup->vdev_cleanup->
>>> rte_vdev_driver->ipsec_mb_remove->ipsec_mb_qp_release->
>>> ipsec_mb_secondary_qp_op->rte_mp_request_sync->mp_request_sync->
>>> send_msg->sendmsg(mp_fd, &msgh, 0);
>>>
>>> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
>>> Cc: kevin.laatz@intel.com
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
>> Looks good, could there be a test?
>>
>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>>
> Hi Thanks Stephen,
>
> We have some findings for this change in our product test line. I will 
> do more investigation for this patch.
>
> Brs,
>
> Yang Ming
>
>
Hi,

This patch has been tested and functions correctly under normal 
conditions. However, during testing, we observed some new error logs in 
specific cases. Upon investigation, we identified that these logs 
originate from a separate issue, which will be addressed in the next 
version of this patch. Additionally, a similar issue may affect FreeBSD, 
and I plan to include a fix for that as well in the upcoming patch 
series. Please note that the entire patch series has been thoroughly tested.

Brs,

Yang Ming

[PATCH v2 1/2] eal: prevent socket closure before MP sync

[Yang Ming]

The secordary process should not close socket file for MP
channel before performing MP request synchronization.
This prevents error logs when the secondary process exits
without any operation on the crypto device while the primary
process starts the device.

Case situation:
eal_bus_cleanup has been added in rte_eal_cleanup. But for the
secondary process, rte_eal_cleanup firstly performs
rte_mp_channel_cleanup, which closes socket file for the MP
channel, making mp_fd invalid. Subsequently, eal_bus_cleanup
triggers vdev_cleanup, which calls mp_request_sync to send a
message via the MP channel. Since mp_fd is invalid, error logs
occur.

Error logs occur as below when the secordary process exit:
EAL: failed to send to (/tmp/dpdk/l2hicu/mp_socket) due to Bad
file descriptor
EAL: Fail to send request /tmp/dpdk/l2hicu/mp_socket:
ipsec_mb_mp_msg
USER1: Create MR request to primary process failed.

Function call trace:
1. rte_eal_cleanup->rte_mp_channel_cleanup->close_socket_fd
2. rte_eal_cleanup->eal_bus_cleanup->vdev_cleanup->
rte_vdev_driver->ipsec_mb_remove->ipsec_mb_qp_release->
ipsec_mb_secondary_qp_op->rte_mp_request_sync->mp_request_sync->
send_msg->sendmsg(mp_fd, &msgh, 0);

Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
Cc: kevin.laatz@intel.com
Cc: stable@dpdk.org

Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
---
 lib/eal/freebsd/eal.c | 2 +-
 lib/eal/linux/eal.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index d07cff8651..d81f12a7b1 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -899,8 +899,8 @@ rte_eal_cleanup(void)
 	struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 	rte_service_finalize();
-	rte_mp_channel_cleanup();
 	eal_bus_cleanup();
+	rte_mp_channel_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
 	rte_eal_alarm_cleanup();
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index b1e63e37fc..73ea47b12d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1318,11 +1318,11 @@ rte_eal_cleanup(void)
 		rte_memseg_walk(mark_freeable, NULL);
 
 	rte_service_finalize();
+	eal_bus_cleanup();
 #ifdef VFIO_PRESENT
 	vfio_mp_sync_cleanup();
 #endif
 	rte_mp_channel_cleanup();
-	eal_bus_cleanup();
 	rte_trace_save();
 	eal_trace_fini();
 	eal_mp_dev_hotplug_cleanup();
-- 
2.34.1

[PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

[Yang Ming]

From: myang <ming.1.yang@nokia-sbell.com>

When a secondary process tries to release a queue pair (QP) that
does not belong to it, error logs occur:
CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release
qp_id=0
EAL: Message data is too long
EAL: Fail to handle message: ipsec_mb_mp_msg
EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket:
ipsec_mb_mp_msg

This patch ensures that a secondary process only frees a QP if
it actually owns it, preventing conflicts and resolving the
issue.

Signed-off-by: myang <ming.1.yang@nokia-sbell.com>
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index 910efb1a97..50ee140ccd 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -138,6 +138,7 @@ int
 ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 {
 	struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
+	uint16_t process_id = (uint16_t)getpid();
 
 	if (!qp)
 		return 0;
@@ -152,8 +153,10 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
 	} else { /* secondary process */
-		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
-					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
+		if (qp->qp_used_by_pid == process_id)
+			return ipsec_mb_secondary_qp_op(dev->data->dev_id,
+						qp_id, NULL, 0,
+						RTE_IPSEC_MB_MP_REQ_QP_FREE);
 	}
 	return 0;
 }
-- 
2.34.1

Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

[Yang Ming]

Hi,

On 2025/4/7 13:25, Yang Ming wrote:
> From: myang <ming.1.yang@nokia-sbell.com>
>
> When a secondary process tries to release a queue pair (QP) that
> does not belong to it, error logs occur:
> CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release
> qp_id=0
> EAL: Message data is too long
> EAL: Fail to handle message: ipsec_mb_mp_msg
> EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket:
> ipsec_mb_mp_msg
>
> This patch ensures that a secondary process only frees a QP if
> it actually owns it, preventing conflicts and resolving the
> issue.
>
> Signed-off-by: myang <ming.1.yang@nokia-sbell.com>
> ---
>   drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> index 910efb1a97..50ee140ccd 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> @@ -138,6 +138,7 @@ int
>   ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
>   {
>   	struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
> +	uint16_t process_id = (uint16_t)getpid();
>   
>   	if (!qp)
>   		return 0;
> @@ -152,8 +153,10 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
>   		rte_free(qp);
>   		dev->data->queue_pairs[qp_id] = NULL;
>   	} else { /* secondary process */
> -		return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
> -					NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
> +		if (qp->qp_used_by_pid == process_id)
> +			return ipsec_mb_secondary_qp_op(dev->data->dev_id,
> +						qp_id, NULL, 0,
> +						RTE_IPSEC_MB_MP_REQ_QP_FREE);
>   	}
>   	return 0;
>   }

Hi Experts,

Is there any chance to review and accept this patch?

Brs,

Yang Ming

Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

[David Marchand]

Hello,

On Thu, Apr 24, 2025 at 4:26 PM Yang Ming <ming.1.yang@nokia-sbell.com> wrote:
>
> Hi,
>
> On 2025/4/7 13:25, Yang Ming wrote:
> > From: myang <ming.1.yang@nokia-sbell.com>
> >
> > When a secondary process tries to release a queue pair (QP) that
> > does not belong to it, error logs occur:
> > CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release
> > qp_id=0
> > EAL: Message data is too long
> > EAL: Fail to handle message: ipsec_mb_mp_msg
> > EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket:
> > ipsec_mb_mp_msg
> >
> > This patch ensures that a secondary process only frees a QP if
> > it actually owns it, preventing conflicts and resolving the
> > issue.
> >
> > Signed-off-by: myang <ming.1.yang@nokia-sbell.com>
> > ---
> >   drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> > index 910efb1a97..50ee140ccd 100644
> > --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> > +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> > @@ -138,6 +138,7 @@ int
> >   ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
> >   {
> >       struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
> > +     uint16_t process_id = (uint16_t)getpid();
> >
> >       if (!qp)
> >               return 0;
> > @@ -152,8 +153,10 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
> >               rte_free(qp);
> >               dev->data->queue_pairs[qp_id] = NULL;
> >       } else { /* secondary process */
> > -             return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
> > -                                     NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
> > +             if (qp->qp_used_by_pid == process_id)
> > +                     return ipsec_mb_secondary_qp_op(dev->data->dev_id,
> > +                                             qp_id, NULL, 0,
> > +                                             RTE_IPSEC_MB_MP_REQ_QP_FREE);
> >       }
> >       return 0;
> >   }
>
> Hi Experts,
>
> Is there any chance to review and accept this patch?

Don't forget to Cc maintainers.


-- 
David Marchand

Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

[Ji, Kai]

[-- Attachment #1: Type: text/plain, Size: 2468 bytes --]

Hi Yangming,

PID check is implemented here:
https://github.com/DPDK/dpdk/blob/75f179ebe347b6098cf3af26d3d3b7168fe3fe24/drivers/crypto/ipsec_mb/ipsec_mb_ops.c#L376

Can you share the steps to re-produce the error ?

Regards

Kai

________________________________
From: Yang Ming
Sent: Thursday, April 24, 2025 15:26
To: dev@dpdk.org
Subject: Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

Hi,

On 2025/4/7 13:25, Yang Ming wrote:
> From: myang <ming.1.yang@nokia-sbell.com>
>
> When a secondary process tries to release a queue pair (QP) that
> does not belong to it, error logs occur:
> CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release
> qp_id=0
> EAL: Message data is too long
> EAL: Fail to handle message: ipsec_mb_mp_msg
> EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket:
> ipsec_mb_mp_msg
>
> This patch ensures that a secondary process only frees a QP if
> it actually owns it, preventing conflicts and resolving the
> issue.
>
> Signed-off-by: myang <ming.1.yang@nokia-sbell.com>
> ---
>   drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> index 910efb1a97..50ee140ccd 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> @@ -138,6 +138,7 @@ int
>   ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
>   {
>        struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
> +     uint16_t process_id = (uint16_t)getpid();
>
>        if (!qp)
>                return 0;
> @@ -152,8 +153,10 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
>                rte_free(qp);
>                dev->data->queue_pairs[qp_id] = NULL;
>        } else { /* secondary process */
> -             return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
> -                                     NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
> +             if (qp->qp_used_by_pid == process_id)
> +                     return ipsec_mb_secondary_qp_op(dev->data->dev_id,
> +                                             qp_id, NULL, 0,
> +                                             RTE_IPSEC_MB_MP_REQ_QP_FREE);
>        }
>        return 0;
>   }

Hi Experts,

Is there any chance to review and accept this patch?

Brs,

Yang Ming


[-- Attachment #2: Type: text/html, Size: 6417 bytes --]

Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

[Moses Young]

[-- Attachment #1: Type: text/plain, Size: 4104 bytes --]

On 5/7/2025 11:25 PM, Ji, Kai wrote:

> Hi Yangming,
>
> PID check is implemented here:
> https://github.com/DPDK/dpdk/blob/75f179ebe347b6098cf3af26d3d3b7168fe3fe24/drivers/crypto/ipsec_mb/ipsec_mb_ops.c#L376 
> <https://github.com/DPDK/dpdk/blob/75f179ebe347b6098cf3af26d3d3b7168fe3fe24/drivers/crypto/ipsec_mb/ipsec_mb_ops.c#L376>
>
> Can you share the steps to re-produce the error ?
>
> Regards
>
> Kai
>
> ------------------------------------------------------------------------
> *From:* Yang Ming
> *Sent:* Thursday, April 24, 2025 15:26
> *To:* dev@dpdk.org
> *Subject:* Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in 
> secondary
>
> Hi,
>
> On 2025/4/7 13:25, Yang Ming wrote:
> > From: myang <ming.1.yang@nokia-sbell.com>
> >
> > When a secondary process tries to release a queue pair (QP) that
> > does not belong to it, error logs occur:
> > CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release
> > qp_id=0
> > EAL: Message data is too long
> > EAL: Fail to handle message: ipsec_mb_mp_msg
> > EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket:
> > ipsec_mb_mp_msg
> >
> > This patch ensures that a secondary process only frees a QP if
> > it actually owns it, preventing conflicts and resolving the
> > issue.
> >
> > Signed-off-by: myang <ming.1.yang@nokia-sbell.com>
> > ---
> >   drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c 
> b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> > index 910efb1a97..50ee140ccd 100644
> > --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> > +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> > @@ -138,6 +138,7 @@ int
> >   ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
> >   {
> >        struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
> > +     uint16_t process_id = (uint16_t)getpid();
> >
> >        if (!qp)
> >                return 0;
> > @@ -152,8 +153,10 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, 
> uint16_t qp_id)
> >                rte_free(qp);
> >                dev->data->queue_pairs[qp_id] = NULL;
> >        } else { /* secondary process */
> > -             return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
> > -                                     NULL, 0, 
> RTE_IPSEC_MB_MP_REQ_QP_FREE);
> > +             if (qp->qp_used_by_pid == process_id)
> > +                     return ipsec_mb_secondary_qp_op(dev->data->dev_id,
> > +                                             qp_id, NULL, 0,
> > + RTE_IPSEC_MB_MP_REQ_QP_FREE);
> >        }
> >        return 0;
> >   }
>
> Hi Experts,
>
> Is there any chance to review and accept this patch?
>
> Brs,
>
> Yang Ming
>
Hi,

David. Thanks for your feedback. I will Cc maintainers in next version.

Kai, Thanks for your feedback. Here's our test steps after applying the 
previous patch called "eal: prevent socket closure before MP sync" in 
this serious:
1. Start the primary process: Run the DPDK primary process with the 
IPsec MB crypto device.
2. Launch the secondary process: Start a DPDK secondary process using 
the same device parameters.
3. Exit the secondary normally.

On secondary exit, error logs show below as my commit log's description:
CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release qp_id=0
EAL: Message data is too long
EAL: Fail to handle message: ipsec_mb_mp_msg
EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket: 
ipsec_mb_mp_msg

This message corresponds exactly to the PID check in the code you 
referenced:
if (qp->qp_used_by_pid != req_param->process_id) {
     CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
     goto out;
}

In our view, this log indicates an abnormal condition: a secondary 
process should be able to release its own QPs without triggering an 
error during normal shutdown.

Thanks for your help.

Best,
Yang Ming

[-- Attachment #2: Type: text/html, Size: 7996 bytes --]

Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

[Ji, Kai]

[-- Attachment #1: Type: text/plain, Size: 4530 bytes --]

Hi Yangming,

How did you configure the queue pairs differently between the primary and secondary processes?

The application must call rte_cryptodev_queue_pair_setup() with a unique qp_id for each process. This implies that each process should receive distinct queue pair configurations at runtime. From what I can tell, it’s likely that the secondary process is still using the same queue pair as the primary process.

Regards

Kai




________________________________
From: Moses Young <mosesyyoung@gmail.com>
Sent: Monday, May 12, 2025 11:10
To: Ji, Kai <kai.ji@intel.com>; Yang Ming <ming.1.yang@nokia-sbell.com>; dev@dpdk.org <dev@dpdk.org>
Subject: Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary


On 5/7/2025 11:25 PM, Ji, Kai wrote:

Hi Yangming,

PID check is implemented here:
https://github.com/DPDK/dpdk/blob/75f179ebe347b6098cf3af26d3d3b7168fe3fe24/drivers/crypto/ipsec_mb/ipsec_mb_ops.c#L376

Can you share the steps to re-produce the error ?

Regards

Kai

________________________________
From: Yang Ming
Sent: Thursday, April 24, 2025 15:26
To: dev@dpdk.org<mailto:dev@dpdk.org>
Subject: Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP release in secondary

Hi,

On 2025/4/7 13:25, Yang Ming wrote:
> From: myang <ming.1.yang@nokia-sbell.com><mailto:ming.1.yang@nokia-sbell.com>
>
> When a secondary process tries to release a queue pair (QP) that
> does not belong to it, error logs occur:
> CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release
> qp_id=0
> EAL: Message data is too long
> EAL: Fail to handle message: ipsec_mb_mp_msg
> EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket:
> ipsec_mb_mp_msg
>
> This patch ensures that a secondary process only frees a QP if
> it actually owns it, preventing conflicts and resolving the
> issue.
>
> Signed-off-by: myang <ming.1.yang@nokia-sbell.com><mailto:ming.1.yang@nokia-sbell.com>
> ---
>   drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> index 910efb1a97..50ee140ccd 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> @@ -138,6 +138,7 @@ int
>   ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
>   {
>        struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
> +     uint16_t process_id = (uint16_t)getpid();
>
>        if (!qp)
>                return 0;
> @@ -152,8 +153,10 @@ ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
>                rte_free(qp);
>                dev->data->queue_pairs[qp_id] = NULL;
>        } else { /* secondary process */
> -             return ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,
> -                                     NULL, 0, RTE_IPSEC_MB_MP_REQ_QP_FREE);
> +             if (qp->qp_used_by_pid == process_id)
> +                     return ipsec_mb_secondary_qp_op(dev->data->dev_id,
> +                                             qp_id, NULL, 0,
> +                                             RTE_IPSEC_MB_MP_REQ_QP_FREE);
>        }
>        return 0;
>   }

Hi Experts,

Is there any chance to review and accept this patch?

Brs,

Yang Ming

Hi,

David. Thanks for your feedback. I will Cc maintainers in next version.

Kai, Thanks for your feedback. Here's our test steps after applying the previous patch called "eal: prevent socket closure before MP sync" in this serious:
1. Start the primary process: Run the DPDK primary process with the IPsec MB crypto device.
2. Launch the secondary process: Start a DPDK secondary process using the same device parameters.
3. Exit the secondary normally.

On secondary exit, error logs show below as my commit log's description:
CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release qp_id=0
EAL: Message data is too long
EAL: Fail to handle message: ipsec_mb_mp_msg
EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket: ipsec_mb_mp_msg

This message corresponds exactly to the PID check in the code you referenced:
if (qp->qp_used_by_pid != req_param->process_id) {
    CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);
    goto out;
}

In our view, this log indicates an abnormal condition: a secondary process should be able to release its own QPs without triggering an error during normal shutdown.

Thanks for your help.

Best,
Yang Ming

[-- Attachment #2: Type: text/html, Size: 10717 bytes --]