[dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[Andrew Rybchenko]

By default RSS hash delivery (offload) is bound to RSS mode and
it is incorrect to advertise it as enabled if Rx multi-queue mode
has no RSS.

Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/bnxt/bnxt_ethdev.c       | 3 ++-
 drivers/net/cxgbe/cxgbe_ethdev.c     | 4 +++-
 drivers/net/e1000/igb_ethdev.c       | 6 ++++--
 drivers/net/enic/enic_ethdev.c       | 4 +++-
 drivers/net/fm10k/fm10k_ethdev.c     | 3 ++-
 drivers/net/hinic/hinic_pmd_ethdev.c | 3 ++-
 drivers/net/i40e/i40e_ethdev.c       | 3 ++-
 drivers/net/iavf/iavf_ethdev.c       | 3 ++-
 drivers/net/ice/ice_ethdev.c         | 3 ++-
 drivers/net/ixgbe/ixgbe_ethdev.c     | 6 ++++--
 drivers/net/liquidio/lio_ethdev.c    | 4 +++-
 drivers/net/mlx4/mlx4.c              | 3 ++-
 drivers/net/mlx5/mlx5_ethdev.c       | 3 ++-
 drivers/net/netvsc/hn_ethdev.c       | 3 ++-
 drivers/net/nfp/nfp_net.c            | 3 ++-
 drivers/net/qede/qede_ethdev.c       | 3 ++-
 drivers/net/sfc/sfc_rx.c             | 2 +-
 drivers/net/thunderx/nicvf_ethdev.c  | 3 ++-
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 3 ++-
 19 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 393706b800..06033e3b8f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -674,7 +674,8 @@ 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;
 
-	rx_offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (eth_dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		rx_offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 	eth_dev->data->dev_conf.rxmode.offloads = rx_offloads;
 
 	if (rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 3dc9351a13..51b63ef574 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -426,7 +426,9 @@ int cxgbe_dev_configure(struct rte_eth_dev *eth_dev)
 
 	CXGBE_FUNC_TRACE();
 
-	eth_dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (eth_dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		eth_dev->data->dev_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_RSS_HASH;
 
 	if (!(adapter->flags & FW_QUEUE_BOUND)) {
 		err = cxgbe_setup_sge_fwevtq(adapter);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 369a978adc..2d2dc9cbef 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1182,7 +1182,8 @@ eth_igb_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* multipe queue mode checking */
 	ret  = igb_check_mq_mode(dev);
@@ -3259,7 +3260,8 @@ igbvf_dev_configure(struct rte_eth_dev *dev)
 	PMD_INIT_LOG(DEBUG, "Configured Virtual Function port id: %d",
 		     dev->data->port_id);
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/*
 	 * VF has no ability to enable/disable HW CRC
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 683fed28b1..a7a178e41b 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -405,7 +405,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev)
 		return ret;
 	}
 
-	eth_dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (eth_dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		eth_dev->data->dev_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_RSS_HASH;
 
 	enic->mc_count = 0;
 	enic->hw_ip_checksum = !!(eth_dev->data->dev_conf.rxmode.offloads &
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 3114825220..407baa16c3 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -461,7 +461,8 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* multipe queue mode checking */
 	ret  = fm10k_check_mq_mode(dev);
diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c b/drivers/net/hinic/hinic_pmd_ethdev.c
index 072fec3398..ddfe082142 100644
--- a/drivers/net/hinic/hinic_pmd_ethdev.c
+++ b/drivers/net/hinic/hinic_pmd_ethdev.c
@@ -318,7 +318,8 @@ static int hinic_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* mtu size is 256~9600 */
 	if (dev->data->dev_conf.rxmode.max_rx_pkt_len < HINIC_MIN_FRAME_SIZE ||
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 9a81f995d8..ca668dd8f9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1813,7 +1813,8 @@ i40e_dev_configure(struct rte_eth_dev *dev)
 	ad->tx_simple_allowed = true;
 	ad->tx_vec_allowed = true;
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* Only legacy filter API needs the following fdir config. So when the
 	 * legacy filter API is deprecated, the following codes should also be
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index c23a8fc675..a39ba1466c 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -147,7 +147,8 @@ iavf_dev_configure(struct rte_eth_dev *dev)
 	ad->rx_vec_allowed = true;
 	ad->tx_vec_allowed = true;
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* Vlan stripping setting */
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN) {
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 7fd4bfe082..837a26fd9d 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -2445,7 +2445,8 @@ ice_dev_configure(struct rte_eth_dev *dev)
 	ad->rx_bulk_alloc_allowed = true;
 	ad->tx_simple_allowed = true;
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	return 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 797f321e0e..0084c3335c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2405,7 +2405,8 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* multipe queue mode checking */
 	ret  = ixgbe_check_mq_mode(dev);
@@ -5160,7 +5161,8 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
 	PMD_INIT_LOG(DEBUG, "Configured Virtual Function port id: %d",
 		     dev->data->port_id);
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/*
 	 * VF has no ability to enable/disable HW CRC
diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
index cdf979edbf..ad4a51ecda 100644
--- a/drivers/net/liquidio/lio_ethdev.c
+++ b/drivers/net/liquidio/lio_ethdev.c
@@ -1736,7 +1736,9 @@ lio_dev_configure(struct rte_eth_dev *eth_dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	eth_dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (eth_dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		eth_dev->data->dev_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* Inform firmware about change in number of queues to use.
 	 * Disable IO queues and reset registers for re-configuration.
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 9b5c5477da..ab5e6c66cb 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -248,7 +248,8 @@ mlx4_dev_configure(struct rte_eth_dev *dev)
 	struct rte_flow_error error;
 	int ret;
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* Prepare internal flow rules. */
 	ret = mlx4_flow_sync(priv, &error);
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 11efb2d8ad..1d6c084119 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -405,7 +405,8 @@ mlx5_dev_configure(struct rte_eth_dev *dev)
 		return -rte_errno;
 	}
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	memcpy(priv->rss_conf.rss_key,
 	       use_app_rss_key ?
diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 6589355090..164e9ad174 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -532,7 +532,8 @@ static int hn_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	dev_conf->rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev_conf->rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	unsupported = txmode->offloads & ~HN_TX_OFFLOAD_CAPS;
 	if (unsupported) {
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 7941248c01..450875ce2f 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -407,7 +407,8 @@ nfp_net_configure(struct rte_eth_dev *dev)
 	rxmode = &dev_conf->rxmode;
 	txmode = &dev_conf->txmode;
 
-	rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (rxmode->mq_mode & ETH_MQ_RX_RSS_FLAG)
+		rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* Checking TX mode */
 	if (txmode->mq_mode) {
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 853748afed..19d2e96191 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1186,7 +1186,8 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
 
 	PMD_INIT_FUNC_TRACE(edev);
 
-	rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (rxmode->mq_mode & ETH_MQ_RX_RSS_FLAG)
+		rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	/* We need to have min 1 RX queue.There is no min check in
 	 * rte_eth_dev_configure(), so we are checking it here.
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index 10fa1e5ff3..74218296cd 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -1558,7 +1558,7 @@ sfc_rx_check_mode(struct sfc_adapter *sa, struct rte_eth_rxmode *rxmode)
 	}
 
 	if ((offloads_supported & DEV_RX_OFFLOAD_RSS_HASH) &&
-	    (~rxmode->offloads & DEV_RX_OFFLOAD_RSS_HASH))
+	    (rxmode->mq_mode & ETH_MQ_RX_RSS_FLAG))
 		rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	return rc;
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index 062eec3f25..2cf0ffe13b 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -1920,7 +1920,8 @@ nicvf_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (rxmode->mq_mode & ETH_MQ_RX_RSS_FLAG)
+		rxmode->offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	if (!rte_eal_has_hugepages()) {
 		PMD_INIT_LOG(INFO, "Huge page is not configured");
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 53fbd2bd03..6e6efa9603 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -408,7 +408,8 @@ vmxnet3_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+	if (dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
+		dev->data->dev_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
 
 	if (dev->data->nb_tx_queues > VMXNET3_MAX_TX_QUEUES ||
 	    dev->data->nb_rx_queues > VMXNET3_MAX_RX_QUEUES) {
-- 
2.17.1

[dpdk-dev] [PATCH 2/3] ethdev: decrease verbosity of not disabled offload logs

[Andrew Rybchenko]

Right now a PMD decides if it is critical that an offload cannot
be disabled (i.e. not requested, but still enabled). If PMD treaks
it as OK, we should not spam logs with corresponding messsages
by default. Default log level in ethdev is INFO, so change the
message level to DEBUG.

Fixes: 1daa33805824 ("ethdev: validate offloads set by PMD")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index c43f9f5d14..19ca08b04a 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1204,7 +1204,7 @@ validate_offloads(uint16_t port_id, uint64_t req_offloads,
 
 		/* Chech if offload couldn't be disabled. */
 		if (offload & set_offloads) {
-			RTE_ETHDEV_LOG(INFO,
+			RTE_ETHDEV_LOG(DEBUG,
 				"Port %u failed to disable %s offload %s\n",
 				port_id, offload_type, offload_name(offload));
 		}
-- 
2.17.1

[dpdk-dev] [PATCH 3/3] ethdev: improve message about not disabled offload

[Andrew Rybchenko]

Avoid usaged of "failed" in the message about not requested but
enabled offload, since it is not a failure.

Fixes: 1daa33805824 ("ethdev: validate offloads set by PMD")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 19ca08b04a..76d4a20ed3 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1205,7 +1205,7 @@ validate_offloads(uint16_t port_id, uint64_t req_offloads,
 		/* Chech if offload couldn't be disabled. */
 		if (offload & set_offloads) {
 			RTE_ETHDEV_LOG(DEBUG,
-				"Port %u failed to disable %s offload %s\n",
+				"Port %u %s offload %s is not requested but enabled\n",
 				port_id, offload_type, offload_name(offload));
 		}
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH 2/3] ethdev: decrease verbosity of not disabled offload logs

[Ferruh Yigit]

On 11/14/2019 4:40 PM, Andrew Rybchenko wrote:
> Right now a PMD decides if it is critical that an offload cannot
> be disabled (i.e. not requested, but still enabled). If PMD treaks
> it as OK, we should not spam logs with corresponding messsages
> by default. Default log level in ethdev is INFO, so change the
> message level to DEBUG.
> 
> Fixes: 1daa33805824 ("ethdev: validate offloads set by PMD")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH 3/3] ethdev: improve message about not disabled offload

[Ferruh Yigit]

On 11/14/2019 4:40 PM, Andrew Rybchenko wrote:
> Avoid usaged of "failed" in the message about not requested but
> enabled offload, since it is not a failure.
> 
> Fixes: 1daa33805824 ("ethdev: validate offloads set by PMD")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[Stephen Hemminger]

On Thu, 14 Nov 2019 16:40:50 +0000
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> By default RSS hash delivery (offload) is bound to RSS mode and
> it is incorrect to advertise it as enabled if Rx multi-queue mode
> has no RSS.
> 
> Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

If you have to change so many drivers, why not just handle it in
common rte_ethdev code?

Re: [dpdk-dev] [PATCH 2/3] ethdev: decrease verbosity of not disabled offload logs

[Stephen Hemminger]

On Thu, 14 Nov 2019 16:40:51 +0000
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> Right now a PMD decides if it is critical that an offload cannot
> be disabled (i.e. not requested, but still enabled). If PMD treaks
> it as OK, we should not spam logs with corresponding messsages
> by default. Default log level in ethdev is INFO, so change the
> message level to DEBUG.
> 
> Fixes: 1daa33805824 ("ethdev: validate offloads set by PMD")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [dpdk-dev] [PATCH 3/3] ethdev: improve message about not disabled offload

[Stephen Hemminger]

On Thu, 14 Nov 2019 16:40:52 +0000
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> Avoid usaged of "failed" in the message about not requested but
> enabled offload, since it is not a failure.
> 
> Fixes: 1daa33805824 ("ethdev: validate offloads set by PMD")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[Andrew Rybchenko]

Hi Stephen,

On 11/14/19 7:56 PM, Stephen Hemminger wrote:
> On Thu, 14 Nov 2019 16:40:50 +0000
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
>> By default RSS hash delivery (offload) is bound to RSS mode and
>> it is incorrect to advertise it as enabled if Rx multi-queue mode
>> has no RSS.
>>
>> Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> If you have to change so many drivers, why not just handle it in
> common rte_ethdev code?

rte_ethdev cannot set the offload itself since it has no knowledge that
the offload cannot be disabled.
rte_ethdev can blacklist the automatically set offloads in the case of
non-RSS Rx multi-queue mode, but I don't like it since it adds to many
layers where we change offloads.
That's why I prefer this way.

Andrew.

Re: [dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[Ferruh Yigit]

On 11/15/2019 9:41 AM, Andrew Rybchenko wrote:
> Hi Stephen,
> 
> On 11/14/19 7:56 PM, Stephen Hemminger wrote:
>> On Thu, 14 Nov 2019 16:40:50 +0000
>> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>>
>>> By default RSS hash delivery (offload) is bound to RSS mode and
>>> it is incorrect to advertise it as enabled if Rx multi-queue mode
>>> has no RSS.
>>>
>>> Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")
>>>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>
>> If you have to change so many drivers, why not just handle it in
>> common rte_ethdev code?
> 
> rte_ethdev cannot set the offload itself since it has no knowledge that
> the offload cannot be disabled.
> rte_ethdev can blacklist the automatically set offloads in the case of
> non-RSS Rx multi-queue mode, but I don't like it since it adds to many
> layers where we change offloads.

+1

> That's why I prefer this way.

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[Ferruh Yigit]

On 11/14/2019 4:40 PM, Andrew Rybchenko wrote:
> By default RSS hash delivery (offload) is bound to RSS mode and
> it is incorrect to advertise it as enabled if Rx multi-queue mode
> has no RSS.
> 
> Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[Stephen Hemminger]

On Fri, 15 Nov 2019 12:41:16 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> Hi Stephen,
> 
> On 11/14/19 7:56 PM, Stephen Hemminger wrote:
> > On Thu, 14 Nov 2019 16:40:50 +0000
> > Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> >   
> >> By default RSS hash delivery (offload) is bound to RSS mode and
> >> it is incorrect to advertise it as enabled if Rx multi-queue mode
> >> has no RSS.
> >>
> >> Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")
> >>
> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>  
> > 
> > If you have to change so many drivers, why not just handle it in
> > common rte_ethdev code?  
> 
> rte_ethdev cannot set the offload itself since it has no knowledge that
> the offload cannot be disabled.
> rte_ethdev can blacklist the automatically set offloads in the case of
> non-RSS Rx multi-queue mode, but I don't like it since it adds to many
> layers where we change offloads.
> That's why I prefer this way.
> 
> Andrew.

Makes sense, just concerned that other (or new) drivers will
have same issue.

Re: [dpdk-dev] [PATCH 1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only

[Andrew Rybchenko]

On 11/15/19 7:35 PM, Stephen Hemminger wrote:
> On Fri, 15 Nov 2019 12:41:16 +0300
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>
>> Hi Stephen,
>>
>> On 11/14/19 7:56 PM, Stephen Hemminger wrote:
>>> On Thu, 14 Nov 2019 16:40:50 +0000
>>> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>>>    
>>>> By default RSS hash delivery (offload) is bound to RSS mode and
>>>> it is incorrect to advertise it as enabled if Rx multi-queue mode
>>>> has no RSS.
>>>>
>>>> Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")
>>>>
>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> If you have to change so many drivers, why not just handle it in
>>> common rte_ethdev code?
>> rte_ethdev cannot set the offload itself since it has no knowledge that
>> the offload cannot be disabled.
>> rte_ethdev can blacklist the automatically set offloads in the case of
>> non-RSS Rx multi-queue mode, but I don't like it since it adds to many
>> layers where we change offloads.
>> That's why I prefer this way.
>>
>> Andrew.
> Makes sense, just concerned that other (or new) drivers will
> have same issue.

Yes, that's true, we should try to find a better solution, but I don't
see any better solution right now. I'll keep it in my head.