* [PATCH v6 net-next 1/9] net: move ethtool-related netdev state into its own struct
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 2/9] net: ethtool: attach an XArray of custom RSS contexts to a netdevice edward.cree
` (7 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
net_dev->ethtool is a pointer to new struct ethtool_netdev_state, which
currently contains only the wol_enabled field.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
drivers/net/ethernet/realtek/r8169_main.c | 4 ++--
drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c | 4 ++--
drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 2 +-
drivers/net/phy/phy.c | 2 +-
drivers/net/phy/phy_device.c | 5 +++--
drivers/net/phy/phylink.c | 2 +-
include/linux/ethtool.h | 8 ++++++++
include/linux/netdevice.h | 7 ++++---
net/core/dev.c | 4 ++++
net/ethtool/ioctl.c | 2 +-
net/ethtool/wol.c | 2 +-
11 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 9246ea2118ff..714d2e804694 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1608,7 +1608,7 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
if (!tp->dash_enabled) {
rtl_set_d3_pll_down(tp, !wolopts);
- tp->dev->wol_enabled = wolopts ? 1 : 0;
+ tp->dev->ethtool->wol_enabled = wolopts ? 1 : 0;
}
}
@@ -5478,7 +5478,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
rtl_set_d3_pll_down(tp, true);
} else {
rtl_set_d3_pll_down(tp, false);
- dev->wol_enabled = 1;
+ dev->ethtool->wol_enabled = 1;
}
jumbo_max = rtl_jumbo_max(tp);
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c
index 46a5a3e95202..e868f7ef4920 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_ethtool.c
@@ -37,9 +37,9 @@ static int ngbe_set_wol(struct net_device *netdev,
wx->wol = 0;
if (wol->wolopts & WAKE_MAGIC)
wx->wol = WX_PSR_WKUP_CTL_MAG;
- netdev->wol_enabled = !!(wx->wol);
+ netdev->ethtool->wol_enabled = !!(wx->wol);
wr32(wx, WX_PSR_WKUP_CTL, wx->wol);
- device_set_wakeup_enable(&pdev->dev, netdev->wol_enabled);
+ device_set_wakeup_enable(&pdev->dev, netdev->ethtool->wol_enabled);
return 0;
}
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index e894e01d030d..a8119de60deb 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -650,7 +650,7 @@ static int ngbe_probe(struct pci_dev *pdev,
if (wx->wol_hw_supported)
wx->wol = NGBE_PSR_WKUP_CTL_MAG;
- netdev->wol_enabled = !!(wx->wol);
+ netdev->ethtool->wol_enabled = !!(wx->wol);
wr32(wx, NGBE_PSR_WKUP_CTL, wx->wol);
device_set_wakeup_enable(&pdev->dev, wx->wol);
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c4236564c1cd..785182fa5fe0 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1309,7 +1309,7 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
if (netdev) {
struct device *parent = netdev->dev.parent;
- if (netdev->wol_enabled)
+ if (netdev->ethtool->wol_enabled)
pm_system_wakeup();
else if (device_may_wakeup(&netdev->dev))
pm_wakeup_dev_event(&netdev->dev, 0, true);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 6c6ec9475709..473cbc1d497b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -296,7 +296,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
if (!netdev)
goto out;
- if (netdev->wol_enabled)
+ if (netdev->ethtool->wol_enabled)
return false;
/* As long as not all affected network drivers support the
@@ -1984,7 +1984,8 @@ int phy_suspend(struct phy_device *phydev)
return 0;
phy_ethtool_get_wol(phydev, &wol);
- phydev->wol_enabled = wol.wolopts || (netdev && netdev->wol_enabled);
+ phydev->wol_enabled = wol.wolopts ||
+ (netdev && netdev->ethtool->wol_enabled);
/* If the device has WOL enabled, we cannot suspend the PHY */
if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
return -EBUSY;
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 02427378acfd..38e4e7c0f7d5 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2275,7 +2275,7 @@ void phylink_suspend(struct phylink *pl, bool mac_wol)
{
ASSERT_RTNL();
- if (mac_wol && (!pl->netdev || pl->netdev->wol_enabled)) {
+ if (mac_wol && (!pl->netdev || pl->netdev->ethtool->wol_enabled)) {
/* Wake-on-Lan enabled, MAC handling */
mutex_lock(&pl->state_mutex);
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 6fd9107d3cc0..8cd6b3c993f1 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -998,6 +998,14 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
const struct ethtool_link_ksettings *cmd,
u32 *dev_speed, u8 *dev_duplex);
+/**
+ * struct ethtool_netdev_state - per-netdevice state for ethtool features
+ * @wol_enabled: Wake-on-LAN is enabled
+ */
+struct ethtool_netdev_state {
+ unsigned wol_enabled:1;
+};
+
struct phy_device;
struct phy_tdr_config;
struct phy_plca_cfg;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 85111502cf8f..bd88ecb61598 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -79,6 +79,7 @@ struct xdp_buff;
struct xdp_frame;
struct xdp_metadata_ops;
struct xdp_md;
+struct ethtool_netdev_state;
typedef u32 xdp_features_t;
@@ -1985,8 +1986,6 @@ enum netdev_reg_state {
* switch driver and used to set the phys state of the
* switch port.
*
- * @wol_enabled: Wake-on-LAN is enabled
- *
* @threaded: napi threaded mode is enabled
*
* @net_notifier_list: List of per-net netdev notifier block
@@ -1998,6 +1997,7 @@ enum netdev_reg_state {
* @udp_tunnel_nic_info: static structure describing the UDP tunnel
* offload capabilities of the device
* @udp_tunnel_nic: UDP tunnel offload state
+ * @ethtool: ethtool related state
* @xdp_state: stores info on attached XDP BPF programs
*
* @nested_level: Used as a parameter of spin_lock_nested() of
@@ -2372,7 +2372,6 @@ struct net_device {
struct lock_class_key *qdisc_tx_busylock;
bool proto_down;
bool threaded;
- unsigned wol_enabled:1;
struct list_head net_notifier_list;
@@ -2383,6 +2382,8 @@ struct net_device {
const struct udp_tunnel_nic_info *udp_tunnel_nic_info;
struct udp_tunnel_nic *udp_tunnel_nic;
+ struct ethtool_netdev_state *ethtool;
+
/* protected by rtnl_lock */
struct bpf_xdp_entity xdp_state[__MAX_XDP_MODE];
diff --git a/net/core/dev.c b/net/core/dev.c
index c361a7b69da8..29351bbea803 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11065,6 +11065,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
dev->real_num_rx_queues = rxqs;
if (netif_alloc_rx_queues(dev))
goto free_all;
+ dev->ethtool = kzalloc(sizeof(*dev->ethtool), GFP_KERNEL_ACCOUNT);
+ if (!dev->ethtool)
+ goto free_all;
strcpy(dev->name, name);
dev->name_assign_type = name_assign_type;
@@ -11115,6 +11118,7 @@ void free_netdev(struct net_device *dev)
return;
}
+ kfree(dev->ethtool);
netif_free_tx_queues(dev);
netif_free_rx_queues(dev);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index e645d751a5e8..998571f05deb 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1503,7 +1503,7 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr)
if (ret)
return ret;
- dev->wol_enabled = !!wol.wolopts;
+ dev->ethtool->wol_enabled = !!wol.wolopts;
ethtool_notify(dev, ETHTOOL_MSG_WOL_NTF, NULL);
return 0;
diff --git a/net/ethtool/wol.c b/net/ethtool/wol.c
index 0ed56c9ac1bc..a39d8000d808 100644
--- a/net/ethtool/wol.c
+++ b/net/ethtool/wol.c
@@ -137,7 +137,7 @@ ethnl_set_wol(struct ethnl_req_info *req_info, struct genl_info *info)
ret = dev->ethtool_ops->set_wol(dev, &wol);
if (ret)
return ret;
- dev->wol_enabled = !!wol.wolopts;
+ dev->ethtool->wol_enabled = !!wol.wolopts;
return 1;
}
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v6 net-next 2/9] net: ethtool: attach an XArray of custom RSS contexts to a netdevice
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 1/9] net: move ethtool-related netdev state into its own struct edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray edward.cree
` (6 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
Each context stores the RXFH settings (indir, key, and hfunc) as well
as optionally some driver private data.
Delete any still-existing contexts at netdev unregister time.
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
include/linux/ethtool.h | 42 +++++++++++++++++++++++++++++++++++++++++
net/core/dev.c | 27 ++++++++++++++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 8cd6b3c993f1..13c9c819de58 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -159,6 +159,46 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
return index % n_rx_rings;
}
+/**
+ * struct ethtool_rxfh_context - a custom RSS context configuration
+ * @indir_size: Number of u32 entries in indirection table
+ * @key_size: Size of hash key, in bytes
+ * @priv_size: Size of driver private data, in bytes
+ * @hfunc: RSS hash function identifier. One of the %ETH_RSS_HASH_*
+ * @input_xfrm: Defines how the input data is transformed. Valid values are one
+ * of %RXH_XFRM_*.
+ * @indir_configured: indir has been specified (at create time or subsequently)
+ * @key_configured: hkey has been specified (at create time or subsequently)
+ */
+struct ethtool_rxfh_context {
+ u32 indir_size;
+ u32 key_size;
+ u16 priv_size;
+ u8 hfunc;
+ u8 input_xfrm;
+ u8 indir_configured:1;
+ u8 key_configured:1;
+ /* private: driver private data, indirection table, and hash key are
+ * stored sequentially in @data area. Use below helpers to access.
+ */
+ u8 data[] __aligned(sizeof(void *));
+};
+
+static inline void *ethtool_rxfh_context_priv(struct ethtool_rxfh_context *ctx)
+{
+ return ctx->data;
+}
+
+static inline u32 *ethtool_rxfh_context_indir(struct ethtool_rxfh_context *ctx)
+{
+ return (u32 *)(ctx->data + ALIGN(ctx->priv_size, sizeof(u32)));
+}
+
+static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
+{
+ return (u8 *)(ethtool_rxfh_context_indir(ctx) + ctx->indir_size);
+}
+
/* declare a link mode bitmap */
#define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \
DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
@@ -1000,9 +1040,11 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
/**
* struct ethtool_netdev_state - per-netdevice state for ethtool features
+ * @rss_ctx: XArray of custom RSS contexts
* @wol_enabled: Wake-on-LAN is enabled
*/
struct ethtool_netdev_state {
+ struct xarray rss_ctx;
unsigned wol_enabled:1;
};
diff --git a/net/core/dev.c b/net/core/dev.c
index 29351bbea803..78465b69dc68 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10285,6 +10285,9 @@ int register_netdevice(struct net_device *dev)
if (ret)
return ret;
+ /* rss ctx ID 0 is reserved for the default context, start from 1 */
+ xa_init_flags(&dev->ethtool->rss_ctx, XA_FLAGS_ALLOC1);
+
spin_lock_init(&dev->addr_list_lock);
netdev_set_addr_lockdep_class(dev);
@@ -11184,6 +11187,28 @@ void synchronize_net(void)
}
EXPORT_SYMBOL(synchronize_net);
+static void netdev_rss_contexts_free(struct net_device *dev)
+{
+ struct ethtool_rxfh_context *ctx;
+ unsigned long context;
+
+ xa_for_each(&dev->ethtool->rss_ctx, context, ctx) {
+ struct ethtool_rxfh_param rxfh;
+
+ rxfh.indir = ethtool_rxfh_context_indir(ctx);
+ rxfh.key = ethtool_rxfh_context_key(ctx);
+ rxfh.hfunc = ctx->hfunc;
+ rxfh.input_xfrm = ctx->input_xfrm;
+ rxfh.rss_context = context;
+ rxfh.rss_delete = true;
+
+ xa_erase(&dev->ethtool->rss_ctx, context);
+ dev->ethtool_ops->set_rxfh(dev, &rxfh, NULL);
+ kfree(ctx);
+ }
+ xa_destroy(&dev->ethtool->rss_ctx);
+}
+
/**
* unregister_netdevice_queue - remove device from the kernel
* @dev: device
@@ -11287,6 +11312,8 @@ void unregister_netdevice_many_notify(struct list_head *head,
netdev_name_node_alt_flush(dev);
netdev_name_node_free(dev->name_node);
+ netdev_rss_contexts_free(dev);
+
call_netdevice_notifiers(NETDEV_PRE_UNINIT, dev);
if (dev->netdev_ops->ndo_uninit)
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 1/9] net: move ethtool-related netdev state into its own struct edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 2/9] net: ethtool: attach an XArray of custom RSS contexts to a netdevice edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 6:32 ` Przemek Kitszel
2024-06-20 5:47 ` [PATCH v6 net-next 4/9] net: ethtool: let the core choose RSS context IDs edward.cree
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
Since drivers are still choosing the context IDs, we have to force the
XArray to use the ID they've chosen rather than picking one ourselves,
and handle the case where they give us an ID that's already in use.
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
include/linux/ethtool.h | 14 ++++++++
net/ethtool/ioctl.c | 74 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 13c9c819de58..283ba4aff623 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -199,6 +199,17 @@ static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
return (u8 *)(ethtool_rxfh_context_indir(ctx) + ctx->indir_size);
}
+static inline size_t ethtool_rxfh_context_size(u32 indir_size, u32 key_size,
+ u16 priv_size)
+{
+ size_t indir_bytes = array_size(indir_size, sizeof(u32));
+ size_t flex_len;
+
+ flex_len = size_add(size_add(indir_bytes, key_size),
+ ALIGN(priv_size, sizeof(u32)));
+ return struct_size((struct ethtool_rxfh_context *)0, data, flex_len);
+}
+
/* declare a link mode bitmap */
#define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \
DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
@@ -709,6 +720,8 @@ struct ethtool_rxfh_param {
* contexts.
* @cap_rss_sym_xor_supported: indicates if the driver supports symmetric-xor
* RSS.
+ * @rxfh_priv_size: size of the driver private data area the core should
+ * allocate for an RSS context (in &struct ethtool_rxfh_context).
* @supported_coalesce_params: supported types of interrupt coalescing.
* @supported_ring_params: supported ring params.
* @get_drvinfo: Report driver/device information. Modern drivers no
@@ -892,6 +905,7 @@ struct ethtool_ops {
u32 cap_link_lanes_supported:1;
u32 cap_rss_ctx_supported:1;
u32 cap_rss_sym_xor_supported:1;
+ u16 rxfh_priv_size;
u32 supported_coalesce_params;
u32 supported_ring_params;
void (*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 998571f05deb..f879deb6ac4e 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1278,10 +1278,12 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
const struct ethtool_ops *ops = dev->ethtool_ops;
u32 dev_indir_size = 0, dev_key_size = 0, i;
struct ethtool_rxfh_param rxfh_dev = {};
+ struct ethtool_rxfh_context *ctx = NULL;
struct netlink_ext_ack *extack = NULL;
struct ethtool_rxnfc rx_rings;
struct ethtool_rxfh rxfh;
u32 indir_bytes = 0;
+ bool create = false;
u8 *rss_config;
int ret;
@@ -1309,6 +1311,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
if ((rxfh.input_xfrm & RXH_XFRM_SYM_XOR) &&
!ops->cap_rss_sym_xor_supported)
return -EOPNOTSUPP;
+ create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;
/* If either indir, hash key or function is valid, proceed further.
* Must request at least one change: indir size, hash key, function
@@ -1374,13 +1377,42 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
}
}
+ if (create) {
+ if (rxfh_dev.rss_delete) {
+ ret = -EINVAL;
+ goto out;
+ }
+ ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
+ dev_key_size,
+ ops->rxfh_priv_size),
+ GFP_KERNEL_ACCOUNT);
+ if (!ctx) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ ctx->indir_size = dev_indir_size;
+ ctx->key_size = dev_key_size;
+ ctx->hfunc = rxfh.hfunc;
+ ctx->input_xfrm = rxfh.input_xfrm;
+ ctx->priv_size = ops->rxfh_priv_size;
+ } else if (rxfh.rss_context) {
+ ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
+ if (!ctx) {
+ ret = -ENOENT;
+ goto out;
+ }
+ }
rxfh_dev.hfunc = rxfh.hfunc;
rxfh_dev.rss_context = rxfh.rss_context;
rxfh_dev.input_xfrm = rxfh.input_xfrm;
ret = ops->set_rxfh(dev, &rxfh_dev, extack);
- if (ret)
+ if (ret) {
+ if (create)
+ /* failed to create, free our new tracking entry */
+ kfree(ctx);
goto out;
+ }
if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, rss_context),
&rxfh_dev.rss_context, sizeof(rxfh_dev.rss_context)))
@@ -1393,6 +1425,46 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
else if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
dev->priv_flags |= IFF_RXFH_CONFIGURED;
}
+ /* Update rss_ctx tracking */
+ if (create) {
+ /* Ideally this should happen before calling the driver,
+ * so that we can fail more cleanly; but we don't have the
+ * context ID until the driver picks it, so we have to
+ * wait until after.
+ */
+ if (WARN_ON(xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context))) {
+ /* context ID reused, our tracking is screwed */
+ kfree(ctx);
+ goto out;
+ }
+ /* Allocate the exact ID the driver gave us */
+ if (xa_is_err(xa_store(&dev->ethtool->rss_ctx, rxfh.rss_context,
+ ctx, GFP_KERNEL))) {
+ kfree(ctx);
+ goto out;
+ }
+ ctx->indir_configured = rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE;
+ ctx->key_configured = !!rxfh.key_size;
+ }
+ if (rxfh_dev.rss_delete) {
+ WARN_ON(xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context) != ctx);
+ kfree(ctx);
+ } else if (ctx) {
+ if (rxfh_dev.indir) {
+ for (i = 0; i < dev_indir_size; i++)
+ ethtool_rxfh_context_indir(ctx)[i] = rxfh_dev.indir[i];
+ ctx->indir_configured = 1;
+ }
+ if (rxfh_dev.key) {
+ memcpy(ethtool_rxfh_context_key(ctx), rxfh_dev.key,
+ dev_key_size);
+ ctx->key_configured = 1;
+ }
+ if (rxfh_dev.hfunc != ETH_RSS_HASH_NO_CHANGE)
+ ctx->hfunc = rxfh_dev.hfunc;
+ if (rxfh_dev.input_xfrm != RXH_XFRM_NO_CHANGE)
+ ctx->input_xfrm = rxfh_dev.input_xfrm;
+ }
out:
kfree(rss_config);
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-20 5:47 ` [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray edward.cree
@ 2024-06-20 6:32 ` Przemek Kitszel
2024-06-20 6:37 ` Edward Cree
2024-06-25 13:39 ` Edward Cree
0 siblings, 2 replies; 20+ messages in thread
From: Przemek Kitszel @ 2024-06-20 6:32 UTC (permalink / raw)
To: edward.cree
Cc: Edward Cree, linux-net-drivers, netdev, habetsm.xilinx,
sudheer.mogilappagari, jdamato, mw, linux, sgoutham, gakula,
sbhatta, hkelam, saeedm, leon, jacob.e.keller, andrew, ahmed.zaki,
davem, kuba, edumazet, pabeni
On 6/20/24 07:47, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> Since drivers are still choosing the context IDs, we have to force the
> XArray to use the ID they've chosen rather than picking one ourselves,
> and handle the case where they give us an ID that's already in use.
Q: This is a new API, perhaps you could force adopters to convert to
not choosing ID and switching to allocated one?
>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
> include/linux/ethtool.h | 14 ++++++++
> net/ethtool/ioctl.c | 74 ++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 87 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 13c9c819de58..283ba4aff623 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -199,6 +199,17 @@ static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
> return (u8 *)(ethtool_rxfh_context_indir(ctx) + ctx->indir_size);
> }
>
> +static inline size_t ethtool_rxfh_context_size(u32 indir_size, u32 key_size,
> + u16 priv_size)
> +{
> + size_t indir_bytes = array_size(indir_size, sizeof(u32));
> + size_t flex_len;
> +
> + flex_len = size_add(size_add(indir_bytes, key_size),
> + ALIGN(priv_size, sizeof(u32)));
> + return struct_size((struct ethtool_rxfh_context *)0, data, flex_len);
struct_size_t
> +}
> +
> /* declare a link mode bitmap */
> #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \
> DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
> @@ -709,6 +720,8 @@ struct ethtool_rxfh_param {
> * contexts.
> * @cap_rss_sym_xor_supported: indicates if the driver supports symmetric-xor
> * RSS.
> + * @rxfh_priv_size: size of the driver private data area the core should
> + * allocate for an RSS context (in &struct ethtool_rxfh_context).
> * @supported_coalesce_params: supported types of interrupt coalescing.
> * @supported_ring_params: supported ring params.
> * @get_drvinfo: Report driver/device information. Modern drivers no
> @@ -892,6 +905,7 @@ struct ethtool_ops {
> u32 cap_link_lanes_supported:1;
> u32 cap_rss_ctx_supported:1;
> u32 cap_rss_sym_xor_supported:1;
> + u16 rxfh_priv_size;
> u32 supported_coalesce_params;
> u32 supported_ring_params;
> void (*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 998571f05deb..f879deb6ac4e 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -1278,10 +1278,12 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
> const struct ethtool_ops *ops = dev->ethtool_ops;
> u32 dev_indir_size = 0, dev_key_size = 0, i;
> struct ethtool_rxfh_param rxfh_dev = {};
> + struct ethtool_rxfh_context *ctx = NULL;
> struct netlink_ext_ack *extack = NULL;
> struct ethtool_rxnfc rx_rings;
> struct ethtool_rxfh rxfh;
> u32 indir_bytes = 0;
> + bool create = false;
> u8 *rss_config;
> int ret;
>
> @@ -1309,6 +1311,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
> if ((rxfh.input_xfrm & RXH_XFRM_SYM_XOR) &&
> !ops->cap_rss_sym_xor_supported)
> return -EOPNOTSUPP;
> + create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;
>
> /* If either indir, hash key or function is valid, proceed further.
> * Must request at least one change: indir size, hash key, function
> @@ -1374,13 +1377,42 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
> }
> }
>
> + if (create) {
> + if (rxfh_dev.rss_delete) {
> + ret = -EINVAL;
> + goto out;
> + }
> + ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
> + dev_key_size,
> + ops->rxfh_priv_size),
> + GFP_KERNEL_ACCOUNT);
> + if (!ctx) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + ctx->indir_size = dev_indir_size;
> + ctx->key_size = dev_key_size;
> + ctx->hfunc = rxfh.hfunc;
> + ctx->input_xfrm = rxfh.input_xfrm;
> + ctx->priv_size = ops->rxfh_priv_size;
> + } else if (rxfh.rss_context) {
> + ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
> + if (!ctx) {
> + ret = -ENOENT;
> + goto out;
> + }
> + }
> rxfh_dev.hfunc = rxfh.hfunc;
> rxfh_dev.rss_context = rxfh.rss_context;
> rxfh_dev.input_xfrm = rxfh.input_xfrm;
>
> ret = ops->set_rxfh(dev, &rxfh_dev, extack);
> - if (ret)
> + if (ret) {
> + if (create)
> + /* failed to create, free our new tracking entry */
> + kfree(ctx);
> goto out;
> + }
>
> if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, rss_context),
> &rxfh_dev.rss_context, sizeof(rxfh_dev.rss_context)))
> @@ -1393,6 +1425,46 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
> else if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
> dev->priv_flags |= IFF_RXFH_CONFIGURED;
> }
> + /* Update rss_ctx tracking */
> + if (create) {
> + /* Ideally this should happen before calling the driver,
> + * so that we can fail more cleanly; but we don't have the
> + * context ID until the driver picks it, so we have to
> + * wait until after.
> + */
> + if (WARN_ON(xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context))) {
> + /* context ID reused, our tracking is screwed */
why no error code set?
> + kfree(ctx);
> + goto out;
> + }
> + /* Allocate the exact ID the driver gave us */
> + if (xa_is_err(xa_store(&dev->ethtool->rss_ctx, rxfh.rss_context,
> + ctx, GFP_KERNEL))) {
this is racy - assuming it is possible that context was set by other
means (otherwisce you would not xa_load() a few lines above) -
a concurrent writer could have done this just after you xa_load() call.
so, instead of xa_load() + xa_store() just use xa_insert()
anyway I feel the pain of trying to support both driver-selected IDs
and your own
> + kfree(ctx);
> + goto out;
> + }
> + ctx->indir_configured = rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE;
> + ctx->key_configured = !!rxfh.key_size;
> + }
> + if (rxfh_dev.rss_delete) {
> + WARN_ON(xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context) != ctx);
> + kfree(ctx);
> + } else if (ctx) {
> + if (rxfh_dev.indir) {
> + for (i = 0; i < dev_indir_size; i++)
> + ethtool_rxfh_context_indir(ctx)[i] = rxfh_dev.indir[i];
> + ctx->indir_configured = 1;
> + }
> + if (rxfh_dev.key) {
> + memcpy(ethtool_rxfh_context_key(ctx), rxfh_dev.key,
> + dev_key_size);
> + ctx->key_configured = 1;
> + }
> + if (rxfh_dev.hfunc != ETH_RSS_HASH_NO_CHANGE)
> + ctx->hfunc = rxfh_dev.hfunc;
> + if (rxfh_dev.input_xfrm != RXH_XFRM_NO_CHANGE)
> + ctx->input_xfrm = rxfh_dev.input_xfrm;
> + }
>
> out:
> kfree(rss_config);
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-20 6:32 ` Przemek Kitszel
@ 2024-06-20 6:37 ` Edward Cree
2024-06-25 7:17 ` Przemek Kitszel
2024-06-25 13:39 ` Edward Cree
1 sibling, 1 reply; 20+ messages in thread
From: Edward Cree @ 2024-06-20 6:37 UTC (permalink / raw)
To: Przemek Kitszel, edward.cree
Cc: linux-net-drivers, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki, davem, kuba, edumazet,
pabeni
On 20/06/2024 07:32, Przemek Kitszel wrote:
> On 6/20/24 07:47, edward.cree@amd.com wrote:
>> From: Edward Cree <ecree.xilinx@gmail.com>
>>
>> Since drivers are still choosing the context IDs, we have to force the
>> XArray to use the ID they've chosen rather than picking one ourselves,
>> and handle the case where they give us an ID that's already in use.
>
> Q: This is a new API, perhaps you could force adopters to convert to
> not choosing ID and switching to allocated one?
Yes, that's exactly what the next patch (#4) does, when it introduces
the new API.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-20 6:37 ` Edward Cree
@ 2024-06-25 7:17 ` Przemek Kitszel
2024-06-25 9:27 ` Edward Cree
0 siblings, 1 reply; 20+ messages in thread
From: Przemek Kitszel @ 2024-06-25 7:17 UTC (permalink / raw)
To: Edward Cree, edward.cree
Cc: linux-net-drivers, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki, davem, kuba, edumazet,
pabeni
On 6/20/24 08:37, Edward Cree wrote:
> On 20/06/2024 07:32, Przemek Kitszel wrote:
>> On 6/20/24 07:47, edward.cree@amd.com wrote:
>>> From: Edward Cree <ecree.xilinx@gmail.com>
>>>
>>> Since drivers are still choosing the context IDs, we have to force the
>>> XArray to use the ID they've chosen rather than picking one ourselves,
>>> and handle the case where they give us an ID that's already in use.
>>
>> Q: This is a new API, perhaps you could force adopters to convert to
>> not choosing ID and switching to allocated one?
>
> Yes, that's exactly what the next patch (#4) does, when it introduces
> the new API.
Thanks, I noticed it only after reading this patch.
I know that there are is v7 already, but I don't know if you just missed
my other comments to this v6 patch, or they are not relevant after you
answering the first question? (Code is not removed in subsequent patch,
so I guess you just missed).
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-25 7:17 ` Przemek Kitszel
@ 2024-06-25 9:27 ` Edward Cree
0 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2024-06-25 9:27 UTC (permalink / raw)
To: Przemek Kitszel, edward.cree
Cc: linux-net-drivers, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki, davem, kuba, edumazet,
pabeni
On 25/06/2024 08:17, Przemek Kitszel wrote:
> I know that there are is v7 already, but I don't know if you just missed
> my other comments to this v6 patch, or they are not relevant after you
> answering the first question? (Code is not removed in subsequent patch,
> so I guess you just missed).
Didn't spot them, sorry.
Will look through and address.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-20 6:32 ` Przemek Kitszel
2024-06-20 6:37 ` Edward Cree
@ 2024-06-25 13:39 ` Edward Cree
2024-06-26 9:05 ` Przemek Kitszel
1 sibling, 1 reply; 20+ messages in thread
From: Edward Cree @ 2024-06-25 13:39 UTC (permalink / raw)
To: Przemek Kitszel, edward.cree
Cc: linux-net-drivers, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki, davem, kuba, edumazet,
pabeni
On 20/06/2024 07:32, Przemek Kitszel wrote:
> On 6/20/24 07:47, edward.cree@amd.com wrote:
>> + return struct_size((struct ethtool_rxfh_context *)0, data, flex_len);
>
> struct_size_t
Yup, will do, thanks for the suggestion.
Don't think that existed yet when I wrote v1 :-D
>> + /* Update rss_ctx tracking */
>> + if (create) {
>> + /* Ideally this should happen before calling the driver,
>> + * so that we can fail more cleanly; but we don't have the
>> + * context ID until the driver picks it, so we have to
>> + * wait until after.
>> + */
>> + if (WARN_ON(xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context))) {
>> + /* context ID reused, our tracking is screwed */
>
> why no error code set?
Because at this point the driver *has* created the context, it's
in the hardware. If we wanted to return failure we'd have to
call the driver again to delete it, and that would still leave
an ugly case where that call fails.
>
>> + kfree(ctx);
>> + goto out;
>> + }
>> + /* Allocate the exact ID the driver gave us */
>> + if (xa_is_err(xa_store(&dev->ethtool->rss_ctx, rxfh.rss_context,
>> + ctx, GFP_KERNEL))) {
>
> this is racy - assuming it is possible that context was set by other
> means (otherwisce you would not xa_load() a few lines above) -
> a concurrent writer could have done this just after you xa_load() call.
I don't expect a concurrent writer - this is all under RTNL.
The xa_load() is there in case we create two contexts
consecutively and the driver gives us the same ID both times.
> so, instead of xa_load() + xa_store() just use xa_insert()
The reason for splitting it up is for the WARN_ON on the
xa_load(). I guess with xa_insert() it would have to be
WARN_ON(xa_insert() == -EBUSY)?
> anyway I feel the pain of trying to support both driver-selected IDs
> and your own
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-25 13:39 ` Edward Cree
@ 2024-06-26 9:05 ` Przemek Kitszel
2024-06-27 14:24 ` Edward Cree
0 siblings, 1 reply; 20+ messages in thread
From: Przemek Kitszel @ 2024-06-26 9:05 UTC (permalink / raw)
To: Edward Cree, edward.cree
Cc: linux-net-drivers, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki, davem, kuba, edumazet,
pabeni
On 6/25/24 15:39, Edward Cree wrote:
> On 20/06/2024 07:32, Przemek Kitszel wrote:
>> On 6/20/24 07:47, edward.cree@amd.com wrote:
>>> + return struct_size((struct ethtool_rxfh_context *)0, data, flex_len);
>>
>> struct_size_t
>
> Yup, will do, thanks for the suggestion.
> Don't think that existed yet when I wrote v1 :-D
yeah, but it's just a nitpick ATM
>
>>> + /* Update rss_ctx tracking */
>>> + if (create) {
>>> + /* Ideally this should happen before calling the driver,
>>> + * so that we can fail more cleanly; but we don't have the
>>> + * context ID until the driver picks it, so we have to
>>> + * wait until after.
>>> + */
>>> + if (WARN_ON(xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context))) {
>>> + /* context ID reused, our tracking is screwed */
>>
>> why no error code set?
>
> Because at this point the driver *has* created the context, it's
> in the hardware. If we wanted to return failure we'd have to
> call the driver again to delete it, and that would still leave
> an ugly case where that call fails.
driver is creating both HW context and ID at the same time, after
you call it from ethtool, eh :(
then my only concern is why do we want to keep old context instead of
update? (my only and last concern for this series by now)
say dumb driver always says "ctx=1" because it does not now better,
but wants to update the context
>
>>
>>> + kfree(ctx);
>>> + goto out;
>>> + }
>>> + /* Allocate the exact ID the driver gave us */
>>> + if (xa_is_err(xa_store(&dev->ethtool->rss_ctx, rxfh.rss_context,
>>> + ctx, GFP_KERNEL))) {
>>
>> this is racy - assuming it is possible that context was set by other
>> means (otherwisce you would not xa_load() a few lines above) -
>> a concurrent writer could have done this just after you xa_load() call.
>
> I don't expect a concurrent writer - this is all under RTNL.
> The xa_load() is there in case we create two contexts
> consecutively and the driver gives us the same ID both times.
thanks, makes sense with the other part of explanation :)
>
>> so, instead of xa_load() + xa_store() just use xa_insert()
>
> The reason for splitting it up is for the WARN_ON on the
> xa_load(). I guess with xa_insert() it would have to be
> WARN_ON(xa_insert() == -EBUSY)?
you need to handle both EBUSY and ENOMEM, both of which you are handling
by separate calls right now, but it is just stylistics at this point
>
>> anyway I feel the pain of trying to support both driver-selected IDs
>> and your own
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-26 9:05 ` Przemek Kitszel
@ 2024-06-27 14:24 ` Edward Cree
2024-06-28 12:15 ` Przemek Kitszel
0 siblings, 1 reply; 20+ messages in thread
From: Edward Cree @ 2024-06-27 14:24 UTC (permalink / raw)
To: Przemek Kitszel, edward.cree
Cc: linux-net-drivers, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki, davem, kuba, edumazet,
pabeni
On 26/06/2024 10:05, Przemek Kitszel wrote:
> On 6/25/24 15:39, Edward Cree wrote:
>> On 20/06/2024 07:32, Przemek Kitszel wrote:
>>> why no error code set?
>>
>> Because at this point the driver *has* created the context, it's
>> in the hardware. If we wanted to return failure we'd have to
>> call the driver again to delete it, and that would still leave
>> an ugly case where that call fails.
>
> driver is creating both HW context and ID at the same time, after
> you call it from ethtool, eh :(
>
> then my only concern is why do we want to keep old context instead of
> update? (my only and last concern for this series by now)
> say dumb driver always says "ctx=1" because it does not now better,
> but wants to update the context
Tbh I'm not sure there's a clear case either way, if driver is
screwing up we don't know why or how. The old context could
still be present too for all we know. So my preference is to
say "we don't know what happened, let's just not touch the
xarray at all".
In any case the WARN_ON should hopefully quickly catch any
drivers that are hitting this, and going forward new drivers
using this API shouldn't get added.
If you still feel strongly this should be changed, please
elaborate further on the reasoning.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray
2024-06-27 14:24 ` Edward Cree
@ 2024-06-28 12:15 ` Przemek Kitszel
0 siblings, 0 replies; 20+ messages in thread
From: Przemek Kitszel @ 2024-06-28 12:15 UTC (permalink / raw)
To: Edward Cree, edward.cree
Cc: linux-net-drivers, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki, davem, kuba, edumazet,
pabeni
On 6/27/24 16:24, Edward Cree wrote:
> On 26/06/2024 10:05, Przemek Kitszel wrote:
>> On 6/25/24 15:39, Edward Cree wrote:
>>> On 20/06/2024 07:32, Przemek Kitszel wrote:
>>>> why no error code set?
>>>
>>> Because at this point the driver *has* created the context, it's
>>> in the hardware. If we wanted to return failure we'd have to
>>> call the driver again to delete it, and that would still leave
>>> an ugly case where that call fails.
>>
>> driver is creating both HW context and ID at the same time, after
>> you call it from ethtool, eh :(
>>
>> then my only concern is why do we want to keep old context instead of
>> update? (my only and last concern for this series by now)
>> say dumb driver always says "ctx=1" because it does not now better,
>> but wants to update the context
>
> Tbh I'm not sure there's a clear case either way, if driver is
> screwing up we don't know why or how. The old context could
> still be present too for all we know. So my preference is to
> say "we don't know what happened, let's just not touch the
> xarray at all".
> In any case the WARN_ON should hopefully quickly catch any
> drivers that are hitting this, and going forward new drivers
> using this API shouldn't get added.
>
> If you still feel strongly this should be changed, please
> elaborate further on the reasoning.
Thanks, it makes sense as currently in the code, works for me!
I'll review v8
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v6 net-next 4/9] net: ethtool: let the core choose RSS context IDs
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
` (2 preceding siblings ...)
2024-06-20 5:47 ` [PATCH v6 net-next 3/9] net: ethtool: record custom RSS contexts in the XArray edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 5/9] net: ethtool: add an extack parameter to new rxfh_context APIs edward.cree
` (4 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
Add a new API to create/modify/remove RSS contexts, that passes in the
newly-chosen context ID (not as a pointer) rather than leaving the
driver to choose it on create. Also pass in the ctx, allowing drivers
to easily use its private data area to store their hardware-specific
state.
Keep the existing .set_rxfh API for now as a fallback, but deprecate it
for custom contexts (rss_context != 0).
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
include/linux/ethtool.h | 40 +++++++++++++++++++++++++++++++++++
net/core/dev.c | 6 +++++-
net/ethtool/ioctl.c | 46 ++++++++++++++++++++++++++++++-----------
3 files changed, 79 insertions(+), 13 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 283ba4aff623..d9570b97154b 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -722,6 +722,10 @@ struct ethtool_rxfh_param {
* RSS.
* @rxfh_priv_size: size of the driver private data area the core should
* allocate for an RSS context (in &struct ethtool_rxfh_context).
+ * @rxfh_max_context_id: maximum (exclusive) supported RSS context ID. If this
+ * is zero then the core may choose any (nonzero) ID, otherwise the core
+ * will only use IDs strictly less than this value, as the @rss_context
+ * argument to @create_rxfh_context and friends.
* @supported_coalesce_params: supported types of interrupt coalescing.
* @supported_ring_params: supported ring params.
* @get_drvinfo: Report driver/device information. Modern drivers no
@@ -818,6 +822,32 @@ struct ethtool_rxfh_param {
* will remain unchanged.
* Returns a negative error code or zero. An error code must be returned
* if at least one unsupported change was requested.
+ * @create_rxfh_context: Create a new RSS context with the specified RX flow
+ * hash indirection table, hash key, and hash function.
+ * The &struct ethtool_rxfh_context for this context is passed in @ctx;
+ * note that the indir table, hkey and hfunc are not yet populated as
+ * of this call. The driver does not need to update these; the core
+ * will do so if this op succeeds.
+ * However, if @rxfh.indir is set to %NULL, the driver must update the
+ * indir table in @ctx with the (default or inherited) table actually in
+ * use; similarly, if @rxfh.key is %NULL, @rxfh.hfunc is
+ * %ETH_RSS_HASH_NO_CHANGE, or @rxfh.input_xfrm is %RXH_XFRM_NO_CHANGE,
+ * the driver should update the corresponding information in @ctx.
+ * If the driver provides this method, it must also provide
+ * @modify_rxfh_context and @remove_rxfh_context.
+ * Returns a negative error code or zero.
+ * @modify_rxfh_context: Reconfigure the specified RSS context. Allows setting
+ * the contents of the RX flow hash indirection table, hash key, and/or
+ * hash function associated with the given context.
+ * Parameters which are set to %NULL or zero will remain unchanged.
+ * The &struct ethtool_rxfh_context for this context is passed in @ctx;
+ * note that it will still contain the *old* settings. The driver does
+ * not need to update these; the core will do so if this op succeeds.
+ * Returns a negative error code or zero. An error code must be returned
+ * if at least one unsupported change was requested.
+ * @remove_rxfh_context: Remove the specified RSS context.
+ * The &struct ethtool_rxfh_context for this context is passed in @ctx.
+ * Returns a negative error code or zero.
* @get_channels: Get number of channels.
* @set_channels: Set number of channels. Returns a negative error code or
* zero.
@@ -906,6 +936,7 @@ struct ethtool_ops {
u32 cap_rss_ctx_supported:1;
u32 cap_rss_sym_xor_supported:1;
u16 rxfh_priv_size;
+ u32 rxfh_max_context_id;
u32 supported_coalesce_params;
u32 supported_ring_params;
void (*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
@@ -968,6 +999,15 @@ struct ethtool_ops {
int (*get_rxfh)(struct net_device *, struct ethtool_rxfh_param *);
int (*set_rxfh)(struct net_device *, struct ethtool_rxfh_param *,
struct netlink_ext_ack *extack);
+ int (*create_rxfh_context)(struct net_device *,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh);
+ int (*modify_rxfh_context)(struct net_device *,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh);
+ int (*remove_rxfh_context)(struct net_device *,
+ struct ethtool_rxfh_context *ctx,
+ u32 rss_context);
void (*get_channels)(struct net_device *, struct ethtool_channels *);
int (*set_channels)(struct net_device *, struct ethtool_channels *);
int (*get_dump_flag)(struct net_device *, struct ethtool_dump *);
diff --git a/net/core/dev.c b/net/core/dev.c
index 78465b69dc68..e875b0a579b4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11203,7 +11203,11 @@ static void netdev_rss_contexts_free(struct net_device *dev)
rxfh.rss_delete = true;
xa_erase(&dev->ethtool->rss_ctx, context);
- dev->ethtool_ops->set_rxfh(dev, &rxfh, NULL);
+ if (dev->ethtool_ops->create_rxfh_context)
+ dev->ethtool_ops->remove_rxfh_context(dev, ctx,
+ context);
+ else
+ dev->ethtool_ops->set_rxfh(dev, &rxfh, NULL);
kfree(ctx);
}
xa_destroy(&dev->ethtool->rss_ctx);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index f879deb6ac4e..2b75d84c3078 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1392,9 +1392,24 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
}
ctx->indir_size = dev_indir_size;
ctx->key_size = dev_key_size;
- ctx->hfunc = rxfh.hfunc;
- ctx->input_xfrm = rxfh.input_xfrm;
ctx->priv_size = ops->rxfh_priv_size;
+ /* Initialise to an empty context */
+ ctx->hfunc = ETH_RSS_HASH_NO_CHANGE;
+ ctx->input_xfrm = RXH_XFRM_NO_CHANGE;
+ if (ops->create_rxfh_context) {
+ u32 limit = ops->rxfh_max_context_id ?: U32_MAX;
+ u32 ctx_id;
+
+ /* driver uses new API, core allocates ID */
+ ret = xa_alloc(&dev->ethtool->rss_ctx, &ctx_id, ctx,
+ XA_LIMIT(1, limit), GFP_KERNEL_ACCOUNT);
+ if (ret < 0) {
+ kfree(ctx);
+ goto out;
+ }
+ WARN_ON(!ctx_id); /* can't happen */
+ rxfh.rss_context = ctx_id;
+ }
} else if (rxfh.rss_context) {
ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
if (!ctx) {
@@ -1406,11 +1421,24 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
rxfh_dev.rss_context = rxfh.rss_context;
rxfh_dev.input_xfrm = rxfh.input_xfrm;
- ret = ops->set_rxfh(dev, &rxfh_dev, extack);
- if (ret) {
+ if (rxfh.rss_context && ops->create_rxfh_context) {
if (create)
+ ret = ops->create_rxfh_context(dev, ctx, &rxfh_dev);
+ else if (rxfh_dev.rss_delete)
+ ret = ops->remove_rxfh_context(dev, ctx,
+ rxfh.rss_context);
+ else
+ ret = ops->modify_rxfh_context(dev, ctx, &rxfh_dev);
+ } else {
+ ret = ops->set_rxfh(dev, &rxfh_dev, extack);
+ }
+ if (ret) {
+ if (create) {
/* failed to create, free our new tracking entry */
+ if (ops->create_rxfh_context)
+ xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context);
kfree(ctx);
+ }
goto out;
}
@@ -1426,12 +1454,8 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
dev->priv_flags |= IFF_RXFH_CONFIGURED;
}
/* Update rss_ctx tracking */
- if (create) {
- /* Ideally this should happen before calling the driver,
- * so that we can fail more cleanly; but we don't have the
- * context ID until the driver picks it, so we have to
- * wait until after.
- */
+ if (create && !ops->create_rxfh_context) {
+ /* driver uses old API, it chose context ID */
if (WARN_ON(xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context))) {
/* context ID reused, our tracking is screwed */
kfree(ctx);
@@ -1443,8 +1467,6 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
kfree(ctx);
goto out;
}
- ctx->indir_configured = rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE;
- ctx->key_configured = !!rxfh.key_size;
}
if (rxfh_dev.rss_delete) {
WARN_ON(xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context) != ctx);
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v6 net-next 5/9] net: ethtool: add an extack parameter to new rxfh_context APIs
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
` (3 preceding siblings ...)
2024-06-20 5:47 ` [PATCH v6 net-next 4/9] net: ethtool: let the core choose RSS context IDs edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 6/9] net: ethtool: add a mutex protecting RSS contexts edward.cree
` (3 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
Currently passed as NULL, but will allow drivers to report back errors
when ethnl support for these ops is added.
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
include/linux/ethtool.h | 9 ++++++---
net/core/dev.c | 2 +-
net/ethtool/ioctl.c | 9 ++++++---
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index d9570b97154b..f8688e77ca62 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -1001,13 +1001,16 @@ struct ethtool_ops {
struct netlink_ext_ack *extack);
int (*create_rxfh_context)(struct net_device *,
struct ethtool_rxfh_context *ctx,
- const struct ethtool_rxfh_param *rxfh);
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack);
int (*modify_rxfh_context)(struct net_device *,
struct ethtool_rxfh_context *ctx,
- const struct ethtool_rxfh_param *rxfh);
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack);
int (*remove_rxfh_context)(struct net_device *,
struct ethtool_rxfh_context *ctx,
- u32 rss_context);
+ u32 rss_context,
+ struct netlink_ext_ack *extack);
void (*get_channels)(struct net_device *, struct ethtool_channels *);
int (*set_channels)(struct net_device *, struct ethtool_channels *);
int (*get_dump_flag)(struct net_device *, struct ethtool_dump *);
diff --git a/net/core/dev.c b/net/core/dev.c
index e875b0a579b4..9ca9424ea8b1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11205,7 +11205,7 @@ static void netdev_rss_contexts_free(struct net_device *dev)
xa_erase(&dev->ethtool->rss_ctx, context);
if (dev->ethtool_ops->create_rxfh_context)
dev->ethtool_ops->remove_rxfh_context(dev, ctx,
- context);
+ context, NULL);
else
dev->ethtool_ops->set_rxfh(dev, &rxfh, NULL);
kfree(ctx);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 2b75d84c3078..244e565e1365 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1423,12 +1423,15 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
if (rxfh.rss_context && ops->create_rxfh_context) {
if (create)
- ret = ops->create_rxfh_context(dev, ctx, &rxfh_dev);
+ ret = ops->create_rxfh_context(dev, ctx, &rxfh_dev,
+ extack);
else if (rxfh_dev.rss_delete)
ret = ops->remove_rxfh_context(dev, ctx,
- rxfh.rss_context);
+ rxfh.rss_context,
+ extack);
else
- ret = ops->modify_rxfh_context(dev, ctx, &rxfh_dev);
+ ret = ops->modify_rxfh_context(dev, ctx, &rxfh_dev,
+ extack);
} else {
ret = ops->set_rxfh(dev, &rxfh_dev, extack);
}
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v6 net-next 6/9] net: ethtool: add a mutex protecting RSS contexts
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
` (4 preceding siblings ...)
2024-06-20 5:47 ` [PATCH v6 net-next 5/9] net: ethtool: add an extack parameter to new rxfh_context APIs edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 7/9] sfc: use new rxfh_context API edward.cree
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
While this is not needed to serialise the ethtool entry points (which
are all under RTNL), drivers may have cause to asynchronously access
dev->ethtool->rss_ctx; taking dev->ethtool->rss_lock allows them to
do this safely without needing to take the RTNL.
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
include/linux/ethtool.h | 3 +++
net/core/dev.c | 5 +++++
net/ethtool/ioctl.c | 7 +++++++
3 files changed, 15 insertions(+)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f8688e77ca62..0d27e13952ad 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -1098,10 +1098,13 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
/**
* struct ethtool_netdev_state - per-netdevice state for ethtool features
* @rss_ctx: XArray of custom RSS contexts
+ * @rss_lock: Protects entries in @rss_ctx. May be taken from
+ * within RTNL.
* @wol_enabled: Wake-on-LAN is enabled
*/
struct ethtool_netdev_state {
struct xarray rss_ctx;
+ struct mutex rss_lock;
unsigned wol_enabled:1;
};
diff --git a/net/core/dev.c b/net/core/dev.c
index 9ca9424ea8b1..4cd988b8a74a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10287,6 +10287,7 @@ int register_netdevice(struct net_device *dev)
/* rss ctx ID 0 is reserved for the default context, start from 1 */
xa_init_flags(&dev->ethtool->rss_ctx, XA_FLAGS_ALLOC1);
+ mutex_init(&dev->ethtool->rss_lock);
spin_lock_init(&dev->addr_list_lock);
netdev_set_addr_lockdep_class(dev);
@@ -11192,6 +11193,7 @@ static void netdev_rss_contexts_free(struct net_device *dev)
struct ethtool_rxfh_context *ctx;
unsigned long context;
+ mutex_lock(&dev->ethtool->rss_lock);
xa_for_each(&dev->ethtool->rss_ctx, context, ctx) {
struct ethtool_rxfh_param rxfh;
@@ -11211,6 +11213,7 @@ static void netdev_rss_contexts_free(struct net_device *dev)
kfree(ctx);
}
xa_destroy(&dev->ethtool->rss_ctx);
+ mutex_unlock(&dev->ethtool->rss_lock);
}
/**
@@ -11323,6 +11326,8 @@ void unregister_netdevice_many_notify(struct list_head *head,
if (dev->netdev_ops->ndo_uninit)
dev->netdev_ops->ndo_uninit(dev);
+ mutex_destroy(&dev->ethtool->rss_lock);
+
if (skb)
rtmsg_ifinfo_send(skb, dev, GFP_KERNEL, portid, nlh);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 244e565e1365..9d2d677770db 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1282,6 +1282,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
struct netlink_ext_ack *extack = NULL;
struct ethtool_rxnfc rx_rings;
struct ethtool_rxfh rxfh;
+ bool locked = false; /* dev->ethtool->rss_lock taken */
u32 indir_bytes = 0;
bool create = false;
u8 *rss_config;
@@ -1377,6 +1378,10 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
}
}
+ if (rxfh.rss_context) {
+ mutex_lock(&dev->ethtool->rss_lock);
+ locked = true;
+ }
if (create) {
if (rxfh_dev.rss_delete) {
ret = -EINVAL;
@@ -1492,6 +1497,8 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
}
out:
+ if (locked)
+ mutex_unlock(&dev->ethtool->rss_lock);
kfree(rss_config);
return ret;
}
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v6 net-next 7/9] sfc: use new rxfh_context API
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
` (5 preceding siblings ...)
2024-06-20 5:47 ` [PATCH v6 net-next 6/9] net: ethtool: add a mutex protecting RSS contexts edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for get_rxfh on custom RSS contexts edward.cree
2024-06-20 5:47 ` [PATCH v6 net-next 9/9] sfc: remove get_rxfh_context dead code edward.cree
8 siblings, 0 replies; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
The core is now responsible for allocating IDs and a memory region for
us to store our state (struct efx_rss_context_priv), so we no longer
need efx_alloc_rss_context_entry() and friends.
Since the contexts are now maintained by the core, use the core's lock
(net_dev->ethtool->rss_lock), rather than our own mutex (efx->rss_lock),
to serialise access against changes; and remove the now-unused
efx->rss_lock from struct efx_nic.
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
drivers/net/ethernet/sfc/ef10.c | 2 +-
drivers/net/ethernet/sfc/ef100_ethtool.c | 4 +
drivers/net/ethernet/sfc/efx.c | 2 +-
drivers/net/ethernet/sfc/efx.h | 2 +-
drivers/net/ethernet/sfc/efx_common.c | 10 +-
drivers/net/ethernet/sfc/ethtool.c | 4 +
drivers/net/ethernet/sfc/ethtool_common.c | 152 ++++++++++++----------
drivers/net/ethernet/sfc/ethtool_common.h | 12 ++
drivers/net/ethernet/sfc/mcdi_filters.c | 135 ++++++++++---------
drivers/net/ethernet/sfc/mcdi_filters.h | 8 +-
drivers/net/ethernet/sfc/net_driver.h | 28 ++--
drivers/net/ethernet/sfc/rx_common.c | 64 ++-------
drivers/net/ethernet/sfc/rx_common.h | 8 +-
13 files changed, 212 insertions(+), 219 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 8fa6c0e9195b..7d69302ffa0a 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -1396,7 +1396,7 @@ static void efx_ef10_table_reset_mc_allocations(struct efx_nic *efx)
efx_mcdi_filter_table_reset_mc_allocations(efx);
nic_data->must_restore_piobufs = true;
efx_ef10_forget_old_piobufs(efx);
- efx->rss_context.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+ efx->rss_context.priv.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
/* Driver-created vswitches and vports must be re-created */
nic_data->must_probe_vswitching = true;
diff --git a/drivers/net/ethernet/sfc/ef100_ethtool.c b/drivers/net/ethernet/sfc/ef100_ethtool.c
index cf55202b3a7b..896ffca4aee2 100644
--- a/drivers/net/ethernet/sfc/ef100_ethtool.c
+++ b/drivers/net/ethernet/sfc/ef100_ethtool.c
@@ -59,8 +59,12 @@ const struct ethtool_ops ef100_ethtool_ops = {
.get_rxfh_indir_size = efx_ethtool_get_rxfh_indir_size,
.get_rxfh_key_size = efx_ethtool_get_rxfh_key_size,
+ .rxfh_priv_size = sizeof(struct efx_rss_context_priv),
.get_rxfh = efx_ethtool_get_rxfh,
.set_rxfh = efx_ethtool_set_rxfh,
+ .create_rxfh_context = efx_ethtool_create_rxfh_context,
+ .modify_rxfh_context = efx_ethtool_modify_rxfh_context,
+ .remove_rxfh_context = efx_ethtool_remove_rxfh_context,
.get_module_info = efx_ethtool_get_module_info,
.get_module_eeprom = efx_ethtool_get_module_eeprom,
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index e9d9de8e648a..6f1a01ded7d4 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -299,7 +299,7 @@ static int efx_probe_nic(struct efx_nic *efx)
if (efx->n_channels > 1)
netdev_rss_key_fill(efx->rss_context.rx_hash_key,
sizeof(efx->rss_context.rx_hash_key));
- efx_set_default_rx_indir_table(efx, &efx->rss_context);
+ efx_set_default_rx_indir_table(efx, efx->rss_context.rx_indir_table);
/* Initialise the interrupt moderation settings */
efx->irq_mod_step_us = DIV_ROUND_UP(efx->timer_quantum_ns, 1000);
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index 48d3623735ba..7a6cab883d66 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -158,7 +158,7 @@ static inline s32 efx_filter_get_rx_ids(struct efx_nic *efx,
}
/* RSS contexts */
-static inline bool efx_rss_active(struct efx_rss_context *ctx)
+static inline bool efx_rss_active(struct efx_rss_context_priv *ctx)
{
return ctx->context_id != EFX_MCDI_RSS_CONTEXT_INVALID;
}
diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
index 4ebd5ae23eca..13cf647051af 100644
--- a/drivers/net/ethernet/sfc/efx_common.c
+++ b/drivers/net/ethernet/sfc/efx_common.c
@@ -714,7 +714,7 @@ void efx_reset_down(struct efx_nic *efx, enum reset_type method)
mutex_lock(&efx->mac_lock);
down_write(&efx->filter_sem);
- mutex_lock(&efx->rss_lock);
+ mutex_lock(&efx->net_dev->ethtool->rss_lock);
efx->type->fini(efx);
}
@@ -777,7 +777,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
if (efx->type->rx_restore_rss_contexts)
efx->type->rx_restore_rss_contexts(efx);
- mutex_unlock(&efx->rss_lock);
+ mutex_unlock(&efx->net_dev->ethtool->rss_lock);
efx->type->filter_table_restore(efx);
up_write(&efx->filter_sem);
@@ -793,7 +793,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
fail:
efx->port_initialized = false;
- mutex_unlock(&efx->rss_lock);
+ mutex_unlock(&efx->net_dev->ethtool->rss_lock);
up_write(&efx->filter_sem);
mutex_unlock(&efx->mac_lock);
@@ -1000,9 +1000,7 @@ int efx_init_struct(struct efx_nic *efx, struct pci_dev *pci_dev)
efx->type->rx_hash_offset - efx->type->rx_prefix_size;
efx->rx_packet_ts_offset =
efx->type->rx_ts_offset - efx->type->rx_prefix_size;
- INIT_LIST_HEAD(&efx->rss_context.list);
- efx->rss_context.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
- mutex_init(&efx->rss_lock);
+ efx->rss_context.priv.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
efx->vport_id = EVB_PORT_ID_ASSIGNED;
spin_lock_init(&efx->stats_lock);
efx->vi_stride = EFX_DEFAULT_VI_STRIDE;
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 37c69c8d90b1..0f5c68b8bab7 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -268,8 +268,12 @@ const struct ethtool_ops efx_ethtool_ops = {
.set_rxnfc = efx_ethtool_set_rxnfc,
.get_rxfh_indir_size = efx_ethtool_get_rxfh_indir_size,
.get_rxfh_key_size = efx_ethtool_get_rxfh_key_size,
+ .rxfh_priv_size = sizeof(struct efx_rss_context_priv),
.get_rxfh = efx_ethtool_get_rxfh,
.set_rxfh = efx_ethtool_set_rxfh,
+ .create_rxfh_context = efx_ethtool_create_rxfh_context,
+ .modify_rxfh_context = efx_ethtool_modify_rxfh_context,
+ .remove_rxfh_context = efx_ethtool_remove_rxfh_context,
.get_ts_info = efx_ethtool_get_ts_info,
.get_module_info = efx_ethtool_get_module_info,
.get_module_eeprom = efx_ethtool_get_module_eeprom,
diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
index 7d5e5db4eac5..0a8d2c9ffce6 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/ethtool_common.c
@@ -820,10 +820,10 @@ int efx_ethtool_get_rxnfc(struct net_device *net_dev,
return 0;
case ETHTOOL_GRXFH: {
- struct efx_rss_context *ctx = &efx->rss_context;
+ struct efx_rss_context_priv *ctx = &efx->rss_context.priv;
__u64 data;
- mutex_lock(&efx->rss_lock);
+ mutex_lock(&net_dev->ethtool->rss_lock);
if (info->flow_type & FLOW_RSS && info->rss_context) {
ctx = efx_find_rss_context_entry(efx, info->rss_context);
if (!ctx) {
@@ -864,7 +864,7 @@ int efx_ethtool_get_rxnfc(struct net_device *net_dev,
out_setdata_unlock:
info->data = data;
out_unlock:
- mutex_unlock(&efx->rss_lock);
+ mutex_unlock(&net_dev->ethtool->rss_lock);
return rc;
}
@@ -1167,31 +1167,33 @@ static int efx_ethtool_get_rxfh_context(struct net_device *net_dev,
struct ethtool_rxfh_param *rxfh)
{
struct efx_nic *efx = efx_netdev_priv(net_dev);
- struct efx_rss_context *ctx;
+ struct efx_rss_context_priv *ctx_priv;
+ struct efx_rss_context ctx;
int rc = 0;
if (!efx->type->rx_pull_rss_context_config)
return -EOPNOTSUPP;
- mutex_lock(&efx->rss_lock);
- ctx = efx_find_rss_context_entry(efx, rxfh->rss_context);
- if (!ctx) {
+ mutex_lock(&net_dev->ethtool->rss_lock);
+ ctx_priv = efx_find_rss_context_entry(efx, rxfh->rss_context);
+ if (!ctx_priv) {
rc = -ENOENT;
goto out_unlock;
}
- rc = efx->type->rx_pull_rss_context_config(efx, ctx);
+ ctx.priv = *ctx_priv;
+ rc = efx->type->rx_pull_rss_context_config(efx, &ctx);
if (rc)
goto out_unlock;
rxfh->hfunc = ETH_RSS_HASH_TOP;
if (rxfh->indir)
- memcpy(rxfh->indir, ctx->rx_indir_table,
- sizeof(ctx->rx_indir_table));
+ memcpy(rxfh->indir, ctx.rx_indir_table,
+ sizeof(ctx.rx_indir_table));
if (rxfh->key)
- memcpy(rxfh->key, ctx->rx_hash_key,
+ memcpy(rxfh->key, ctx.rx_hash_key,
efx->type->rx_hash_key_size);
out_unlock:
- mutex_unlock(&efx->rss_lock);
+ mutex_unlock(&net_dev->ethtool->rss_lock);
return rc;
}
@@ -1218,68 +1220,85 @@ int efx_ethtool_get_rxfh(struct net_device *net_dev,
return 0;
}
-static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
- struct ethtool_rxfh_param *rxfh,
- struct netlink_ext_ack *extack)
+int efx_ethtool_modify_rxfh_context(struct net_device *net_dev,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack)
{
struct efx_nic *efx = efx_netdev_priv(net_dev);
- u32 *rss_context = &rxfh->rss_context;
- struct efx_rss_context *ctx;
- u32 *indir = rxfh->indir;
- bool allocated = false;
- u8 *key = rxfh->key;
- int rc;
+ struct efx_rss_context_priv *priv;
+ const u32 *indir = rxfh->indir;
+ const u8 *key = rxfh->key;
- if (!efx->type->rx_push_rss_context_config)
+ if (!efx->type->rx_push_rss_context_config) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "NIC type does not support custom contexts");
return -EOPNOTSUPP;
-
- mutex_lock(&efx->rss_lock);
-
- if (*rss_context == ETH_RXFH_CONTEXT_ALLOC) {
- if (rxfh->rss_delete) {
- /* alloc + delete == Nothing to do */
- rc = -EINVAL;
- goto out_unlock;
- }
- ctx = efx_alloc_rss_context_entry(efx);
- if (!ctx) {
- rc = -ENOMEM;
- goto out_unlock;
- }
- ctx->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
- /* Initialise indir table and key to defaults */
- efx_set_default_rx_indir_table(efx, ctx);
- netdev_rss_key_fill(ctx->rx_hash_key, sizeof(ctx->rx_hash_key));
- allocated = true;
- } else {
- ctx = efx_find_rss_context_entry(efx, *rss_context);
- if (!ctx) {
- rc = -ENOENT;
- goto out_unlock;
- }
}
-
- if (rxfh->rss_delete) {
- /* delete this context */
- rc = efx->type->rx_push_rss_context_config(efx, ctx, NULL, NULL);
- if (!rc)
- efx_free_rss_context_entry(ctx);
- goto out_unlock;
+ /* Hash function is Toeplitz, cannot be changed */
+ if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
+ rxfh->hfunc != ETH_RSS_HASH_TOP) {
+ NL_SET_ERR_MSG_MOD(extack, "Only Toeplitz hash is supported");
+ return -EOPNOTSUPP;
}
+ priv = ethtool_rxfh_context_priv(ctx);
+
if (!key)
- key = ctx->rx_hash_key;
+ key = ethtool_rxfh_context_key(ctx);
if (!indir)
- indir = ctx->rx_indir_table;
+ indir = ethtool_rxfh_context_indir(ctx);
- rc = efx->type->rx_push_rss_context_config(efx, ctx, indir, key);
- if (rc && allocated)
- efx_free_rss_context_entry(ctx);
- else
- *rss_context = ctx->user_id;
-out_unlock:
- mutex_unlock(&efx->rss_lock);
- return rc;
+ return efx->type->rx_push_rss_context_config(efx, priv, indir, key,
+ false);
+}
+
+int efx_ethtool_create_rxfh_context(struct net_device *net_dev,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack)
+{
+ struct efx_nic *efx = efx_netdev_priv(net_dev);
+ struct efx_rss_context_priv *priv;
+
+ priv = ethtool_rxfh_context_priv(ctx);
+
+ priv->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+ priv->rx_hash_udp_4tuple = false;
+ /* Generate default indir table and/or key if not specified.
+ * We use ctx as a place to store these; this is fine because
+ * we're doing a create, so if we fail then the ctx will just
+ * be deleted.
+ */
+ if (!rxfh->indir)
+ efx_set_default_rx_indir_table(efx, ethtool_rxfh_context_indir(ctx));
+ if (!rxfh->key)
+ netdev_rss_key_fill(ethtool_rxfh_context_key(ctx),
+ ctx->key_size);
+ if (rxfh->hfunc == ETH_RSS_HASH_NO_CHANGE)
+ ctx->hfunc = ETH_RSS_HASH_TOP;
+ if (rxfh->input_xfrm == RXH_XFRM_NO_CHANGE)
+ ctx->input_xfrm = 0;
+ return efx_ethtool_modify_rxfh_context(net_dev, ctx, rxfh, extack);
+}
+
+int efx_ethtool_remove_rxfh_context(struct net_device *net_dev,
+ struct ethtool_rxfh_context *ctx,
+ u32 rss_context,
+ struct netlink_ext_ack *extack)
+{
+ struct efx_nic *efx = efx_netdev_priv(net_dev);
+ struct efx_rss_context_priv *priv;
+
+ if (!efx->type->rx_push_rss_context_config) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "NIC type does not support custom contexts");
+ return -EOPNOTSUPP;
+ }
+
+ priv = ethtool_rxfh_context_priv(ctx);
+ return efx->type->rx_push_rss_context_config(efx, priv, NULL, NULL,
+ true);
}
int efx_ethtool_set_rxfh(struct net_device *net_dev,
@@ -1295,8 +1314,9 @@ int efx_ethtool_set_rxfh(struct net_device *net_dev,
rxfh->hfunc != ETH_RSS_HASH_TOP)
return -EOPNOTSUPP;
- if (rxfh->rss_context)
- return efx_ethtool_set_rxfh_context(net_dev, rxfh, extack);
+ /* Custom contexts should use new API */
+ if (WARN_ON_ONCE(rxfh->rss_context))
+ return -EIO;
if (!indir && !key)
return 0;
diff --git a/drivers/net/ethernet/sfc/ethtool_common.h b/drivers/net/ethernet/sfc/ethtool_common.h
index a680e5980213..fc52e891637d 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.h
+++ b/drivers/net/ethernet/sfc/ethtool_common.h
@@ -49,6 +49,18 @@ int efx_ethtool_get_rxfh(struct net_device *net_dev,
int efx_ethtool_set_rxfh(struct net_device *net_dev,
struct ethtool_rxfh_param *rxfh,
struct netlink_ext_ack *extack);
+int efx_ethtool_create_rxfh_context(struct net_device *net_dev,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack);
+int efx_ethtool_modify_rxfh_context(struct net_device *net_dev,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack);
+int efx_ethtool_remove_rxfh_context(struct net_device *net_dev,
+ struct ethtool_rxfh_context *ctx,
+ u32 rss_context,
+ struct netlink_ext_ack *extack);
int efx_ethtool_reset(struct net_device *net_dev, u32 *flags);
int efx_ethtool_get_module_eeprom(struct net_device *net_dev,
struct ethtool_eeprom *ee,
diff --git a/drivers/net/ethernet/sfc/mcdi_filters.c b/drivers/net/ethernet/sfc/mcdi_filters.c
index 4ff6586116ee..6ef96292909a 100644
--- a/drivers/net/ethernet/sfc/mcdi_filters.c
+++ b/drivers/net/ethernet/sfc/mcdi_filters.c
@@ -194,7 +194,7 @@ efx_mcdi_filter_push_prep_set_match_fields(struct efx_nic *efx,
static void efx_mcdi_filter_push_prep(struct efx_nic *efx,
const struct efx_filter_spec *spec,
efx_dword_t *inbuf, u64 handle,
- struct efx_rss_context *ctx,
+ struct efx_rss_context_priv *ctx,
bool replacing)
{
u32 flags = spec->flags;
@@ -245,7 +245,7 @@ static void efx_mcdi_filter_push_prep(struct efx_nic *efx,
static int efx_mcdi_filter_push(struct efx_nic *efx,
const struct efx_filter_spec *spec, u64 *handle,
- struct efx_rss_context *ctx, bool replacing)
+ struct efx_rss_context_priv *ctx, bool replacing)
{
MCDI_DECLARE_BUF(inbuf, MC_CMD_FILTER_OP_EXT_IN_LEN);
MCDI_DECLARE_BUF(outbuf, MC_CMD_FILTER_OP_EXT_OUT_LEN);
@@ -345,9 +345,9 @@ static s32 efx_mcdi_filter_insert_locked(struct efx_nic *efx,
bool replace_equal)
{
DECLARE_BITMAP(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT);
+ struct efx_rss_context_priv *ctx = NULL;
struct efx_mcdi_filter_table *table;
struct efx_filter_spec *saved_spec;
- struct efx_rss_context *ctx = NULL;
unsigned int match_pri, hash;
unsigned int priv_flags;
bool rss_locked = false;
@@ -380,12 +380,12 @@ static s32 efx_mcdi_filter_insert_locked(struct efx_nic *efx,
bitmap_zero(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT);
if (spec->flags & EFX_FILTER_FLAG_RX_RSS) {
- mutex_lock(&efx->rss_lock);
+ mutex_lock(&efx->net_dev->ethtool->rss_lock);
rss_locked = true;
if (spec->rss_context)
ctx = efx_find_rss_context_entry(efx, spec->rss_context);
else
- ctx = &efx->rss_context;
+ ctx = &efx->rss_context.priv;
if (!ctx) {
rc = -ENOENT;
goto out_unlock;
@@ -548,7 +548,7 @@ static s32 efx_mcdi_filter_insert_locked(struct efx_nic *efx,
out_unlock:
if (rss_locked)
- mutex_unlock(&efx->rss_lock);
+ mutex_unlock(&efx->net_dev->ethtool->rss_lock);
up_write(&table->lock);
return rc;
}
@@ -611,13 +611,13 @@ static int efx_mcdi_filter_remove_internal(struct efx_nic *efx,
new_spec.priority = EFX_FILTER_PRI_AUTO;
new_spec.flags = (EFX_FILTER_FLAG_RX |
- (efx_rss_active(&efx->rss_context) ?
+ (efx_rss_active(&efx->rss_context.priv) ?
EFX_FILTER_FLAG_RX_RSS : 0));
new_spec.dmaq_id = 0;
new_spec.rss_context = 0;
rc = efx_mcdi_filter_push(efx, &new_spec,
&table->entry[filter_idx].handle,
- &efx->rss_context,
+ &efx->rss_context.priv,
true);
if (rc == 0)
@@ -764,7 +764,7 @@ static int efx_mcdi_filter_insert_addr_list(struct efx_nic *efx,
ids = vlan->uc;
}
- filter_flags = efx_rss_active(&efx->rss_context) ? EFX_FILTER_FLAG_RX_RSS : 0;
+ filter_flags = efx_rss_active(&efx->rss_context.priv) ? EFX_FILTER_FLAG_RX_RSS : 0;
/* Insert/renew filters */
for (i = 0; i < addr_count; i++) {
@@ -833,7 +833,7 @@ static int efx_mcdi_filter_insert_def(struct efx_nic *efx,
int rc;
u16 *id;
- filter_flags = efx_rss_active(&efx->rss_context) ? EFX_FILTER_FLAG_RX_RSS : 0;
+ filter_flags = efx_rss_active(&efx->rss_context.priv) ? EFX_FILTER_FLAG_RX_RSS : 0;
efx_filter_init_rx(&spec, EFX_FILTER_PRI_AUTO, filter_flags, 0);
@@ -1375,8 +1375,8 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
struct efx_mcdi_filter_table *table = efx->filter_state;
unsigned int invalid_filters = 0, failed = 0;
struct efx_mcdi_filter_vlan *vlan;
+ struct efx_rss_context_priv *ctx;
struct efx_filter_spec *spec;
- struct efx_rss_context *ctx;
unsigned int filter_idx;
u32 mcdi_flags;
int match_pri;
@@ -1388,7 +1388,7 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
return;
down_write(&table->lock);
- mutex_lock(&efx->rss_lock);
+ mutex_lock(&efx->net_dev->ethtool->rss_lock);
for (filter_idx = 0; filter_idx < EFX_MCDI_FILTER_TBL_ROWS; filter_idx++) {
spec = efx_mcdi_filter_entry_spec(table, filter_idx);
@@ -1407,7 +1407,7 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
if (spec->rss_context)
ctx = efx_find_rss_context_entry(efx, spec->rss_context);
else
- ctx = &efx->rss_context;
+ ctx = &efx->rss_context.priv;
if (spec->flags & EFX_FILTER_FLAG_RX_RSS) {
if (!ctx) {
netif_warn(efx, drv, efx->net_dev,
@@ -1444,7 +1444,7 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
}
}
- mutex_unlock(&efx->rss_lock);
+ mutex_unlock(&efx->net_dev->ethtool->rss_lock);
up_write(&table->lock);
/*
@@ -1861,7 +1861,8 @@ bool efx_mcdi_filter_rfs_expire_one(struct efx_nic *efx, u32 flow_id,
RSS_MODE_HASH_ADDRS << MC_CMD_RSS_CONTEXT_GET_FLAGS_OUT_UDP_IPV6_RSS_MODE_LBN |\
RSS_MODE_HASH_ADDRS << MC_CMD_RSS_CONTEXT_GET_FLAGS_OUT_OTHER_IPV6_RSS_MODE_LBN)
-int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context, u32 *flags)
+static int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context,
+ u32 *flags)
{
/*
* Firmware had a bug (sfc bug 61952) where it would not actually
@@ -1909,8 +1910,8 @@ int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context, u32 *flags)
* Defaults are 4-tuple for TCP and 2-tuple for UDP and other-IP, so we
* just need to set the UDP ports flags (for both IP versions).
*/
-void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
- struct efx_rss_context *ctx)
+static void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
+ struct efx_rss_context_priv *ctx)
{
MCDI_DECLARE_BUF(inbuf, MC_CMD_RSS_CONTEXT_SET_FLAGS_IN_LEN);
u32 flags;
@@ -1931,7 +1932,7 @@ void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
}
static int efx_mcdi_filter_alloc_rss_context(struct efx_nic *efx, bool exclusive,
- struct efx_rss_context *ctx,
+ struct efx_rss_context_priv *ctx,
unsigned *context_size)
{
MCDI_DECLARE_BUF(inbuf, MC_CMD_RSS_CONTEXT_ALLOC_IN_LEN);
@@ -2032,25 +2033,26 @@ void efx_mcdi_rx_free_indir_table(struct efx_nic *efx)
{
int rc;
- if (efx->rss_context.context_id != EFX_MCDI_RSS_CONTEXT_INVALID) {
- rc = efx_mcdi_filter_free_rss_context(efx, efx->rss_context.context_id);
+ if (efx->rss_context.priv.context_id != EFX_MCDI_RSS_CONTEXT_INVALID) {
+ rc = efx_mcdi_filter_free_rss_context(efx, efx->rss_context.priv.context_id);
WARN_ON(rc != 0);
}
- efx->rss_context.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+ efx->rss_context.priv.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
}
static int efx_mcdi_filter_rx_push_shared_rss_config(struct efx_nic *efx,
unsigned *context_size)
{
struct efx_mcdi_filter_table *table = efx->filter_state;
- int rc = efx_mcdi_filter_alloc_rss_context(efx, false, &efx->rss_context,
- context_size);
+ int rc = efx_mcdi_filter_alloc_rss_context(efx, false,
+ &efx->rss_context.priv,
+ context_size);
if (rc != 0)
return rc;
table->rx_rss_context_exclusive = false;
- efx_set_default_rx_indir_table(efx, &efx->rss_context);
+ efx_set_default_rx_indir_table(efx, efx->rss_context.rx_indir_table);
return 0;
}
@@ -2058,26 +2060,27 @@ static int efx_mcdi_filter_rx_push_exclusive_rss_config(struct efx_nic *efx,
const u32 *rx_indir_table,
const u8 *key)
{
+ u32 old_rx_rss_context = efx->rss_context.priv.context_id;
struct efx_mcdi_filter_table *table = efx->filter_state;
- u32 old_rx_rss_context = efx->rss_context.context_id;
int rc;
- if (efx->rss_context.context_id == EFX_MCDI_RSS_CONTEXT_INVALID ||
+ if (efx->rss_context.priv.context_id == EFX_MCDI_RSS_CONTEXT_INVALID ||
!table->rx_rss_context_exclusive) {
- rc = efx_mcdi_filter_alloc_rss_context(efx, true, &efx->rss_context,
- NULL);
+ rc = efx_mcdi_filter_alloc_rss_context(efx, true,
+ &efx->rss_context.priv,
+ NULL);
if (rc == -EOPNOTSUPP)
return rc;
else if (rc != 0)
goto fail1;
}
- rc = efx_mcdi_filter_populate_rss_table(efx, efx->rss_context.context_id,
- rx_indir_table, key);
+ rc = efx_mcdi_filter_populate_rss_table(efx, efx->rss_context.priv.context_id,
+ rx_indir_table, key);
if (rc != 0)
goto fail2;
- if (efx->rss_context.context_id != old_rx_rss_context &&
+ if (efx->rss_context.priv.context_id != old_rx_rss_context &&
old_rx_rss_context != EFX_MCDI_RSS_CONTEXT_INVALID)
WARN_ON(efx_mcdi_filter_free_rss_context(efx, old_rx_rss_context) != 0);
table->rx_rss_context_exclusive = true;
@@ -2091,9 +2094,9 @@ static int efx_mcdi_filter_rx_push_exclusive_rss_config(struct efx_nic *efx,
return 0;
fail2:
- if (old_rx_rss_context != efx->rss_context.context_id) {
- WARN_ON(efx_mcdi_filter_free_rss_context(efx, efx->rss_context.context_id) != 0);
- efx->rss_context.context_id = old_rx_rss_context;
+ if (old_rx_rss_context != efx->rss_context.priv.context_id) {
+ WARN_ON(efx_mcdi_filter_free_rss_context(efx, efx->rss_context.priv.context_id) != 0);
+ efx->rss_context.priv.context_id = old_rx_rss_context;
}
fail1:
netif_err(efx, hw, efx->net_dev, "%s: failed rc=%d\n", __func__, rc);
@@ -2101,33 +2104,28 @@ static int efx_mcdi_filter_rx_push_exclusive_rss_config(struct efx_nic *efx,
}
int efx_mcdi_rx_push_rss_context_config(struct efx_nic *efx,
- struct efx_rss_context *ctx,
+ struct efx_rss_context_priv *ctx,
const u32 *rx_indir_table,
- const u8 *key)
+ const u8 *key, bool delete)
{
int rc;
- WARN_ON(!mutex_is_locked(&efx->rss_lock));
+ WARN_ON(!mutex_is_locked(&efx->net_dev->ethtool->rss_lock));
if (ctx->context_id == EFX_MCDI_RSS_CONTEXT_INVALID) {
+ if (delete)
+ /* already wasn't in HW, nothing to do */
+ return 0;
rc = efx_mcdi_filter_alloc_rss_context(efx, true, ctx, NULL);
if (rc)
return rc;
}
- if (!rx_indir_table) /* Delete this context */
+ if (delete) /* Delete this context */
return efx_mcdi_filter_free_rss_context(efx, ctx->context_id);
- rc = efx_mcdi_filter_populate_rss_table(efx, ctx->context_id,
- rx_indir_table, key);
- if (rc)
- return rc;
-
- memcpy(ctx->rx_indir_table, rx_indir_table,
- sizeof(efx->rss_context.rx_indir_table));
- memcpy(ctx->rx_hash_key, key, efx->type->rx_hash_key_size);
-
- return 0;
+ return efx_mcdi_filter_populate_rss_table(efx, ctx->context_id,
+ rx_indir_table, key);
}
int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
@@ -2139,16 +2137,16 @@ int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
size_t outlen;
int rc, i;
- WARN_ON(!mutex_is_locked(&efx->rss_lock));
+ WARN_ON(!mutex_is_locked(&efx->net_dev->ethtool->rss_lock));
BUILD_BUG_ON(MC_CMD_RSS_CONTEXT_GET_TABLE_IN_LEN !=
MC_CMD_RSS_CONTEXT_GET_KEY_IN_LEN);
- if (ctx->context_id == EFX_MCDI_RSS_CONTEXT_INVALID)
+ if (ctx->priv.context_id == EFX_MCDI_RSS_CONTEXT_INVALID)
return -ENOENT;
MCDI_SET_DWORD(inbuf, RSS_CONTEXT_GET_TABLE_IN_RSS_CONTEXT_ID,
- ctx->context_id);
+ ctx->priv.context_id);
BUILD_BUG_ON(ARRAY_SIZE(ctx->rx_indir_table) !=
MC_CMD_RSS_CONTEXT_GET_TABLE_OUT_INDIRECTION_TABLE_LEN);
rc = efx_mcdi_rpc(efx, MC_CMD_RSS_CONTEXT_GET_TABLE, inbuf, sizeof(inbuf),
@@ -2164,7 +2162,7 @@ int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
RSS_CONTEXT_GET_TABLE_OUT_INDIRECTION_TABLE)[i];
MCDI_SET_DWORD(inbuf, RSS_CONTEXT_GET_KEY_IN_RSS_CONTEXT_ID,
- ctx->context_id);
+ ctx->priv.context_id);
BUILD_BUG_ON(ARRAY_SIZE(ctx->rx_hash_key) !=
MC_CMD_RSS_CONTEXT_SET_KEY_IN_TOEPLITZ_KEY_LEN);
rc = efx_mcdi_rpc(efx, MC_CMD_RSS_CONTEXT_GET_KEY, inbuf, sizeof(inbuf),
@@ -2186,35 +2184,42 @@ int efx_mcdi_rx_pull_rss_config(struct efx_nic *efx)
{
int rc;
- mutex_lock(&efx->rss_lock);
+ mutex_lock(&efx->net_dev->ethtool->rss_lock);
rc = efx_mcdi_rx_pull_rss_context_config(efx, &efx->rss_context);
- mutex_unlock(&efx->rss_lock);
+ mutex_unlock(&efx->net_dev->ethtool->rss_lock);
return rc;
}
void efx_mcdi_rx_restore_rss_contexts(struct efx_nic *efx)
{
struct efx_mcdi_filter_table *table = efx->filter_state;
- struct efx_rss_context *ctx;
+ struct ethtool_rxfh_context *ctx;
+ unsigned long context;
int rc;
- WARN_ON(!mutex_is_locked(&efx->rss_lock));
+ WARN_ON(!mutex_is_locked(&efx->net_dev->ethtool->rss_lock));
if (!table->must_restore_rss_contexts)
return;
- list_for_each_entry(ctx, &efx->rss_context.list, list) {
+ xa_for_each(&efx->net_dev->ethtool->rss_ctx, context, ctx) {
+ struct efx_rss_context_priv *priv;
+ u32 *indir;
+ u8 *key;
+
+ priv = ethtool_rxfh_context_priv(ctx);
/* previous NIC RSS context is gone */
- ctx->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+ priv->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
/* so try to allocate a new one */
- rc = efx_mcdi_rx_push_rss_context_config(efx, ctx,
- ctx->rx_indir_table,
- ctx->rx_hash_key);
+ indir = ethtool_rxfh_context_indir(ctx);
+ key = ethtool_rxfh_context_key(ctx);
+ rc = efx_mcdi_rx_push_rss_context_config(efx, priv, indir, key,
+ false);
if (rc)
netif_warn(efx, probe, efx->net_dev,
- "failed to restore RSS context %u, rc=%d"
+ "failed to restore RSS context %lu, rc=%d"
"; RSS filters may fail to be applied\n",
- ctx->user_id, rc);
+ context, rc);
}
table->must_restore_rss_contexts = false;
}
@@ -2276,7 +2281,7 @@ int efx_mcdi_vf_rx_push_rss_config(struct efx_nic *efx, bool user,
{
if (user)
return -EOPNOTSUPP;
- if (efx->rss_context.context_id != EFX_MCDI_RSS_CONTEXT_INVALID)
+ if (efx->rss_context.priv.context_id != EFX_MCDI_RSS_CONTEXT_INVALID)
return 0;
return efx_mcdi_filter_rx_push_shared_rss_config(efx, NULL);
}
@@ -2295,7 +2300,7 @@ int efx_mcdi_push_default_indir_table(struct efx_nic *efx,
efx_mcdi_rx_free_indir_table(efx);
if (rss_spread > 1) {
- efx_set_default_rx_indir_table(efx, &efx->rss_context);
+ efx_set_default_rx_indir_table(efx, efx->rss_context.rx_indir_table);
rc = efx->type->rx_push_rss_config(efx, false,
efx->rss_context.rx_indir_table, NULL);
}
diff --git a/drivers/net/ethernet/sfc/mcdi_filters.h b/drivers/net/ethernet/sfc/mcdi_filters.h
index c0d6558b9fd2..11b9f87ed9e1 100644
--- a/drivers/net/ethernet/sfc/mcdi_filters.h
+++ b/drivers/net/ethernet/sfc/mcdi_filters.h
@@ -145,9 +145,9 @@ void efx_mcdi_filter_del_vlan(struct efx_nic *efx, u16 vid);
void efx_mcdi_rx_free_indir_table(struct efx_nic *efx);
int efx_mcdi_rx_push_rss_context_config(struct efx_nic *efx,
- struct efx_rss_context *ctx,
+ struct efx_rss_context_priv *ctx,
const u32 *rx_indir_table,
- const u8 *key);
+ const u8 *key, bool delete);
int efx_mcdi_pf_rx_push_rss_config(struct efx_nic *efx, bool user,
const u32 *rx_indir_table,
const u8 *key);
@@ -161,10 +161,6 @@ int efx_mcdi_push_default_indir_table(struct efx_nic *efx,
int efx_mcdi_rx_pull_rss_config(struct efx_nic *efx);
int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
struct efx_rss_context *ctx);
-int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context,
- u32 *flags);
-void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
- struct efx_rss_context *ctx);
void efx_mcdi_rx_restore_rss_contexts(struct efx_nic *efx);
static inline void efx_mcdi_update_rx_scatter(struct efx_nic *efx)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index f2dd7feb0e0c..b85c51cbe7f9 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -737,21 +737,24 @@ struct vfdi_status;
/* The reserved RSS context value */
#define EFX_MCDI_RSS_CONTEXT_INVALID 0xffffffff
/**
- * struct efx_rss_context - A user-defined RSS context for filtering
- * @list: node of linked list on which this struct is stored
+ * struct efx_rss_context_priv - driver private data for an RSS context
* @context_id: the RSS_CONTEXT_ID returned by MC firmware, or
* %EFX_MCDI_RSS_CONTEXT_INVALID if this context is not present on the NIC.
- * For Siena, 0 if RSS is active, else %EFX_MCDI_RSS_CONTEXT_INVALID.
- * @user_id: the rss_context ID exposed to userspace over ethtool.
* @rx_hash_udp_4tuple: UDP 4-tuple hashing enabled
+ */
+struct efx_rss_context_priv {
+ u32 context_id;
+ bool rx_hash_udp_4tuple;
+};
+
+/**
+ * struct efx_rss_context - an RSS context
+ * @priv: hardware-specific state
* @rx_hash_key: Toeplitz hash key for this RSS context
* @indir_table: Indirection table for this RSS context
*/
struct efx_rss_context {
- struct list_head list;
- u32 context_id;
- u32 user_id;
- bool rx_hash_udp_4tuple;
+ struct efx_rss_context_priv priv;
u8 rx_hash_key[40];
u32 rx_indir_table[128];
};
@@ -883,9 +886,7 @@ struct efx_mae;
* @rx_packet_ts_offset: Offset of timestamp from start of packet data
* (valid only if channel->sync_timestamps_enabled; always negative)
* @rx_scatter: Scatter mode enabled for receives
- * @rss_context: Main RSS context. Its @list member is the head of the list of
- * RSS contexts created by user requests
- * @rss_lock: Protects custom RSS context software state in @rss_context.list
+ * @rss_context: Main RSS context.
* @vport_id: The function's vport ID, only relevant for PFs
* @int_error_count: Number of internal errors seen recently
* @int_error_expire: Time at which error count will be expired
@@ -1052,7 +1053,6 @@ struct efx_nic {
int rx_packet_ts_offset;
bool rx_scatter;
struct efx_rss_context rss_context;
- struct mutex rss_lock;
u32 vport_id;
unsigned int_error_count;
@@ -1416,9 +1416,9 @@ struct efx_nic_type {
const u32 *rx_indir_table, const u8 *key);
int (*rx_pull_rss_config)(struct efx_nic *efx);
int (*rx_push_rss_context_config)(struct efx_nic *efx,
- struct efx_rss_context *ctx,
+ struct efx_rss_context_priv *ctx,
const u32 *rx_indir_table,
- const u8 *key);
+ const u8 *key, bool delete);
int (*rx_pull_rss_context_config)(struct efx_nic *efx,
struct efx_rss_context *ctx);
void (*rx_restore_rss_contexts)(struct efx_nic *efx);
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index dcd901eccfc8..0b7dc75c40f9 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -557,69 +557,25 @@ efx_rx_packet_gro(struct efx_channel *channel, struct efx_rx_buffer *rx_buf,
napi_gro_frags(napi);
}
-/* RSS contexts. We're using linked lists and crappy O(n) algorithms, because
- * (a) this is an infrequent control-plane operation and (b) n is small (max 64)
- */
-struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx)
+struct efx_rss_context_priv *efx_find_rss_context_entry(struct efx_nic *efx,
+ u32 id)
{
- struct list_head *head = &efx->rss_context.list;
- struct efx_rss_context *ctx, *new;
- u32 id = 1; /* Don't use zero, that refers to the master RSS context */
-
- WARN_ON(!mutex_is_locked(&efx->rss_lock));
+ struct ethtool_rxfh_context *ctx;
- /* Search for first gap in the numbering */
- list_for_each_entry(ctx, head, list) {
- if (ctx->user_id != id)
- break;
- id++;
- /* Check for wrap. If this happens, we have nearly 2^32
- * allocated RSS contexts, which seems unlikely.
- */
- if (WARN_ON_ONCE(!id))
- return NULL;
- }
+ WARN_ON(!mutex_is_locked(&efx->net_dev->ethtool->rss_lock));
- /* Create the new entry */
- new = kmalloc(sizeof(*new), GFP_KERNEL);
- if (!new)
+ ctx = xa_load(&efx->net_dev->ethtool->rss_ctx, id);
+ if (!ctx)
return NULL;
- new->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
- new->rx_hash_udp_4tuple = false;
-
- /* Insert the new entry into the gap */
- new->user_id = id;
- list_add_tail(&new->list, &ctx->list);
- return new;
-}
-
-struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id)
-{
- struct list_head *head = &efx->rss_context.list;
- struct efx_rss_context *ctx;
-
- WARN_ON(!mutex_is_locked(&efx->rss_lock));
-
- list_for_each_entry(ctx, head, list)
- if (ctx->user_id == id)
- return ctx;
- return NULL;
-}
-
-void efx_free_rss_context_entry(struct efx_rss_context *ctx)
-{
- list_del(&ctx->list);
- kfree(ctx);
+ return ethtool_rxfh_context_priv(ctx);
}
-void efx_set_default_rx_indir_table(struct efx_nic *efx,
- struct efx_rss_context *ctx)
+void efx_set_default_rx_indir_table(struct efx_nic *efx, u32 *indir)
{
size_t i;
- for (i = 0; i < ARRAY_SIZE(ctx->rx_indir_table); i++)
- ctx->rx_indir_table[i] =
- ethtool_rxfh_indir_default(i, efx->rss_spread);
+ for (i = 0; i < ARRAY_SIZE(efx->rss_context.rx_indir_table); i++)
+ indir[i] = ethtool_rxfh_indir_default(i, efx->rss_spread);
}
/**
diff --git a/drivers/net/ethernet/sfc/rx_common.h b/drivers/net/ethernet/sfc/rx_common.h
index fbd2769307f9..75fa84192362 100644
--- a/drivers/net/ethernet/sfc/rx_common.h
+++ b/drivers/net/ethernet/sfc/rx_common.h
@@ -84,11 +84,9 @@ void
efx_rx_packet_gro(struct efx_channel *channel, struct efx_rx_buffer *rx_buf,
unsigned int n_frags, u8 *eh, __wsum csum);
-struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx);
-struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id);
-void efx_free_rss_context_entry(struct efx_rss_context *ctx);
-void efx_set_default_rx_indir_table(struct efx_nic *efx,
- struct efx_rss_context *ctx);
+struct efx_rss_context_priv *efx_find_rss_context_entry(struct efx_nic *efx,
+ u32 id);
+void efx_set_default_rx_indir_table(struct efx_nic *efx, u32 *indir);
bool efx_filter_is_mc_recipient(const struct efx_filter_spec *spec);
bool efx_filter_spec_equal(const struct efx_filter_spec *left,
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for get_rxfh on custom RSS contexts
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
` (6 preceding siblings ...)
2024-06-20 5:47 ` [PATCH v6 net-next 7/9] sfc: use new rxfh_context API edward.cree
@ 2024-06-20 5:47 ` edward.cree
2024-06-20 19:42 ` Simon Horman
2024-06-20 5:47 ` [PATCH v6 net-next 9/9] sfc: remove get_rxfh_context dead code edward.cree
8 siblings, 1 reply; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
On 'ethtool -x' with rss_context != 0, instead of calling the driver to
read the RSS settings for the context, just get the settings from the
rss_ctx xarray, and return them to the user with no driver involvement.
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
net/ethtool/ioctl.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 9d2d677770db..ac562ee3662e 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1199,6 +1199,7 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
const struct ethtool_ops *ops = dev->ethtool_ops;
struct ethtool_rxfh_param rxfh_dev = {};
u32 user_indir_size, user_key_size;
+ struct ethtool_rxfh_context *ctx;
struct ethtool_rxfh rxfh;
u32 indir_bytes;
u8 *rss_config;
@@ -1246,11 +1247,25 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
if (user_key_size)
rxfh_dev.key = rss_config + indir_bytes;
- rxfh_dev.rss_context = rxfh.rss_context;
-
- ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
- if (ret)
- goto out;
+ if (rxfh.rss_context) {
+ ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
+ if (!ctx) {
+ ret = -ENOENT;
+ goto out;
+ }
+ if (rxfh_dev.indir)
+ memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
+ indir_bytes);
+ if (rxfh_dev.key)
+ memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
+ user_key_size);
+ rxfh_dev.hfunc = ctx->hfunc;
+ rxfh_dev.input_xfrm = ctx->input_xfrm;
+ } else {
+ ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
+ if (ret)
+ goto out;
+ }
if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, hfunc),
&rxfh_dev.hfunc, sizeof(rxfh.hfunc))) {
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for get_rxfh on custom RSS contexts
2024-06-20 5:47 ` [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for get_rxfh on custom RSS contexts edward.cree
@ 2024-06-20 19:42 ` Simon Horman
2024-06-24 13:31 ` Edward Cree
0 siblings, 1 reply; 20+ messages in thread
From: Simon Horman @ 2024-06-20 19:42 UTC (permalink / raw)
To: edward.cree
Cc: linux-net-drivers, davem, kuba, edumazet, pabeni, Edward Cree,
netdev, habetsm.xilinx, sudheer.mogilappagari, jdamato, mw, linux,
sgoutham, gakula, sbhatta, hkelam, saeedm, leon, jacob.e.keller,
andrew, ahmed.zaki
On Thu, Jun 20, 2024 at 06:47:11AM +0100, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> On 'ethtool -x' with rss_context != 0, instead of calling the driver to
> read the RSS settings for the context, just get the settings from the
> rss_ctx xarray, and return them to the user with no driver involvement.
>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
> net/ethtool/ioctl.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 9d2d677770db..ac562ee3662e 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -1199,6 +1199,7 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
> const struct ethtool_ops *ops = dev->ethtool_ops;
> struct ethtool_rxfh_param rxfh_dev = {};
> u32 user_indir_size, user_key_size;
> + struct ethtool_rxfh_context *ctx;
> struct ethtool_rxfh rxfh;
> u32 indir_bytes;
> u8 *rss_config;
> @@ -1246,11 +1247,25 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
> if (user_key_size)
> rxfh_dev.key = rss_config + indir_bytes;
>
> - rxfh_dev.rss_context = rxfh.rss_context;
> -
> - ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
> - if (ret)
> - goto out;
> + if (rxfh.rss_context) {
> + ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
> + if (!ctx) {
> + ret = -ENOENT;
> + goto out;
> + }
> + if (rxfh_dev.indir)
> + memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
> + indir_bytes);
> + if (rxfh_dev.key)
> + memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
> + user_key_size);
> + rxfh_dev.hfunc = ctx->hfunc;
> + rxfh_dev.input_xfrm = ctx->input_xfrm;
Hi Edward,
The last line of this function is:
return ret;
With this patch applied, Smatch complains that ret may be used there
when unintialised.
I think that occurs when the code reaches the line where this
commentary has been placed in this email.
> + } else {
> + ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
> + if (ret)
> + goto out;
> + }
>
> if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, hfunc),
> &rxfh_dev.hfunc, sizeof(rxfh.hfunc))) {
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for get_rxfh on custom RSS contexts
2024-06-20 19:42 ` Simon Horman
@ 2024-06-24 13:31 ` Edward Cree
0 siblings, 0 replies; 20+ messages in thread
From: Edward Cree @ 2024-06-24 13:31 UTC (permalink / raw)
To: Simon Horman, edward.cree
Cc: linux-net-drivers, davem, kuba, edumazet, pabeni, netdev,
habetsm.xilinx, sudheer.mogilappagari, jdamato, mw, linux,
sgoutham, gakula, sbhatta, hkelam, saeedm, leon, jacob.e.keller,
andrew, ahmed.zaki
On 20/06/2024 20:42, Simon Horman wrote:
> On Thu, Jun 20, 2024 at 06:47:11AM +0100, edward.cree@amd.com wrote:
>> From: Edward Cree <ecree.xilinx@gmail.com>
>>
>> On 'ethtool -x' with rss_context != 0, instead of calling the driver to
>> read the RSS settings for the context, just get the settings from the
>> rss_ctx xarray, and return them to the user with no driver involvement.
>>
>> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
>> ---
>> net/ethtool/ioctl.c | 25 ++++++++++++++++++++-----
>> 1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
>> index 9d2d677770db..ac562ee3662e 100644
>> --- a/net/ethtool/ioctl.c
>> +++ b/net/ethtool/ioctl.c
>> @@ -1199,6 +1199,7 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>> const struct ethtool_ops *ops = dev->ethtool_ops;
>> struct ethtool_rxfh_param rxfh_dev = {};
>> u32 user_indir_size, user_key_size;
>> + struct ethtool_rxfh_context *ctx;
>> struct ethtool_rxfh rxfh;
>> u32 indir_bytes;
>> u8 *rss_config;
>> @@ -1246,11 +1247,25 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>> if (user_key_size)
>> rxfh_dev.key = rss_config + indir_bytes;
>>
>> - rxfh_dev.rss_context = rxfh.rss_context;
>> -
>> - ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
>> - if (ret)
>> - goto out;
>> + if (rxfh.rss_context) {
>> + ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
>> + if (!ctx) {
>> + ret = -ENOENT;
>> + goto out;
>> + }
>> + if (rxfh_dev.indir)
>> + memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
>> + indir_bytes);
>> + if (rxfh_dev.key)
>> + memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
>> + user_key_size);
>> + rxfh_dev.hfunc = ctx->hfunc;
>> + rxfh_dev.input_xfrm = ctx->input_xfrm;
>
> Hi Edward,
>
> The last line of this function is:
>
> return ret;
>
> With this patch applied, Smatch complains that ret may be used there
> when unintialised.
>
> I think that occurs when the code reaches the line where this
> commentary has been placed in this email.
You (and Smatch) are quite right. Fixed for v7.
-ed
>
>> + } else {
>> + ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
>> + if (ret)
>> + goto out;
>> + }
>>
>> if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, hfunc),
>> &rxfh_dev.hfunc, sizeof(rxfh.hfunc))) {
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v6 net-next 9/9] sfc: remove get_rxfh_context dead code
2024-06-20 5:47 [PATCH v6 net-next 0/9] ethtool: track custom RSS contexts in the core edward.cree
` (7 preceding siblings ...)
2024-06-20 5:47 ` [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for get_rxfh on custom RSS contexts edward.cree
@ 2024-06-20 5:47 ` edward.cree
8 siblings, 0 replies; 20+ messages in thread
From: edward.cree @ 2024-06-20 5:47 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, edumazet, pabeni
Cc: Edward Cree, netdev, habetsm.xilinx, sudheer.mogilappagari,
jdamato, mw, linux, sgoutham, gakula, sbhatta, hkelam, saeedm,
leon, jacob.e.keller, andrew, ahmed.zaki
From: Edward Cree <ecree.xilinx@gmail.com>
The core now always satisfies 'ethtool -x context nonzero' from its own
tracking, so our lookup code for that case is never called. Remove it.
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
drivers/net/ethernet/sfc/ethtool_common.c | 38 ++---------------------
1 file changed, 2 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
index 0a8d2c9ffce6..6ded44b86052 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/ethtool_common.c
@@ -1163,48 +1163,14 @@ u32 efx_ethtool_get_rxfh_key_size(struct net_device *net_dev)
return efx->type->rx_hash_key_size;
}
-static int efx_ethtool_get_rxfh_context(struct net_device *net_dev,
- struct ethtool_rxfh_param *rxfh)
-{
- struct efx_nic *efx = efx_netdev_priv(net_dev);
- struct efx_rss_context_priv *ctx_priv;
- struct efx_rss_context ctx;
- int rc = 0;
-
- if (!efx->type->rx_pull_rss_context_config)
- return -EOPNOTSUPP;
-
- mutex_lock(&net_dev->ethtool->rss_lock);
- ctx_priv = efx_find_rss_context_entry(efx, rxfh->rss_context);
- if (!ctx_priv) {
- rc = -ENOENT;
- goto out_unlock;
- }
- ctx.priv = *ctx_priv;
- rc = efx->type->rx_pull_rss_context_config(efx, &ctx);
- if (rc)
- goto out_unlock;
-
- rxfh->hfunc = ETH_RSS_HASH_TOP;
- if (rxfh->indir)
- memcpy(rxfh->indir, ctx.rx_indir_table,
- sizeof(ctx.rx_indir_table));
- if (rxfh->key)
- memcpy(rxfh->key, ctx.rx_hash_key,
- efx->type->rx_hash_key_size);
-out_unlock:
- mutex_unlock(&net_dev->ethtool->rss_lock);
- return rc;
-}
-
int efx_ethtool_get_rxfh(struct net_device *net_dev,
struct ethtool_rxfh_param *rxfh)
{
struct efx_nic *efx = efx_netdev_priv(net_dev);
int rc;
- if (rxfh->rss_context)
- return efx_ethtool_get_rxfh_context(net_dev, rxfh);
+ if (rxfh->rss_context) /* core should never call us for these */
+ return -EINVAL;
rc = efx->type->rx_pull_rss_config(efx);
if (rc)
^ permalink raw reply related [flat|nested] 20+ messages in thread