[dpdk-stable] [EXT] patch 'crypto/octeontx2: fix multi-process' has been queued to stable release 19.11.6

[Luca Boccassi]

On Tue, 2020-11-10 at 10:59 +0000, Ankur Dwivedi wrote:
> Hi Luca,
> 
> Please find my reply inline.
> 
> Thanks
> Ankur
> 
> > -----Original Message-----
> > From: luca.boccassi at gmail.com <luca.boccassi at gmail.com>
> > Sent: Tuesday, November 10, 2020 12:10 AM
> > To: Ankur Dwivedi <adwivedi at marvell.com>
> > Cc: Anoob Joseph <anoobj at marvell.com>; dpdk stable <stable at dpdk.org>
> > Subject: [EXT] patch 'crypto/octeontx2: fix multi-process' has been queued to
> > stable release 19.11.6
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > Hi,
> > 
> > FYI, your patch has been queued to stable release 19.11.6
> > 
> > Note it hasn't been pushed to
> > https://urldefense.proofpoint.com/v2/url?u=http-
> > 3A__dpdk.org_browse_dpdk-
> > 2Dstable&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=ILjiNF3GF25y6QdHZUx
> > Ml6JrStU0MIuCtO5dMzn3Ybk&m=WJuhF88JRLbzAE2KFDm7fq_VKrRhCBfJ1zW
> > 9-e8Y9Fs&s=mj0L_xoCfGbVaKeVCZRgKMCSRJvL_VQRL9Ukk60Oyk4&e=  yet.
> > It will be pushed if I get no objections before 11/11/20. So please shout if
> > anyone has objections.
> > 
> > Also note that after the patch there's a diff of the upstream commit vs the
> > patch applied to the branch. This will indicate if there was any rebasing
> > needed to apply to the stable branch. If there were code changes for rebasing
> > (ie: not only metadata diffs), please double check that the rebase was
> > correctly done.
> > 
> > Queued patches are on a temporary branch at:
> > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__github.com_bluca_dpdk-
> > 2Dstable&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=ILjiNF3GF25y6QdHZUx
> > Ml6JrStU0MIuCtO5dMzn3Ybk&m=WJuhF88JRLbzAE2KFDm7fq_VKrRhCBfJ1zW
> > 9-e8Y9Fs&s=7RDdv-ig0gf-Ik31WdzFV8mldqC7ejjFz7eIIlkpPAo&e=
> > 
> > This queued commit can be viewed at:
> > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__github.com_bluca_dpdk-
> > 2Dstable_commit_3415ca530b2ea4acdbc15b3c6eb4c4f4177a09dc&d=DwIDAg
> > &c=nKjWec2b6R0mOyPaz7xtfQ&r=ILjiNF3GF25y6QdHZUxMl6JrStU0MIuCtO5d
> > Mzn3Ybk&m=WJuhF88JRLbzAE2KFDm7fq_VKrRhCBfJ1zW9-
> > e8Y9Fs&s=pEl2YmNRFwnSh57DYLWRaNYvL4ZiOMv_zTkzBlAsRo4&e=
> > 
> > Thanks.
> > 
> > Luca Boccassi
> > 
> > ---
> > From 3415ca530b2ea4acdbc15b3c6eb4c4f4177a09dc Mon Sep 17 00:00:00
> > 2001
> > From: Ankur Dwivedi <adwivedi at marvell.com>
> > Date: Thu, 22 Oct 2020 13:20:00 +0530
> > Subject: [PATCH] crypto/octeontx2: fix multi-process
> > 
> > [ upstream commit 9fd11c1583c9b3249f49755b3216a79698e94700 ]
> > 
> > During crypto device probe few functions should be called only for the
> > primary process. This patch fixes this issue.
> > 
> > Fixes: 818d138bcce9 ("crypto/octeontx2: add init sequence in probe")
> > 
> > Signed-off-by: Ankur Dwivedi <adwivedi at marvell.com>
> > Reviewed-by: Anoob Joseph <anoobj at marvell.com>
> > ---
> > drivers/crypto/octeontx2/otx2_cryptodev.c     | 49 +++++++++++--------
> > drivers/crypto/octeontx2/otx2_cryptodev.h     |  2 +
> > drivers/crypto/octeontx2/otx2_cryptodev_ops.c | 13 +++--
> > 3 files changed, 40 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/crypto/octeontx2/otx2_cryptodev.c
> > b/drivers/crypto/octeontx2/otx2_cryptodev.c
> > index 417eda6de6..2323de1f60 100644
> > --- a/drivers/crypto/octeontx2/otx2_cryptodev.c
> > +++ b/drivers/crypto/octeontx2/otx2_cryptodev.c
> > @@ -70,31 +70,34 @@ otx2_cpt_pci_probe(struct rte_pci_driver *pci_drv
> > __rte_unused,
> > 
> > 	otx2_dev = &vf->otx2_dev;
> > 
> > -	/* Initialize the base otx2_dev object */
> > -	ret = otx2_dev_init(pci_dev, otx2_dev);
> > -	if (ret) {
> > -		CPT_LOG_ERR("Could not initialize otx2_dev");
> > -		goto pmd_destroy;
> > -	}
> > +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> > +		/* Initialize the base otx2_dev object */
> > +		ret = otx2_dev_init(pci_dev, otx2_dev);
> > +		if (ret) {
> > +			CPT_LOG_ERR("Could not initialize otx2_dev");
> > +			goto pmd_destroy;
> > +		}
> > 
> > -	/* Get number of queues available on the device */
> > -	ret = otx2_cpt_available_queues_get(dev, &nb_queues);
> > -	if (ret) {
> > -		CPT_LOG_ERR("Could not determine the number of queues
> > available");
> > -		goto otx2_dev_fini;
> > -	}
> > +		/* Get number of queues available on the device */
> > +		ret = otx2_cpt_available_queues_get(dev, &nb_queues);
> > +		if (ret) {
> > +			CPT_LOG_ERR("Could not determine the number of
> > queues available");
> > +			goto otx2_dev_fini;
> > +		}
> > 
> > -	/* Don't exceed the limits set per VF */
> > -	nb_queues = RTE_MIN(nb_queues,
> > OTX2_CPT_MAX_QUEUES_PER_VF);
> > +		/* Don't exceed the limits set per VF */
> > +		nb_queues = RTE_MIN(nb_queues,
> > OTX2_CPT_MAX_QUEUES_PER_VF);
> > 
> > -	if (nb_queues == 0) {
> > -		CPT_LOG_ERR("No free queues available on the device");
> > -		goto otx2_dev_fini;
> > -	}
> > +		if (nb_queues == 0) {
> > +			CPT_LOG_ERR("No free queues available on the
> > device");
> > +			goto otx2_dev_fini;
> > +		}
> > 
> > -	vf->max_queues = nb_queues;
> > +		vf->max_queues = nb_queues;
> > 
> > -	CPT_LOG_INFO("Max queues supported by device: %d", vf-
> > > max_queues);
> > +		CPT_LOG_INFO("Max queues supported by device: %d",
> > +				vf->max_queues);
> > +	}
> > 
> > 	dev->feature_flags = RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO |
> > 			     RTE_CRYPTODEV_FF_HW_ACCELERATED | @@ -
> > 105,10 +108,14 @@ otx2_cpt_pci_probe(struct rte_pci_driver *pci_drv
> > __rte_unused,
> > 			     RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO |
> > 			     RTE_CRYPTODEV_FF_RSA_PRIV_OP_KEY_QT;
> > 
> > +	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> > +		otx2_cpt_set_enqdeq_fns(dev);
> > +
> > 	return 0;
> > 
> > otx2_dev_fini:
> > -	otx2_dev_fini(pci_dev, otx2_dev);
> > +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> [Ankur] For 19.11 branch the above if check is not required as the otx2_dev_fini label is reached only by the primary process. For 20.11 branch the above if check was required but for 19.11 branch it is not required. So if its possible can you remove the if check during merging?

As a generic process, I try to keep the diff from mainline as minimal
as possible, as it makes the backporting process much simpler and
smoother - more patches apply without changes.

Unless there's a specific reason to remove this (eg: it would break
something if it's left in) I'd leave it as is.

-- 
Kind regards,
Luca Boccassi