[dpdk-dev] [EXT] [PATCH v1 4/4] regexdev: implement regex rte level functions

[Ori Kam]

Hi Pavan,

Thanks for the review, PSB

Thanks,
Ori

> -----Original Message-----
> From: dev <dev-bounces at dpdk.org> On Behalf Of Pavan Nikhilesh Bhagavatula
> 
> >This commit implements all the RegEx public API.
> >
> >Signed-off-by: Ori Kam <orika at mellanox.com>
> >---
> > lib/librte_regexdev/rte_regexdev.c | 298
> >+++++++++++++++++++++++++++++++++++++
> > 1 file changed, 298 insertions(+)
> >
> >diff --git a/lib/librte_regexdev/rte_regexdev.c
> >b/lib/librte_regexdev/rte_regexdev.c
> >index 4396bb5..72f18fb 100644
> >--- a/lib/librte_regexdev/rte_regexdev.c
> >+++ b/lib/librte_regexdev/rte_regexdev.c
> >@@ -76,3 +76,301 @@
> > {
> > 	regex_devices[dev->dev_id] = NULL;
> > }
> >+
> 
> <snip>
> 
> >+
> >+int
> >+rte_regexdev_info_get(uint8_t dev_id, struct rte_regexdev_info
> >*dev_info)
> >+{
> >+	if (dev_id >= RTE_MAX_REGEXDEV_DEVS)
> >+		return -EINVAL;
> 
> We should use macro for this similar to ethdev/eventdev across the file.
> 
> RTE_ETH_VALID_PORTID_OR_ERR_RET
> RTE_FUNC_PTR_OR_ERR_RET
> 

O.K will move to macro.

> 
> >+	if (regex_devices[dev_id] == NULL)
> >+		return -EINVAL;
> >+	if (dev_info == NULL)
> >+		return -EINVAL;
> >+	if (regex_devices[dev_id]->dev_ops->dev_info_get == NULL)
> >+		return -ENOTSUP;
> >+	return regex_devices[dev_id]->dev_ops->dev_info_get
> >+		(regex_devices[dev_id], dev_info);
> >+}
> >+
> >+int
> >+rte_regexdev_configure(uint8_t dev_id, const struct
> >rte_regexdev_config *cfg)
> >+{
> >+	if (dev_id >= RTE_MAX_REGEXDEV_DEVS)
> >+		return -EINVAL;
> >+	if (regex_devices[dev_id] == NULL)
> >+		return -EINVAL;
> >+	if (cfg == NULL)
> >+		return -EINVAL;
> 
> Please handle re-configure cases, add error checks for cfg passed based on dev
> info.
> 

I don't think the checks that you suggest should be done in this level.
The RTE level isn't aware on the specific capabilities of the PMD.
I think it is the responsibility of the PMD to check. 

> >+	if (regex_devices[dev_id]->dev_ops->dev_configure == NULL)
> >+		return -ENOTSUP;
> >+	return regex_devices[dev_id]->dev_ops->dev_configure
> >+		(regex_devices[dev_id], cfg);
> >+}
> >+
> 
> <Snip>
> 
> >+
> >+uint16_t
> >+rte_regexdev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
> >+			   struct rte_regex_ops **ops, uint16_t nb_ops)
> >+{
> >+	return regex_devices[dev_id]-
> >>enqueue(regex_devices[dev_id], qp_id,
> >+					      ops, nb_ops);
> >+}
> 
> Move these functions to .h in-lining them.
> Also, please add debug checks @see rte_eth_rx_burst/rte_eth_tx_burst.
> 

O.K will update.

> >+
> >+uint16_t
> >+rte_regexdev_dequeue_burst(uint8_t dev_id, uint16_t qp_id,
> >+			   struct rte_regex_ops **ops, uint16_t nb_ops)
> >+{
> >+	return regex_devices[dev_id]-
> >>dequeue(regex_devices[dev_id], qp_id,
> >+					      ops, nb_ops);
> >+}
> >--
> >1.8.3.1