Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'net-ethtool-consistently-take-rss_lock-for-all-rxfh-ops'

Jakub Kicinski says:

====================
net: ethtool: consistently take rss_lock for all rxfh ops

I'd like to bring RXFH and RXFHINDIR ioctls under a single set of
Netlink ops. It appears that while core takes the ethtool->rss_lock
around some of the RXFHINDIR ops, drivers (sfc) take it internally
for the RXFH.

Consistently take the lock around all ops and accesses to the XArray
within the core. This should hopefully make the rss_lock a lot less
confusing.
====================

Link: https://patch.msgid.link/20250626202848.104457-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

+59 -41
+2 -7
drivers/net/ethernet/sfc/ethtool_common.c
··· 810 810 811 811 ctx = &efx->rss_context.priv; 812 812 813 - mutex_lock(&net_dev->ethtool->rss_lock); 814 813 if (info->rss_context) { 815 814 ctx = efx_find_rss_context_entry(efx, info->rss_context); 816 - if (!ctx) { 817 - rc = -ENOENT; 818 - goto out_unlock; 819 - } 815 + if (!ctx) 816 + return -ENOENT; 820 817 } 821 818 822 819 data = 0; ··· 847 850 } 848 851 out_setdata_unlock: 849 852 info->data = data; 850 - out_unlock: 851 - mutex_unlock(&net_dev->ethtool->rss_lock); 852 853 return rc; 853 854 } 854 855
+2
net/ethtool/common.c
··· 707 707 if (!rxfh.indir) 708 708 return U32_MAX; 709 709 710 + mutex_lock(&dev->ethtool->rss_lock); 710 711 ret = dev->ethtool_ops->get_rxfh(dev, &rxfh); 712 + mutex_unlock(&dev->ethtool->rss_lock); 711 713 if (ret) { 712 714 current_max = U32_MAX; 713 715 goto out_free;
+38 -28
net/ethtool/ioctl.c
··· 1079 1079 !ops->rxfh_per_ctx_fields) 1080 1080 return -EINVAL; 1081 1081 1082 + mutex_lock(&dev->ethtool->rss_lock); 1082 1083 if (ops->get_rxfh) { 1083 1084 struct ethtool_rxfh_param rxfh = {}; 1084 1085 1085 1086 rc = ops->get_rxfh(dev, &rxfh); 1086 1087 if (rc) 1087 - return rc; 1088 + goto exit_unlock; 1088 1089 1089 1090 rc = ethtool_check_xfrm_rxfh(rxfh.input_xfrm, info.data); 1090 1091 if (rc) 1091 - return rc; 1092 + goto exit_unlock; 1092 1093 } 1093 1094 1094 1095 fields.data = info.data; ··· 1097 1096 if (info.flow_type & FLOW_RSS) 1098 1097 fields.rss_context = info.rss_context; 1099 1098 1100 - return ops->set_rxfh_fields(dev, &fields, NULL); 1099 + rc = ops->set_rxfh_fields(dev, &fields, NULL); 1100 + exit_unlock: 1101 + mutex_unlock(&dev->ethtool->rss_lock); 1102 + return rc; 1101 1103 } 1102 1104 1103 1105 static noinline_for_stack int ··· 1127 1123 if (info.flow_type & FLOW_RSS) 1128 1124 fields.rss_context = info.rss_context; 1129 1125 1126 + mutex_lock(&dev->ethtool->rss_lock); 1130 1127 ret = ops->get_rxfh_fields(dev, &fields); 1128 + mutex_unlock(&dev->ethtool->rss_lock); 1131 1129 if (ret < 0) 1132 1130 return ret; 1133 1131 ··· 1275 1269 if (!rxfh.indir) 1276 1270 return -ENOMEM; 1277 1271 1272 + mutex_lock(&dev->ethtool->rss_lock); 1278 1273 ret = dev->ethtool_ops->get_rxfh(dev, &rxfh); 1274 + mutex_unlock(&dev->ethtool->rss_lock); 1279 1275 if (ret) 1280 1276 goto out; 1281 1277 if (copy_to_user(useraddr + ··· 1342 1334 } 1343 1335 1344 1336 rxfh_dev.hfunc = ETH_RSS_HASH_NO_CHANGE; 1337 + 1338 + mutex_lock(&dev->ethtool->rss_lock); 1345 1339 ret = ops->set_rxfh(dev, &rxfh_dev, extack); 1346 1340 if (ret) 1347 - goto out; 1341 + goto out_unlock; 1348 1342 1349 1343 /* indicate whether rxfh was set to default */ 1350 1344 if (user_size == 0) ··· 1354 1344 else 1355 1345 dev->priv_flags |= IFF_RXFH_CONFIGURED; 1356 1346 1347 + out_unlock: 1348 + mutex_unlock(&dev->ethtool->rss_lock); 1357 1349 out: 1358 1350 kfree(rxfh_dev.indir); 1359 1351 return ret; ··· 1416 1404 if (user_key_size) 1417 1405 rxfh_dev.key = rss_config + indir_bytes; 1418 1406 1407 + mutex_lock(&dev->ethtool->rss_lock); 1419 1408 if (rxfh.rss_context) { 1420 1409 ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context); 1421 1410 if (!ctx) { ··· 1462 1449 ret = -EFAULT; 1463 1450 } 1464 1451 out: 1452 + mutex_unlock(&dev->ethtool->rss_lock); 1465 1453 kfree(rss_config); 1466 1454 1467 1455 return ret; ··· 1514 1500 struct netlink_ext_ack *extack = NULL; 1515 1501 struct ethtool_rxnfc rx_rings; 1516 1502 struct ethtool_rxfh rxfh; 1517 - bool locked = false; /* dev->ethtool->rss_lock taken */ 1518 1503 bool create = false; 1519 1504 bool mod = false; 1520 1505 u8 *rss_config; ··· 1563 1550 rxfh.input_xfrm == RXH_XFRM_NO_CHANGE)) 1564 1551 return -EINVAL; 1565 1552 1566 - ret = ethtool_check_flow_types(dev, rxfh.input_xfrm); 1567 - if (ret) 1568 - return ret; 1569 - 1570 1553 indir_bytes = dev_indir_size * sizeof(rxfh_dev.indir[0]); 1571 1554 1572 1555 /* Check settings which may be global rather than per RSS-context */ ··· 1579 1570 rx_rings.cmd = ETHTOOL_GRXRINGS; 1580 1571 ret = ops->get_rxnfc(dev, &rx_rings, NULL); 1581 1572 if (ret) 1582 - goto out; 1573 + goto out_free; 1583 1574 1584 1575 /* rxfh.indir_size == 0 means reset the indir table to default (master 1585 1576 * context) or delete the context (other RSS contexts). ··· 1595 1586 &rx_rings, 1596 1587 rxfh.indir_size); 1597 1588 if (ret) 1598 - goto out; 1589 + goto out_free; 1599 1590 } else if (rxfh.indir_size == 0) { 1600 1591 if (rxfh.rss_context == 0) { 1601 1592 u32 *indir; ··· 1617 1608 useraddr + rss_cfg_offset + user_indir_len, 1618 1609 rxfh.key_size)) { 1619 1610 ret = -EFAULT; 1620 - goto out; 1611 + goto out_free; 1621 1612 } 1622 1613 } 1623 1614 1624 - if (rxfh.rss_context) { 1625 - mutex_lock(&dev->ethtool->rss_lock); 1626 - locked = true; 1627 - } 1615 + mutex_lock(&dev->ethtool->rss_lock); 1616 + 1617 + ret = ethtool_check_flow_types(dev, rxfh.input_xfrm); 1618 + if (ret) 1619 + goto out_unlock; 1628 1620 1629 1621 if (rxfh.rss_context && rxfh_dev.rss_delete) { 1630 1622 ret = ethtool_check_rss_ctx_busy(dev, rxfh.rss_context); 1631 1623 if (ret) 1632 - goto out; 1624 + goto out_unlock; 1633 1625 } 1634 1626 1635 1627 if (create) { 1636 1628 if (rxfh_dev.rss_delete) { 1637 1629 ret = -EINVAL; 1638 - goto out; 1630 + goto out_unlock; 1639 1631 } 1640 1632 ctx = ethtool_rxfh_ctx_alloc(ops, dev_indir_size, dev_key_size); 1641 1633 if (!ctx) { 1642 1634 ret = -ENOMEM; 1643 - goto out; 1635 + goto out_unlock; 1644 1636 } 1645 1637 1646 1638 if (ops->create_rxfh_context) { ··· 1654 1644 GFP_KERNEL_ACCOUNT); 1655 1645 if (ret < 0) { 1656 1646 kfree(ctx); 1657 - goto out; 1647 + goto out_unlock; 1658 1648 } 1659 1649 WARN_ON(!ctx_id); /* can't happen */ 1660 1650 rxfh.rss_context = ctx_id; ··· 1663 1653 ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context); 1664 1654 if (!ctx) { 1665 1655 ret = -ENOENT; 1666 - goto out; 1656 + goto out_unlock; 1667 1657 } 1668 1658 } 1669 1659 rxfh_dev.hfunc = rxfh.hfunc; ··· 1697 1687 xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context); 1698 1688 kfree(ctx); 1699 1689 } 1700 - goto out; 1690 + goto out_unlock; 1701 1691 } 1702 1692 mod = !create && !rxfh_dev.rss_delete; 1703 1693 ··· 1718 1708 if (WARN_ON(xa_load(&dev->ethtool->rss_ctx, rxfh_dev.rss_context))) { 1719 1709 /* context ID reused, our tracking is screwed */ 1720 1710 kfree(ctx); 1721 - goto out; 1711 + goto out_unlock; 1722 1712 } 1723 1713 /* Allocate the exact ID the driver gave us */ 1724 1714 if (xa_is_err(xa_store(&dev->ethtool->rss_ctx, rxfh_dev.rss_context, 1725 1715 ctx, GFP_KERNEL))) { 1726 1716 kfree(ctx); 1727 - goto out; 1717 + goto out_unlock; 1728 1718 } 1729 1719 1730 1720 /* Fetch the defaults for the old API, in the new API drivers ··· 1740 1730 if (WARN_ON(ret)) { 1741 1731 xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context); 1742 1732 kfree(ctx); 1743 - goto out; 1733 + goto out_unlock; 1744 1734 } 1745 1735 } 1746 1736 if (rxfh_dev.rss_delete) { ··· 1765 1755 ctx->input_xfrm = rxfh_dev.input_xfrm; 1766 1756 } 1767 1757 1768 - out: 1769 - if (locked) 1770 - mutex_unlock(&dev->ethtool->rss_lock); 1758 + out_unlock: 1759 + mutex_unlock(&dev->ethtool->rss_lock); 1760 + out_free: 1771 1761 kfree(rss_config); 1772 1762 if (mod) 1773 1763 ethtool_rss_notify(dev, rxfh.rss_context);
+17 -6
net/ethtool/rss.c
··· 139 139 } 140 140 141 141 static int 142 + rss_prepare(const struct rss_req_info *request, struct net_device *dev, 143 + struct rss_reply_data *data, const struct genl_info *info) 144 + { 145 + if (request->rss_context) 146 + return rss_prepare_ctx(request, dev, data, info); 147 + return rss_prepare_get(request, dev, data, info); 148 + } 149 + 150 + static int 142 151 rss_prepare_data(const struct ethnl_req_info *req_base, 143 152 struct ethnl_reply_data *reply_base, 144 153 const struct genl_info *info) ··· 156 147 struct rss_req_info *request = RSS_REQINFO(req_base); 157 148 struct net_device *dev = reply_base->dev; 158 149 const struct ethtool_ops *ops; 150 + int ret; 159 151 160 152 ops = dev->ethtool_ops; 161 153 if (!ops->get_rxfh) 162 154 return -EOPNOTSUPP; 163 155 164 156 /* Some drivers don't handle rss_context */ 165 - if (request->rss_context) { 166 - if (!ops->cap_rss_ctx_supported && !ops->create_rxfh_context) 167 - return -EOPNOTSUPP; 157 + if (request->rss_context && 158 + !ops->cap_rss_ctx_supported && !ops->create_rxfh_context) 159 + return -EOPNOTSUPP; 168 160 169 - return rss_prepare_ctx(request, dev, data, info); 170 - } 161 + mutex_lock(&dev->ethtool->rss_lock); 162 + ret = rss_prepare(request, dev, data, info); 163 + mutex_unlock(&dev->ethtool->rss_lock); 171 164 172 - return rss_prepare_get(request, dev, data, info); 165 + return ret; 173 166 } 174 167 175 168 static int