Skip to content

Commit 05d6f44

Browse files
walking-machinePaolo Abeni
authored andcommitted
ice: Interpret .set_channels() input differently
A bug occurs because a safety check guarding AF_XDP-related queues in ethnl_set_channels(), does not trigger. This happens, because kernel and ice driver interpret the ethtool command differently. How the bug occurs: 1. ethtool -l <IFNAME> -> combined: 40 2. Attach AF_XDP to queue 30 3. ethtool -L <IFNAME> rx 15 tx 15 combined number is not specified, so command becomes {rx_count = 15, tx_count = 15, combined_count = 40}. 4. ethnl_set_channels checks, if there are any AF_XDP of queues from the new (combined_count + rx_count) to the old one, so from 55 to 40, check does not trigger. 5. ice interprets `rx 15 tx 15` as 15 combined channels and deletes the queue that AF_XDP is attached to. Interpret the command in a way that is more consistent with ethtool manual [0] (--show-channels and --set-channels). Considering that in the ice driver only the difference between RX and TX queues forms dedicated channels, change the correct way to set number of channels to: ethtool -L <IFNAME> combined 10 /* For symmetric queues */ ethtool -L <IFNAME> combined 8 tx 2 rx 0 /* For asymmetric queues */ [0] https://man7.org/linux/man-pages/man8/ethtool.8.html Fixes: 87324e7 ("ice: Implement ethtool ops for channels") Reviewed-by: Michal Swiatkowski <[email protected]> Signed-off-by: Larysa Zaremba <[email protected]> Tested-by: Chandan Kumar Rout <[email protected]> Tested-by: Pucha Himasekhar Reddy <[email protected]> Acked-by: Maciej Fijalkowski <[email protected]> Signed-off-by: Jacob Keller <[email protected]> Signed-off-by: Paolo Abeni <[email protected]>
1 parent 6671e35 commit 05d6f44

File tree

1 file changed

+2
-17
lines changed

1 file changed

+2
-17
lines changed

drivers/net/ethernet/intel/ice/ice_ethtool.c

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3593,7 +3593,6 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
35933593
struct ice_pf *pf = vsi->back;
35943594
int new_rx = 0, new_tx = 0;
35953595
bool locked = false;
3596-
u32 curr_combined;
35973596
int ret = 0;
35983597

35993598
/* do not support changing channels in Safe Mode */
@@ -3615,22 +3614,8 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch)
36153614
return -EOPNOTSUPP;
36163615
}
36173616

3618-
curr_combined = ice_get_combined_cnt(vsi);
3619-
3620-
/* these checks are for cases where user didn't specify a particular
3621-
* value on cmd line but we get non-zero value anyway via
3622-
* get_channels(); look at ethtool.c in ethtool repository (the user
3623-
* space part), particularly, do_schannels() routine
3624-
*/
3625-
if (ch->rx_count == vsi->num_rxq - curr_combined)
3626-
ch->rx_count = 0;
3627-
if (ch->tx_count == vsi->num_txq - curr_combined)
3628-
ch->tx_count = 0;
3629-
if (ch->combined_count == curr_combined)
3630-
ch->combined_count = 0;
3631-
3632-
if (!(ch->combined_count || (ch->rx_count && ch->tx_count))) {
3633-
netdev_err(dev, "Please specify at least 1 Rx and 1 Tx channel\n");
3617+
if (ch->rx_count && ch->tx_count) {
3618+
netdev_err(dev, "Dedicated RX or TX channels cannot be used simultaneously\n");
36343619
return -EINVAL;
36353620
}
36363621

0 commit comments

Comments
 (0)