Skip to content

Commit e96a79b

Browse files
davem330davem330
committed
Merge branch 'ethtool-rss-fixes' into main
Jakub Kicinski says; ==================== ethtool: more RSS fixes More fixes for RSS setting. First two patches fix my own bugs in bnxt conversion to the new API. The third patch fixes what seems to be a 10 year old issue (present since the Linux RSS API was created). Fourth patch fixes an issue with the XArray state being out of sync. And then a small test. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 9da49aa + 0d6ccfe commit e96a79b

3 files changed

Lines changed: 79 additions & 15 deletions

File tree

‎drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,8 +1863,14 @@ static void bnxt_modify_rss(struct bnxt *bp, struct ethtool_rxfh_context *ctx,
18631863
}
18641864

18651865
static int bnxt_rxfh_context_check(struct bnxt *bp,
1866+
const struct ethtool_rxfh_param *rxfh,
18661867
struct netlink_ext_ack *extack)
18671868
{
1869+
if (rxfh->hfunc && rxfh->hfunc != ETH_RSS_HASH_TOP) {
1870+
NL_SET_ERR_MSG_MOD(extack, "RSS hash function not supported");
1871+
return -EOPNOTSUPP;
1872+
}
1873+
18681874
if (!BNXT_SUPPORTS_MULTI_RSS_CTX(bp)) {
18691875
NL_SET_ERR_MSG_MOD(extack, "RSS contexts not supported");
18701876
return -EOPNOTSUPP;
@@ -1888,7 +1894,7 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
18881894
struct bnxt_vnic_info *vnic;
18891895
int rc;
18901896

1891-
rc = bnxt_rxfh_context_check(bp, extack);
1897+
rc = bnxt_rxfh_context_check(bp, rxfh, extack);
18921898
if (rc)
18931899
return rc;
18941900

@@ -1915,8 +1921,12 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
19151921
if (rc)
19161922
goto out;
19171923

1924+
/* Populate defaults in the context */
19181925
bnxt_set_dflt_rss_indir_tbl(bp, ctx);
1926+
ctx->hfunc = ETH_RSS_HASH_TOP;
19191927
memcpy(vnic->rss_hash_key, bp->rss_hash_key, HW_HASH_KEY_SIZE);
1928+
memcpy(ethtool_rxfh_context_key(ctx),
1929+
bp->rss_hash_key, HW_HASH_KEY_SIZE);
19201930

19211931
rc = bnxt_hwrm_vnic_alloc(bp, vnic, 0, bp->rx_nr_rings);
19221932
if (rc) {
@@ -1953,7 +1963,7 @@ static int bnxt_modify_rxfh_context(struct net_device *dev,
19531963
struct bnxt_rss_ctx *rss_ctx;
19541964
int rc;
19551965

1956-
rc = bnxt_rxfh_context_check(bp, extack);
1966+
rc = bnxt_rxfh_context_check(bp, rxfh, extack);
19571967
if (rc)
19581968
return rc;
19591969

‎net/ethtool/ioctl.c‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,13 +1331,13 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
13311331
u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]);
13321332
const struct ethtool_ops *ops = dev->ethtool_ops;
13331333
u32 dev_indir_size = 0, dev_key_size = 0, i;
1334+
u32 user_indir_len = 0, indir_bytes = 0;
13341335
struct ethtool_rxfh_param rxfh_dev = {};
13351336
struct ethtool_rxfh_context *ctx = NULL;
13361337
struct netlink_ext_ack *extack = NULL;
13371338
struct ethtool_rxnfc rx_rings;
13381339
struct ethtool_rxfh rxfh;
13391340
bool locked = false; /* dev->ethtool->rss_lock taken */
1340-
u32 indir_bytes = 0;
13411341
bool create = false;
13421342
u8 *rss_config;
13431343
int ret;
@@ -1382,10 +1382,9 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
13821382
rxfh.input_xfrm == RXH_XFRM_NO_CHANGE))
13831383
return -EINVAL;
13841384

