<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>On 5/19/2025 6:46 PM, Ji, Kai wrote:</p>
    <blockquote type="cite"
cite="mid:DS0PR11MB74589B3EB4B98A5DE37AF748819CA@DS0PR11MB7458.namprd11.prod.outlook.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        Hi Yangming, </div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        How did you configure the queue pairs differently between the
        primary and secondary processes?</div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        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.</div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        Regards</div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        Kai </div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <div class="elementToProof"
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <div><br>
      </div>
      <div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <hr style="display: inline-block; width: 98%;">
      <div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
        <b>From:</b> Moses Young <a class="moz-txt-link-rfc2396E" href="mailto:mosesyyoung@gmail.com"><mosesyyoung@gmail.com></a><br>
        <b>Sent:</b> Monday, May 12, 2025 11:10<br>
        <b>To:</b> Ji, Kai <a class="moz-txt-link-rfc2396E" href="mailto:kai.ji@intel.com"><kai.ji@intel.com></a>; Yang Ming
        <a class="moz-txt-link-rfc2396E" href="mailto:ming.1.yang@nokia-sbell.com"><ming.1.yang@nokia-sbell.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:dev@dpdk.org">dev@dpdk.org</a>
        <a class="moz-txt-link-rfc2396E" href="mailto:dev@dpdk.org"><dev@dpdk.org></a><br>
        <b>Subject:</b> Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP
        release in secondary </div>
      <div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
        <br>
      </div>
      <p style="margin-top: 0px; margin-bottom: 0px;">On 5/7/2025 11:25
        PM, Ji, Kai wrote:</p>
      <blockquote>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          Hi Yangming, </div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <div
style="line-height: 19px; font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          PID check is implemented here:  </div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          <a
href="https://github.com/DPDK/dpdk/blob/75f179ebe347b6098cf3af26d3d3b7168fe3fe24/drivers/crypto/ipsec_mb/ipsec_mb_ops.c#L376"
            id="OWAce7322dc-19fb-9a60-b9da-0cc56219bc49"
class="x_OWAAutoLink x_moz-txt-link-freetext moz-txt-link-freetext"
            data-auth="NotApplicable" moz-do-not-send="true">https://github.com/DPDK/dpdk/blob/75f179ebe347b6098cf3af26d3d3b7168fe3fe24/drivers/crypto/ipsec_mb/ipsec_mb_ops.c#L376</a></div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          Can you share the steps to re-produce the error ?</div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          Regards</div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <div
style="font-family: "IntelOne Text"; font-size: 10pt; color: rgb(0, 0, 0);">
          Kai  </div>
        <div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <hr style="display: inline-block; width: 98%;">
        <div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
          <b>From:</b> Yang Ming<br>
          <b>Sent:</b> Thursday, April 24, 2025 15:26<br>
          <b>To:</b> <a href="mailto:dev@dpdk.org"
            id="OWAb157fb33-c271-4671-52d4-55ae7fc02603"
class="x_moz-txt-link-abbreviated OWAAutoLink moz-txt-link-freetext"
            moz-do-not-send="true">dev@dpdk.org</a><br>
          <b>Subject:</b> Re: [PATCH v2 2/2] crypto/ipsec_mb: fix QP
          release in secondary</div>
        <div
