| Message ID | 20221216191134.1015946-1-david.m.ertman@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net] ice: Prevent set_channel from changing queues while RDMA active | expand |
Hi Dave,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/intel-lab-lkp/linux/commits/Dave-Ertman/ice-Prevent-set_channel-from-changing-queues-while-RDMA-active/20221217-031226
patch link: https://lore.kernel.org/r/20221216191134.1015946-1-david.m.ertman%40intel.com
patch subject: [Intel-wired-lan] [PATCH net] ice: Prevent set_channel from changing queues while RDMA active
config: i386-randconfig-a013
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/5893e01c4bf31b1a411a69180f5a08e554a53fd2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dave-Ertman/ice-Prevent-set_channel-from-changing-queues-while-RDMA-active/20221217-031226
git checkout 5893e01c4bf31b1a411a69180f5a08e554a53fd2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ethernet/intel/ice/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/net/ethernet/intel/ice/ice_ethtool.c:3710:19: error: member reference type 'struct device' is not a pointer; did you mean to use '.'?
if (pf->adev->dev->driver) {
~~~~~~~~~~~~~^~
.
>> drivers/net/ethernet/intel/ice/ice_ethtool.c:3727:15: error: incompatible pointer types passing 'struct auxiliary_device **' to parameter of type 'struct mutex *' [-Werror,-Wincompatible-pointer-types]
mutex_unlock(&pf->adev);
^~~~~~~~~
include/linux/mutex.h:218:40: note: passing argument to parameter 'lock' here
extern void mutex_unlock(struct mutex *lock);
^
2 errors generated.
vim +3710 drivers/net/ethernet/intel/ice/ice_ethtool.c
3632
3633 /**
3634 * ice_set_channels - set the number channels
3635 * @dev: network interface device structure
3636 * @ch: ethtool channel data structure
3637 */
3638 static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
3639 {
3640 struct ice_netdev_priv *np = netdev_priv(dev);
3641 struct ice_vsi *vsi = np->vsi;
3642 struct ice_pf *pf = vsi->back;
3643 int new_rx = 0, new_tx = 0;
3644 u32 curr_combined;
3645 int ret = 0;
3646
3647 /* do not support changing channels in Safe Mode */
3648 if (ice_is_safe_mode(pf)) {
3649 netdev_err(dev, "Changing channel in Safe Mode is not supported\n");
3650 return -EOPNOTSUPP;
3651 }
3652 /* do not support changing other_count */
3653 if (ch->other_count != (test_bit(ICE_FLAG_FD_ENA, pf->flags) ? 1U : 0U))
3654 return -EINVAL;
3655
3656 if (ice_is_adq_active(pf)) {
3657 netdev_err(dev, "Cannot set channels with ADQ configured.\n");
3658 return -EOPNOTSUPP;
3659 }
3660
3661 if (test_bit(ICE_FLAG_FD_ENA, pf->flags) && pf->hw.fdir_active_fltr) {
3662 netdev_err(dev, "Cannot set channels when Flow Director filters are active\n");
3663 return -EOPNOTSUPP;
3664 }
3665
3666 curr_combined = ice_get_combined_cnt(vsi);
3667
3668 /* these checks are for cases where user didn't specify a particular
3669 * value on cmd line but we get non-zero value anyway via
3670 * get_channels(); look at ethtool.c in ethtool repository (the user
3671 * space part), particularly, do_schannels() routine
3672 */
3673 if (ch->rx_count == vsi->num_rxq - curr_combined)
3674 ch->rx_count = 0;
3675 if (ch->tx_count == vsi->num_txq - curr_combined)
3676 ch->tx_count = 0;
3677 if (ch->combined_count == curr_combined)
3678 ch->combined_count = 0;
3679
3680 if (!(ch->combined_count || (ch->rx_count && ch->tx_count))) {
3681 netdev_err(dev, "Please specify at least 1 Rx and 1 Tx channel\n");
3682 return -EINVAL;
3683 }
3684
3685 new_rx = ch->combined_count + ch->rx_count;
3686 new_tx = ch->combined_count + ch->tx_count;
3687
3688 if (new_rx < vsi->tc_cfg.numtc) {
3689 netdev_err(dev, "Cannot set less Rx channels, than Traffic Classes you have (%u)\n",
3690 vsi->tc_cfg.numtc);
3691 return -EINVAL;
3692 }
3693 if (new_tx < vsi->tc_cfg.numtc) {
3694 netdev_err(dev, "Cannot set less Tx channels, than Traffic Classes you have (%u)\n",
3695 vsi->tc_cfg.numtc);
3696 return -EINVAL;
3697 }
3698 if (new_rx > ice_get_max_rxq(pf)) {
3699 netdev_err(dev, "Maximum allowed Rx channels is %d\n",
3700 ice_get_max_rxq(pf));
3701 return -EINVAL;
3702 }
3703 if (new_tx > ice_get_max_txq(pf)) {
3704 netdev_err(dev, "Maximum allowed Tx channels is %d\n",
3705 ice_get_max_txq(pf));
3706 return -EINVAL;
3707 }
3708
3709 mutex_lock(&pf->adev_mutex);
> 3710 if (pf->adev->dev->driver) {
3711 netdev_err(dev, "Cannot change channels when RDMA is active\n");
3712 ret = -EINVAL;
3713 goto adev_unlock;
3714 }
3715
3716 ice_vsi_recfg_qs(vsi, new_rx, new_tx);
3717
3718 if (!netif_is_rxfh_configured(dev)) {
3719 ret = ice_vsi_set_dflt_rss_lut(vsi, new_rx);
3720 goto adev_unlock;
3721 }
3722
3723 /* Update rss_size due to change in Rx queues */
3724 vsi->rss_size = ice_get_valid_rss_size(&pf->hw, new_rx);
3725
3726 adev_unlock:
> 3727 mutex_unlock(&pf->adev);
3728 return ret;
3729 }
3730
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c index b7be84bbe72d..79704fcc0cb5 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c @@ -3473,6 +3473,7 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch) struct ice_pf *pf = vsi->back; int new_rx = 0, new_tx = 0; u32 curr_combined; + int ret = 0; /* do not support changing channels in Safe Mode */ if (ice_is_safe_mode(pf)) { @@ -3536,15 +3537,26 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch) return -EINVAL; } + mutex_lock(&pf->adev_mutex); + if (pf->adev->dev->driver) { + netdev_err(dev, "Cannot change channels when RDMA is active\n"); + ret = -EINVAL; + goto adev_unlock; + } + ice_vsi_recfg_qs(vsi, new_rx, new_tx); - if (!netif_is_rxfh_configured(dev)) - return ice_vsi_set_dflt_rss_lut(vsi, new_rx); + if (!netif_is_rxfh_configured(dev)) { + ret = ice_vsi_set_dflt_rss_lut(vsi, new_rx); + goto adev_unlock; + } /* Update rss_size due to change in Rx queues */ vsi->rss_size = ice_get_valid_rss_size(&pf->hw, new_rx); - return 0; +adev_unlock: + mutex_unlock(&pf->adev); + return ret; } /**
The PF controls the set of queues that the RDMA auxiliary_driver requests resources from. The set_channel command will alter that pool and trigger a reconfiguration of the VSI, which breaks RDMA functionality. Prevent set_channel from executing when RDMA driver bound to auxiliary device. Signed-off-by: Dave Ertman <david.m.ertman@intel.com> --- drivers/net/ethernet/intel/ice/ice_ethtool.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)