1385-
if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
1386-
indir_bytes = dev_indir_size * sizeof(rxfh_dev.indir[0]);
1385+
indir_bytes = dev_indir_size * sizeof(rxfh_dev.indir[0]);
13871386

1388-
rss_config = kzalloc(indir_bytes + rxfh.key_size, GFP_USER);
1387+
rss_config = kzalloc(indir_bytes + dev_key_size, GFP_USER);
13891388
if (!rss_config)
13901389
return -ENOMEM;
13911390

@@ -1400,6 +1399,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
14001399
*/
14011400
if (rxfh.indir_size &&
14021401
rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE) {
1402+
user_indir_len = indir_bytes;
14031403
rxfh_dev.indir = (u32 *)rss_config;
14041404
rxfh_dev.indir_size = dev_indir_size;
14051405
ret = ethtool_copy_validate_indir(rxfh_dev.indir,
@@ -1426,7 +1426,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
14261426
rxfh_dev.key_size = dev_key_size;
14271427
rxfh_dev.key = rss_config + indir_bytes;
14281428
if (copy_from_user(rxfh_dev.key,
1429-
useraddr + rss_cfg_offset + indir_bytes,
1429+
useraddr + rss_cfg_offset + user_indir_len,
14301430
rxfh.key_size)) {
14311431
ret = -EFAULT;
14321432
goto out;
@@ -1474,16 +1474,21 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
14741474
rxfh_dev.input_xfrm = rxfh.input_xfrm;
14751475

14761476
if (rxfh.rss_context && ops->create_rxfh_context) {
1477-
if (create)
1477+
if (create) {
14781478
ret = ops->create_rxfh_context(dev, ctx, &rxfh_dev,
14791479
extack);
1480-
else if (rxfh_dev.rss_delete)
1480+
/* Make sure driver populates defaults */
1481+
WARN_ON_ONCE(!ret && !rxfh_dev.key &&
1482+
!memchr_inv(ethtool_rxfh_context_key(ctx),
1483+
0, ctx->key_size));
1484+
} else if (rxfh_dev.rss_delete) {
14811485
ret = ops->remove_rxfh_context(dev, ctx,
14821486
rxfh.rss_context,
14831487
extack);
1484-
else
1488+
} else {
14851489
ret = ops->modify_rxfh_context(dev, ctx, &rxfh_dev,
14861490
extack);
1491+
}
14871492
} else {
14881493
ret = ops->set_rxfh(dev, &rxfh_dev, extack);
14891494
}
@@ -1522,6 +1527,22 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
15221527
kfree(ctx);
15231528
goto out;
15241529
}
1530+
1531+
/* Fetch the defaults for the old API, in the new API drivers
1532+
* should write defaults into ctx themselves.
1533+
*/
1534+
rxfh_dev.indir = (u32 *)rss_config;
1535+
rxfh_dev.indir_size = dev_indir_size;
1536+
1537+
rxfh_dev.key = rss_config + indir_bytes;
1538+
rxfh_dev.key_size = dev_key_size;
1539+
1540+
ret = ops->get_rxfh(dev, &rxfh_dev);
1541+
if (WARN_ON(ret)) {
1542+
xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context);
1543+
kfree(ctx);
1544+
goto out;
1545+
}
15251546
}
15261547
if (rxfh_dev.rss_delete) {
15271548
WARN_ON(xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context) != ctx);
@@ -1530,12 +1551,14 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
15301551
if (rxfh_dev.indir) {
15311552
for (i = 0; i < dev_indir_size; i++)
15321553
ethtool_rxfh_context_indir(ctx)[i] = rxfh_dev.indir[i];
1533-
ctx->indir_configured = 1;
1554+
ctx->indir_configured =
1555+
rxfh.indir_size &&
1556+
rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE;
15341557
}
15351558
if (rxfh_dev.key) {
15361559
memcpy(ethtool_rxfh_context_key(ctx), rxfh_dev.key,
15371560
dev_key_size);
1538-
ctx->key_configured = 1;
1561+
ctx->key_configured = !!rxfh.key_size;
15391562
}
15401563
if (rxfh_dev.hfunc != ETH_RSS_HASH_NO_CHANGE)
15411564
ctx->hfunc = rxfh_dev.hfunc;

