[1/3] drivers/net: fix RSS hash offload set if Rx mode is RSS only
Checks
Commit Message
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(-)
Comments
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?
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.
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>
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.
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.
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.
@@ -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) {
@@ -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);
@@ -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
@@ -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 &
@@ -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);
@@ -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 ||
@@ -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
@@ -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) {
@@ -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;
}
@@ -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
@@ -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.
@@ -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);
@@ -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 ?
@@ -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) {
@@ -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) {
@@ -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.
@@ -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;
@@ -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");
@@ -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) {