Re: [dpdk-dev] [PATCH v14 4/7] drivers/net: update Rx RSS hash offload capabilities

[Andrew Rybchenko]

On 10/29/19 8:03 AM, 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/sfc/sfc_ef10_essb_rx.c 
b/drivers/net/sfc/sfc_ef10_essb_rx.c
index 63da807ea..d9d2ce6bd 100644
--- a/drivers/net/sfc/sfc_ef10_essb_rx.c
+++ b/drivers/net/sfc/sfc_ef10_essb_rx.c
@@ -715,7 +715,8 @@ 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,
+       .dev_offload_capa       = DEV_RX_OFFLOAD_CHECKSUM |
+                                 DEV_RX_OFFLOAD_RSS_HASH,
        .queue_offload_capa     = 0,
        .get_dev_info           = sfc_ef10_essb_rx_get_dev_info,
        .pool_ops_supported     = sfc_ef10_essb_rx_pool_ops_supported,
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index f2fc6e70a..9e527b7fb 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -796,7 +796,8 @@ struct sfc_dp_rx sfc_ef10_rx = {
        .features               = SFC_DP_RX_FEAT_MULTI_PROCESS |
                                  SFC_DP_RX_FEAT_INTR,
        .dev_offload_capa       = DEV_RX_OFFLOAD_CHECKSUM |
-                                 DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM,
+                                 DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
+                                 DEV_RX_OFFLOAD_RSS_HASH,
        .queue_offload_capa     = DEV_RX_OFFLOAD_SCATTER,
        .get_dev_info           = sfc_ef10_rx_get_dev_info,
        .qsize_up_rings         = sfc_ef10_rx_qsize_up_rings,
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index e6809bb64..4af8f1977 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -617,7 +617,8 @@ struct sfc_dp_rx sfc_efx_rx = {
                .hw_fw_caps     = 0,
        },
        .features               = SFC_DP_RX_FEAT_INTR,
-       .dev_offload_capa       = DEV_RX_OFFLOAD_CHECKSUM,
+       .dev_offload_capa       = DEV_RX_OFFLOAD_CHECKSUM |
+                                 DEV_RX_OFFLOAD_RSS_HASH,
        .queue_offload_capa     = DEV_RX_OFFLOAD_SCATTER,
        .qsize_up_rings         = sfc_efx_rx_qsize_up_rings,
        .qcreate                = sfc_efx_rx_qcreate,

Thanks  a lot, above looks good to me now.

@@ -1556,6 +1557,9 @@ sfc_rx_check_mode(struct sfc_adapter *sa, struct 
rte_eth_rxmode *rxmode)
                rxmode->offloads |= DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
        }

+ if (!(rxmode->offloads & DEV_RX_OFFLOAD_RSS_HASH))

+               rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+

I've tried to highlight on my previous review notes, but it looks like not
that successful. The right condition should check that offload is really
supported:

if ((offloads_supported & DEV_RX_OFFLOAD_RSS_HASH) &&
    (~rxmode->offloads & DEV_RX_OFFLOAD_RSS_HASH))
          rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;

Basically similar to DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM above
(including style of disabled offload check just to have common style).

Thanks.

        return rc;
  }

[snip]