Hi Pavan,

> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Pavan Nikhilesh Bhagavatula
> 
> >> >+
> >> >+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.
> 
> PMD capabilities are standardized through dev_info.
> All the PMD capabilities needs to be exposed to RTE level through dev_info 
> else
> how would an application using rte_regexdev would know the capabilities of
> the driver.
> 

The capabilities are exposed to the application using rte_regexdev_info_get.

> >I think it is the responsibility of the PMD to check.
> 
> The checks would be same for all the pmds which would just be unnecessary
> code repetition.
> Instead RTE layer should probe dev_info and compare against dev_configure.
> 

I accept your comment I will add the missing checks, and re-configuration cases.

> >
> >> >+ 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

Reply via email to