‎tools/testing/selftests/drivers/net/hw/rss_ctx.py‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ def _rss_key_rand(length):
1919
return [random.randint(0, 255) for _ in range(length)]
2020

2121

22+
def _rss_key_check(cfg, data=None, context=0):
23+
if data is None:
24+
data = get_rss(cfg, context=context)
25+
if 'rss-hash-key' not in data:
26+
return
27+
non_zero = [x for x in data['rss-hash-key'] if x != 0]
28+
ksft_eq(bool(non_zero), True, comment=f"RSS key is all zero {data['rss-hash-key']}")
29+
30+
2231
def get_rss(cfg, context=0):
2332
return ethtool(f"-x {cfg.ifname} context {context}", json=True)[0]
2433

@@ -90,8 +99,9 @@ def _send_traffic_check(cfg, port, name, params):
9099
def test_rss_key_indir(cfg):
91100
"""Test basics like updating the main RSS key and indirection table."""
92101

93-
if len(_get_rx_cnts(cfg)) < 2:
94-
KsftSkipEx("Device has only one queue (or doesn't support queue stats)")
102+
qcnt = len(_get_rx_cnts(cfg))
103+
if qcnt < 3:
104+
KsftSkipEx("Device has fewer than 3 queues (or doesn't support queue stats)")
95105

96106
data = get_rss(cfg)
97107
want_keys = ['rss-hash-key', 'rss-hash-function', 'rss-indirection-table']
@@ -101,6 +111,7 @@ def test_rss_key_indir(cfg):
101111
if not data[k]:
102112
raise KsftFailEx(f"ethtool results empty for '{k}': {data[k]}")
103113

114+
_rss_key_check(cfg, data=data)
104115
key_len = len(data['rss-hash-key'])
105116

106117
# Set the key
@@ -110,9 +121,26 @@ def test_rss_key_indir(cfg):
110121
data = get_rss(cfg)
111122
ksft_eq(key, data['rss-hash-key'])
112123

124+
# Set the indirection table and the key together
125+
key = _rss_key_rand(key_len)
126+
ethtool(f"-X {cfg.ifname} equal 3 hkey " + _rss_key_str(key))
127+
reset_indir = defer(ethtool, f"-X {cfg.ifname} default")
128+
129+
data = get_rss(cfg)
130+
_rss_key_check(cfg, data=data)
131+
ksft_eq(0, min(data['rss-indirection-table']))
132+
ksft_eq(2, max(data['rss-indirection-table']))
133+
134+
# Reset indirection table and set the key
135+
key = _rss_key_rand(key_len)
136+
ethtool(f"-X {cfg.ifname} default hkey " + _rss_key_str(key))
137+
data = get_rss(cfg)
138+
_rss_key_check(cfg, data=data)
139+
ksft_eq(0, min(data['rss-indirection-table']))
140+
ksft_eq(qcnt - 1, max(data['rss-indirection-table']))
141+
113142
# Set the indirection table
114143
ethtool(f"-X {cfg.ifname} equal 2")
115-
reset_indir = defer(ethtool, f"-X {cfg.ifname} default")
116144
data = get_rss(cfg)
117145
ksft_eq(0, min(data['rss-indirection-table']))
118146
ksft_eq(1, max(data['rss-indirection-table']))
@@ -317,8 +345,11 @@ def test_rss_context(cfg, ctx_cnt=1, create_with_cfg=None):
317345
ctx_cnt = i
318346
break
319347

348+
_rss_key_check(cfg, context=ctx_id)
349+
320350
if not create_with_cfg:
321351
ethtool(f"-X {cfg.ifname} context {ctx_id} {want_cfg}")
352+
_rss_key_check(cfg, context=ctx_id)
322353

323354
# Sanity check the context we just created
324355
data = get_rss(cfg, ctx_id)

0 commit comments

Comments
 (0)