>On 10/25/19 5:33 PM, pbhagavat...@marvell.com wrote:
>> From: Pavan Nikhilesh <pbhagavat...@marvell.com>
>>
>> Add DEV_RX_OFFLOAD_RSS_HASH flag for all PMDs that support RSS
>hash
>> delivery.
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavat...@marvell.com>
>> Reviewed-by: Andrew Rybchenko <arybche...@solarflare.com>
>> Reviewed-by: Hemant Agrawal <hemant.agra...@nxp.com>
>> Acked-by: Jerin Jacob <jer...@marvell.com>
>> Acked-by: Ajit Khaparde <ajit.khapa...@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.
>> + rx_offloads |= DEV_RX_OFFLOAD_RSS_HASH;
>> + eth_dev->data->dev_conf.rxmode.offloads =
>rx_offloads;
>> + }
>> +
>> if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
>> eth_dev->data->mtu =
>> eth_dev->data-
>>dev_conf.rxmode.max_rx_pkt_len -
>
>[snip]
>
>> diff --git a/drivers/net/sfc/sfc_ef10_essb_rx.c
>b/drivers/net/sfc/sfc_ef10_essb_rx.c
>> index 63da807ea..220ef0e47 100644
>> --- a/drivers/net/sfc/sfc_ef10_essb_rx.c
>> +++ b/drivers/net/sfc/sfc_ef10_essb_rx.c
>> @@ -716,7 +716,7 @@ struct sfc_dp_rx sfc_ef10_essb_rx = {
>> .features = SFC_DP_RX_FEAT_FLOW_FLAG |
>> SFC_DP_RX_FEAT_FLOW_MARK,
>> .dev_offload_capa = DEV_RX_OFFLOAD_CHECKSUM,
>> - .queue_offload_capa = 0,
>> + .queue_offload_capa = DEV_RX_OFFLOAD_RSS_HASH,
>
>Please, move it dev_offload_capa to be sure that it cannot
>be requested on queue level and no checks are required
>on queue level that the offload cannot be disabled.
>We'll move to queue level when/if it is really supported on queue level.
>
Sure will fix all sfc related comments in v14.
>> .get_dev_info = sfc_ef10_essb_rx_get_dev_info,
>> .pool_ops_supported =
>sfc_ef10_essb_rx_pool_ops_supported,
>> .qsize_up_rings =
>sfc_ef10_essb_rx_qsize_up_rings,
>> diff --git a/drivers/net/sfc/sfc_ef10_rx.c
>b/drivers/net/sfc/sfc_ef10_rx.c
>> index f2fc6e70a..85b5df466 100644
>> --- a/drivers/net/sfc/sfc_ef10_rx.c
>> +++ b/drivers/net/sfc/sfc_ef10_rx.c
>> @@ -797,7 +797,8 @@ struct sfc_dp_rx sfc_ef10_rx = {
>> SFC_DP_RX_FEAT_INTR,
>> .dev_offload_capa = DEV_RX_OFFLOAD_CHECKSUM |
>>
>DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM,
>> - .queue_offload_capa = DEV_RX_OFFLOAD_SCATTER,
>> + .queue_offload_capa = DEV_RX_OFFLOAD_SCATTER |
>> + DEV_RX_OFFLOAD_RSS_HASH,
>
>Same here
>
>> .get_dev_info = sfc_ef10_rx_get_dev_info,
>> .qsize_up_rings = sfc_ef10_rx_qsize_up_rings,
>> .qcreate = sfc_ef10_rx_qcreate,
>> diff --git a/drivers/net/sfc/sfc_ethdev.c
>b/drivers/net/sfc/sfc_ethdev.c
>> index 454b8956a..403711ca0 100644
>> --- a/drivers/net/sfc/sfc_ethdev.c
>> +++ b/drivers/net/sfc/sfc_ethdev.c
>> @@ -206,6 +206,11 @@ sfc_dev_configure(struct rte_eth_dev *dev)
>> sfc_log_init(sa, "entry n_rxq=%u n_txq=%u",
>> dev_data->nb_rx_queues, dev_data-
>>nb_tx_queues);
>>
>> + if (!(dev_data->dev_conf.rxmode.offloads &
>DEV_RX_OFFLOAD_RSS_HASH)) {
>> + sfc_info(sa, "RX_OFFLOAD_RSS_HASH cannot be
>disabled");
>> + dev_data->dev_conf.rxmode.offloads |=
>DEV_RX_OFFLOAD_RSS_HASH;
>> + }
>> +
>
>It should be in drivers/net/sfc/sfc_rx.c sfc_rx_check_mode() close to
>the
>end of the function and similar to DEV_RX_OFFLOAD_CHECKSUM.
>
>> sfc_adapter_lock(sa);
>> switch (sa->state) {
>> case SFC_ADAPTER_CONFIGURED:
>> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
>> index e6809bb64..695580b22 100644
>> --- a/drivers/net/sfc/sfc_rx.c
>> +++ b/drivers/net/sfc/sfc_rx.c
>> @@ -618,7 +618,8 @@ struct sfc_dp_rx sfc_efx_rx = {
>> },
>> .features = SFC_DP_RX_FEAT_INTR,
>> .dev_offload_capa = DEV_RX_OFFLOAD_CHECKSUM,
>> - .queue_offload_capa = DEV_RX_OFFLOAD_SCATTER,
>> + .queue_offload_capa = DEV_RX_OFFLOAD_SCATTER |
>> + DEV_RX_OFFLOAD_RSS_HASH,
>
>Please, move to dev_offload_capa.
>
>> .qsize_up_rings = sfc_efx_rx_qsize_up_rings,
>> .qcreate = sfc_efx_rx_qcreate,
>> .qdestroy = sfc_efx_rx_qdestroy,
>
>[snip]
