Re: [net-next 06/10] net/mlx5e: change Mellanox references in DIM code

[Andy Gospodarek]

On Fri, Jan 05, 2018 at 10:04:50AM +0200, Tal Gilboa wrote:
> On 1/4/2018 10:21 PM, Andy Gospodarek wrote:
> > From: Andy Gospodarek 
> > 
> > Change all mlx5_am* and MLX_AM* references to net_dim and NET_DIM,
> MLX_AM->MLX5_AM
> 
> > cq_period_mode = enable ?
> > -   MLX5_CQ_PERIOD_MODE_START_FROM_CQE :
> > -   MLX5_CQ_PERIOD_MODE_START_FROM_EQE;
> > +   NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE :
> > +   NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> I'm not sure about this part. CQE/EQE based moderation is a feature in
> Mellanox's chips, which isn't necessarily coupled with adaptive moderation.
> net_dim lib should know which values to choose according to the selected
> mode, but I don't think mlx5 driver should use an enum from net_dim for
> enabling/disabling HW features. Another issue is that we use the enum value
> as an argument for the command to HW (0=EQE, 1=CQE). If someone would change
> the values it would break the HW feature. I think it would be safer to use
> the NET_DIM_XXX enum only when using functions from net_dim lib.

[Please ignore my eariler response, I'm not sure I fully read/parsed what you
were saying.  Sorry about that.]

I like your suggestion, so I'm going to refactor this a bit based on that.  I
made all the other suggested changes, so this should be the last one