style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <div style="font-size: 11pt;">Hi,<br>
          <br>
          On 2025/4/7 13:25, Yang Ming wrote:<br>
          > From: myang <a href="mailto:ming.1.yang@nokia-sbell.com"
            id="OWA3f9efb88-ef9c-2a54-dceb-f658b1698fba"
            class="x_moz-txt-link-rfc2396E OWAAutoLink"
            moz-do-not-send="true">
            <ming.1.yang@nokia-sbell.com></a><br>
          ><br>
          > When a secondary process tries to release a queue pair
          (QP) that<br>
          > does not belong to it, error logs occur:<br>
          > CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to
          release<br>
          > qp_id=0<br>
          > EAL: Message data is too long<br>
          > EAL: Fail to handle message: ipsec_mb_mp_msg<br>
          > EAL: Fail to recv reply for request
          /tmp/dpdk/l2hi/mp_socket:<br>
          > ipsec_mb_mp_msg<br>
          ><br>
          > This patch ensures that a secondary process only frees a
          QP if<br>
          > it actually owns it, preventing conflicts and resolving
          the<br>
          > issue.<br>
          ><br>
          > Signed-off-by: myang <a
            href="mailto:ming.1.yang@nokia-sbell.com"
            id="OWAb2264f81-2e4b-4c96-ab14-a31b1a26ffdf"
            class="x_moz-txt-link-rfc2396E OWAAutoLink"
            moz-do-not-send="true">
            <ming.1.yang@nokia-sbell.com></a><br>
          > ---<br>
          >   drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--<br>
          >   1 file changed, 5 insertions(+), 2 deletions(-)<br>
          ><br>
          > diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
          b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c<br>
          > index 910efb1a97..50ee140ccd 100644<br>
          > --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c<br>
          > +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c<br>
          > @@ -138,6 +138,7 @@ int<br>
          >   ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t
          qp_id)<br>
          >   {<br>
          >        struct ipsec_mb_qp *qp =
          dev->data->queue_pairs[qp_id];<br>
          > +     uint16_t process_id = (uint16_t)getpid();<br>
          >  <br>
          >        if (!qp)<br>
          >                return 0;<br>
          > @@ -152,8 +153,10 @@ ipsec_mb_qp_release(struct
          rte_cryptodev *dev, uint16_t qp_id)<br>
          >                rte_free(qp);<br>
          >                dev->data->queue_pairs[qp_id] =
          NULL;<br>
          >        } else { /* secondary process */<br>
          > -             return
          ipsec_mb_secondary_qp_op(dev->data->dev_id, qp_id,<br>
          > -                                     NULL, 0,
          RTE_IPSEC_MB_MP_REQ_QP_FREE);<br>
          > +             if (qp->qp_used_by_pid == process_id)<br>
          > +                     return
          ipsec_mb_secondary_qp_op(dev->data->dev_id,<br>
          > +                                             qp_id,
          NULL, 0,<br>
          > +                                            
          RTE_IPSEC_MB_MP_REQ_QP_FREE);<br>
          >        }<br>
          >        return 0;<br>
          >   }<br>
          <br>
          Hi Experts,<br>
          <br>
          Is there any chance to review and accept this patch?<br>
          <br>
          Brs,<br>
          <br>
          Yang Ming<br>
          <br>
        </div>
      </blockquote>
      <div>Hi,<br>
        <br>
        David. Thanks for your feedback. I will Cc maintainers in next
        version.<br>
        <br>
        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:<br>
        1. Start the primary process: Run the DPDK primary process with
        the IPsec MB crypto device.<br>
        2. Launch the secondary process: Start a DPDK secondary process
        using the same device parameters.<br>
        3. Exit the secondary normally.<br>
        <br>
        On secondary exit, error logs show below as my commit log's
        description:<br>
        CRYPTODEV: ipsec_mb_ipc_request() line 373: Unable to release
        qp_id=0<br>
        EAL: Message data is too long<br>
        EAL: Fail to handle message: ipsec_mb_mp_msg<br>
        EAL: Fail to recv reply for request /tmp/dpdk/l2hi/mp_socket:
        ipsec_mb_mp_msg<br>
        <br>
        This message corresponds exactly to the PID check in the code
        you referenced:<br>
        if (qp->qp_used_by_pid != req_param->process_id) {<br>
            CDEV_LOG_ERR("Unable to release qp_id=%d", qp_id);<br>
            goto out;<br>
        }<br>
        <br>
        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.<br>
        <br>
        Thanks for your help.<br>
        <br>
        Best,<br>
        Yang Ming</div>
    </blockquote>
    <p><br>
    </p>
    Hi Kai,<br>
    <br>
    The queue pairs is shared between the primary and secondary
    processes. Even if each process calls
    rte_cryptodev_queue_pair_setup() with a unique qp_id, the primary
    and secondary still end up using the same shared queue pair
    structure, because cryptodev->data is shared between them.<br>
    <br>
    From the code path, cryptodev->data is allocated in the primary
    via rte_cryptodev_data_alloc() (inside
ipsec_mb_create-->rte_cryptodev_pmd_create-->rte_cryptodev_pmd_allocate-->rte_cryptodev_data_alloc).
    This memory is placed in a shared memzone (rte_cryptodev_data_%u),
    so both primary and secondary processes reference the same
    cryptodev->data, including nb_queue_pairs and queue_pairs[].<br>
    As a result, when the secondary process exits, ipsec_mb_remove() is
    called (inside
rte_eal_cleanup-->eal_bus_cleanup-->vdev_cleanup-->rte_vdev_driver-->ipsec_mb_remove-->ipsec_mb_qp_release-->ipsec_mb_secondary_qp_op)
    and it loops through all queue pairs using:<br>
    for (qp_id = 0; qp_id < cryptodev->data->nb_queue_pairs;
    qp_id++)<br>
        ipsec_mb_qp_release(cryptodev, qp_id);<br>
    <br>
    This causes the secondary to attempt releasing queue pairs it
    doesn't own, triggering the error logs mentioned in previous mail.<br>
    <br>
    Best regards,<br>
    Yang Ming
  </body>
</html>