* [PATCH net-next 00/11] eth: bnxt: use the new RSS API
@ 2024-07-02 23:47 Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts Jakub Kicinski
` (11 more replies)
0 siblings, 12 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Convert bnxt from using the set_rxfh API to separate create/modify/remove
callbacks.
Two small extensions to the core APIs are necessary:
- the ability to discard contexts if for some catastrophic reasons
device can no longer provide them;
- the ability to reserve space in the context for RSS table growth.
The driver is adjusted to store indirection tables on u32 to make
it easier to use core structs directly.
With that out of the way the conversion is fairly straightforward.
Jakub Kicinski (11):
net: ethtool: let drivers remove lost RSS contexts
net: ethtool: let drivers declare max size of RSS indir table and key
eth: bnxt: allow deleting RSS contexts when the device is down
eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
eth: bnxt: remove rss_ctx_bmap
eth: bnxt: depend on core cleaning up RSS contexts
eth: bnxt: use context priv for struct bnxt_rss_ctx
eth: bnxt: use the RSS context XArray instead of the local list
eth: bnxt: bump the entry size in indir tables to u32
eth: bnxt: use the indir table from ethtool context
eth: bnxt: pad out the correct indirection table
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 115 +++++--------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 14 +-
.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 159 ++++++++++--------
include/linux/ethtool.h | 25 +--
net/ethtool/ioctl.c | 46 +++--
net/ethtool/rss.c | 14 ++
6 files changed, 201 insertions(+), 172 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-03 11:08 ` Edward Cree
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let driver declare max size of RSS indir table and key Jakub Kicinski
` (10 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
RSS contexts may get lost from a device, in various extreme circumstances.
Specifically if the firmware leaks resources and resets, or crashes and
either recovers in partially working state or the crash causes a
different FW version to run - creating the context again may fail.
Drivers should do their absolute best to prevent this from happening.
When it does, however, telling user that a context exists, when it can't
possibly be used any more is counter productive. Add a helper for
drivers to discard contexts. Print an error, in the future netlink
notification will also be sent.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
include/linux/ethtool.h | 2 ++
| 14 ++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f74bb0cf8ed1..3ce5be0d168a 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -210,6 +210,8 @@ static inline size_t ethtool_rxfh_context_size(u32 indir_size, u32 key_size,
return struct_size_t(struct ethtool_rxfh_context, data, flex_len);
}
+void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id);
+
/* declare a link mode bitmap */
#define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \
DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
--git a/net/ethtool/rss.c b/net/ethtool/rss.c
index 71679137eff2..e2e5bab56a6b 100644
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -159,3 +159,17 @@ const struct ethnl_request_ops ethnl_rss_request_ops = {
.fill_reply = rss_fill_reply,
.cleanup_data = rss_cleanup_data,
};
+
+void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id)
+{
+ struct ethtool_rxfh_context *ctx;
+
+ WARN_ONCE(!rtnl_is_locked() &&
+ !lockdep_is_held_type(&dev->ethtool->rss_lock, -1),
+ "RSS context lock assertion failed\n");
+
+ netdev_err(dev, "device error, RSS context %d lost\n", context_id);
+ ctx = xa_erase(&dev->ethtool->rss_ctx, context_id);
+ kfree(ctx);
+}
+EXPORT_SYMBOL(ethtool_rxfh_context_lost);
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 02/11] net: ethtool: let driver declare max size of RSS indir table and key
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-04 7:34 ` Simon Horman
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let drivers " Jakub Kicinski
` (9 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Some drivers (bnxt but I think also mlx5 from ML discussions) change
the size of the indirection table depending on the number of Rx rings.
Decouple the max table size from the size of the currently used table,
so that we can reserve space in the context for table growth.
Static members in ethtool_ops are good enough for now, we can add
callbacks to read the max size more dynamically if someone needs
that.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
include/linux/ethtool.h | 20 +++++++-----------
net/ethtool/ioctl.c | 46 ++++++++++++++++++++++++++++++++---------
2 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3ce5be0d168a..dc8ed93097c3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -173,6 +173,7 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
struct ethtool_rxfh_context {
u32 indir_size;
u32 key_size;
+ u32 key_off;
u16 priv_size;
u8 hfunc;
u8 input_xfrm;
@@ -196,18 +197,7 @@ static inline u32 *ethtool_rxfh_context_indir(struct ethtool_rxfh_context *ctx)
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_t(struct ethtool_rxfh_context, data, flex_len);
+ return &ctx->data[ctx->key_off];
}
void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id);
@@ -723,6 +713,10 @@ struct ethtool_rxfh_param {
* contexts.
* @cap_rss_sym_xor_supported: indicates if the driver supports symmetric-xor
* RSS.
+ * @rxfh_indir_space: max size of RSS indirection tables, if indirection table
+ * size as returned by @get_rxfh_indir_size may change during lifetime
+ * of the device. Leave as 0 if the table size is constant.
+ * @rxfh_key_space: same as @rxfh_indir_space, but for the key.
* @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
@@ -940,6 +934,8 @@ struct ethtool_ops {
u32 cap_link_lanes_supported:1;
u32 cap_rss_ctx_supported:1;
u32 cap_rss_sym_xor_supported:1;
+ u32 rxfh_indir_space;
+ u16 rxfh_key_space;
u16 rxfh_priv_size;
u32 rxfh_max_context_id;
u32 supported_coalesce_params;
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 46f0497ae6bc..7e0fa9fafc7d 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1290,6 +1290,40 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
return ret;
}
+static struct ethtool_rxfh_context *
+ethtool_rxfh_ctx_alloc(const struct ethtool_ops *ops,
+ u32 indir_size, u32 key_size)
+{
+ size_t indir_bytes, flex_len, key_off, size;
+ struct ethtool_rxfh_context *ctx;
+ u32 priv_bytes, indir_max;
+ u16 key_max;
+
+ key_max = max(key_size, ops->rxfh_key_space);
+ indir_max = max(indir_size, ops->rxfh_indir_space);
+
+ priv_bytes = ALIGN(ops->rxfh_priv_size, sizeof(u32));
+ indir_bytes = array_size(indir_max, sizeof(u32));
+
+ key_off = size_add(priv_bytes, indir_bytes);
+ flex_len = size_add(key_off, key_max);
+ size = struct_size_t(struct ethtool_rxfh_context, data, flex_len);
+
+ ctx = kzalloc(size, GFP_KERNEL_ACCOUNT);
+ if (!ctx)
+ return NULL;
+
+ ctx->indir_size = indir_size;
+ ctx->key_size = key_size;
+ ctx->key_off = key_off;
+ ctx->priv_size = ops->rxfh_priv_size;
+
+ ctx->hfunc = ETH_RSS_HASH_NO_CHANGE;
+ ctx->input_xfrm = RXH_XFRM_NO_CHANGE;
+
+ return ctx;
+}
+
static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
void __user *useraddr)
{
@@ -1406,20 +1440,12 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
ret = -EINVAL;
goto out;
}
- ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
- dev_key_size,
- ops->rxfh_priv_size),
- GFP_KERNEL_ACCOUNT);
+ ctx = ethtool_rxfh_ctx_alloc(ops, dev_indir_size, dev_key_size);
if (!ctx) {
ret = -ENOMEM;
goto out;
}
- ctx->indir_size = dev_indir_size;
- ctx->key_size = dev_key_size;
- 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;
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 02/11] net: ethtool: let drivers declare max size of RSS indir table and key
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let driver declare max size of RSS indir table and key Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-03 11:16 ` Edward Cree
2024-07-02 23:47 ` [PATCH net-next 03/11] eth: bnxt: allow deleting RSS contexts when the device is down Jakub Kicinski
` (8 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Some drivers (bnxt but I think also mlx5 from ML discussions) change
the size of the indirection table depending on the number of Rx rings.
Decouple the max table size from the size of the currently used table,
so that we can reserve space in the context for table growth.
Static members in ethtool_ops are good enough for now, we can add
callbacks to read the max size more dynamically if someone needs
that.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
include/linux/ethtool.h | 20 +++++++-----------
net/ethtool/ioctl.c | 46 ++++++++++++++++++++++++++++++++---------
2 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3ce5be0d168a..dc8ed93097c3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -173,6 +173,7 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
struct ethtool_rxfh_context {
u32 indir_size;
u32 key_size;
+ u32 key_off;
u16 priv_size;
u8 hfunc;
u8 input_xfrm;
@@ -196,18 +197,7 @@ static inline u32 *ethtool_rxfh_context_indir(struct ethtool_rxfh_context *ctx)
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_t(struct ethtool_rxfh_context, data, flex_len);
+ return &ctx->data[ctx->key_off];
}
void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id);
@@ -723,6 +713,10 @@ struct ethtool_rxfh_param {
* contexts.
* @cap_rss_sym_xor_supported: indicates if the driver supports symmetric-xor
* RSS.
+ * @rxfh_indir_space: max size of RSS indirection tables, if indirection table
+ * size as returned by @get_rxfh_indir_size may change during lifetime
+ * of the device. Leave as 0 if the table size is constant.
+ * @rxfh_key_space: same as @rxfh_indir_space, but for the key.
* @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
@@ -940,6 +934,8 @@ struct ethtool_ops {
u32 cap_link_lanes_supported:1;
u32 cap_rss_ctx_supported:1;
u32 cap_rss_sym_xor_supported:1;
+ u32 rxfh_indir_space;
+ u16 rxfh_key_space;
u16 rxfh_priv_size;
u32 rxfh_max_context_id;
u32 supported_coalesce_params;
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 46f0497ae6bc..7e0fa9fafc7d 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1290,6 +1290,40 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
return ret;
}
+static struct ethtool_rxfh_context *
+ethtool_rxfh_ctx_alloc(const struct ethtool_ops *ops,
+ u32 indir_size, u32 key_size)
+{
+ size_t indir_bytes, flex_len, key_off, size;
+ struct ethtool_rxfh_context *ctx;
+ u32 priv_bytes, indir_max;
+ u16 key_max;
+
+ key_max = max(key_size, ops->rxfh_key_space);
+ indir_max = max(indir_size, ops->rxfh_indir_space);
+
+ priv_bytes = ALIGN(ops->rxfh_priv_size, sizeof(u32));
+ indir_bytes = array_size(indir_max, sizeof(u32));
+
+ key_off = size_add(priv_bytes, indir_bytes);
+ flex_len = size_add(key_off, key_max);
+ size = struct_size_t(struct ethtool_rxfh_context, data, flex_len);
+
+ ctx = kzalloc(size, GFP_KERNEL_ACCOUNT);
+ if (!ctx)
+ return NULL;
+
+ ctx->indir_size = indir_size;
+ ctx->key_size = key_size;
+ ctx->key_off = key_off;
+ ctx->priv_size = ops->rxfh_priv_size;
+
+ ctx->hfunc = ETH_RSS_HASH_NO_CHANGE;
+ ctx->input_xfrm = RXH_XFRM_NO_CHANGE;
+
+ return ctx;
+}
+
static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
void __user *useraddr)
{
@@ -1406,20 +1440,12 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
ret = -EINVAL;
goto out;
}
- ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
- dev_key_size,
- ops->rxfh_priv_size),
- GFP_KERNEL_ACCOUNT);
+ ctx = ethtool_rxfh_ctx_alloc(ops, dev_indir_size, dev_key_size);
if (!ctx) {
ret = -ENOMEM;
goto out;
}
- ctx->indir_size = dev_indir_size;
- ctx->key_size = dev_key_size;
- 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;
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 03/11] eth: bnxt: allow deleting RSS contexts when the device is down
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (2 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let drivers " Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends Jakub Kicinski
` (7 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Contexts get deleted from FW when the device is down, but they
are kept in SW and re-added back on open. bnxt_set_rxfh_context()
apparently does not want to deal with complexity of dealing with
both the device down and device up cases. This is perhaps acceptable
for creating new contexts, but not being able to delete contexts
makes core-driven cleanups messy. Specifically with the new RSS
API core will try to delete contexts automatically after bringing
the device down.
Support the delete-while-down case. Skip the FW logic and delete
just the driver state.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 10 ++++++----
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 6 ++++--
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6fc34ccb86e3..ab6dae416532 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10195,10 +10195,12 @@ void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
struct bnxt_ntuple_filter *ntp_fltr;
int i;
- bnxt_hwrm_vnic_free_one(bp, &rss_ctx->vnic);
- for (i = 0; i < BNXT_MAX_CTX_PER_VNIC; i++) {
- if (vnic->fw_rss_cos_lb_ctx[i] != INVALID_HW_RING_ID)
- bnxt_hwrm_vnic_ctx_free_one(bp, vnic, i);
+ if (netif_running(bp->dev)) {
+ bnxt_hwrm_vnic_free_one(bp, &rss_ctx->vnic);
+ for (i = 0; i < BNXT_MAX_CTX_PER_VNIC; i++) {
+ if (vnic->fw_rss_cos_lb_ctx[i] != INVALID_HW_RING_ID)
+ bnxt_hwrm_vnic_ctx_free_one(bp, vnic, i);
+ }
}
if (!all)
return;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index bf157f6cc042..0a7524cba5c3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1868,6 +1868,7 @@ static int bnxt_set_rxfh_context(struct bnxt *bp,
struct bnxt_rss_ctx *rss_ctx;
struct bnxt_vnic_info *vnic;
bool modify = false;
+ bool delete;
int bit_id;
int rc;
@@ -1876,7 +1877,8 @@ static int bnxt_set_rxfh_context(struct bnxt *bp,
return -EOPNOTSUPP;
}
- if (!netif_running(bp->dev)) {
+ delete = *rss_context != ETH_RXFH_CONTEXT_ALLOC && rxfh->rss_delete;
+ if (!netif_running(bp->dev) && !delete) {
NL_SET_ERR_MSG_MOD(extack, "Unable to set RSS contexts when interface is down");
return -EAGAIN;
}
@@ -1888,7 +1890,7 @@ static int bnxt_set_rxfh_context(struct bnxt *bp,
*rss_context);
return -EINVAL;
}
- if (*rss_context && rxfh->rss_delete) {
+ if (delete) {
bnxt_del_one_rss_ctx(bp, rss_ctx, true);
return 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (3 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 03/11] eth: bnxt: allow deleting RSS contexts when the device is down Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-03 12:06 ` Edward Cree
` (2 more replies)
2024-07-02 23:47 ` [PATCH net-next 05/11] eth: bnxt: remove rss_ctx_bmap Jakub Kicinski
` (6 subsequent siblings)
11 siblings, 3 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Use the new ethtool ops for RSS context management. The conversion
is pretty straightforward cut / paste of the right chunks of the
combined handler. Main change is that we let the core pick the IDs
(bitmap will be removed separately for ease of review), so we need
to tell the core when we lose a context.
Since the new API passes rxfh as const, change bnxt_modify_rss()
to also take const.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 +
.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 119 +++++++++++-------
2 files changed, 75 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index ab6dae416532..3dc43c263b91 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10242,6 +10242,7 @@ static void bnxt_hwrm_realloc_rss_ctx_vnic(struct bnxt *bp)
netdev_err(bp->dev, "Failed to restore RSS ctx %d\n",
rss_ctx->index);
bnxt_del_one_rss_ctx(bp, rss_ctx, true);
+ ethtool_rxfh_context_lost(bp->dev, rss_ctx->index);
}
}
}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 0a7524cba5c3..a7f71ebca2fe 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1835,7 +1835,7 @@ static int bnxt_get_rxfh(struct net_device *dev,
}
static void bnxt_modify_rss(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
- struct ethtool_rxfh_param *rxfh)
+ const struct ethtool_rxfh_param *rxfh)
{
if (rxfh->key) {
if (rss_ctx) {
@@ -1860,44 +1860,35 @@ static void bnxt_modify_rss(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
}
}
-static int bnxt_set_rxfh_context(struct bnxt *bp,
- struct ethtool_rxfh_param *rxfh,
- struct netlink_ext_ack *extack)
+static int bnxt_rxfh_context_check(struct bnxt *bp,
+ struct netlink_ext_ack *extack)
{
- u32 *rss_context = &rxfh->rss_context;
- struct bnxt_rss_ctx *rss_ctx;
- struct bnxt_vnic_info *vnic;
- bool modify = false;
- bool delete;
- int bit_id;
- int rc;
-
if (!BNXT_SUPPORTS_MULTI_RSS_CTX(bp)) {
NL_SET_ERR_MSG_MOD(extack, "RSS contexts not supported");
return -EOPNOTSUPP;
}
- delete = *rss_context != ETH_RXFH_CONTEXT_ALLOC && rxfh->rss_delete;
- if (!netif_running(bp->dev) && !delete) {
+ if (!netif_running(bp->dev)) {
NL_SET_ERR_MSG_MOD(extack, "Unable to set RSS contexts when interface is down");
return -EAGAIN;
}
- if (*rss_context != ETH_RXFH_CONTEXT_ALLOC) {
- rss_ctx = bnxt_get_rss_ctx_from_index(bp, *rss_context);
- if (!rss_ctx) {
- NL_SET_ERR_MSG_FMT_MOD(extack, "RSS context %u not found",
- *rss_context);
- return -EINVAL;
- }
- if (delete) {
- bnxt_del_one_rss_ctx(bp, rss_ctx, true);
- return 0;
- }
- modify = true;
- vnic = &rss_ctx->vnic;
- goto modify_context;
- }
+ return 0;
+}
+
+static int bnxt_create_rxfh_context(struct net_device *dev,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack)
+{
+ struct bnxt *bp = netdev_priv(dev);
+ struct bnxt_rss_ctx *rss_ctx;
+ struct bnxt_vnic_info *vnic;
+ int rc;
+
+ rc = bnxt_rxfh_context_check(bp, extack);
+ if (rc)
+ return rc;
if (bp->num_rss_ctx >= BNXT_MAX_ETH_RSS_CTX) {
NL_SET_ERR_MSG_FMT_MOD(extack, "Out of RSS contexts, maximum %u",
@@ -1905,6 +1896,11 @@ static int bnxt_set_rxfh_context(struct bnxt *bp,
return -EINVAL;
}
+ if (test_and_set_bit(rxfh->rss_context, bp->rss_ctx_bmap)) {
+ NL_SET_ERR_MSG_MOD(extack, "Context ID conflict");
+ return -EINVAL;
+ }
+
if (!bnxt_rfs_capable(bp, true)) {
NL_SET_ERR_MSG_MOD(extack, "Out hardware resources");
return -ENOMEM;
@@ -1939,33 +1935,65 @@ static int bnxt_set_rxfh_context(struct bnxt *bp,
NL_SET_ERR_MSG_MOD(extack, "Unable to setup TPA");
goto out;
}
-modify_context:
bnxt_modify_rss(bp, rss_ctx, rxfh);
- if (modify)
- return bnxt_hwrm_vnic_rss_cfg_p5(bp, vnic);
-
rc = __bnxt_setup_vnic_p5(bp, vnic);
if (rc) {
NL_SET_ERR_MSG_MOD(extack, "Unable to setup TPA");
goto out;
}
- bit_id = bitmap_find_free_region(bp->rss_ctx_bmap,
- BNXT_RSS_CTX_BMAP_LEN, 0);
- if (bit_id < 0) {
- rc = -ENOMEM;
- goto out;
- }
- rss_ctx->index = (u16)bit_id;
- *rss_context = rss_ctx->index;
-
+ rss_ctx->index = rxfh->rss_context;
return 0;
out:
bnxt_del_one_rss_ctx(bp, rss_ctx, true);
return rc;
}
+static int bnxt_modify_rxfh_context(struct net_device *dev,
+ struct ethtool_rxfh_context *ctx,
+ const struct ethtool_rxfh_param *rxfh,
+ struct netlink_ext_ack *extack)
+{
+ struct bnxt *bp = netdev_priv(dev);
+ struct bnxt_rss_ctx *rss_ctx;
+ int rc;
+
+ rc = bnxt_rxfh_context_check(bp, extack);
+ if (rc)
+ return rc;
+
+ rss_ctx = bnxt_get_rss_ctx_from_index(bp, rxfh->rss_context);
+ if (!rss_ctx) {
+ NL_SET_ERR_MSG_FMT_MOD(extack, "RSS context %u not found",
+ rxfh->rss_context);
+ return -EINVAL;
+ }
+
+ bnxt_modify_rss(bp, rss_ctx, rxfh);
+
+ return bnxt_hwrm_vnic_rss_cfg_p5(bp, &rss_ctx->vnic);
+}
+
+static int bnxt_remove_rxfh_context(struct net_device *dev,
+ struct ethtool_rxfh_context *ctx,
+ u32 rss_context,
+ struct netlink_ext_ack *extack)
+{
+ struct bnxt *bp = netdev_priv(dev);
+ struct bnxt_rss_ctx *rss_ctx;
+
+ rss_ctx = bnxt_get_rss_ctx_from_index(bp, rss_context);
+ if (!rss_ctx) {
+ NL_SET_ERR_MSG_FMT_MOD(extack, "RSS context %u not found",
+ rss_context);
+ return -EINVAL;
+ }
+
+ bnxt_del_one_rss_ctx(bp, rss_ctx, true);
+ return 0;
+}
+
static int bnxt_set_rxfh(struct net_device *dev,
struct ethtool_rxfh_param *rxfh,
struct netlink_ext_ack *extack)
@@ -1976,9 +2004,6 @@ static int bnxt_set_rxfh(struct net_device *dev,
if (rxfh->hfunc && rxfh->hfunc != ETH_RSS_HASH_TOP)
return -EOPNOTSUPP;
- if (rxfh->rss_context)
- return bnxt_set_rxfh_context(bp, rxfh, extack);
-
bnxt_modify_rss(bp, NULL, rxfh);
bnxt_clear_usr_fltrs(bp, false);
@@ -5271,6 +5296,7 @@ void bnxt_ethtool_free(struct bnxt *bp)
const struct ethtool_ops bnxt_ethtool_ops = {
.cap_link_lanes_supported = 1,
.cap_rss_ctx_supported = 1,
+ .rxfh_max_context_id = BNXT_MAX_ETH_RSS_CTX,
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
ETHTOOL_COALESCE_MAX_FRAMES |
ETHTOOL_COALESCE_USECS_IRQ |
@@ -5308,6 +5334,9 @@ const struct ethtool_ops bnxt_ethtool_ops = {
.get_rxfh_key_size = bnxt_get_rxfh_key_size,
.get_rxfh = bnxt_get_rxfh,
.set_rxfh = bnxt_set_rxfh,
+ .create_rxfh_context = bnxt_create_rxfh_context,
+ .modify_rxfh_context = bnxt_modify_rxfh_context,
+ .remove_rxfh_context = bnxt_remove_rxfh_context,
.flash_device = bnxt_flash_device,
.get_eeprom_len = bnxt_get_eeprom_len,
.get_eeprom = bnxt_get_eeprom,
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 05/11] eth: bnxt: remove rss_ctx_bmap
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (4 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 06/11] eth: bnxt: depend on core cleaning up RSS contexts Jakub Kicinski
` (5 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Core will allocate IDs for the driver, from the range
[1, BNXT_MAX_ETH_RSS_CTX], no need to track the allocations.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 ++-----------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 --
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 5 -----
3 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 3dc43c263b91..02aeba4b5df5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10224,7 +10224,6 @@ void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
kfree(rss_ctx->rss_indir_tbl);
list_del(&rss_ctx->list);
bp->num_rss_ctx--;
- clear_bit(rss_ctx->index, bp->rss_ctx_bmap);
kfree(rss_ctx);
}
@@ -10266,20 +10265,12 @@ void bnxt_clear_rss_ctxs(struct bnxt *bp, bool all)
list_for_each_entry_safe(rss_ctx, tmp, &bp->rss_ctx_list, list)
bnxt_del_one_rss_ctx(bp, rss_ctx, all);
-
- if (all)
- bitmap_free(bp->rss_ctx_bmap);
}
static void bnxt_init_multi_rss_ctx(struct bnxt *bp)
{
- bp->rss_ctx_bmap = bitmap_zalloc(BNXT_RSS_CTX_BMAP_LEN, GFP_KERNEL);
- if (bp->rss_ctx_bmap) {
- /* burn index 0 since we cannot have context 0 */
- __set_bit(0, bp->rss_ctx_bmap);
- INIT_LIST_HEAD(&bp->rss_ctx_list);
- bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX;
- }
+ INIT_LIST_HEAD(&bp->rss_ctx_list);
+ bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX;
}
/* Allow PF, trusted VFs and VFs with default VLAN to be in promiscuous mode */
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index e46bd11e52b0..f4365a840e3a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1298,7 +1298,6 @@ struct bnxt_rss_ctx {
};
#define BNXT_MAX_ETH_RSS_CTX 32
-#define BNXT_RSS_CTX_BMAP_LEN (BNXT_MAX_ETH_RSS_CTX + 1)
#define BNXT_VNIC_ID_INVALID 0xffffffff
struct bnxt_hw_rings {
@@ -2332,7 +2331,6 @@ struct bnxt {
struct bnxt_ring_grp_info *grp_info;
struct bnxt_vnic_info *vnic_info;
struct list_head rss_ctx_list;
- unsigned long *rss_ctx_bmap;
u32 num_rss_ctx;
int nr_vnics;
u16 *rss_indir_tbl;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index a7f71ebca2fe..48f8e14685a1 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1896,11 +1896,6 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
return -EINVAL;
}
- if (test_and_set_bit(rxfh->rss_context, bp->rss_ctx_bmap)) {
- NL_SET_ERR_MSG_MOD(extack, "Context ID conflict");
- return -EINVAL;
- }
-
if (!bnxt_rfs_capable(bp, true)) {
NL_SET_ERR_MSG_MOD(extack, "Out hardware resources");
return -ENOMEM;
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 06/11] eth: bnxt: depend on core cleaning up RSS contexts
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (5 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 05/11] eth: bnxt: remove rss_ctx_bmap Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 07/11] eth: bnxt: use context priv for struct bnxt_rss_ctx Jakub Kicinski
` (4 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
New RSS context API removes old contexts on netdev unregister.
No need to wipe them manually.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 11 ++++-------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +-
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 02aeba4b5df5..b6915261c15d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10259,12 +10259,12 @@ struct bnxt_rss_ctx *bnxt_alloc_rss_ctx(struct bnxt *bp)
return rss_ctx;
}
-void bnxt_clear_rss_ctxs(struct bnxt *bp, bool all)
+void bnxt_clear_rss_ctxs(struct bnxt *bp)
{
struct bnxt_rss_ctx *rss_ctx, *tmp;
list_for_each_entry_safe(rss_ctx, tmp, &bp->rss_ctx_list, list)
- bnxt_del_one_rss_ctx(bp, rss_ctx, all);
+ bnxt_del_one_rss_ctx(bp, rss_ctx, false);
}
static void bnxt_init_multi_rss_ctx(struct bnxt *bp)
@@ -12316,7 +12316,7 @@ static void __bnxt_close_nic(struct bnxt *bp, bool irq_re_init,
msleep(20);
if (BNXT_SUPPORTS_MULTI_RSS_CTX(bp))
- bnxt_clear_rss_ctxs(bp, false);
+ bnxt_clear_rss_ctxs(bp);
/* Flush rings and disable interrupts */
bnxt_shutdown_nic(bp, irq_re_init);
@@ -15227,8 +15227,7 @@ static void bnxt_remove_one(struct pci_dev *pdev)
bnxt_free_l2_filters(bp, true);
bnxt_free_ntp_fltrs(bp, true);
- if (BNXT_SUPPORTS_MULTI_RSS_CTX(bp))
- bnxt_clear_rss_ctxs(bp, true);
+ WARN_ON(bp->num_rss_ctx);
clear_bit(BNXT_STATE_IN_FW_RESET, &bp->state);
/* Flush any pending tasks */
cancel_work_sync(&bp->sp_task);
@@ -15879,8 +15878,6 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
bnxt_clear_int_mode(bp);
init_err_pci_clean:
- if (BNXT_SUPPORTS_MULTI_RSS_CTX(bp))
- bnxt_clear_rss_ctxs(bp, true);
bnxt_hwrm_func_drv_unrgtr(bp);
bnxt_free_hwrm_resources(bp);
bnxt_hwmon_uninit(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index f4365a840e3a..04c4ff7b9052 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2846,7 +2846,7 @@ int __bnxt_setup_vnic_p5(struct bnxt *bp, struct bnxt_vnic_info *vnic);
void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
bool all);
struct bnxt_rss_ctx *bnxt_alloc_rss_ctx(struct bnxt *bp);
-void bnxt_clear_rss_ctxs(struct bnxt *bp, bool all);
+void bnxt_clear_rss_ctxs(struct bnxt *bp);
int bnxt_open_nic(struct bnxt *, bool, bool);
int bnxt_half_open_nic(struct bnxt *bp);
void bnxt_half_close_nic(struct bnxt *bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 48f8e14685a1..397aedad3d4f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -970,7 +970,7 @@ static int bnxt_set_channels(struct net_device *dev,
bnxt_clear_usr_fltrs(bp, true);
if (BNXT_SUPPORTS_MULTI_RSS_CTX(bp))
- bnxt_clear_rss_ctxs(bp, false);
+ bnxt_clear_rss_ctxs(bp);
if (netif_running(dev)) {
if (BNXT_PF(bp)) {
/* TODO CHIMP_FW: Send message to all VF's
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 07/11] eth: bnxt: use context priv for struct bnxt_rss_ctx
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (6 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 06/11] eth: bnxt: depend on core cleaning up RSS contexts Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 08/11] eth: bnxt: use the RSS context XArray instead of the local list Jakub Kicinski
` (3 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Core can allocate space for per-context driver-private data,
use it for struct bnxt_rss_ctx. Inline bnxt_alloc_rss_ctx()
at this point, most of the init (as in the actions bnxt_del_one_rss_ctx()
will undo) is open coded in bnxt_create_rxfh_context(), anyway.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 -----------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 -
.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 24 +++++++------------
3 files changed, 9 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index b6915261c15d..39876feae1a4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10224,7 +10224,6 @@ void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
kfree(rss_ctx->rss_indir_tbl);
list_del(&rss_ctx->list);
bp->num_rss_ctx--;
- kfree(rss_ctx);
}
static void bnxt_hwrm_realloc_rss_ctx_vnic(struct bnxt *bp)
@@ -10246,19 +10245,6 @@ static void bnxt_hwrm_realloc_rss_ctx_vnic(struct bnxt *bp)
}
}
-struct bnxt_rss_ctx *bnxt_alloc_rss_ctx(struct bnxt *bp)
-{
- struct bnxt_rss_ctx *rss_ctx = NULL;
-
- rss_ctx = kzalloc(sizeof(*rss_ctx), GFP_KERNEL);
- if (rss_ctx) {
- rss_ctx->vnic.rss_ctx = rss_ctx;
- list_add_tail(&rss_ctx->list, &bp->rss_ctx_list);
- bp->num_rss_ctx++;
- }
- return rss_ctx;
-}
-
void bnxt_clear_rss_ctxs(struct bnxt *bp)
{
struct bnxt_rss_ctx *rss_ctx, *tmp;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 04c4ff7b9052..21c3296cf6d9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2845,7 +2845,6 @@ int bnxt_hwrm_vnic_rss_cfg_p5(struct bnxt *bp, struct bnxt_vnic_info *vnic);
int __bnxt_setup_vnic_p5(struct bnxt *bp, struct bnxt_vnic_info *vnic);
void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
bool all);
-struct bnxt_rss_ctx *bnxt_alloc_rss_ctx(struct bnxt *bp);
void bnxt_clear_rss_ctxs(struct bnxt *bp);
int bnxt_open_nic(struct bnxt *, bool, bool);
int bnxt_half_open_nic(struct bnxt *bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 397aedad3d4f..2e6e060e2b44 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1901,11 +1901,13 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
return -ENOMEM;
}
- rss_ctx = bnxt_alloc_rss_ctx(bp);
- if (!rss_ctx)
- return -ENOMEM;
+ rss_ctx = ethtool_rxfh_context_priv(ctx);
+
+ list_add_tail(&rss_ctx->list, &bp->rss_ctx_list);
+ bp->num_rss_ctx++;
vnic = &rss_ctx->vnic;
+ vnic->rss_ctx = rss_ctx;
vnic->flags |= BNXT_VNIC_RSSCTX_FLAG;
vnic->vnic_id = BNXT_VNIC_ID_INVALID;
rc = bnxt_alloc_rss_ctx_rss_table(bp, rss_ctx);
@@ -1958,12 +1960,7 @@ static int bnxt_modify_rxfh_context(struct net_device *dev,
if (rc)
return rc;
- rss_ctx = bnxt_get_rss_ctx_from_index(bp, rxfh->rss_context);
- if (!rss_ctx) {
- NL_SET_ERR_MSG_FMT_MOD(extack, "RSS context %u not found",
- rxfh->rss_context);
- return -EINVAL;
- }
+ rss_ctx = ethtool_rxfh_context_priv(ctx);
bnxt_modify_rss(bp, rss_ctx, rxfh);
@@ -1978,12 +1975,7 @@ static int bnxt_remove_rxfh_context(struct net_device *dev,
struct bnxt *bp = netdev_priv(dev);
struct bnxt_rss_ctx *rss_ctx;
- rss_ctx = bnxt_get_rss_ctx_from_index(bp, rss_context);
- if (!rss_ctx) {
- NL_SET_ERR_MSG_FMT_MOD(extack, "RSS context %u not found",
- rss_context);
- return -EINVAL;
- }
+ rss_ctx = ethtool_rxfh_context_priv(ctx);
bnxt_del_one_rss_ctx(bp, rss_ctx, true);
return 0;
@@ -5292,6 +5284,8 @@ const struct ethtool_ops bnxt_ethtool_ops = {
.cap_link_lanes_supported = 1,
.cap_rss_ctx_supported = 1,
.rxfh_max_context_id = BNXT_MAX_ETH_RSS_CTX,
+ .rxfh_indir_space = BNXT_MAX_RSS_TABLE_ENTRIES_P5,
+ .rxfh_priv_size = sizeof(struct bnxt_rss_ctx),
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
ETHTOOL_COALESCE_MAX_FRAMES |
ETHTOOL_COALESCE_USECS_IRQ |
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 08/11] eth: bnxt: use the RSS context XArray instead of the local list
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (7 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 07/11] eth: bnxt: use context priv for struct bnxt_rss_ctx Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32 Jakub Kicinski
` (2 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Core already maintains all RSS contexts in an XArray, no need
to keep a second list in the driver.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 41 ++++++++++---------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 -
.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 11 +++--
3 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 39876feae1a4..6b8966d3ecb6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5970,17 +5970,20 @@ bnxt_cfg_rfs_ring_tbl_idx(struct bnxt *bp,
struct hwrm_cfa_ntuple_filter_alloc_input *req,
struct bnxt_ntuple_filter *fltr)
{
- struct bnxt_rss_ctx *rss_ctx, *tmp;
u16 rxq = fltr->base.rxq;
if (fltr->base.flags & BNXT_ACT_RSS_CTX) {
- list_for_each_entry_safe(rss_ctx, tmp, &bp->rss_ctx_list, list) {
- if (rss_ctx->index == fltr->base.fw_vnic_id) {
- struct bnxt_vnic_info *vnic = &rss_ctx->vnic;
+ struct ethtool_rxfh_context *ctx;
+ struct bnxt_rss_ctx *rss_ctx;
+ struct bnxt_vnic_info *vnic;
- req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
- break;
- }
+ ctx = xa_load(&bp->dev->ethtool->rss_ctx,
+ fltr->base.fw_vnic_id);
+ if (ctx) {
+ rss_ctx = ethtool_rxfh_context_priv(ctx);
+ vnic = &rss_ctx->vnic;
+
+ req->dst_id = cpu_to_le16(vnic->fw_vnic_id);
}
return;
}
@@ -10222,16 +10225,17 @@ void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
vnic->rss_table,
vnic->rss_table_dma_addr);
kfree(rss_ctx->rss_indir_tbl);
- list_del(&rss_ctx->list);
bp->num_rss_ctx--;
}
static void bnxt_hwrm_realloc_rss_ctx_vnic(struct bnxt *bp)
{
bool set_tpa = !!(bp->flags & BNXT_FLAG_TPA);
- struct bnxt_rss_ctx *rss_ctx, *tmp;
+ struct ethtool_rxfh_context *ctx;
+ unsigned long context;
- list_for_each_entry_safe(rss_ctx, tmp, &bp->rss_ctx_list, list) {
+ xa_for_each(&bp->dev->ethtool->rss_ctx, context, ctx) {
+ struct bnxt_rss_ctx *rss_ctx = ethtool_rxfh_context_priv(ctx);
struct bnxt_vnic_info *vnic = &rss_ctx->vnic;
if (bnxt_hwrm_vnic_alloc(bp, vnic, 0, bp->rx_nr_rings) ||
@@ -10247,16 +10251,14 @@ static void bnxt_hwrm_realloc_rss_ctx_vnic(struct bnxt *bp)
void bnxt_clear_rss_ctxs(struct bnxt *bp)
{
- struct bnxt_rss_ctx *rss_ctx, *tmp;
+ struct ethtool_rxfh_context *ctx;
+ unsigned long context;
+
+ xa_for_each(&bp->dev->ethtool->rss_ctx, context, ctx) {
+ struct bnxt_rss_ctx *rss_ctx = ethtool_rxfh_context_priv(ctx);
- list_for_each_entry_safe(rss_ctx, tmp, &bp->rss_ctx_list, list)
bnxt_del_one_rss_ctx(bp, rss_ctx, false);
-}
-
-static void bnxt_init_multi_rss_ctx(struct bnxt *bp)
-{
- INIT_LIST_HEAD(&bp->rss_ctx_list);
- bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX;
+ }
}
/* Allow PF, trusted VFs and VFs with default VLAN to be in promiscuous mode */
@@ -15840,8 +15842,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
INIT_LIST_HEAD(&bp->usr_fltr_list);
if (BNXT_SUPPORTS_NTUPLE_VNIC(bp))
- bnxt_init_multi_rss_ctx(bp);
-
+ bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX;
rc = register_netdev(dev);
if (rc)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 21c3296cf6d9..be40e0513777 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1291,7 +1291,6 @@ struct bnxt_vnic_info {
};
struct bnxt_rss_ctx {
- struct list_head list;
struct bnxt_vnic_info vnic;
u16 *rss_indir_tbl;
u8 index;
@@ -2330,7 +2329,6 @@ struct bnxt {
/* grp_info indexed by completion ring index */
struct bnxt_ring_grp_info *grp_info;
struct bnxt_vnic_info *vnic_info;
- struct list_head rss_ctx_list;
u32 num_rss_ctx;
int nr_vnics;
u16 *rss_indir_tbl;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 2e6e060e2b44..74765583405b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1210,12 +1210,12 @@ static int bnxt_grxclsrule(struct bnxt *bp, struct ethtool_rxnfc *cmd)
static struct bnxt_rss_ctx *bnxt_get_rss_ctx_from_index(struct bnxt *bp,
u32 index)
{
- struct bnxt_rss_ctx *rss_ctx, *tmp;
+ struct ethtool_rxfh_context *ctx;
- list_for_each_entry_safe(rss_ctx, tmp, &bp->rss_ctx_list, list)
- if (rss_ctx->index == index)
- return rss_ctx;
- return NULL;
+ ctx = xa_load(&bp->dev->ethtool->rss_ctx, index);
+ if (!ctx)
+ return NULL;
+ return ethtool_rxfh_context_priv(ctx);
}
static int bnxt_alloc_rss_ctx_rss_table(struct bnxt *bp,
@@ -1903,7 +1903,6 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
rss_ctx = ethtool_rxfh_context_priv(ctx);
- list_add_tail(&rss_ctx->list, &bp->rss_ctx_list);
bp->num_rss_ctx++;
vnic = &rss_ctx->vnic;
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (8 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 08/11] eth: bnxt: use the RSS context XArray instead of the local list Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-03 10:51 ` Przemek Kitszel
2024-07-02 23:47 ` [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 11/11] eth: bnxt: pad out the correct indirection table Jakub Kicinski
11 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Ethtool core stores indirection table with u32 entries, "just to be safe".
Switch the type in the driver, so that it's easier to swap local tables
for the core ones.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 8 ++++----
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 4 ++--
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6b8966d3ecb6..4176459921b5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6225,7 +6225,7 @@ static u16 bnxt_cp_ring_for_tx(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
{
int entries;
- u16 *tbl;
+ u32 *tbl;
if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS)
entries = BNXT_MAX_RSS_TABLE_ENTRIES_P5;
@@ -6248,7 +6248,7 @@ int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
{
u16 max_rings, max_entries, pad, i;
- u16 *rss_indir_tbl;
+ u32 *rss_indir_tbl;
if (!bp->rx_nr_rings)
return;
@@ -6269,12 +6269,12 @@ void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
pad = bp->rss_indir_tbl_entries - max_entries;
if (pad)
- memset(&rss_indir_tbl[i], 0, pad * sizeof(u16));
+ memset(&rss_indir_tbl[i], 0, pad * sizeof(u32));
}
static u16 bnxt_get_max_rss_ring(struct bnxt *bp)
{
- u16 i, tbl_size, max_ring = 0;
+ u32 i, tbl_size, max_ring = 0;
if (!bp->rss_indir_tbl)
return 0;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index be40e0513777..1a33824a32a8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1292,7 +1292,7 @@ struct bnxt_vnic_info {
struct bnxt_rss_ctx {
struct bnxt_vnic_info vnic;
- u16 *rss_indir_tbl;
+ u32 *rss_indir_tbl;
u8 index;
};
@@ -2331,7 +2331,7 @@ struct bnxt {
struct bnxt_vnic_info *vnic_info;
u32 num_rss_ctx;
int nr_vnics;
- u16 *rss_indir_tbl;
+ u32 *rss_indir_tbl;
u16 rss_indir_tbl_entries;
u32 rss_hash_cfg;
u32 rss_hash_delta;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 74765583405b..e5f687d4a455 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1804,7 +1804,7 @@ static int bnxt_get_rxfh(struct net_device *dev,
u32 rss_context = rxfh->rss_context;
struct bnxt_rss_ctx *rss_ctx = NULL;
struct bnxt *bp = netdev_priv(dev);
- u16 *indir_tbl = bp->rss_indir_tbl;
+ u32 *indir_tbl = bp->rss_indir_tbl;
struct bnxt_vnic_info *vnic;
u32 i, tbl_size;
@@ -1848,7 +1848,7 @@ static void bnxt_modify_rss(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
}
if (rxfh->indir) {
u32 i, pad, tbl_size = bnxt_get_rxfh_indir_size(bp->dev);
- u16 *indir_tbl = bp->rss_indir_tbl;
+ u32 *indir_tbl = bp->rss_indir_tbl;
if (rss_ctx)
indir_tbl = rss_ctx->rss_indir_tbl;
@@ -1856,7 +1856,7 @@ static void bnxt_modify_rss(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
indir_tbl[i] = rxfh->indir[i];
pad = bp->rss_indir_tbl_entries - tbl_size;
if (pad)
- memset(&bp->rss_indir_tbl[i], 0, pad * sizeof(u16));
+ memset(&bp->rss_indir_tbl[i], 0, pad * sizeof(u32));
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (9 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32 Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
2024-07-03 11:08 ` Przemek Kitszel
2024-07-03 11:39 ` Edward Cree
2024-07-02 23:47 ` [PATCH net-next 11/11] eth: bnxt: pad out the correct indirection table Jakub Kicinski
11 siblings, 2 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
Instead of allocating a separate indir table in the vnic use
the one already present in the RSS context allocated by the core.
This doesn't save much LoC but we won't have to worry about syncing
the local version back to the core, once core learns how to dump
contexts.
Add ethtool_rxfh_priv_context() for converting from priv pointer
to the context. The cast is a bit ugly (understatement) and some
driver paths make carrying the context pointer in addition to
driver priv pointer quite tedious.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 27 +++++++--------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 5 ++-
.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 34 +++++++++----------
include/linux/ethtool.h | 5 +++
4 files changed, 35 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4176459921b5..8ee57b07ffb6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6222,10 +6222,9 @@ static u16 bnxt_cp_ring_for_tx(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
return bnxt_cp_ring_from_grp(bp, &txr->tx_ring_struct);
}
-int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
+static int bnxt_alloc_rss_indir_tbl(struct bnxt *bp)
{
int entries;
- u32 *tbl;
if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS)
entries = BNXT_MAX_RSS_TABLE_ENTRIES_P5;
@@ -6233,19 +6232,16 @@ int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
entries = HW_HASH_INDEX_SIZE;
bp->rss_indir_tbl_entries = entries;
- tbl = kmalloc_array(entries, sizeof(*bp->rss_indir_tbl), GFP_KERNEL);
- if (!tbl)
+ bp->rss_indir_tbl =
+ kmalloc_array(entries, sizeof(*bp->rss_indir_tbl), GFP_KERNEL);
+ if (!bp->rss_indir_tbl)
return -ENOMEM;
- if (rss_ctx)
- rss_ctx->rss_indir_tbl = tbl;
- else
- bp->rss_indir_tbl = tbl;
-
return 0;
}
-void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
+void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp,
+ struct ethtool_rxfh_context *ctx)
{
u16 max_rings, max_entries, pad, i;
u32 *rss_indir_tbl;
@@ -6259,8 +6255,8 @@ void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
max_rings = bp->rx_nr_rings;
max_entries = bnxt_get_rxfh_indir_size(bp->dev);
- if (rss_ctx)
- rss_indir_tbl = &rss_ctx->rss_indir_tbl[0];
+ if (ctx)
+ rss_indir_tbl = ethtool_rxfh_context_indir(ctx);
else
rss_indir_tbl = &bp->rss_indir_tbl[0];
@@ -6315,10 +6311,12 @@ static void bnxt_fill_hw_rss_tbl_p5(struct bnxt *bp,
struct bnxt_vnic_info *vnic)
{
__le16 *ring_tbl = vnic->rss_table;
+ struct ethtool_rxfh_context *ctx;
struct bnxt_rx_ring_info *rxr;
u16 tbl_size, i;
tbl_size = bnxt_get_rxfh_indir_size(bp->dev);
+ ctx = ethtool_rxfh_priv_context(vnic->rss_ctx);
for (i = 0; i < tbl_size; i++) {
u16 ring_id, j;
@@ -6326,7 +6324,7 @@ static void bnxt_fill_hw_rss_tbl_p5(struct bnxt *bp,
if (vnic->flags & BNXT_VNIC_NTUPLE_FLAG)
j = ethtool_rxfh_indir_default(i, bp->rx_nr_rings);
else if (vnic->flags & BNXT_VNIC_RSSCTX_FLAG)
- j = vnic->rss_ctx->rss_indir_tbl[i];
+ j = ethtool_rxfh_context_indir(ctx)[i];
else
j = bp->rss_indir_tbl[i];
rxr = &bp->rx_ring[j];
@@ -10224,7 +10222,6 @@ void bnxt_del_one_rss_ctx(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
dma_free_coherent(&bp->pdev->dev, vnic->rss_table_size,
vnic->rss_table,
vnic->rss_table_dma_addr);
- kfree(rss_ctx->rss_indir_tbl);
bp->num_rss_ctx--;
}
@@ -15685,7 +15682,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
bp->flags |= BNXT_FLAG_CHIP_P7;
}
- rc = bnxt_alloc_rss_indir_tbl(bp, NULL);
+ rc = bnxt_alloc_rss_indir_tbl(bp);
if (rc)
goto init_err_pci_clean;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 1a33824a32a8..a4420db55de9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1292,7 +1292,6 @@ struct bnxt_vnic_info {
struct bnxt_rss_ctx {
struct bnxt_vnic_info vnic;
- u32 *rss_indir_tbl;
u8 index;
};
@@ -2808,8 +2807,8 @@ int bnxt_hwrm_cfa_ntuple_filter_alloc(struct bnxt *bp,
int bnxt_hwrm_vnic_set_tpa(struct bnxt *bp, struct bnxt_vnic_info *vnic,
u32 tpa_flags);
void bnxt_fill_ipv6_mask(__be32 mask[4]);
-int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx);
-void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx);
+void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp,
+ struct ethtool_rxfh_context *ctx);
int bnxt_get_nr_rss_ctxs(struct bnxt *bp, int rx_rings);
int bnxt_hwrm_vnic_cfg(struct bnxt *bp, struct bnxt_vnic_info *vnic);
int bnxt_hwrm_vnic_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic,
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index e5f687d4a455..13e9b3b26f09 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1218,11 +1218,10 @@ static struct bnxt_rss_ctx *bnxt_get_rss_ctx_from_index(struct bnxt *bp,
return ethtool_rxfh_context_priv(ctx);
}
-static int bnxt_alloc_rss_ctx_rss_table(struct bnxt *bp,
- struct bnxt_rss_ctx *rss_ctx)
+static int bnxt_alloc_vnic_rss_table(struct bnxt *bp,
+ struct bnxt_vnic_info *vnic)
{
int size = L1_CACHE_ALIGN(BNXT_MAX_RSS_TABLE_SIZE_P5);
- struct bnxt_vnic_info *vnic = &rss_ctx->vnic;
vnic->rss_table_size = size + HW_HASH_KEY_SIZE;
vnic->rss_table = dma_alloc_coherent(&bp->pdev->dev,
@@ -1801,7 +1800,6 @@ static u32 bnxt_get_rxfh_key_size(struct net_device *dev)
static int bnxt_get_rxfh(struct net_device *dev,
struct ethtool_rxfh_param *rxfh)
{
- u32 rss_context = rxfh->rss_context;
struct bnxt_rss_ctx *rss_ctx = NULL;
struct bnxt *bp = netdev_priv(dev);
u32 *indir_tbl = bp->rss_indir_tbl;
@@ -1815,10 +1813,13 @@ static int bnxt_get_rxfh(struct net_device *dev,
vnic = &bp->vnic_info[BNXT_VNIC_DEFAULT];
if (rxfh->rss_context) {
- rss_ctx = bnxt_get_rss_ctx_from_index(bp, rss_context);
- if (!rss_ctx)
+ struct ethtool_rxfh_context *ctx;
+
+ ctx = xa_load(&bp->dev->ethtool->rss_ctx, rxfh->rss_context);
+ if (!ctx)
return -EINVAL;
- indir_tbl = rss_ctx->rss_indir_tbl;
+ indir_tbl = ethtool_rxfh_context_indir(ctx);
+ rss_ctx = ethtool_rxfh_context_priv(ctx);
vnic = &rss_ctx->vnic;
}
@@ -1834,7 +1835,8 @@ static int bnxt_get_rxfh(struct net_device *dev,
return 0;
}
-static void bnxt_modify_rss(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
+static void bnxt_modify_rss(struct bnxt *bp, struct ethtool_rxfh_context *ctx,
+ struct bnxt_rss_ctx *rss_ctx,
const struct ethtool_rxfh_param *rxfh)
{
if (rxfh->key) {
@@ -1851,7 +1853,7 @@ static void bnxt_modify_rss(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx,
u32 *indir_tbl = bp->rss_indir_tbl;
if (rss_ctx)
- indir_tbl = rss_ctx->rss_indir_tbl;
+ indir_tbl = ethtool_rxfh_context_indir(ctx);
for (i = 0; i < tbl_size; i++)
indir_tbl[i] = rxfh->indir[i];
pad = bp->rss_indir_tbl_entries - tbl_size;
@@ -1909,15 +1911,11 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
vnic->rss_ctx = rss_ctx;
vnic->flags |= BNXT_VNIC_RSSCTX_FLAG;
vnic->vnic_id = BNXT_VNIC_ID_INVALID;
- rc = bnxt_alloc_rss_ctx_rss_table(bp, rss_ctx);
+ rc = bnxt_alloc_vnic_rss_table(bp, vnic);
if (rc)
goto out;
- rc = bnxt_alloc_rss_indir_tbl(bp, rss_ctx);
- if (rc)
- goto out;
-
- bnxt_set_dflt_rss_indir_tbl(bp, rss_ctx);
+ bnxt_set_dflt_rss_indir_tbl(bp, ctx);
memcpy(vnic->rss_hash_key, bp->rss_hash_key, HW_HASH_KEY_SIZE);
rc = bnxt_hwrm_vnic_alloc(bp, vnic, 0, bp->rx_nr_rings);
@@ -1931,7 +1929,7 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
NL_SET_ERR_MSG_MOD(extack, "Unable to setup TPA");
goto out;
}
- bnxt_modify_rss(bp, rss_ctx, rxfh);
+ bnxt_modify_rss(bp, ctx, rss_ctx, rxfh);
rc = __bnxt_setup_vnic_p5(bp, vnic);
if (rc) {
@@ -1961,7 +1959,7 @@ static int bnxt_modify_rxfh_context(struct net_device *dev,
rss_ctx = ethtool_rxfh_context_priv(ctx);
- bnxt_modify_rss(bp, rss_ctx, rxfh);
+ bnxt_modify_rss(bp, ctx, rss_ctx, rxfh);
return bnxt_hwrm_vnic_rss_cfg_p5(bp, &rss_ctx->vnic);
}
@@ -1990,7 +1988,7 @@ static int bnxt_set_rxfh(struct net_device *dev,
if (rxfh->hfunc && rxfh->hfunc != ETH_RSS_HASH_TOP)
return -EOPNOTSUPP;
- bnxt_modify_rss(bp, NULL, rxfh);
+ bnxt_modify_rss(bp, NULL, NULL, rxfh);
bnxt_clear_usr_fltrs(bp, false);
if (netif_running(bp->dev)) {
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index dc8ed93097c3..5b28407d0619 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -190,6 +190,11 @@ static inline void *ethtool_rxfh_context_priv(struct ethtool_rxfh_context *ctx)
return ctx->data;
}
+static inline struct ethtool_rxfh_context *ethtool_rxfh_priv_context(void *priv)
+{
+ return container_of((u8(*)[])priv, struct ethtool_rxfh_context, data);
+}
+
static inline u32 *ethtool_rxfh_context_indir(struct ethtool_rxfh_context *ctx)
{
return (u32 *)(ctx->data + ALIGN(ctx->priv_size, sizeof(u32)));
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next 11/11] eth: bnxt: pad out the correct indirection table
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
` (10 preceding siblings ...)
2024-07-02 23:47 ` [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context Jakub Kicinski
@ 2024-07-02 23:47 ` Jakub Kicinski
11 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-02 23:47 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, ecree.xilinx, michael.chan,
Jakub Kicinski
bnxt allocates tables of max size, and changes the used size
based on number of active rings. The unused entries get padded
out with zeros. bnxt_modify_rss() seems to always pad out
the table of the main / default RSS context, instead of
the table of the modified context.
I haven't observed any behavior change due to this patch,
so I don't think it's a fix. Not entirely sure what role
the padding plays, 0 is a valid queue ID.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 13e9b3b26f09..1d587846c394 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1858,7 +1858,7 @@ static void bnxt_modify_rss(struct bnxt *bp, struct ethtool_rxfh_context *ctx,
indir_tbl[i] = rxfh->indir[i];
pad = bp->rss_indir_tbl_entries - tbl_size;
if (pad)
- memset(&bp->rss_indir_tbl[i], 0, pad * sizeof(u32));
+ memset(&indir_tbl[i], 0, pad * sizeof(u32));
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32
2024-07-02 23:47 ` [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32 Jakub Kicinski
@ 2024-07-03 10:51 ` Przemek Kitszel
2024-07-03 13:49 ` Jakub Kicinski
0 siblings, 1 reply; 30+ messages in thread
From: Przemek Kitszel @ 2024-07-03 10:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, edumazet, davem, pabeni, ecree.xilinx, michael.chan
On 7/3/24 01:47, Jakub Kicinski wrote:
> Ethtool core stores indirection table with u32 entries, "just to be safe".
> Switch the type in the driver, so that it's easier to swap local tables
> for the core ones.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 8 ++++----
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 4 ++--
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 6 +++---
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 6b8966d3ecb6..4176459921b5 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -6225,7 +6225,7 @@ static u16 bnxt_cp_ring_for_tx(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
> int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
> {
> int entries;
> - u16 *tbl;
> + u32 *tbl;
>
> if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS)
> entries = BNXT_MAX_RSS_TABLE_ENTRIES_P5;
it's a shame that default git context is so small, a few lines below we
have tbl alloc:
tbl = kmalloc_array(entries, sizeof(*bp->rss_indir_tbl), GFP_KERNEL);
> @@ -6248,7 +6248,7 @@ int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
> void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
> {
> u16 max_rings, max_entries, pad, i;
> - u16 *rss_indir_tbl;
> + u32 *rss_indir_tbl;
>
> if (!bp->rx_nr_rings)
> return;
> @@ -6269,12 +6269,12 @@ void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx)
>
> pad = bp->rss_indir_tbl_entries - max_entries;
> if (pad)
> - memset(&rss_indir_tbl[i], 0, pad * sizeof(u16));
> + memset(&rss_indir_tbl[i], 0, pad * sizeof(u32));
> }
without the above allocation line (perhaps you could mention necessary
bit of that in commit msg in case of v2) this patch could not be proved
correct :)
with due diligence done:
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
OTOH, I assume we need this in-driver table only to keep it over the
up→down→up cycle. Could we just keep it as inactive in the core?
(And xa_mark() it as inactive to avoid reporting to the user or any
other actions that we want to avoid)
PS. patch2 is duplicated for me
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts
2024-07-02 23:47 ` [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts Jakub Kicinski
@ 2024-07-03 11:08 ` Edward Cree
2024-07-03 13:43 ` Jakub Kicinski
0 siblings, 1 reply; 30+ messages in thread
From: Edward Cree @ 2024-07-03 11:08 UTC (permalink / raw)
To: Jakub Kicinski, davem; +Cc: netdev, edumazet, pabeni, michael.chan
On 03/07/2024 00:47, Jakub Kicinski wrote:
> RSS contexts may get lost from a device, in various extreme circumstances.
> Specifically if the firmware leaks resources and resets, or crashes and
> either recovers in partially working state or the crash causes a
> different FW version to run - creating the context again may fail.
So, I deliberately *didn't* do this, on the grounds that if the user
fixed things by updating FW and resetting again, their contexts could
get restored. I suppose big users like Meta will have orchestration
doing all that work anyway so it doesn't matter.
> Drivers should do their absolute best to prevent this from happening.
> When it does, however, telling user that a context exists, when it can't
> possibly be used any more is counter productive. Add a helper for
> drivers to discard contexts. Print an error, in the future netlink
> notification will also be sent.
Possibility of a netlink notification makes the idea of a broken flag
a bit more workable imho. But it's up to you which way to go.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context
2024-07-02 23:47 ` [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context Jakub Kicinski
@ 2024-07-03 11:08 ` Przemek Kitszel
2024-07-03 11:39 ` Edward Cree
1 sibling, 0 replies; 30+ messages in thread
From: Przemek Kitszel @ 2024-07-03 11:08 UTC (permalink / raw)
To: Jakub Kicinski, ecree.xilinx
Cc: netdev, edumazet, pabeni, michael.chan, davem
On 7/3/24 01:47, Jakub Kicinski wrote:
> Instead of allocating a separate indir table in the vnic use
> the one already present in the RSS context allocated by the core.
> This doesn't save much LoC but we won't have to worry about syncing
> the local version back to the core, once core learns how to dump
> contexts.
>
> Add ethtool_rxfh_priv_context() for converting from priv pointer
> to the context. The cast is a bit ugly (understatement) and some
could we wrap flex u8 data[] array of struct ethtool_rxfh_context
in an union with void *priv?, then no cast will be needed.
but I still would prefer to don't abuse ownership over in-core data,
and just keep it as inactive there, at the very end this could be
moved to devm_ to avoid leaks
> driver paths make carrying the context pointer in addition to
> driver priv pointer quite tedious.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> @@ -190,6 +190,11 @@ static inline void *ethtool_rxfh_context_priv(struct ethtool_rxfh_context *ctx)
> return ctx->data;
> }
>
> +static inline struct ethtool_rxfh_context *ethtool_rxfh_priv_context(void *priv)
> +{
> + return container_of((u8(*)[])priv, struct ethtool_rxfh_context, data);
> +}
> +
> static inline u32 *ethtool_rxfh_context_indir(struct ethtool_rxfh_context *ctx)
> {
> return (u32 *)(ctx->data + ALIGN(ctx->priv_size, sizeof(u32)));
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 02/11] net: ethtool: let drivers declare max size of RSS indir table and key
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let drivers " Jakub Kicinski
@ 2024-07-03 11:16 ` Edward Cree
0 siblings, 0 replies; 30+ messages in thread
From: Edward Cree @ 2024-07-03 11:16 UTC (permalink / raw)
To: Jakub Kicinski, davem; +Cc: netdev, edumazet, pabeni, michael.chan
On 03/07/2024 00:47, Jakub Kicinski wrote:
> Some drivers (bnxt but I think also mlx5 from ML discussions) change
> the size of the indirection table depending on the number of Rx rings.
> Decouple the max table size from the size of the currently used table,
> so that we can reserve space in the context for table growth.
>
> Static members in ethtool_ops are good enough for now, we can add
> callbacks to read the max size more dynamically if someone needs
> that.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> include/linux/ethtool.h | 20 +++++++-----------
> net/ethtool/ioctl.c | 46 ++++++++++++++++++++++++++++++++---------
> 2 files changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 3ce5be0d168a..dc8ed93097c3 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -173,6 +173,7 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
> struct ethtool_rxfh_context {
> u32 indir_size;
> u32 key_size;
> + u32 key_off;
kdoc needed.
Other than that LGTM.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context
2024-07-02 23:47 ` [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context Jakub Kicinski
2024-07-03 11:08 ` Przemek Kitszel
@ 2024-07-03 11:39 ` Edward Cree
2024-07-03 13:51 ` Jakub Kicinski
1 sibling, 1 reply; 30+ messages in thread
From: Edward Cree @ 2024-07-03 11:39 UTC (permalink / raw)
To: Jakub Kicinski, davem; +Cc: netdev, edumazet, pabeni, michael.chan
On 03/07/2024 00:47, Jakub Kicinski wrote:
> Instead of allocating a separate indir table in the vnic use
> the one already present in the RSS context allocated by the core.
> This doesn't save much LoC but we won't have to worry about syncing
> the local version back to the core, once core learns how to dump
> contexts.
>
> Add ethtool_rxfh_priv_context() for converting from priv pointer
> to the context. The cast is a bit ugly (understatement) and some
> driver paths make carrying the context pointer in addition to
> driver priv pointer quite tedious.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
...
> @@ -6315,10 +6311,12 @@ static void bnxt_fill_hw_rss_tbl_p5(struct bnxt *bp,
> struct bnxt_vnic_info *vnic)
> {
> __le16 *ring_tbl = vnic->rss_table;
> + struct ethtool_rxfh_context *ctx;
> struct bnxt_rx_ring_info *rxr;
> u16 tbl_size, i;
>
> tbl_size = bnxt_get_rxfh_indir_size(bp->dev);
> + ctx = ethtool_rxfh_priv_context(vnic->rss_ctx);
Not super familiar with this driver or why this need arises, but
would it be simpler to just store ctx in vnic instead of priv?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
2024-07-02 23:47 ` [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends Jakub Kicinski
@ 2024-07-03 12:06 ` Edward Cree
2024-07-03 12:49 ` Pavan Chebbi
2024-07-04 6:19 ` kernel test robot
2024-07-05 0:34 ` kernel test robot
2 siblings, 1 reply; 30+ messages in thread
From: Edward Cree @ 2024-07-03 12:06 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, michael.chan, Pavan Chebbi
On 03/07/2024 00:47, Jakub Kicinski wrote:
> Use the new ethtool ops for RSS context management. The conversion
> is pretty straightforward cut / paste of the right chunks of the
> combined handler. Main change is that we let the core pick the IDs
> (bitmap will be removed separately for ease of review), so we need
> to tell the core when we lose a context.
> Since the new API passes rxfh as const, change bnxt_modify_rss()
> to also take const.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
...
> @@ -5271,6 +5296,7 @@ void bnxt_ethtool_free(struct bnxt *bp)
> const struct ethtool_ops bnxt_ethtool_ops = {
> .cap_link_lanes_supported = 1,
> .cap_rss_ctx_supported = 1,
> + .rxfh_max_context_id = BNXT_MAX_ETH_RSS_CTX,
According to Pavan [1], this limit only existed for the sake of the
SW side (presumably the rss_ctx_bmap), so probably it can be removed
in patch #5.
The higher FW limit Pavan mentions appears to be on number rather
than index; at least I can't see anything in the driver feeding the
user-facing context ID to the device. But I don't know whether FW
has any opportunity to say ENOMEM, or whether the driver needs to
validate against the hardware limit itself. Hopefully Pavan (CCed)
can elaborate.
-ed
[1] https://lore.kernel.org/netdev/CALs4sv2dyy3uy+Xznm41M3uOkv1TSoGMwVBL5Cwzv=_E=+L_4A@mail.gmail.com/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
2024-07-03 12:06 ` Edward Cree
@ 2024-07-03 12:49 ` Pavan Chebbi
2024-07-03 13:46 ` Jakub Kicinski
0 siblings, 1 reply; 30+ messages in thread
From: Pavan Chebbi @ 2024-07-03 12:49 UTC (permalink / raw)
To: Edward Cree; +Cc: Jakub Kicinski, davem, netdev, edumazet, pabeni, michael.chan
[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]
On Wed, Jul 3, 2024 at 5:36 PM Edward Cree <ecree.xilinx@gmail.com> wrote:
>
> On 03/07/2024 00:47, Jakub Kicinski wrote:
> > Use the new ethtool ops for RSS context management. The conversion
> > is pretty straightforward cut / paste of the right chunks of the
> > combined handler. Main change is that we let the core pick the IDs
> > (bitmap will be removed separately for ease of review), so we need
> > to tell the core when we lose a context.
> > Since the new API passes rxfh as const, change bnxt_modify_rss()
> > to also take const.
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ...
> > @@ -5271,6 +5296,7 @@ void bnxt_ethtool_free(struct bnxt *bp)
> > const struct ethtool_ops bnxt_ethtool_ops = {
> > .cap_link_lanes_supported = 1,
> > .cap_rss_ctx_supported = 1,
> > + .rxfh_max_context_id = BNXT_MAX_ETH_RSS_CTX,
>
> According to Pavan [1], this limit only existed for the sake of the
> SW side (presumably the rss_ctx_bmap), so probably it can be removed
> in patch #5.
> The higher FW limit Pavan mentions appears to be on number rather
> than index; at least I can't see anything in the driver feeding the
> user-facing context ID to the device. But I don't know whether FW
Hi Ed, you are right. It's a SW side number and the device has no
connection with the ID.
> has any opportunity to say ENOMEM, or whether the driver needs to
> validate against the hardware limit itself. Hopefully Pavan (CCed)
> can elaborate.
Because the driver is not aware of the hardware limit, and the limit
is dynamic, we can rely on FW to know if the resource request we made
was honored (there is no direct ENOMEM mechanism)
The driver already does this when we make a runtime check for
resources using bnxt_rfs_capable() when an RSS ctx is being created.
But for this version of the driver, I would prefer to keep a limit
because we have some FW improvements coming in, in the area of
resource management.
Though removing the limit may not break anything, I'd prefer to have
it removed once a FW with improvements (indicated by a query
flag/caps) is available.
Michael may also add his thoughts on this.
>
> -ed
>
> [1] https://lore.kernel.org/netdev/CALs4sv2dyy3uy+Xznm41M3uOkv1TSoGMwVBL5Cwzv=_E=+L_4A@mail.gmail.com/
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts
2024-07-03 11:08 ` Edward Cree
@ 2024-07-03 13:43 ` Jakub Kicinski
2024-07-03 14:15 ` Edward Cree
0 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-03 13:43 UTC (permalink / raw)
To: Edward Cree; +Cc: davem, netdev, edumazet, pabeni, michael.chan
On Wed, 3 Jul 2024 12:08:36 +0100 Edward Cree wrote:
> On 03/07/2024 00:47, Jakub Kicinski wrote:
> > RSS contexts may get lost from a device, in various extreme circumstances.
> > Specifically if the firmware leaks resources and resets, or crashes and
> > either recovers in partially working state or the crash causes a
> > different FW version to run - creating the context again may fail.
>
> So, I deliberately *didn't* do this, on the grounds that if the user
> fixed things by updating FW and resetting again, their contexts could
> get restored. I suppose big users like Meta will have orchestration
> doing all that work anyway so it doesn't matter.
"We" don't reset FW while workload is running. I'm speculating why bnxt
may lose the contexts. From my perspective if contexts get lost the
machine should get taken out of production and at least power cycled.
> > Drivers should do their absolute best to prevent this from happening.
> > When it does, however, telling user that a context exists, when it can't
> > possibly be used any more is counter productive. Add a helper for
> > drivers to discard contexts. Print an error, in the future netlink
> > notification will also be sent.
>
> Possibility of a netlink notification makes the idea of a broken flag
> a bit more workable imho. But it's up to you which way to go.
Oh, have we talked about this? Now that you mention the broken flag
I recall talking about devlink health reporter.. a while back.
I don't have a preference on how we deal with the lost contexts.
The more obvious we make it to orchestration that the machine is broken
the better. Can you point me to the discussion / describe the broken
flag?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
2024-07-03 12:49 ` Pavan Chebbi
@ 2024-07-03 13:46 ` Jakub Kicinski
0 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-03 13:46 UTC (permalink / raw)
To: Pavan Chebbi; +Cc: Edward Cree, davem, netdev, edumazet, pabeni, michael.chan
On Wed, 3 Jul 2024 18:19:18 +0530 Pavan Chebbi wrote:
> > has any opportunity to say ENOMEM, or whether the driver needs to
> > validate against the hardware limit itself. Hopefully Pavan (CCed)
> > can elaborate.
>
> Because the driver is not aware of the hardware limit, and the limit
> is dynamic, we can rely on FW to know if the resource request we made
> was honored (there is no direct ENOMEM mechanism)
> The driver already does this when we make a runtime check for
> resources using bnxt_rfs_capable() when an RSS ctx is being created.
> But for this version of the driver, I would prefer to keep a limit
> because we have some FW improvements coming in, in the area of
> resource management.
> Though removing the limit may not break anything, I'd prefer to have
> it removed once a FW with improvements (indicated by a query
> flag/caps) is available.
I like keeping the limit too, FWIW, it'd be great to give this info
to user space in due course to let it make more informed decisions.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32
2024-07-03 10:51 ` Przemek Kitszel
@ 2024-07-03 13:49 ` Jakub Kicinski
2024-07-03 14:02 ` Przemek Kitszel
0 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-03 13:49 UTC (permalink / raw)
To: Przemek Kitszel
Cc: netdev, edumazet, davem, pabeni, ecree.xilinx, michael.chan
On Wed, 3 Jul 2024 12:51:58 +0200 Przemek Kitszel wrote:
> OTOH, I assume we need this in-driver table only to keep it over the
> up→down→up cycle. Could we just keep it as inactive in the core?
> (And xa_mark() it as inactive to avoid reporting to the user or any
> other actions that we want to avoid)
Do you mean keep the table for the default context / context 0
in the core as well?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context
2024-07-03 11:39 ` Edward Cree
@ 2024-07-03 13:51 ` Jakub Kicinski
0 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-03 13:51 UTC (permalink / raw)
To: Edward Cree; +Cc: davem, netdev, edumazet, pabeni, michael.chan
On Wed, 3 Jul 2024 12:39:09 +0100 Edward Cree wrote:
> > tbl_size = bnxt_get_rxfh_indir_size(bp->dev);
> > + ctx = ethtool_rxfh_priv_context(vnic->rss_ctx);
>
> Not super familiar with this driver or why this need arises, but
> would it be simpler to just store ctx in vnic instead of priv?
Yup, I think that should work!
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32
2024-07-03 13:49 ` Jakub Kicinski
@ 2024-07-03 14:02 ` Przemek Kitszel
2024-07-03 16:02 ` Jakub Kicinski
0 siblings, 1 reply; 30+ messages in thread
From: Przemek Kitszel @ 2024-07-03 14:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, edumazet, davem, pabeni, ecree.xilinx, michael.chan
On 7/3/24 15:49, Jakub Kicinski wrote:
> On Wed, 3 Jul 2024 12:51:58 +0200 Przemek Kitszel wrote:
>> OTOH, I assume we need this in-driver table only to keep it over the
>> up→down→up cycle. Could we just keep it as inactive in the core?
>> (And xa_mark() it as inactive to avoid reporting to the user or any
>> other actions that we want to avoid)
>
> Do you mean keep the table for the default context / context 0
> in the core as well?
>
it was not obvious for me that this is only about the default one,
but that would yield more uniformity and cleanups I guess
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts
2024-07-03 13:43 ` Jakub Kicinski
@ 2024-07-03 14:15 ` Edward Cree
0 siblings, 0 replies; 30+ messages in thread
From: Edward Cree @ 2024-07-03 14:15 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, michael.chan
On 03/07/2024 14:43, Jakub Kicinski wrote:
> On Wed, 3 Jul 2024 12:08:36 +0100 Edward Cree wrote:
>> Possibility of a netlink notification makes the idea of a broken flag
>> a bit more workable imho. But it's up to you which way to go.
>
> Oh, have we talked about this? Now that you mention the broken flag
> I recall talking about devlink health reporter.. a while back.
>
> I don't have a preference on how we deal with the lost contexts.
> The more obvious we make it to orchestration that the machine is broken
> the better. Can you point me to the discussion / describe the broken
> flag?
We discussed it briefly on v4 back in October [1][2]. Idea was
there'd be a flag in struct ethtool_rxfh_context with meaning of
"this context is not present in hw owing to reinsertion failure
after a device reset", reported in ethtool -x and perhaps also
devlink health. Driver could set this flag (which would trigger
a netlink notification) but could also clear it if a second
reset (or some runtime configuration change) triggers another
round of reinsertion which succeeds this time.
Fwiw I don't have a strong preference either — like I say, you do
what you think is best.
-ed
[1]: https://lore.kernel.org/all/2ea45188-5554-8067-820d-378cada735ee@gmail.com/T/#ma8fce7df8b65601009551839d9d102c49e79803a
[2]: https://lore.kernel.org/all/2ea45188-5554-8067-820d-378cada735ee@gmail.com/T/#m073cb990982d89e35d78edf364389de62256664b
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32
2024-07-03 14:02 ` Przemek Kitszel
@ 2024-07-03 16:02 ` Jakub Kicinski
0 siblings, 0 replies; 30+ messages in thread
From: Jakub Kicinski @ 2024-07-03 16:02 UTC (permalink / raw)
To: Przemek Kitszel
Cc: netdev, edumazet, davem, pabeni, ecree.xilinx, michael.chan
On Wed, 3 Jul 2024 16:02:38 +0200 Przemek Kitszel wrote:
> On 7/3/24 15:49, Jakub Kicinski wrote:
> > On Wed, 3 Jul 2024 12:51:58 +0200 Przemek Kitszel wrote:
> >> OTOH, I assume we need this in-driver table only to keep it over the
> >> up→down→up cycle. Could we just keep it as inactive in the core?
> >> (And xa_mark() it as inactive to avoid reporting to the user or any
> >> other actions that we want to avoid)
> >
> > Do you mean keep the table for the default context / context 0
> > in the core as well?
>
> it was not obvious for me that this is only about the default one,
> but that would yield more uniformity and cleanups I guess
I'll add a note to the commit message. Initially I was hoping to add
context 0, too, but there's a number of gaps. We will hopefully
converge to that over time.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
2024-07-02 23:47 ` [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends Jakub Kicinski
2024-07-03 12:06 ` Edward Cree
@ 2024-07-04 6:19 ` kernel test robot
2024-07-05 0:34 ` kernel test robot
2 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2024-07-04 6:19 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: llvm, oe-kbuild-all, netdev, edumazet, pabeni, ecree.xilinx,
michael.chan, Jakub Kicinski
Hi Jakub,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Jakub-Kicinski/net-ethtool-let-drivers-remove-lost-RSS-contexts/20240703-163816
base: net-next/main
patch link: https://lore.kernel.org/r/20240702234757.4188344-6-kuba%40kernel.org
patch subject: [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
config: i386-randconfig-014-20240704 (https://download.01.org/0day-ci/archive/20240704/202407041406.1dVK7Qf1-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240704/202407041406.1dVK7Qf1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407041406.1dVK7Qf1-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_dynamic_debug.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_printf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_bitmap.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_uuid.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_xarray.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_blackhole_dev.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_meminit.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_free_pages.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/test_objpool.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/ts_kmp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/ts_bm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/ts_fsm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pinctrl/pinctrl-mcp23s08_i2c.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pinctrl/pinctrl-mcp23s08.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/video/backlight/rt4831-backlight.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/regulator/da9121-regulator.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/regulator/max20411-regulator.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/regulator/tps6286x-regulator.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/tty/n_hdlc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/tty/n_gsm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/tty/ttynull.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/char/agp/amd-k7-agp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/char/agp/amd64-agp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/char/agp/sis-agp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/base/regmap/regmap-i2c.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/base/regmap/regmap-spmi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/block/brd.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/block/ublk_drv.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mfd/arizona.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mfd/pcf50633-gpio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mfd/rt4831.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_pmem.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvdimm/nd_btt.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/dax/dax.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/usb/class/usbtmc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/usb/misc/isight_firmware.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/usb/misc/yurex.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/input/matrix-keymap.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/i2c/busses/i2c-ccgx-ucsi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/i2c/busses/i2c-ali1563.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/i2c/busses/i2c-pxa.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/i2c/uda1342.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/rc/rc-core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/usb/dvb-usb/dvb-usb-dibusb-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/usb/dvb-usb/dvb-usb-dibusb-mc-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/usb/go7007/go7007.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/dvb-frontends/au8522_decoder.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/dvb-frontends/mb86a16.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/v4l2-core/v4l2-async.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/v4l2-core/v4l2-fwnode.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/radio/si470x/radio-si470x-common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/corsair-cpro.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/mr75203.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/watchdog/simatic-ipc-wdt.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cpufreq/cpufreq-dt-platdev.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/leds/flash/leds-rt4505.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/leds/simple/simatic-ipc-leds.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/leds/simple/simatic-ipc-leds-gpio-core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/firmware/google/coreboot_table.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/firmware/google/memconsole.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/firmware/google/memconsole-coreboot.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/firmware/google/vpd-sysfs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/crypto/atmel-sha204a.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-a4tech.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-aureal.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-cypress.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-elecom.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-evision.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-ezkey.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-gyration.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-kensington.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-lenovo.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-logitech-dj.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-magicmouse.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-megaworld.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-ntrig.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-pl.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-primax.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-redragon.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-retrode.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-samsung.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-topseed.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-viewsonic.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/platform/x86/siemens/simatic-ipc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_performance.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/nvmem/nvmem_u-boot-env.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_util.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_cmdset_0020.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/maps/map_funcs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/hisi-spmi-controller.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_cif.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/amd/yc/snd-soc-acp6x-mach.o
WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/amd/renoir/snd-acp3x-rn.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mtty.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/vfio-mdev/mdpy.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/configfs/configfs_sample.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kprobes/kprobe_example.o
WARNING: modpost: missing MODULE_DESCRIPTION() in samples/kprobes/kretprobe_example.o
>> ERROR: modpost: "ethtool_rxfh_context_lost" [drivers/net/ethernet/broadcom/bnxt/bnxt_en.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 02/11] net: ethtool: let driver declare max size of RSS indir table and key
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let driver declare max size of RSS indir table and key Jakub Kicinski
@ 2024-07-04 7:34 ` Simon Horman
0 siblings, 0 replies; 30+ messages in thread
From: Simon Horman @ 2024-07-04 7:34 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, ecree.xilinx, michael.chan
On Tue, Jul 02, 2024 at 04:47:47PM -0700, Jakub Kicinski wrote:
> Some drivers (bnxt but I think also mlx5 from ML discussions) change
> the size of the indirection table depending on the number of Rx rings.
> Decouple the max table size from the size of the currently used table,
> so that we can reserve space in the context for table growth.
>
> Static members in ethtool_ops are good enough for now, we can add
> callbacks to read the max size more dynamically if someone needs
> that.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> include/linux/ethtool.h | 20 +++++++-----------
> net/ethtool/ioctl.c | 46 ++++++++++++++++++++++++++++++++---------
> 2 files changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 3ce5be0d168a..dc8ed93097c3 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -173,6 +173,7 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
> struct ethtool_rxfh_context {
> u32 indir_size;
> u32 key_size;
> + u32 key_off;
> u16 priv_size;
> u8 hfunc;
> u8 input_xfrm;
nit: key_off should have an entry in the kernel doc for
struct ethtool_rxfh_context.
...
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
2024-07-02 23:47 ` [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends Jakub Kicinski
2024-07-03 12:06 ` Edward Cree
2024-07-04 6:19 ` kernel test robot
@ 2024-07-05 0:34 ` kernel test robot
2 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2024-07-05 0:34 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: oe-kbuild-all, netdev, edumazet, pabeni, ecree.xilinx,
michael.chan, Jakub Kicinski
Hi Jakub,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Jakub-Kicinski/net-ethtool-let-drivers-remove-lost-RSS-contexts/20240703-163816
base: net-next/main
patch link: https://lore.kernel.org/r/20240702234757.4188344-6-kuba%40kernel.org
patch subject: [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends
config: csky-randconfig-r063-20240704 (https://download.01.org/0day-ci/archive/20240705/202407050831.3SjQAiZw-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240705/202407050831.3SjQAiZw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407050831.3SjQAiZw-lkp@intel.com/
All errors (new ones prefixed by >>):
csky-linux-ld: drivers/net/ethernet/broadcom/bnxt/bnxt.o: in function `__bnxt_open_nic':
>> bnxt.c:(.text+0x11e32): undefined reference to `ethtool_rxfh_context_lost'
>> csky-linux-ld: bnxt.c:(.text+0x11f0c): undefined reference to `ethtool_rxfh_context_lost'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2024-07-05 0:35 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-02 23:47 [PATCH net-next 00/11] eth: bnxt: use the new RSS API Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 01/11] net: ethtool: let drivers remove lost RSS contexts Jakub Kicinski
2024-07-03 11:08 ` Edward Cree
2024-07-03 13:43 ` Jakub Kicinski
2024-07-03 14:15 ` Edward Cree
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let driver declare max size of RSS indir table and key Jakub Kicinski
2024-07-04 7:34 ` Simon Horman
2024-07-02 23:47 ` [PATCH net-next 02/11] net: ethtool: let drivers " Jakub Kicinski
2024-07-03 11:16 ` Edward Cree
2024-07-02 23:47 ` [PATCH net-next 03/11] eth: bnxt: allow deleting RSS contexts when the device is down Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 04/11] eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends Jakub Kicinski
2024-07-03 12:06 ` Edward Cree
2024-07-03 12:49 ` Pavan Chebbi
2024-07-03 13:46 ` Jakub Kicinski
2024-07-04 6:19 ` kernel test robot
2024-07-05 0:34 ` kernel test robot
2024-07-02 23:47 ` [PATCH net-next 05/11] eth: bnxt: remove rss_ctx_bmap Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 06/11] eth: bnxt: depend on core cleaning up RSS contexts Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 07/11] eth: bnxt: use context priv for struct bnxt_rss_ctx Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 08/11] eth: bnxt: use the RSS context XArray instead of the local list Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 09/11] eth: bnxt: bump the entry size in indir tables to u32 Jakub Kicinski
2024-07-03 10:51 ` Przemek Kitszel
2024-07-03 13:49 ` Jakub Kicinski
2024-07-03 14:02 ` Przemek Kitszel
2024-07-03 16:02 ` Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 10/11] eth: bnxt: use the indir table from ethtool context Jakub Kicinski
2024-07-03 11:08 ` Przemek Kitszel
2024-07-03 11:39 ` Edward Cree
2024-07-03 13:51 ` Jakub Kicinski
2024-07-02 23:47 ` [PATCH net-next 11/11] eth: bnxt: pad out the correct indirection table Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).