> 
> > current_cq_period_mode = is_rx_cq ?
> > priv->channels.params.rx_cq_moderation.cq_period_mode :
> > priv->channels.params.tx_cq_moderation.cq_period_mode;
> > mode_changed = cq_period_mode != current_cq_period_mode;
> > -   if (cq_period_mode == MLX5_CQ_PERIOD_MODE_START_FROM_CQE &&
> > +   if (cq_period_mode == NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE &&
> > !MLX5_CAP_GEN(mdev, cq_period_start_from_cqe))
> > return -EOPNOTSUPP;
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c 
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > index 3aa1c90..edd4077 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > @@ -674,8 +674,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> > wqe->data.lkey = rq->mkey_be;
> > }
> > -   INIT_WORK(>am.work, mlx5e_rx_am_work);
> > -   rq->am.mode = params->rx_cq_moderation.cq_period_mode;
> > +   INIT_WORK(>dim.work, mlx5e_rx_dim_work);
> > +   rq->dim.mode = params->rx_cq_moderation.cq_period_mode;
> > rq->page_cache.head = 0;
> > rq->page_cache.tail = 0;
> > @@ -919,7 +919,7 @@ static int mlx5e_open_rq(struct mlx5e_channel *c,
> > if (err)
> > goto err_destroy_rq;
> > -   if (params->rx_am_enabled)
> > +   if (params->rx_dim_enabled)
> > c->rq.state |= BIT(MLX5E_RQ_STATE_AM);
> > return 0;
> > @@ -952,7 +952,7 @@ static void mlx5e_deactivate_rq(struct mlx5e_rq *rq)
> >   static void mlx5e_close_rq(struct mlx5e_rq *rq)
> >   {
> > -   cancel_work_sync(>am.work);
> > +   cancel_work_sync(>dim.work);
> > mlx5e_destroy_rq(rq);
> > mlx5e_free_rx_descs(rq);
> > mlx5e_free_rq(rq);
> > @@ -1565,7 +1565,7 @@ static void mlx5e_destroy_cq(struct mlx5e_cq *cq)
> >   }
> >   static int mlx5e_open_cq(struct mlx5e_channel *c,
> > -struct mlx5e_cq_moder moder,
> > +struct net_dim_cq_moder moder,
> >  struct mlx5e_cq_param *param,
> >  struct mlx5e_cq *cq)
> >   {
> > @@ -1747,7 +1747,7 @@ static int mlx5e_open_channel(struct mlx5e_priv 
> > *priv, int ix,
> >   struct mlx5e_channel_param *cparam,
> >   struct mlx5e_channel **cp)
> >   {
> > -   struct mlx5e_cq_moder icocq_moder = {0, 0};
> > +   struct net_dim_cq_moder icocq_moder = {0, 0};
> > struct net_device *netdev = priv->netdev;
> > int cpu = mlx5e_get_cpu(priv, ix);
> > struct mlx5e_channel *c;
> > @@ -1999,7 +1999,7 @@ static void mlx5e_build_ico_cq_param(struct 
> > mlx5e_priv *priv,
> > mlx5e_build_common_cq_param(priv, param);
> > -   param->cq_period_mode = MLX5_CQ_PERIOD_MODE_START_FROM_EQE;
> > +   param->cq_period_mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> >   }
> >   static void mlx5e_build_icosq_param(struct mlx5e_priv *priv,
> > @@ -4016,13 +4016,13 @@ void mlx5e_set_tx_cq_mode_params(struct 
> > mlx5e_params *params, u8 cq_period_mode)
> > params->tx_cq_moderation.usec =
> > MLX5E_PARAMS_DEFAULT_TX_CQ_MODERATION_USEC;
> > -   if (cq_period_mode == MLX5_CQ_PERIOD_MODE_START_FROM_CQE)
> > +   if (cq_period_mode == NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE)
> > params->tx_cq_moderation.usec =
> > MLX5E_PARAMS_DEFAULT_TX_CQ_MODERATION_USEC_FROM_CQE;
> > MLX5E_SET_PFLAG(params, MLX5E_PFLAG_TX_CQE_BASED_MODER,
> > params->tx_cq_moderation.cq_period_mode ==
> > -   MLX5_CQ_PERIOD_MODE_START_FROM_CQE);
> > +   

Re: [net-next 06/10] net/mlx5e: change Mellanox references in DIM code

[Andy Gospodarek]

On Fri, Jan 05, 2018 at 10:04:50AM +0200, Tal Gilboa wrote:
> On 1/4/2018 10:21 PM, Andy Gospodarek wrote:
> > From: Andy Gospodarek 
> > 
> > Change all mlx5_am* and MLX_AM* references to net_dim and NET_DIM,
> MLX_AM->MLX5_AM
> 
> > cq_period_mode = enable ?
> > -   MLX5_CQ_PERIOD_MODE_START_FROM_CQE :
> > -   MLX5_CQ_PERIOD_MODE_START_FROM_EQE;
> > +   NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE :
> > +   NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> I'm not sure about this part. CQE/EQE based moderation is a feature in
> Mellanox's chips, which isn't necessarily coupled with adaptive moderation.
> net_dim lib should know which values to choose according to the selected
> mode, but I don't think mlx5 driver should use an enum from net_dim for
> enabling/disabling HW features. Another issue is that we use the enum value
> as an argument for the command to HW (0=EQE, 1=CQE). If someone would change
> the values it would break the HW feature. I think it would be safer to use
> the NET_DIM_XXX enum only when using functions from net_dim lib.

I've gone back and forth about when to do this (now vs later).

One of the future improments I'd planned was actually to allow profiles
to live in en_dim.c and bnxt_dim.c or have some additional profiles
in net_dim.h.  This is specifially to address some different hardware
limits that might exist as hardware with a maximum that is smaller than
256 usecs (for example) might not find much benefit from how quickly
these existing profiles scale-up.

I'll play with this today for a bit and see what falls out.  My
preference is not to change this for v2 not so as to not hold up this
set too long.

> 
> > current_cq_period_mode = is_rx_cq ?
> > priv->channels.params.rx_cq_moderation.cq_period_mode :
> > priv->channels.params.tx_cq_moderation.cq_period_mode;
> > mode_changed = cq_period_mode != current_cq_period_mode;
> > -   if (cq_period_mode == MLX5_CQ_PERIOD_MODE_START_FROM_CQE &&
> > +   if (cq_period_mode == NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE &&
> > !MLX5_CAP_GEN(mdev, cq_period_start_from_cqe))
> > return -EOPNOTSUPP;
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c 
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > index 3aa1c90..edd4077 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > @@ -674,8 +674,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> > wqe->data.lkey = rq->mkey_be;
> > }
> > -   INIT_WORK(>am.work, mlx5e_rx_am_work);
> > -   rq->am.mode = params->rx_cq_moderation.cq_period_mode;
> > +   INIT_WORK(>dim.work, mlx5e_rx_dim_work);
> > +   rq->dim.mode = params->rx_cq_moderation.cq_period_mode;
> > rq->page_cache.head = 0;
> > rq->page_cache.tail = 0;
> > @@ -919,7 +919,7 @@ static int mlx5e_open_rq(struct mlx5e_channel *c,
> > if (err)
> > goto err_destroy_rq;
> > -   if (params->rx_am_enabled)
> > +   if (params->rx_dim_enabled)
> > c->rq.state |= BIT(MLX5E_RQ_STATE_AM);
> > return 0;
> > @@ -952,7 +952,7 @@ static void mlx5e_deactivate_rq(struct mlx5e_rq *rq)
> >   static void mlx5e_close_rq(struct mlx5e_rq *rq)
> >   {
> > -   cancel_work_sync(>am.work);
> > +   cancel_work_sync(>dim.work);
> > mlx5e_destroy_rq(rq);
> > mlx5e_free_rx_descs(rq);
> > mlx5e_free_rq(rq);
> > @@ -1565,7 +1565,7 @@ static void mlx5e_destroy_cq(struct mlx5e_cq *cq)
> >   }
> >   static int mlx5e_open_cq(struct mlx5e_channel *c,
> > -struct mlx5e_cq_moder moder,
> > +struct net_dim_cq_moder moder,
> >  struct mlx5e_cq_param *param,
> >  struct mlx5e_cq *cq)
> >   {
> > @@ -1747,7 +1747,7 @@ static int mlx5e_open_channel(struct mlx5e_priv 
> > *priv, int ix,
> >   struct mlx5e_channel_param *cparam,
> >   struct mlx5e_channel **cp)
> >   {
> > -   struct mlx5e_cq_moder icocq_moder = {0, 0};
> > +   struct net_dim_cq_moder icocq_moder = {0, 0};
> > struct net_device *netdev = priv->netdev;
> > int cpu = mlx5e_get_cpu(priv, ix);
> > struct mlx5e_channel *c;
> > @@ -1999,7 +1999,7 @@ static void mlx5e_build_ico_cq_param(struct 
> > mlx5e_priv *priv,
> > mlx5e_build_common_cq_param(priv, param);
> > -   param->cq_period_mode = MLX5_CQ_PERIOD_MODE_START_FROM_EQE;
> > +   param->cq_period_mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> >   }
> >   static void mlx5e_build_icosq_param(struct mlx5e_priv *priv,
> > @@ -4016,13 +4016,13 @@ void mlx5e_set_tx_cq_mode_params(struct 
> > mlx5e_params *params, u8 cq_period_mode)
> > params->tx_cq_moderation.usec =
> > MLX5E_PARAMS_DEFAULT_TX_CQ_MODERATION_USEC;
> > -   if (cq_period_mode == MLX5_CQ_PERIOD_MODE_START_FROM_CQE)
> > +   if (cq_period_mode == NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE)
> >   

Re: [net-next 06/10] net/mlx5e: change Mellanox references in DIM code

[Tal Gilboa]

On 1/4/2018 10:21 PM, Andy Gospodarek wrote:

From: Andy Gospodarek 

Change all mlx5_am* and MLX_AM* references to net_dim and NET_DIM,

MLX_AM->MLX5_AM


cq_period_mode = enable ?
-   MLX5_CQ_PERIOD_MODE_START_FROM_CQE :
-   MLX5_CQ_PERIOD_MODE_START_FROM_EQE;
+   NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE :
+   NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
I'm not sure about this part. CQE/EQE based moderation is a feature in 
Mellanox's chips, which isn't necessarily coupled with adaptive 
moderation. net_dim lib should know which values to choose according to 
the selected mode, but I don't think mlx5 driver should use an enum from 
net_dim for enabling/disabling HW features. Another issue is that we use 
the enum value as an argument for the command to HW (0=EQE, 1=CQE). If 
someone would change the values it would break the HW feature. I think 
it would be safer to use the NET_DIM_XXX enum only when using functions 
from net_dim lib.



current_cq_period_mode = is_rx_cq ?
priv->channels.params.rx_cq_moderation.cq_period_mode :
priv->channels.params.tx_cq_moderation.cq_period_mode;
mode_changed = cq_period_mode != current_cq_period_mode;
  
-	if (cq_period_mode == MLX5_CQ_PERIOD_MODE_START_FROM_CQE &&

+   if (cq_period_mode == NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE &&
!MLX5_CAP_GEN(mdev, cq_period_start_from_cqe))
return -EOPNOTSUPP;
  
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c

index 3aa1c90..edd4077 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -674,8 +674,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
wqe->data.lkey = rq->mkey_be;
}
  
-	INIT_WORK(>am.work, mlx5e_rx_am_work);

-   rq->am.mode = params->rx_cq_moderation.cq_period_mode;
+   INIT_WORK(>dim.work, mlx5e_rx_dim_work);
+   rq->dim.mode = params->rx_cq_moderation.cq_period_mode;
rq->page_cache.head = 0;
rq->page_cache.tail = 0;
  
@@ -919,7 +919,7 @@ static int mlx5e_open_rq(struct mlx5e_channel *c,

if (err)
goto err_destroy_rq;
  
-	if (params->rx_am_enabled)

+   if (params->rx_dim_enabled)
c->rq.state |= BIT(MLX5E_RQ_STATE_AM);
  
  	return 0;

@@ -952,7 +952,7 @@ static void mlx5e_deactivate_rq(struct mlx5e_rq *rq)
  
  static void mlx5e_close_rq(struct mlx5e_rq *rq)

  {
-   cancel_work_sync(>am.work);
+   cancel_work_sync(>dim.work);
mlx5e_destroy_rq(rq);
mlx5e_free_rx_descs(rq);
mlx5e_free_rq(rq);
@@ -1565,7 +1565,7 @@ static void mlx5e_destroy_cq(struct mlx5e_cq *cq)
  }
  
  static int mlx5e_open_cq(struct mlx5e_channel *c,

-struct mlx5e_cq_moder moder,
+struct net_dim_cq_moder moder,
 struct mlx5e_cq_param *param,
 struct mlx5e_cq *cq)
  {
@@ -1747,7 +1747,7 @@ static int mlx5e_open_channel(struct mlx5e_priv *priv, 
int ix,
  struct mlx5e_channel_param *cparam,
  struct mlx5e_channel **cp)
  {
-   struct mlx5e_cq_moder icocq_moder = {0, 0};
+   struct net_dim_cq_moder icocq_moder = {0, 0};
struct net_device *netdev = priv->netdev;
int cpu = mlx5e_get_cpu(priv, ix);
struct mlx5e_channel *c;
@@ -1999,7 +1999,7 @@ static void mlx5e_build_ico_cq_param(struct mlx5e_priv 
*priv,
  
  	mlx5e_build_common_cq_param(priv, param);
  
-	param->cq_period_mode = MLX5_CQ_PERIOD_MODE_START_FROM_EQE;

+   param->cq_period_mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
  }
  
  static void mlx5e_build_icosq_param(struct mlx5e_priv *priv,

@@ -4016,13 +4016,13 @@ void mlx5e_set_tx_cq_mode_params(struct mlx5e_params 
*params, u8 cq_period_mode)
params->tx_cq_moderation.usec =
MLX5E_PARAMS_DEFAULT_TX_CQ_MODERATION_USEC;
  
-	if (cq_period_mode == MLX5_CQ_PERIOD_MODE_START_FROM_CQE)

+   if (cq_period_mode == NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE)
params->tx_cq_moderation.usec =
MLX5E_PARAMS_DEFAULT_TX_CQ_MODERATION_USEC_FROM_CQE;
  
  	MLX5E_SET_PFLAG(params, MLX5E_PFLAG_TX_CQE_BASED_MODER,

params->tx_cq_moderation.cq_period_mode ==
-   MLX5_CQ_PERIOD_MODE_START_FROM_CQE);
+   NET_DIM_CQ_PERIOD_MODE_START_FROM_CQE);
  }
  
  void mlx5e_set_rx_cq_mode_params(struct mlx5e_params *params, u8 cq_period_mode)

@@ -4034,17 +4034,17 @@ void mlx5e_set_rx_cq_mode_params(struct mlx5e_params 
*params, u8 cq_period_mode)
params->rx_cq_moderation.usec =
MLX5E_PARAMS_DEFAULT_RX_CQ_MODERATION_USEC;
  
-	if (cq_period_mode == MLX5_CQ_PERIOD_MODE_START_FROM_CQE)

+   if 

[net-next 06/10] net/mlx5e: change Mellanox references in DIM code

[Andy Gospodarek]

From: Andy Gospodarek 

Change all mlx5_am* and MLX_AM* references to net_dim and NET_DIM,
respectively, in code that handles dynamic interrupt moderation.  Also
change all references from 'am' to 'dim' when used as local variables.

Signed-off-by: Andy Gospodarek 
Acked-by: Tal Gilboa 
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h   |  10 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_dim.c   |  14 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_dim.h   |  20 +-
 .../net/ethernet/mellanox/mlx5/core/en_ethtool.c   |  12 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |  32 +--
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c   |   4 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c  |   2 +-
 drivers/net/ethernet/mellanox/mlx5/core/net_dim.c  | 284 ++---
 drivers/net/ethernet/mellanox/mlx5/core/net_dim.h  |  63 +++--
 9 files changed, 219 insertions(+), 222 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h 
b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 2ccedf6..da2d5e7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -50,6 +50,7 @@
 #include "mlx5_core.h"
 #include "en_stats.h"
 #include "en_dim.h"
+#include "net_dim.h"
 
 #define MLX5_SET_CFG(p, f, v) MLX5_SET(create_flow_group_in, p, f, v)
 
@@ -237,8 +238,8 @@ struct mlx5e_params {
u16 num_channels;
u8  num_tc;
bool rx_cqe_compress_def;
-   struct mlx5e_cq_moder rx_cq_moderation;
-   struct mlx5e_cq_moder tx_cq_moderation;
+   struct net_dim_cq_moder rx_cq_moderation;
+   struct net_dim_cq_moder tx_cq_moderation;
bool lro_en;
u32 lro_wqe_sz;
u16 tx_max_inline;
@@ -248,7 +249,7 @@ struct mlx5e_params {
u32 indirection_rqt[MLX5E_INDIR_RQT_SIZE];
bool vlan_strip_disable;
bool scatter_fcs_en;
-   bool rx_am_enabled;
+   bool rx_dim_enabled;
u32 lro_timeout;
u32 pflags;
struct bpf_prog *xdp_prog;
@@ -527,7 +528,7 @@ struct mlx5e_rq {
unsigned long  state;
intix;
 
-   struct mlx5e_rx_am am; /* Adaptive Moderation */
+   struct net_dim dim; /* Dynamic Interrupt Moderation */
 
/* XDP */
struct bpf_prog   *xdp_prog;
@@ -1075,4 +1076,5 @@ void mlx5e_build_nic_params(struct mlx5_core_dev *mdev,
struct mlx5e_params *params,
u16 max_channels);
 u8 mlx5e_params_calculate_tx_min_inline(struct mlx5_core_dev *mdev);
+void mlx5e_rx_dim_work(struct work_struct *work);
 #endif /* __MLX5_EN_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c
index b9b434b..f620325 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dim.c
@@ -32,17 +32,17 @@
 
 #include "en.h"
 
-void mlx5e_rx_am_work(struct work_struct *work)
+void mlx5e_rx_dim_work(struct work_struct *work)
 {
-   struct mlx5e_rx_am *am = container_of(work, struct mlx5e_rx_am,
- work);
-   struct mlx5e_rq *rq = container_of(am, struct mlx5e_rq, am);
-   struct mlx5e_cq_moder cur_profile = mlx5e_am_get_profile(am->mode,
-
am->profile_ix);
+   struct net_dim *dim = container_of(work, struct net_dim,
+  work);
+   struct mlx5e_rq *rq = container_of(dim, struct mlx5e_rq, dim);
+   struct net_dim_cq_moder cur_profile = net_dim_get_profile(dim->mode,
+ 
dim->profile_ix);
 
mlx5_core_modify_cq_moderation(rq->mdev, >cq.mcq,
   cur_profile.usec, cur_profile.pkts);
 
-   am->state = MLX5E_AM_START_MEASURE;
+   dim->state = NET_DIM_START_MEASURE;
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dim.h 
b/drivers/net/ethernet/mellanox/mlx5/core/en_dim.h
index 5ce8e54..21219de 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_dim.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dim.h
@@ -40,23 +40,23 @@ struct mlx5e_cq_moder {
u8 cq_period_mode;
 };
 
-struct mlx5e_rx_am_sample {
+struct mlx5e_rx_dim_sample {
ktime_t time;
u32 pkt_ctr;
u32 byte_ctr;
u16 event_ctr;
 };
 
-struct mlx5e_rx_am_stats {
+struct mlx5e_rx_dim_stats {
int ppms; /* packets per msec */
int bpms; /* bytes per msec */
int epms; /* events per msec */
 };
 
-struct mlx5e_rx_am { /* Adaptive Moderation */
+struct mlx5e_rx_dim { /* Adaptive Moderation */
u8  state;
-   struct mlx5e_rx_am_statsprev_stats;
-   struct mlx5e_rx_am_sample