Re: [dpdk-dev] [PATCH v13 3/6] drivers/net: update Rx RSS hash offload capabilities

[Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>]

>On 10/25/19 7:10 PM, Pavan Nikhilesh Bhagavatula wrote:
>>> On 10/25/19 5:33 PM, pbhagavatula@marvell.com wrote:
>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>
>>>> Add DEV_RX_OFFLOAD_RSS_HASH flag for all PMDs that support
>RSS
>>> hash
>>>> delivery.
>>>>
>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> Reviewed-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> [snip]
>>>
>>>> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
>>> b/drivers/net/bnxt/bnxt_ethdev.c
>>>> index e7ec99e15..d4f8cc92a 100644
>>>> --- a/drivers/net/bnxt/bnxt_ethdev.c
>>>> +++ b/drivers/net/bnxt/bnxt_ethdev.c
>>>> @@ -117,7 +117,8 @@ static const struct rte_pci_id
>>> bnxt_pci_id_map[] = {
>>>>    				     DEV_RX_OFFLOAD_KEEP_CRC | \
>>>>    				     DEV_RX_OFFLOAD_VLAN_EXTEND |
>>> \
>>>>    				     DEV_RX_OFFLOAD_TCP_LRO | \
>>>> -				     DEV_RX_OFFLOAD_SCATTER)
>>>> +				     DEV_RX_OFFLOAD_SCATTER | \
>>>> +				     DEV_RX_OFFLOAD_RSS_HASH)
>>>>
>>>>    static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int
>>> mask);
>>>>    static void bnxt_print_link_info(struct rte_eth_dev *eth_dev);
>>>> @@ -681,6 +682,12 @@ static int bnxt_dev_configure_op(struct
>>> rte_eth_dev *eth_dev)
>>>>    	bp->rx_cp_nr_rings = bp->rx_nr_rings;
>>>>    	bp->tx_cp_nr_rings = bp->tx_nr_rings;
>>>>
>>>> +	if (!(rx_offloads & DEV_RX_OFFLOAD_RSS_HASH)) {
>>>> +		PMD_DRV_LOG(INFO, "RX_OFFLOAD_RSS_HASH
>>> cannot be disabled\n");
>>>
>>> Shouldn't logging be done from rte_eth_dev_configure()?
>>> I.e. a generic function which is called after dev_configure callback
>and
>>> take a look at dev_conf->rx_mode.offloads and
>>> dev->data->dev_conf.rxmode.offloads and for each bit which
>differs
>>> log message using rte_eth_dev_rx_offload_name().
>>> Same for Tx while we are on the page. I.e. two more patch just
>before
>>> this one.
>>>
>> Just to be clear this log would effect all offloads which can't be
>disabled for
>> a give PMD.
>
>I think it is perfectly fine and it is my goal in fact.

Ack let me roll up v14.

>
>Thanks,
>Andrew.

Thanks,
Pavan.