[dpdk-dev] [PATCH] examples/ipsec-secgw: add cryptodev mask option

[dpdk-dev] [PATCH] examples/ipsec-secgw: add cryptodev mask option

[Akhil Goyal]

Previously, ipsec-secgw application did not give user the
flexibility to decide which crypto device(s) will be used.

In this patch, a new cryptodev_mask option is added to the
application. Same as portmask, the cryptodev_mask avails the
user to mask out the unwanted crypto devices in the system.

This patch is similar to the support added in l2fwd-crypto
(d2797f51cc63: examples/l2fwd-crypto: add cryptodev mask option)

Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index c98454a..dd0c7d1 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -89,6 +89,7 @@
 
 #define OPTION_CONFIG		"config"
 #define OPTION_SINGLE_SA	"single-sa"
+#define OPTION_CRYPTODEV_MASK	"cryptodev_mask"
 
 #define BURST_TX_DRAIN_US 100 /* TX drain every ~100us */
 
@@ -154,6 +155,7 @@ struct ethaddr_info ethaddr_tbl[RTE_MAX_ETHPORTS] = {
 
 /* mask of enabled ports */
 static uint32_t enabled_port_mask;
+static uint64_t enabled_cryptodev_mask = UINT64_MAX;
 static uint32_t unprotected_port_mask;
 static int32_t promiscuous_on = 1;
 static int32_t numa_on = 1; /**< NUMA is enabled by default. */
@@ -848,6 +850,8 @@ print_usage(const char *prgname)
 		"rx queues configuration\n"
 		"  --single-sa SAIDX: use single SA index for outbound, "
 		"bypassing the SP\n"
+		"  --cryptodev_mask MASK: hexadecimal bitmask of the "
+		"crypto devices to configure\n"
 		"  -f CONFIG_FILE: Configuration file path\n",
 		prgname);
 }
@@ -962,6 +966,14 @@ parse_args_long_options(struct option *lgopts, int32_t option_index)
 		}
 	}
 
+	if (__STRNCMP(optname, OPTION_CRYPTODEV_MASK)) {
+		ret = parse_portmask(optarg);
+		if (ret != -1) {
+			enabled_cryptodev_mask = ret;
+			ret = 0;
+		}
+	}
+
 	return ret;
 }
 #undef __STRNCMP
@@ -976,6 +988,7 @@ parse_args(int32_t argc, char **argv)
 	static struct option lgopts[] = {
 		{OPTION_CONFIG, 1, 0, 0},
 		{OPTION_SINGLE_SA, 1, 0, 0},
+		{OPTION_CRYPTODEV_MASK, 1, 0, 0},
 		{NULL, 0, 0, 0}
 	};
 	int32_t f_present = 0;
@@ -1238,6 +1251,16 @@ add_cdev_mapping(struct rte_cryptodev_info *dev_info, uint16_t cdev_id,
 	return ret;
 }
 
+/* Check if the device is enabled by cryptodev_mask */
+static int
+check_cryptodev_mask(uint8_t cdev_id)
+{
+	if (enabled_cryptodev_mask & (1 << cdev_id))
+		return 0;
+
+	return -1;
+}
+
 static int32_t
 cryptodevs_init(void)
 {
@@ -1275,10 +1298,12 @@ cryptodevs_init(void)
 	}
 
 	idx = 0;
-	/* Start from last cdev id to give HW priority */
-	for (cdev_id = rte_cryptodev_count() - 1; cdev_id >= 0; cdev_id--) {
+	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
 		struct rte_cryptodev_info cdev_info;
 
+		if (check_cryptodev_mask((uint8_t)cdev_id))
+			continue;
+
 		rte_cryptodev_info_get(cdev_id, &cdev_info);
 
 		if (nb_lcore_params > cdev_info.max_nb_queue_pairs)
-- 
2.9.3

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: add cryptodev mask option

[De Lara Guarch, Pablo]

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, December 14, 2017 6:52 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> hemant.agrawal@nxp.com; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH] examples/ipsec-secgw: add cryptodev mask option
> 
> Previously, ipsec-secgw application did not give user the flexibility to decide
> which crypto device(s) will be used.
> 
> In this patch, a new cryptodev_mask option is added to the application.
> Same as portmask, the cryptodev_mask avails the user to mask out the
> unwanted crypto devices in the system.
> 
> This patch is similar to the support added in l2fwd-crypto
> (d2797f51cc63: examples/l2fwd-crypto: add cryptodev mask option)
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> ---

...

Not sure if you should change the order of the crypto devices that was set previously
(starting from the end and not from the beginning). Shouldn't we keep it as it was?

