Hello,
This is v2 of my previous patch:
https://lore.kernel.org/lkml/[email protected]/.
Saeed: Thanks for reviewing v1. I made significant changes to support
per-channel DIM settings. Is this ready for an official v1 submission or
are there other major changes you'd like to see before I do that?
***************************************************************************
Version History
---------------
* v1: Initial draft, individual channel DIM changes not supported.
* v2: Support individual channel DIM changes.
***************************************************************************
Currently, only gobal coalescing configuration queries or changes are
supported in the `mlx5` driver. However, per-queue operations are not, and
result in `EOPNOTSUPP` errors when attempted with `ethtool`. This patch
adds support for per-queue coalesce operations.
Here's an example use case:
- A mlx5 NIC is configured with 8 queues, each queue has its IRQ pinned to
a unique CPU.
- Two custom RSS contexts are created: context 1 and context 2. Each
context has a different set of queues where flows are distributed. For
example, context 1 may distribute flows to queues 0-3, and context 2 may
distribute flows to queues 4-7.
- A series of ntuple filters are installed which direct matching flows to
RSS contexts. For example, perhaps port 80 is directed to context 1 and
port 443 to context 2.
- Applications which receive network data associated with either context
are pinned to the CPUs where the queues in the matching context have
their IRQs pinned to maximize locality.
The apps themselves, however, may have different requirements on latency vs
CPU usage and so setting the per queue IRQ coalesce values would be very
helpful.
This patch would support this. In v1 DIM mode changes could only be changed
NIC-wide. However, in this iteration, DIM mode changes are supported
globally as well as on a per-queue basis.
Here's an example:
```
$ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
Queue: 2
Adaptive RX: on TX: on
stats-block-usecs: 0
sample-interval: 0
pkt-rate-low: 0
pkt-rate-high: 0
rx-usecs: 8
rx-frames: 128
rx-usecs-irq: 0
rx-frames-irq: 0
tx-usecs: 8
tx-frames: 128
tx-usecs-irq: 0
tx-frames-irq: 0
```
Now, let's try to set adaptive-rx off rx-usecs 16 for queue 2:
```
$ sudo ethtool --per-queue eth0 queue_mask 0x4 --coalesce adaptive-rx off
$ sudo ethtool --per-queue eth0 queue_mask 0x4 --coalesce rx-usecs 16
```
Confirm that the operation succeeded:
```
$ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
Queue: 2
Adaptive RX: off TX: on
stats-block-usecs: 0
sample-interval: 0
pkt-rate-low: 0
pkt-rate-high: 0
rx-usecs: 16
rx-frames: 32
rx-usecs-irq: 0
rx-frames-irq: 0
tx-usecs: 8
tx-frames: 128
tx-usecs-irq: 0
tx-frames-irq: 0
```
The individual channel settings do not overwrite the global ones. However
Setting the global parameters will also reset all of the individual channel
options. For example, after we set the options for queue 2, we'll see that
the global options remain unchanged:
```
$ sudo ethtool --show-coalesce eth0
Coalesce parameters for eth0:
Adaptive RX: on TX: on
stats-block-usecs: 0
sample-interval: 0
pkt-rate-low: 0
pkt-rate-high: 0
rx-usecs: 8
rx-frames: 128
rx-usecs-irq: 0
rx-frames-irq: 0
tx-usecs: 16
tx-frames: 32
tx-usecs-irq: 0
tx-frames-irq: 0
```
But then if we set them, we'll see that the options for queue 2 have been
reset as well:
```
$ sudo ethtool --coalesce eth0 adaptive-tx off
$ sudo ethtool --show-coalesce eth0
Coalesce parameters for eth0:
Adaptive RX: on TX: off
stats-block-usecs: 0
sample-interval: 0
pkt-rate-low: 0
pkt-rate-high: 0
rx-usecs: 8
rx-frames: 128
rx-usecs-irq: 0
rx-frames-irq: 0
tx-usecs: 16
tx-frames: 32
tx-usecs-irq: 0
tx-frames-irq: 0
$ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
Queue: 2
Adaptive RX: on TX: off
stats-block-usecs: 0
sample-interval: 0
pkt-rate-low: 0
pkt-rate-high: 0
rx-usecs: 8
rx-frames: 128
rx-usecs-irq: 0
rx-frames-irq: 0
tx-usecs: 16
tx-frames: 32
tx-usecs-irq: 0
tx-frames-irq: 0
```
Previously a global `struct mlx5e_params` stored the options in
`struct mlx5e_priv.channels.params`. That was preserved, but a channel-
specific instance was added as well, in `struct mlx5e_channel.params`.
Best Regards,
***************************************************************************
Nabil S. Alramli (4):
mlx5: Add mlx5e_param to individual mlx5e_channel and preserve them
through mlx5e_open_channels()
mlx5: Add queue number parameter to mlx5e_safe_switch_params()
mlx5: Implement mlx5e_ethtool_{get,set}_per_queue_coalesce() to
support per-queue operations
mlx5: Add {get,set}_per_queue_coalesce()
drivers/net/ethernet/mellanox/mlx5/core/en.h | 6 +-
.../ethernet/mellanox/mlx5/core/en_dcbnl.c | 2 +-
.../ethernet/mellanox/mlx5/core/en_ethtool.c | 214 +++++++++++++-----
.../net/ethernet/mellanox/mlx5/core/en_main.c | 76 +++++--
.../ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
5 files changed, 222 insertions(+), 78 deletions(-)
--
2.35.1
Implment two new methods, mlx5e_ethtool_get_per_queue_coalesce() and
mlx5e_ethtool_set_per_queue_coalesce() and update global coalescing
request handlers to call into them for an extra level of indirection to
allow support for per-queue operations.
Signed-off-by: Nabil S. Alramli <[email protected]>
---
.../ethernet/mellanox/mlx5/core/en_ethtool.c | 152 +++++++++++++-----
1 file changed, 112 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 13e5838ff1ee..daa0aa833a42 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -513,33 +513,55 @@ static int mlx5e_set_channels(struct net_device *dev,
return mlx5e_ethtool_set_channels(priv, ch);
}
-int mlx5e_ethtool_get_coalesce(struct mlx5e_priv *priv,
+static int mlx5e_ethtool_get_per_queue_coalesce(struct mlx5e_priv *priv,
+ int queue,
struct ethtool_coalesce *coal,
struct kernel_ethtool_coalesce *kernel_coal)
{
struct dim_cq_moder *rx_moder, *tx_moder;
+ struct mlx5e_params *params;
if (!MLX5_CAP_GEN(priv->mdev, cq_moderation))
return -EOPNOTSUPP;
- rx_moder = &priv->channels.params.rx_cq_moderation;
+ if (queue != -1 && queue >= priv->channels.num) {
+ netdev_err(priv->netdev, "%s: Invalid queue ID [%d]",
+ __func__, queue);
+ return -EINVAL;
+ }
+
+ if (queue == -1)
+ params = &priv->channels.params;
+ else
+ params = &priv->channels.c[queue]->params;
+
+ rx_moder = ¶ms->rx_cq_moderation;
coal->rx_coalesce_usecs = rx_moder->usec;
coal->rx_max_coalesced_frames = rx_moder->pkts;
- coal->use_adaptive_rx_coalesce = priv->channels.params.rx_dim_enabled;
+ coal->use_adaptive_rx_coalesce = params->rx_dim_enabled;
- tx_moder = &priv->channels.params.tx_cq_moderation;
+ tx_moder = ¶ms->tx_cq_moderation;
coal->tx_coalesce_usecs = tx_moder->usec;
coal->tx_max_coalesced_frames = tx_moder->pkts;
- coal->use_adaptive_tx_coalesce = priv->channels.params.tx_dim_enabled;
+ coal->use_adaptive_tx_coalesce = params->tx_dim_enabled;
- kernel_coal->use_cqe_mode_rx =
- MLX5E_GET_PFLAG(&priv->channels.params, MLX5E_PFLAG_RX_CQE_BASED_MODER);
- kernel_coal->use_cqe_mode_tx =
- MLX5E_GET_PFLAG(&priv->channels.params, MLX5E_PFLAG_TX_CQE_BASED_MODER);
+ if (kernel_coal) {
+ kernel_coal->use_cqe_mode_rx =
+ MLX5E_GET_PFLAG(&priv->channels.params, MLX5E_PFLAG_RX_CQE_BASED_MODER);
+ kernel_coal->use_cqe_mode_tx =
+ MLX5E_GET_PFLAG(&priv->channels.params, MLX5E_PFLAG_TX_CQE_BASED_MODER);
+ }
return 0;
}
+int mlx5e_ethtool_get_coalesce(struct mlx5e_priv *priv,
+ struct ethtool_coalesce *coal,
+ struct kernel_ethtool_coalesce *kernel_coal)
+{
+ return mlx5e_ethtool_get_per_queue_coalesce(priv, -1, coal, kernel_coal);
+}
+
static int mlx5e_get_coalesce(struct net_device *netdev,
struct ethtool_coalesce *coal,
struct kernel_ethtool_coalesce *kernel_coal,
@@ -554,32 +576,55 @@ static int mlx5e_get_coalesce(struct net_device *netdev,
#define MLX5E_MAX_COAL_FRAMES MLX5_MAX_CQ_COUNT
static void
-mlx5e_set_priv_channels_tx_coalesce(struct mlx5e_priv *priv, struct ethtool_coalesce *coal)
+mlx5e_set_priv_channels_tx_coalesce(struct mlx5e_priv *priv,
+ int queue,
+ struct ethtool_coalesce *coal)
{
struct mlx5_core_dev *mdev = priv->mdev;
int tc;
int i;
- for (i = 0; i < priv->channels.num; ++i) {
- struct mlx5e_channel *c = priv->channels.c[i];
+ if (queue == -1) {
+ for (i = 0; i < priv->channels.num; ++i) {
+ struct mlx5e_channel *c = priv->channels.c[i];
+
+ for (tc = 0; tc < c->num_tc; tc++) {
+ mlx5_core_modify_cq_moderation(mdev,
+ &c->sq[tc].cq.mcq,
+ coal->tx_coalesce_usecs,
+ coal->tx_max_coalesced_frames);
+ }
+ }
+ } else {
+ struct mlx5e_channel *c = priv->channels.c[queue];
for (tc = 0; tc < c->num_tc; tc++) {
mlx5_core_modify_cq_moderation(mdev,
- &c->sq[tc].cq.mcq,
- coal->tx_coalesce_usecs,
- coal->tx_max_coalesced_frames);
+ &c->sq[tc].cq.mcq,
+ coal->tx_coalesce_usecs,
+ coal->tx_max_coalesced_frames);
}
}
}
static void
-mlx5e_set_priv_channels_rx_coalesce(struct mlx5e_priv *priv, struct ethtool_coalesce *coal)
+mlx5e_set_priv_channels_rx_coalesce(struct mlx5e_priv *priv,
+ int queue,
+ struct ethtool_coalesce *coal)
{
struct mlx5_core_dev *mdev = priv->mdev;
int i;
- for (i = 0; i < priv->channels.num; ++i) {
- struct mlx5e_channel *c = priv->channels.c[i];
+ if (queue == -1) {
+ for (i = 0; i < priv->channels.num; ++i) {
+ struct mlx5e_channel *c = priv->channels.c[i];
+
+ mlx5_core_modify_cq_moderation(mdev, &c->rq.cq.mcq,
+ coal->rx_coalesce_usecs,
+ coal->rx_max_coalesced_frames);
+ }
+ } else {
+ struct mlx5e_channel *c = priv->channels.c[queue];
mlx5_core_modify_cq_moderation(mdev, &c->rq.cq.mcq,
coal->rx_coalesce_usecs,
@@ -596,15 +641,17 @@ static int cqe_mode_to_period_mode(bool val)
return val ? MLX5_CQ_PERIOD_MODE_START_FROM_CQE : MLX5_CQ_PERIOD_MODE_START_FROM_EQE;
}
-int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv,
+static int mlx5e_ethtool_set_per_queue_coalesce(struct mlx5e_priv *priv,
+ int queue,
struct ethtool_coalesce *coal,
struct kernel_ethtool_coalesce *kernel_coal,
struct netlink_ext_ack *extack)
{
struct dim_cq_moder *rx_moder, *tx_moder;
struct mlx5_core_dev *mdev = priv->mdev;
- struct mlx5e_params new_params;
- bool reset_rx, reset_tx;
+ bool reset_rx = false, reset_tx = false;
+ struct mlx5e_params new_params = {0};
+ struct mlx5e_params *old_params;
bool reset = true;
u8 cq_period_mode;
int err = 0;
@@ -626,14 +673,29 @@ int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv,
return -ERANGE;
}
- if ((kernel_coal->use_cqe_mode_rx || kernel_coal->use_cqe_mode_tx) &&
- !MLX5_CAP_GEN(priv->mdev, cq_period_start_from_cqe)) {
- NL_SET_ERR_MSG_MOD(extack, "cqe_mode_rx/tx is not supported on this device");
- return -EOPNOTSUPP;
+ if (kernel_coal) {
+ if ((kernel_coal->use_cqe_mode_rx || kernel_coal->use_cqe_mode_tx) &&
+ !MLX5_CAP_GEN(priv->mdev, cq_period_start_from_cqe)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "cqe_mode_rx/tx is not supported on this device");
+ return -EOPNOTSUPP;
+ }
+ }
+
+ if (queue != -1 && queue >= priv->channels.num) {
+ netdev_err(priv->netdev, "%s: Invalid queue ID [%d]",
+ __func__, queue);
+ return -EINVAL;
}
mutex_lock(&priv->state_lock);
- new_params = priv->channels.params;
+
+ if (queue == -1)
+ old_params = &priv->channels.params;
+ else
+ old_params = &priv->channels.c[queue]->params;
+
+ new_params = *old_params;
rx_moder = &new_params.rx_cq_moderation;
rx_moder->usec = coal->rx_coalesce_usecs;
@@ -645,19 +707,21 @@ int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv,
tx_moder->pkts = coal->tx_max_coalesced_frames;
new_params.tx_dim_enabled = !!coal->use_adaptive_tx_coalesce;
- reset_rx = !!coal->use_adaptive_rx_coalesce != priv->channels.params.rx_dim_enabled;
- reset_tx = !!coal->use_adaptive_tx_coalesce != priv->channels.params.tx_dim_enabled;
+ reset_rx = !!coal->use_adaptive_rx_coalesce != old_params->rx_dim_enabled;
+ reset_tx = !!coal->use_adaptive_tx_coalesce != old_params->tx_dim_enabled;
- cq_period_mode = cqe_mode_to_period_mode(kernel_coal->use_cqe_mode_rx);
- if (cq_period_mode != rx_moder->cq_period_mode) {
- mlx5e_set_rx_cq_mode_params(&new_params, cq_period_mode);
- reset_rx = true;
- }
+ if (kernel_coal) {
+ cq_period_mode = cqe_mode_to_period_mode(kernel_coal->use_cqe_mode_rx);
+ if (cq_period_mode != rx_moder->cq_period_mode) {
+ mlx5e_set_rx_cq_mode_params(&new_params, cq_period_mode);
+ reset_rx = true;
+ }
- cq_period_mode = cqe_mode_to_period_mode(kernel_coal->use_cqe_mode_tx);
- if (cq_period_mode != tx_moder->cq_period_mode) {
- mlx5e_set_tx_cq_mode_params(&new_params, cq_period_mode);
- reset_tx = true;
+ cq_period_mode = cqe_mode_to_period_mode(kernel_coal->use_cqe_mode_tx);
+ if (cq_period_mode != tx_moder->cq_period_mode) {
+ mlx5e_set_tx_cq_mode_params(&new_params, cq_period_mode);
+ reset_tx = true;
+ }
}
if (reset_rx) {
@@ -678,18 +742,26 @@ int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv,
*/
if (!reset_rx && !reset_tx && test_bit(MLX5E_STATE_OPENED, &priv->state)) {
if (!coal->use_adaptive_rx_coalesce)
- mlx5e_set_priv_channels_rx_coalesce(priv, coal);
+ mlx5e_set_priv_channels_rx_coalesce(priv, queue, coal);
if (!coal->use_adaptive_tx_coalesce)
- mlx5e_set_priv_channels_tx_coalesce(priv, coal);
+ mlx5e_set_priv_channels_tx_coalesce(priv, queue, coal);
reset = false;
}
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset, -1);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset, queue);
mutex_unlock(&priv->state_lock);
return err;
}
+int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv,
+ struct ethtool_coalesce *coal,
+ struct kernel_ethtool_coalesce *kernel_coal,
+ struct netlink_ext_ack *extack)
+{
+ return mlx5e_ethtool_set_per_queue_coalesce(priv, -1, coal, kernel_coal, extack);
+}
+
static int mlx5e_set_coalesce(struct net_device *netdev,
struct ethtool_coalesce *coal,
struct kernel_ethtool_coalesce *kernel_coal,
--
2.35.1
Currently mlx5e_safe_switch_params() does not take a queue number and
assumes a global operation. This allows the function to handle individual
channels. Note that Global operations reset the individual channel
settings.
Signed-off-by: Nabil S. Alramli <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 3 +-
.../ethernet/mellanox/mlx5/core/en_dcbnl.c | 2 +-
.../ethernet/mellanox/mlx5/core/en_ethtool.c | 18 ++++----
.../net/ethernet/mellanox/mlx5/core/en_main.c | 42 +++++++++++++------
.../ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
5 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 3657839b88f8..ba8eb2b30251 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -1063,7 +1063,8 @@ int mlx5e_safe_reopen_channels(struct mlx5e_priv *priv);
int mlx5e_safe_switch_params(struct mlx5e_priv *priv,
struct mlx5e_params *new_params,
mlx5e_fp_preactivate preactivate,
- void *context, bool reset);
+ void *context, bool reset,
+ int queue);
int mlx5e_update_tx_netdev_queues(struct mlx5e_priv *priv);
int mlx5e_num_channels_changed_ctx(struct mlx5e_priv *priv, void *context);
void mlx5e_activate_priv_channels(struct mlx5e_priv *priv);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
index 8705cffc747f..8c0704beb25c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
@@ -1159,7 +1159,7 @@ static int mlx5e_set_trust_state(struct mlx5e_priv *priv, u8 trust_state)
err = mlx5e_safe_switch_params(priv, &new_params,
mlx5e_update_trust_state_hw,
- &trust_state, reset);
+ &trust_state, reset, -1);
mutex_unlock(&priv->state_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index dff02434ff45..13e5838ff1ee 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -389,7 +389,7 @@ int mlx5e_ethtool_set_ringparam(struct mlx5e_priv *priv,
if (err)
goto unlock;
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true, -1);
unlock:
mutex_unlock(&priv->state_lock);
@@ -489,7 +489,7 @@ int mlx5e_ethtool_set_channels(struct mlx5e_priv *priv,
/* Switch to new channels, set new parameters and close old ones */
err = mlx5e_safe_switch_params(priv, &new_params,
- mlx5e_num_channels_changed_ctx, NULL, true);
+ mlx5e_num_channels_changed_ctx, NULL, true, -1);
if (arfs_enabled) {
int err2 = mlx5e_arfs_enable(priv->fs);
@@ -684,7 +684,7 @@ int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv,
reset = false;
}
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset, -1);
mutex_unlock(&priv->state_lock);
return err;
@@ -1893,7 +1893,7 @@ static int set_pflag_cqe_based_moder(struct net_device *netdev, bool enable,
else
mlx5e_set_tx_cq_mode_params(&new_params, cq_period_mode);
- return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+ return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true, -1);
}
static int set_pflag_tx_cqe_based_moder(struct net_device *netdev, bool enable)
@@ -1935,10 +1935,10 @@ int mlx5e_modify_rx_cqe_compression_locked(struct mlx5e_priv *priv, bool new_val
new_params.ptp_rx = new_val;
if (new_params.ptp_rx == priv->channels.params.ptp_rx)
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true, -1);
else
err = mlx5e_safe_switch_params(priv, &new_params, mlx5e_ptp_rx_manage_fs_ctx,
- &new_params.ptp_rx, true);
+ &new_params.ptp_rx, true, -1);
if (err)
return err;
@@ -1996,7 +1996,7 @@ static int set_pflag_rx_striding_rq(struct net_device *netdev, bool enable)
MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_RX_STRIDING_RQ, enable);
mlx5e_set_rq_type(mdev, &new_params);
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true, -1);
if (err)
return err;
@@ -2041,7 +2041,7 @@ static int set_pflag_tx_mpwqe_common(struct net_device *netdev, u32 flag, bool e
MLX5E_SET_PFLAG(&new_params, flag, enable);
- return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+ return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true, -1);
}
static int set_pflag_xdp_tx_mpwqe(struct net_device *netdev, bool enable)
@@ -2094,7 +2094,7 @@ static int set_pflag_tx_port_ts(struct net_device *netdev, bool enable)
*/
err = mlx5e_safe_switch_params(priv, &new_params,
- mlx5e_num_channels_changed_ctx, NULL, true);
+ mlx5e_num_channels_changed_ctx, NULL, true, -1);
if (!err)
priv->tx_ptp_opened = true;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index a1578219187b..d7325fc4f2c0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3055,19 +3055,35 @@ static int mlx5e_switch_priv_channels(struct mlx5e_priv *priv,
int mlx5e_safe_switch_params(struct mlx5e_priv *priv,
struct mlx5e_params *params,
mlx5e_fp_preactivate preactivate,
- void *context, bool reset)
+ void *context, bool reset,
+ int queue)
{
struct mlx5e_channels *new_chs;
int err;
+ int i;
+
+ if (queue == -1) {
+ for (i = 0; i < priv->channels.num; i++)
+ priv->channels.c[i]->params = *params;
+ } else {
+ priv->channels.c[queue]->params = *params;
+ }
reset &= test_bit(MLX5E_STATE_OPENED, &priv->state);
- if (!reset)
- return mlx5e_switch_priv_params(priv, params, preactivate, context);
+ if (!reset) {
+ if (queue == -1)
+ return mlx5e_switch_priv_params(priv, params, preactivate, context);
+ else
+ return 0;
+ }
new_chs = kzalloc(sizeof(*new_chs), GFP_KERNEL);
if (!new_chs)
return -ENOMEM;
- new_chs->params = *params;
+ if (queue == -1)
+ new_chs->params = *params;
+ else
+ new_chs->params = priv->channels.params;
mlx5e_selq_prepare_params(&priv->selq, &new_chs->params);
@@ -3093,7 +3109,7 @@ int mlx5e_safe_switch_params(struct mlx5e_priv *priv,
int mlx5e_safe_reopen_channels(struct mlx5e_priv *priv)
{
- return mlx5e_safe_switch_params(priv, &priv->channels.params, NULL, NULL, true);
+ return mlx5e_safe_switch_params(priv, &priv->channels.params, NULL, NULL, true, -1);
}
void mlx5e_timestamp_init(struct mlx5e_priv *priv)
@@ -3486,7 +3502,7 @@ static int mlx5e_setup_tc_mqprio_dcb(struct mlx5e_priv *priv,
mlx5e_params_mqprio_dcb_set(&new_params, tc ? tc : 1);
err = mlx5e_safe_switch_params(priv, &new_params,
- mlx5e_num_channels_changed_ctx, NULL, true);
+ mlx5e_num_channels_changed_ctx, NULL, true, -1);
if (!err && priv->mqprio_rl) {
mlx5e_mqprio_rl_cleanup(priv->mqprio_rl);
@@ -3607,7 +3623,7 @@ static int mlx5e_setup_tc_mqprio_channel(struct mlx5e_priv *priv,
nch_changed = mlx5e_get_dcb_num_tc(&priv->channels.params) > 1;
preactivate = nch_changed ? mlx5e_num_channels_changed_ctx :
mlx5e_update_netdev_queues_ctx;
- err = mlx5e_safe_switch_params(priv, &new_params, preactivate, NULL, true);
+ err = mlx5e_safe_switch_params(priv, &new_params, preactivate, NULL, true, -1);
if (err) {
if (rl) {
mlx5e_mqprio_rl_cleanup(rl);
@@ -3849,7 +3865,7 @@ static int set_feature_lro(struct net_device *netdev, bool enable)
}
err = mlx5e_safe_switch_params(priv, &new_params,
- mlx5e_modify_tirs_packet_merge_ctx, NULL, reset);
+ mlx5e_modify_tirs_packet_merge_ctx, NULL, reset, -1);
out:
mutex_unlock(&priv->state_lock);
return err;
@@ -3877,7 +3893,7 @@ static int set_feature_hw_gro(struct net_device *netdev, bool enable)
goto out;
}
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset, -1);
out:
mutex_unlock(&priv->state_lock);
return err;
@@ -3975,7 +3991,7 @@ static int set_feature_rx_fcs(struct net_device *netdev, bool enable)
new_params = chs->params;
new_params.scatter_fcs_en = enable;
err = mlx5e_safe_switch_params(priv, &new_params, mlx5e_set_rx_port_ts_wrap,
- &new_params.scatter_fcs_en, true);
+ &new_params.scatter_fcs_en, true, -1);
mutex_unlock(&priv->state_lock);
return err;
}
@@ -4349,7 +4365,7 @@ int mlx5e_change_mtu(struct net_device *netdev, int new_mtu,
reset = false;
}
- err = mlx5e_safe_switch_params(priv, &new_params, preactivate, NULL, reset);
+ err = mlx5e_safe_switch_params(priv, &new_params, preactivate, NULL, reset, -1);
out:
netdev->mtu = params->sw_mtu;
@@ -4400,7 +4416,7 @@ static int mlx5e_hwstamp_config_ptp_rx(struct mlx5e_priv *priv, bool ptp_rx)
new_params = priv->channels.params;
new_params.ptp_rx = ptp_rx;
return mlx5e_safe_switch_params(priv, &new_params, mlx5e_ptp_rx_manage_fs_ctx,
- &new_params.ptp_rx, true);
+ &new_params.ptp_rx, true, -1);
}
int mlx5e_hwstamp_set(struct mlx5e_priv *priv, struct ifreq *ifr)
@@ -4831,7 +4847,7 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
old_prog = priv->channels.params.xdp_prog;
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, reset, -1);
if (err)
goto unlock;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
index baa7ef812313..877719c4454a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
@@ -522,7 +522,7 @@ static int mlx5i_change_mtu(struct net_device *netdev, int new_mtu)
new_params = priv->channels.params;
new_params.sw_mtu = new_mtu;
- err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+ err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true, -1);
if (err)
goto out;
--
2.35.1
The mlx-5 driver currently only implements querying or modifying coalesce
configurations globally. It does not allow per-queue operations. This
change is to implement per-queue coalesce operations in the driver.
Signed-off-by: Nabil S. Alramli <[email protected]>
---
.../ethernet/mellanox/mlx5/core/en_ethtool.c | 46 +++++++++++++++++--
1 file changed, 41 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index daa0aa833a42..492c03c3d5f4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -515,8 +515,8 @@ static int mlx5e_set_channels(struct net_device *dev,
static int mlx5e_ethtool_get_per_queue_coalesce(struct mlx5e_priv *priv,
int queue,
- struct ethtool_coalesce *coal,
- struct kernel_ethtool_coalesce *kernel_coal)
+ struct ethtool_coalesce *coal,
+ struct kernel_ethtool_coalesce *kernel_coal)
{
struct dim_cq_moder *rx_moder, *tx_moder;
struct mlx5e_params *params;
@@ -572,6 +572,23 @@ static int mlx5e_get_coalesce(struct net_device *netdev,
return mlx5e_ethtool_get_coalesce(priv, coal, kernel_coal);
}
+/**
+ * mlx5e_get_per_queue_coalesce - gets coalesce settings for particular queue
+ * @netdev: netdev structure
+ * @coal: ethtool's coalesce settings
+ * @queue: the particular queue to read
+ *
+ * Reads a specific queue's coalesce settings
+ **/
+static int mlx5e_get_per_queue_coalesce(struct net_device *netdev,
+ u32 queue,
+ struct ethtool_coalesce *coal)
+{
+ struct mlx5e_priv *priv = netdev_priv(netdev);
+
+ return mlx5e_ethtool_get_per_queue_coalesce(priv, queue, coal, NULL);
+}
+
#define MLX5E_MAX_COAL_TIME MLX5_MAX_CQ_PERIOD
#define MLX5E_MAX_COAL_FRAMES MLX5_MAX_CQ_COUNT
@@ -643,9 +660,9 @@ static int cqe_mode_to_period_mode(bool val)
static int mlx5e_ethtool_set_per_queue_coalesce(struct mlx5e_priv *priv,
int queue,
- struct ethtool_coalesce *coal,
- struct kernel_ethtool_coalesce *kernel_coal,
- struct netlink_ext_ack *extack)
+ struct ethtool_coalesce *coal,
+ struct kernel_ethtool_coalesce *kernel_coal,
+ struct netlink_ext_ack *extack)
{
struct dim_cq_moder *rx_moder, *tx_moder;
struct mlx5_core_dev *mdev = priv->mdev;
@@ -772,6 +789,23 @@ static int mlx5e_set_coalesce(struct net_device *netdev,
return mlx5e_ethtool_set_coalesce(priv, coal, kernel_coal, extack);
}
+/**
+ * mlx5e_set_per_queue_coalesce - set specific queue's coalesce settings
+ * @netdev: the netdev to change
+ * @coal: ethtool's coalesce settings
+ * @queue: the queue to change
+ *
+ * Sets the specified queue's coalesce settings.
+ **/
+static int mlx5e_set_per_queue_coalesce(struct net_device *netdev,
+ u32 queue,
+ struct ethtool_coalesce *coal)
+{
+ struct mlx5e_priv *priv = netdev_priv(netdev);
+
+ return mlx5e_ethtool_set_per_queue_coalesce(priv, queue, coal, NULL, NULL);
+}
+
static void ptys2ethtool_supported_link(struct mlx5_core_dev *mdev,
unsigned long *supported_modes,
u32 eth_proto_cap)
@@ -2506,6 +2540,8 @@ const struct ethtool_ops mlx5e_ethtool_ops = {
.flash_device = mlx5e_flash_device,
.get_priv_flags = mlx5e_get_priv_flags,
.set_priv_flags = mlx5e_set_priv_flags,
+ .get_per_queue_coalesce = mlx5e_get_per_queue_coalesce,
+ .set_per_queue_coalesce = mlx5e_set_per_queue_coalesce,
.self_test = mlx5e_self_test,
.get_fec_stats = mlx5e_get_fec_stats,
.get_fecparam = mlx5e_get_fecparam,
--
2.35.1
Hi Nabil,
On Mon, 18 Sep, 2023 18:29:51 -0400 "Nabil S. Alramli" <[email protected]> wrote:
> Hello,
>
> This is v2 of my previous patch:
> https://lore.kernel.org/lkml/[email protected]/.
>
> Saeed: Thanks for reviewing v1. I made significant changes to support
> per-channel DIM settings. Is this ready for an official v1 submission or
> are there other major changes you'd like to see before I do that?
>
> ***************************************************************************
> Version History
> ---------------
> * v1: Initial draft, individual channel DIM changes not supported.
> * v2: Support individual channel DIM changes.
> ***************************************************************************
We actually began working on a patch set for the feature internally
inspired by your initial RFC. If it is alright with you, would it be ok
to have you as a co-author of that series that we should have prepared
in the coming days? We have some minor enhancements that we think will
improve the general architecture for how we handle both the global and
per-queue settings.
>
> Currently, only gobal coalescing configuration queries or changes are
> supported in the `mlx5` driver. However, per-queue operations are not, and
> result in `EOPNOTSUPP` errors when attempted with `ethtool`. This patch
> adds support for per-queue coalesce operations.
>
> Here's an example use case:
>
> - A mlx5 NIC is configured with 8 queues, each queue has its IRQ pinned to
> a unique CPU.
> - Two custom RSS contexts are created: context 1 and context 2. Each
> context has a different set of queues where flows are distributed. For
> example, context 1 may distribute flows to queues 0-3, and context 2 may
> distribute flows to queues 4-7.
> - A series of ntuple filters are installed which direct matching flows to
> RSS contexts. For example, perhaps port 80 is directed to context 1 and
> port 443 to context 2.
> - Applications which receive network data associated with either context
> are pinned to the CPUs where the queues in the matching context have
> their IRQs pinned to maximize locality.
>
> The apps themselves, however, may have different requirements on latency vs
> CPU usage and so setting the per queue IRQ coalesce values would be very
> helpful.
>
> This patch would support this. In v1 DIM mode changes could only be changed
> NIC-wide. However, in this iteration, DIM mode changes are supported
> globally as well as on a per-queue basis.
>
> Here's an example:
>
> ```
> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
> Queue: 2
> Adaptive RX: on TX: on
> stats-block-usecs: 0
> sample-interval: 0
> pkt-rate-low: 0
> pkt-rate-high: 0
>
> rx-usecs: 8
> rx-frames: 128
> rx-usecs-irq: 0
> rx-frames-irq: 0
>
> tx-usecs: 8
> tx-frames: 128
> tx-usecs-irq: 0
> tx-frames-irq: 0
> ```
>
> Now, let's try to set adaptive-rx off rx-usecs 16 for queue 2:
>
> ```
> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --coalesce adaptive-rx off
> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --coalesce rx-usecs 16
> ```
>
> Confirm that the operation succeeded:
>
> ```
> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
> Queue: 2
> Adaptive RX: off TX: on
> stats-block-usecs: 0
> sample-interval: 0
> pkt-rate-low: 0
> pkt-rate-high: 0
>
> rx-usecs: 16
> rx-frames: 32
> rx-usecs-irq: 0
> rx-frames-irq: 0
>
> tx-usecs: 8
> tx-frames: 128
> tx-usecs-irq: 0
> tx-frames-irq: 0
> ```
>
> The individual channel settings do not overwrite the global ones. However
> Setting the global parameters will also reset all of the individual channel
> options. For example, after we set the options for queue 2, we'll see that
> the global options remain unchanged:
> ```
> $ sudo ethtool --show-coalesce eth0
> Coalesce parameters for eth0:
> Adaptive RX: on TX: on
> stats-block-usecs: 0
> sample-interval: 0
> pkt-rate-low: 0
> pkt-rate-high: 0
>
> rx-usecs: 8
> rx-frames: 128
> rx-usecs-irq: 0
> rx-frames-irq: 0
>
> tx-usecs: 16
> tx-frames: 32
> tx-usecs-irq: 0
> tx-frames-irq: 0
> ```
>
> But then if we set them, we'll see that the options for queue 2 have been
> reset as well:
> ```
> $ sudo ethtool --coalesce eth0 adaptive-tx off
>
> $ sudo ethtool --show-coalesce eth0
> Coalesce parameters for eth0:
> Adaptive RX: on TX: off
> stats-block-usecs: 0
> sample-interval: 0
> pkt-rate-low: 0
> pkt-rate-high: 0
>
> rx-usecs: 8
> rx-frames: 128
> rx-usecs-irq: 0
> rx-frames-irq: 0
>
> tx-usecs: 16
> tx-frames: 32
> tx-usecs-irq: 0
> tx-frames-irq: 0
>
> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
> Queue: 2
> Adaptive RX: on TX: off
> stats-block-usecs: 0
> sample-interval: 0
> pkt-rate-low: 0
> pkt-rate-high: 0
>
> rx-usecs: 8
> rx-frames: 128
> rx-usecs-irq: 0
> rx-frames-irq: 0
>
> tx-usecs: 16
> tx-frames: 32
> tx-usecs-irq: 0
> tx-frames-irq: 0
> ```
>
> Previously a global `struct mlx5e_params` stored the options in
> `struct mlx5e_priv.channels.params`. That was preserved, but a channel-
> specific instance was added as well, in `struct mlx5e_channel.params`.
>
> Best Regards,
>
> ***************************************************************************
>
> Nabil S. Alramli (4):
> mlx5: Add mlx5e_param to individual mlx5e_channel and preserve them
> through mlx5e_open_channels()
> mlx5: Add queue number parameter to mlx5e_safe_switch_params()
We currently are working on a variation of this without needing to use
mlx5e_safe_switch_params for updating individual channel states (our
variation of the feature avoids needing to place an instance of
mlx5e_params per channel).
> mlx5: Implement mlx5e_ethtool_{get,set}_per_queue_coalesce() to
> support per-queue operations
> mlx5: Add {get,set}_per_queue_coalesce()
>
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 6 +-
> .../ethernet/mellanox/mlx5/core/en_dcbnl.c | 2 +-
> .../ethernet/mellanox/mlx5/core/en_ethtool.c | 214 +++++++++++++-----
> .../net/ethernet/mellanox/mlx5/core/en_main.c | 76 +++++--
> .../ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
> 5 files changed, 222 insertions(+), 78 deletions(-)
--
Thanks,
Rahul Rameshbabu
Hi Rahul,
Thank you for your response.
On 9/19/23 14:55, Rahul Rameshbabu wrote:
> Hi Nabil,
>
> On Mon, 18 Sep, 2023 18:29:51 -0400 "Nabil S. Alramli" <[email protected]> wrote:
>> Hello,
>>
>> This is v2 of my previous patch:
>> https://lore.kernel.org/lkml/[email protected]/.
>>
>> Saeed: Thanks for reviewing v1. I made significant changes to support
>> per-channel DIM settings. Is this ready for an official v1 submission or
>> are there other major changes you'd like to see before I do that?
>>
>> ***************************************************************************
>> Version History
>> ---------------
>> * v1: Initial draft, individual channel DIM changes not supported.
>> * v2: Support individual channel DIM changes.
>> ***************************************************************************
>
> We actually began working on a patch set for the feature internally
> inspired by your initial RFC. If it is alright with you, would it be ok
> to have you as a co-author of that series that we should have prepared
> in the coming days? We have some minor enhancements that we think will
> improve the general architecture for how we handle both the global and
> per-queue settings.
>
Yes. Please feel free to add me as a co-author. Actually, I'm new to
submitting mlx-5 patches and a lot of credit goes to Joe Damato
<[email protected]> who had this initial idea and helped me develop it
into this patch, so would you mind adding him as well? If you would like
you could start with my patch-set and then revert it and add your own,
or if you think that's too much trouble then I'm fine with however you'd
like to proceed. I'd be happy to test your patch whenever it's ready.
>>
>> Currently, only gobal coalescing configuration queries or changes are
>> supported in the `mlx5` driver. However, per-queue operations are not, and
>> result in `EOPNOTSUPP` errors when attempted with `ethtool`. This patch
>> adds support for per-queue coalesce operations.
>>
>> Here's an example use case:
>>
>> - A mlx5 NIC is configured with 8 queues, each queue has its IRQ pinned to
>> a unique CPU.
>> - Two custom RSS contexts are created: context 1 and context 2. Each
>> context has a different set of queues where flows are distributed. For
>> example, context 1 may distribute flows to queues 0-3, and context 2 may
>> distribute flows to queues 4-7.
>> - A series of ntuple filters are installed which direct matching flows to
>> RSS contexts. For example, perhaps port 80 is directed to context 1 and
>> port 443 to context 2.
>> - Applications which receive network data associated with either context
>> are pinned to the CPUs where the queues in the matching context have
>> their IRQs pinned to maximize locality.
>>
>> The apps themselves, however, may have different requirements on latency vs
>> CPU usage and so setting the per queue IRQ coalesce values would be very
>> helpful.
>>
>> This patch would support this. In v1 DIM mode changes could only be changed
>> NIC-wide. However, in this iteration, DIM mode changes are supported
>> globally as well as on a per-queue basis.
>>
>> Here's an example:
>>
>> ```
>> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
>> Queue: 2
>> Adaptive RX: on TX: on
>> stats-block-usecs: 0
>> sample-interval: 0
>> pkt-rate-low: 0
>> pkt-rate-high: 0
>>
>> rx-usecs: 8
>> rx-frames: 128
>> rx-usecs-irq: 0
>> rx-frames-irq: 0
>>
>> tx-usecs: 8
>> tx-frames: 128
>> tx-usecs-irq: 0
>> tx-frames-irq: 0
>> ```
>>
>> Now, let's try to set adaptive-rx off rx-usecs 16 for queue 2:
>>
>> ```
>> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --coalesce adaptive-rx off
>> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --coalesce rx-usecs 16
>> ```
>>
>> Confirm that the operation succeeded:
>>
>> ```
>> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
>> Queue: 2
>> Adaptive RX: off TX: on
>> stats-block-usecs: 0
>> sample-interval: 0
>> pkt-rate-low: 0
>> pkt-rate-high: 0
>>
>> rx-usecs: 16
>> rx-frames: 32
>> rx-usecs-irq: 0
>> rx-frames-irq: 0
>>
>> tx-usecs: 8
>> tx-frames: 128
>> tx-usecs-irq: 0
>> tx-frames-irq: 0
>> ```
>>
>> The individual channel settings do not overwrite the global ones. However
>> Setting the global parameters will also reset all of the individual channel
>> options. For example, after we set the options for queue 2, we'll see that
>> the global options remain unchanged:
>> ```
>> $ sudo ethtool --show-coalesce eth0
>> Coalesce parameters for eth0:
>> Adaptive RX: on TX: on
>> stats-block-usecs: 0
>> sample-interval: 0
>> pkt-rate-low: 0
>> pkt-rate-high: 0
>>
>> rx-usecs: 8
>> rx-frames: 128
>> rx-usecs-irq: 0
>> rx-frames-irq: 0
>>
>> tx-usecs: 16
>> tx-frames: 32
>> tx-usecs-irq: 0
>> tx-frames-irq: 0
>> ```
>>
>> But then if we set them, we'll see that the options for queue 2 have been
>> reset as well:
>> ```
>> $ sudo ethtool --coalesce eth0 adaptive-tx off
>>
>> $ sudo ethtool --show-coalesce eth0
>> Coalesce parameters for eth0:
>> Adaptive RX: on TX: off
>> stats-block-usecs: 0
>> sample-interval: 0
>> pkt-rate-low: 0
>> pkt-rate-high: 0
>>
>> rx-usecs: 8
>> rx-frames: 128
>> rx-usecs-irq: 0
>> rx-frames-irq: 0
>>
>> tx-usecs: 16
>> tx-frames: 32
>> tx-usecs-irq: 0
>> tx-frames-irq: 0
>>
>> $ sudo ethtool --per-queue eth0 queue_mask 0x4 --show-coalesce
>> Queue: 2
>> Adaptive RX: on TX: off
>> stats-block-usecs: 0
>> sample-interval: 0
>> pkt-rate-low: 0
>> pkt-rate-high: 0
>>
>> rx-usecs: 8
>> rx-frames: 128
>> rx-usecs-irq: 0
>> rx-frames-irq: 0
>>
>> tx-usecs: 16
>> tx-frames: 32
>> tx-usecs-irq: 0
>> tx-frames-irq: 0
>> ```
>>
>> Previously a global `struct mlx5e_params` stored the options in
>> `struct mlx5e_priv.channels.params`. That was preserved, but a channel-
>> specific instance was added as well, in `struct mlx5e_channel.params`.
>>
>> Best Regards,
>>
>> ***************************************************************************
>>
>> Nabil S. Alramli (4):
>> mlx5: Add mlx5e_param to individual mlx5e_channel and preserve them
>> through mlx5e_open_channels()
>> mlx5: Add queue number parameter to mlx5e_safe_switch_params()
>
> We currently are working on a variation of this without needing to use
> mlx5e_safe_switch_params for updating individual channel states (our
> variation of the feature avoids needing to place an instance of
> mlx5e_params per channel).
>
Oh I'm curious to see how this solution works. I look forward to your
upcoming patch, and would be happy to review it as well.
>> mlx5: Implement mlx5e_ethtool_{get,set}_per_queue_coalesce() to
>> support per-queue operations
>> mlx5: Add {get,set}_per_queue_coalesce()
>>
>> drivers/net/ethernet/mellanox/mlx5/core/en.h | 6 +-
>> .../ethernet/mellanox/mlx5/core/en_dcbnl.c | 2 +-
>> .../ethernet/mellanox/mlx5/core/en_ethtool.c | 214 +++++++++++++-----
>> .../net/ethernet/mellanox/mlx5/core/en_main.c | 76 +++++--
>> .../ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
>> 5 files changed, 222 insertions(+), 78 deletions(-)
>
> --
> Thanks,
>
> Rahul Rameshbabu
Best Regards,
-- Nabil S. Alramli
On Tue, 19 Sep, 2023 16:42:27 -0400 "Nabil S. Alramli" <[email protected]> wrote:
> Hi Rahul,
>
> Thank you for your response.
>
> On 9/19/23 14:55, Rahul Rameshbabu wrote:
>> Hi Nabil,
>> On Mon, 18 Sep, 2023 18:29:51 -0400 "Nabil S. Alramli" <[email protected]>
>> wrote:
>>> Hello,
>>>
>>> This is v2 of my previous patch:
>>> https://lore.kernel.org/lkml/[email protected]/.
>>>
>>> Saeed: Thanks for reviewing v1. I made significant changes to support
>>> per-channel DIM settings. Is this ready for an official v1 submission or
>>> are there other major changes you'd like to see before I do that?
>>>
>>> ***************************************************************************
>>> Version History
>>> ---------------
>>> * v1: Initial draft, individual channel DIM changes not supported.
>>> * v2: Support individual channel DIM changes.
>>> ***************************************************************************
>> We actually began working on a patch set for the feature internally
>> inspired by your initial RFC. If it is alright with you, would it be ok
>> to have you as a co-author of that series that we should have prepared
>> in the coming days? We have some minor enhancements that we think will
>> improve the general architecture for how we handle both the global and
>> per-queue settings.
>>
>
> Yes. Please feel free to add me as a co-author. Actually, I'm new to
> submitting mlx-5 patches and a lot of credit goes to Joe Damato
> <[email protected]> who had this initial idea and helped me develop it
> into this patch, so would you mind adding him as well? If you would like
> you could start with my patch-set and then revert it and add your own,
> or if you think that's too much trouble then I'm fine with however you'd
> like to proceed. I'd be happy to test your patch whenever it's ready.
Thanks for letting me know. Adding Joe as well as a co-author.
>
>>>
>>> Nabil S. Alramli (4):
>>> mlx5: Add mlx5e_param to individual mlx5e_channel and preserve them
>>> through mlx5e_open_channels()
>>> mlx5: Add queue number parameter to mlx5e_safe_switch_params()
>> We currently are working on a variation of this without needing to use
>> mlx5e_safe_switch_params for updating individual channel states (our
>> variation of the feature avoids needing to place an instance of
>> mlx5e_params per channel).
>>
>
> Oh I'm curious to see how this solution works. I look forward to your
> upcoming patch, and would be happy to review it as well.
I just wanted to let you know the patch series we are developing is
going through internal review. There are some bits we will need to add
to this series for supporting changing coalescing parameters such as the
cq_period_mode *without* needing to create an entirely new CQ (modifying
the existing CQ).
--
Thanks,
Rahul Rameshbabu