On 7/6/21 10:53 AM, Jiawen Wu wrote:
> On July 5, 2021 5:08 PM, Andrew Rybchenko wrote:
>> On 7/5/21 11:36 AM, Jiawen Wu wrote:
>>> On July 3, 2021 12:36 AM, Andrew Rybchenko wrote:
>>>> On 6/17/21 1:59 PM, Jiawen Wu wrote:
>>>>> Setup device Rx queue and release Rx queue.
>>>>>
>>>>> Signed-off-by: Jiawen Wu <jiawe...@trustnetic.com>
>>>>> ---
>>>>> drivers/net/ngbe/meson.build | 1 +
>>>>> drivers/net/ngbe/ngbe_ethdev.c | 37 +++-
>>>>> drivers/net/ngbe/ngbe_ethdev.h | 16 ++
>>>>> drivers/net/ngbe/ngbe_rxtx.c | 308 +++++++++++++++++++++++++++++++++
>>>>> drivers/net/ngbe/ngbe_rxtx.h | 96 ++++++++++
>>>>> 5 files changed, 457 insertions(+), 1 deletion(-) create mode
>>>>> 100644 drivers/net/ngbe/ngbe_rxtx.c create mode 100644
>>>>> drivers/net/ngbe/ngbe_rxtx.h
>>>>>
>>>>> a/drivers/net/ngbe/ngbe_ethdev.c b/drivers/net/ngbe/ngbe_ethdev.c
>>>>> index c952023e8b..e73606c5f3 100644
>>>>> --- a/drivers/net/ngbe/ngbe_ethdev.c
>>>>> +++ b/drivers/net/ngbe/ngbe_ethdev.c
>>>>> @@ -37,6 +38,12 @@ static const struct rte_pci_id pci_id_ngbe_map[] = {
>>>>> { .vendor_id = 0, /* sentinel */ }, };
>>>>>
>>>>> +static const struct rte_eth_desc_lim rx_desc_lim = {
>>>>> + .nb_max = NGBE_RING_DESC_MAX,
>>>>> + .nb_min = NGBE_RING_DESC_MIN,
>>>>> + .nb_align = NGBE_RXD_ALIGN,
>>>>> +};
>>>>> +
>>>>> static const struct eth_dev_ops ngbe_eth_dev_ops;
>>>>>
>>>>> static inline void
>>>>> @@ -266,11 +280,30 @@ ngbe_dev_close(struct rte_eth_dev *dev)
>>>>> static int ngbe_dev_info_get(struct rte_eth_dev *dev, struct
>>>>> rte_eth_dev_info *dev_info) {
>>>>> - RTE_SET_USED(dev);
>>>>> + struct ngbe_hw *hw = ngbe_dev_hw(dev);
>>>>> +
>>>>> + dev_info->max_rx_queues = (uint16_t)hw->mac.max_rx_queues;
>>>>> +
>>>>> + dev_info->default_rxconf = (struct rte_eth_rxconf) {
>>>>> + .rx_thresh = {
>>>>> + .pthresh = NGBE_DEFAULT_RX_PTHRESH,
>>>>> + .hthresh = NGBE_DEFAULT_RX_HTHRESH,
>>>>> + .wthresh = NGBE_DEFAULT_RX_WTHRESH,
>>>>> + },
>>>>> + .rx_free_thresh = NGBE_DEFAULT_RX_FREE_THRESH,
>>>>> + .rx_drop_en = 0,
>>>>> + .offloads = 0,
>>>>> + };
>>>>> +
>>>>> + dev_info->rx_desc_lim = rx_desc_lim;
>>>>>
> <...>
>>>>> +int __rte_cold
>>>>> +ngbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>>>> + uint16_t queue_idx,
>>>>> + uint16_t nb_desc,
>>>>> + unsigned int socket_id,
>>>>> + const struct rte_eth_rxconf *rx_conf,
>>>>> + struct rte_mempool *mp)
>>>>> +{
>>>>> + const struct rte_memzone *rz;
>>>>> + struct ngbe_rx_queue *rxq;
>>>>> + struct ngbe_hw *hw;
>>>>> + uint16_t len;
>>>>> + struct ngbe_adapter *adapter = ngbe_dev_adapter(dev);
>>>>> +
>>>>> + PMD_INIT_FUNC_TRACE();
>>>>> + hw = ngbe_dev_hw(dev);
>>>>> +
>>>>> + /*
>>>>> + * Validate number of receive descriptors.
>>>>> + * It must not exceed hardware maximum, and must be multiple
>>>>> + * of NGBE_ALIGN.
>>>>> + */
>>>>> + if (nb_desc % NGBE_RXD_ALIGN != 0 ||
>>>>> + nb_desc > NGBE_RING_DESC_MAX ||
>>>>> + nb_desc < NGBE_RING_DESC_MIN) {
>>>>> + return -EINVAL;
>>>>> + }
>>>>
>>>> rte_eth_rx_queue_setup cares about it
>>>>
>>>
>>> I don't quite understand.
>>
>> ethdev does the check based dev_info provided by the driver:
>>
>> if (nb_rx_desc > dev_info.rx_desc_lim.nb_max ||
>> nb_rx_desc < dev_info.rx_desc_lim.nb_min ||
>> nb_rx_desc % dev_info.rx_desc_lim.nb_align != 0) {
>> RTE_ETHDEV_LOG(ERR,
>> "Invalid value for nb_rx_desc(=%hu), should be: <= %hu, >= %hu,
>> and a product of %hu\n",
>> nb_rx_desc, dev_info.rx_desc_lim.nb_max,
>> dev_info.rx_desc_lim.nb_min,
>> dev_info.rx_desc_lim.nb_align);
>> return -EINVAL;
>> }
>
> 'dev_info.rx_desc_lim' was set up in function 'ngbe_dev_info_get' with the
> struct 'rx_desc_lim' defined.
> So I think it is appropriate to check with macros. However, the log can be
Sorry, but I don't understand why do you want to duplicate
check which is already done in a generic code.