>  	idx = 0;
> -	/* Start from last cdev id to give HW priority */
> -	for (cdev_id = rte_cryptodev_count() - 1; cdev_id >= 0; cdev_id--) {
> +	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
>  		struct rte_cryptodev_info cdev_info;
> 
> +		if (check_cryptodev_mask((uint8_t)cdev_id))
> +			continue;
> +
>  		rte_cryptodev_info_get(cdev_id, &cdev_info);
> 
>  		if (nb_lcore_params > cdev_info.max_nb_queue_pairs)
> --
> 2.9.3

For the rest, I don't have other objections, so apart from the comment above:

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: add cryptodev mask option

[Akhil Goyal]

Hi Pablo,
On 1/10/2018 6:17 PM, De Lara Guarch, Pablo wrote:
> Hi Akhil,
> 
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Thursday, December 14, 2017 6:52 AM
>> To: dev@dpdk.org
>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> hemant.agrawal@nxp.com; Gonzalez Monroy, Sergio
>> <sergio.gonzalez.monroy@intel.com>; Nicolau, Radu
>> <radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
>> Subject: [PATCH] examples/ipsec-secgw: add cryptodev mask option
>>
>> Previously, ipsec-secgw application did not give user the flexibility to decide
>> which crypto device(s) will be used.
>>
>> In this patch, a new cryptodev_mask option is added to the application.
>> Same as portmask, the cryptodev_mask avails the user to mask out the
>> unwanted crypto devices in the system.
>>
>> This patch is similar to the support added in l2fwd-crypto
>> (d2797f51cc63: examples/l2fwd-crypto: add cryptodev mask option)
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>> ---
> 
> ...
> 
> Not sure if you should change the order of the crypto devices that was set previously
> (starting from the end and not from the beginning). Shouldn't we keep it as it was?
Actually as per the current code base there is no fix order of the 
devices to be available. In bus scan, all(pci,fslmc,vdev) have same 
priority(110), which means whatever is first recognized/linked will come 
first.

So the assumption that last cdev_id is HW doesn't seem to be correct.
I just wanted to make the code similar to l2fwd-crypto and the behavior 
of cryptodev_mask similar to what l2fwd-crypto understands.

Please let me know if my understanding is not correct.

> 
>>   	idx = 0;
>> -	/* Start from last cdev id to give HW priority */
>> -	for (cdev_id = rte_cryptodev_count() - 1; cdev_id >= 0; cdev_id--) {
>> +	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
>>   		struct rte_cryptodev_info cdev_info;
>>
>> +		if (check_cryptodev_mask((uint8_t)cdev_id))
>> +			continue;
>> +
>>   		rte_cryptodev_info_get(cdev_id, &cdev_info);
>>
>>   		if (nb_lcore_params > cdev_info.max_nb_queue_pairs)
>> --
>> 2.9.3
> 
> For the rest, I don't have other objections, so apart from the comment above:
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> 

Thanks,
Akhil

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: add cryptodev mask option

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Wednesday, January 10, 2018 2:22 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> dev@dpdk.org
> Cc: hemant.agrawal@nxp.com; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>
> Subject: Re: [PATCH] examples/ipsec-secgw: add cryptodev mask option
> 
> Hi Pablo,
> On 1/10/2018 6:17 PM, De Lara Guarch, Pablo wrote:
> > Hi Akhil,
> >
> >> -----Original Message-----
> >> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> >> Sent: Thursday, December 14, 2017 6:52 AM
> >> To: dev@dpdk.org
> >> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> >> hemant.agrawal@nxp.com; Gonzalez Monroy, Sergio
> >> <sergio.gonzalez.monroy@intel.com>; Nicolau, Radu
> >> <radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
> >> Subject: [PATCH] examples/ipsec-secgw: add cryptodev mask option
> >>
> >> Previously, ipsec-secgw application did not give user the flexibility
> >> to decide which crypto device(s) will be used.
> >>
> >> In this patch, a new cryptodev_mask option is added to the application.
> >> Same as portmask, the cryptodev_mask avails the user to mask out the
> >> unwanted crypto devices in the system.
> >>
> >> This patch is similar to the support added in l2fwd-crypto
> >> (d2797f51cc63: examples/l2fwd-crypto: add cryptodev mask option)
> >>
> >> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> >> ---
> >
> > ...
> >
> > Not sure if you should change the order of the crypto devices that was
> > set previously (starting from the end and not from the beginning).
> Shouldn't we keep it as it was?
> Actually as per the current code base there is no fix order of the devices to
> be available. In bus scan, all(pci,fslmc,vdev) have same priority(110), which
> means whatever is first recognized/linked will come first.
> 
> So the assumption that last cdev_id is HW doesn't seem to be correct.
> I just wanted to make the code similar to l2fwd-crypto and the behavior of
> cryptodev_mask similar to what l2fwd-crypto understands.
> 
> Please let me know if my understanding is not correct.

Right, actually I am seeing PCI devices first on my system, so clearly the statement below was wrong.

Looks ok to me then :)

Thanks,
Pablo

> 
> >
> >>   	idx = 0;
> >> -	/* Start from last cdev id to give HW priority */
> >> -	for (cdev_id = rte_cryptodev_count() - 1; cdev_id >= 0; cdev_id--) {
> >> +	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
> >>   		struct rte_cryptodev_info cdev_info;
> >>
> >> +		if (check_cryptodev_mask((uint8_t)cdev_id))
> >> +			continue;
> >> +
> >>   		rte_cryptodev_info_get(cdev_id, &cdev_info);
> >>
> >>   		if (nb_lcore_params > cdev_info.max_nb_queue_pairs)
> >> --
> >> 2.9.3
> >
> > For the rest, I don't have other objections, so apart from the comment
> above:
> >
> > Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >
> >
> 
> Thanks,
> Akhil

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: add cryptodev mask option

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, December 14, 2017 6:52 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> hemant.agrawal@nxp.com; Gonzalez Monroy, Sergio
> <sergio.gonzalez.monroy@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH] examples/ipsec-secgw: add cryptodev mask option
> 
> Previously, ipsec-secgw application did not give user the flexibility to decide
> which crypto device(s) will be used.
> 
> In this patch, a new cryptodev_mask option is added to the application.
> Same as portmask, the cryptodev_mask avails the user to mask out the
> unwanted crypto devices in the system.
> 
> This patch is similar to the support added in l2fwd-crypto
> (d2797f51cc63: examples/l2fwd-crypto: add cryptodev mask option)
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>

Applied to dpdk-next-crypto.
Thanks,

Pablo