Hi Pavan,
Thanks for the review, PSB
Thanks,
Ori
> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Pavan Nikhilesh Bhagavatula
>
> >This commit implements all the RegEx public API.
> >
> >Signed-off-by: Ori Kam <or...@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
