lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190123143930.GH2191@nanopsycho> Date: Wed, 23 Jan 2019 15:39:30 +0100 From: Jiri Pirko <jiri@...nulli.us> To: Eran Ben Elisha <eranbe@...lanox.com> Cc: netdev@...r.kernel.org, Jiri Pirko <jiri@...lanox.com>, "David S. Miller" <davem@...emloft.net>, Saeed Mahameed <saeedm@...lanox.com>, Moshe Shemesh <moshe@...lanox.com> Subject: Re: [PATCH net-next 2/7] net/mlx5e: Move driver to use devlink msg API Tue, Jan 22, 2019 at 04:57:19PM CET, eranbe@...lanox.com wrote: >As part of the devlink health reporter diagnose ops callback, the mlx5e TX >reporter used devlink health buffers API. Which will soon be depracated. >Modify the reporter to use the new devlink msg API. > >The actual set of the new diagnose function will be done later in the >downstream patch, only when devlink will actually expose a new diagnose >operation that uses the new API. > >Signed-off-by: Eran Ben Elisha <eranbe@...lanox.com> >Reviewed-by: Moshe Shemesh <moshe@...lanox.com> >--- > .../mellanox/mlx5/core/en/reporter_tx.c | 123 ++++-------------- > 1 file changed, 26 insertions(+), 97 deletions(-) > >diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c >index d9675afbb924..fc92850c214a 100644 >--- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c >+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c >@@ -192,110 +192,47 @@ static int mlx5e_tx_reporter_recover(struct devlink_health_reporter *reporter, > } > > static int >-mlx5e_tx_reporter_build_diagnose_output(struct devlink_health_buffer *buffer, >+mlx5e_tx_reporter_build_diagnose_output(struct devlink_msg_ctx *msg_ctx, > u32 sqn, u8 state, u8 stopped) What is "state" and "stopped"? Is "stopped" a bool? Is "state" an enum. > { >- int err, i; >- int nest = 0; >- char name[20]; >- >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT); >- if (err) >- goto buffer_error; >- nest++; >- >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR); >- if (err) >- goto buffer_error; >- nest++; >- >- sprintf(name, "SQ 0x%x", sqn); >- err = devlink_health_buffer_put_object_name(buffer, name); >- if (err) >- goto buffer_error; >- >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE); >- if (err) >- goto buffer_error; >- nest++; >- >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT); >- if (err) >- goto buffer_error; >- nest++; >- >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR); >- if (err) >- goto buffer_error; >- nest++; >- >- err = devlink_health_buffer_put_object_name(buffer, "HW state"); >- if (err) >- goto buffer_error; >+ int err; > >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE); >+ err = devlink_msg_object_start(msg_ctx, "SQ"); > if (err) >- goto buffer_error; >- nest++; >+ return err; > >- err = devlink_health_buffer_put_value_u8(buffer, state); >+ err = devlink_msg_object_start(msg_ctx, NULL); > if (err) >- goto buffer_error; >- >- devlink_health_buffer_nest_end(buffer); /* DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE */ >- nest--; >- >- devlink_health_buffer_nest_end(buffer); /* DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR */ >- nest--; >+ return err; > >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR); >+ err = devlink_msg_put_name_value_pair(msg_ctx, "sqn", &sqn, >+ sizeof(sqn), NLA_U32); > if (err) >- goto buffer_error; >- nest++; >+ return err; > >- err = devlink_health_buffer_put_object_name(buffer, "stopped"); >+ err = devlink_msg_put_name_value_pair(msg_ctx, "HW state", &state, >+ sizeof(state), NLA_U8); > if (err) >- goto buffer_error; >+ return err; > >- err = devlink_health_buffer_nest_start(buffer, >- DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE); >+ err = devlink_msg_put_name_value_pair(msg_ctx, "stopped", &stopped, >+ sizeof(stopped), NLA_U8); > if (err) >- goto buffer_error; >- nest++; >+ return err; > >- err = devlink_health_buffer_put_value_u8(buffer, stopped); >+ err = devlink_msg_object_end(msg_ctx, NULL); > if (err) >- goto buffer_error; >- >- for (i = 0; i < nest; i++) >- devlink_health_buffer_nest_end(buffer); >- >- return 0; >+ return err; > >-buffer_error: >- for (i = 0; i < nest; i++) >- devlink_health_buffer_nest_cancel(buffer); >+ err = devlink_msg_object_end(msg_ctx, "SQ"); > return err; > } > > static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter, >- struct devlink_health_buffer **buffers_array, >- unsigned int buffer_size, >- unsigned int num_buffers) >+ struct devlink_msg_ctx *msg_ctx) > { > struct mlx5e_priv *priv = devlink_health_reporter_priv(reporter); >- unsigned int buff = 0; >- int i = 0, err = 0; >- >- if (buffer_size < MLX5E_TX_REPORTER_PER_SQ_MAX_LEN) >- return -ENOMEM; >+ int i, err = 0; > > mutex_lock(&priv->state_lock); > >@@ -304,7 +241,8 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter, > return 0; > } > >- while (i < priv->channels.num * priv->channels.params.num_tc) { >+ for (i = 0; i < priv->channels.num * priv->channels.params.num_tc; >+ i++) { > struct mlx5e_txqsq *sq = priv->txq2sq[i]; > u8 state; > >@@ -312,15 +250,11 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter, > if (err) > break; > >- err = mlx5e_tx_reporter_build_diagnose_output(buffers_array[buff], >- sq->sqn, state, >+ err = mlx5e_tx_reporter_build_diagnose_output(msg_ctx, sq->sqn, >+ state, > netif_xmit_stopped(sq->txq)); This should be an array. On SQ entry : one member of array. So if you want to change it, you need to do this in 2 patches. One API, one the actual layout. :/ >- if (err) { >- if (++buff == num_buffers) >- break; >- } else { >- i++; >- } >+ if (err) >+ break; > } > > mutex_unlock(&priv->state_lock); >@@ -330,11 +264,6 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter, > static const struct devlink_health_reporter_ops mlx5_tx_reporter_ops = { > .name = "TX", > .recover = mlx5e_tx_reporter_recover, >- .diagnose_size = MLX5E_MAX_NUM_CHANNELS * MLX5E_MAX_NUM_TC * >- MLX5E_TX_REPORTER_PER_SQ_MAX_LEN, >- .diagnose = mlx5e_tx_reporter_diagnose, So you change the callback, remove it so the dissection is broken. >- .dump_size = 0, >- .dump = NULL, This has 0 relation to the patch. Should be separate. > }; > > #define MLX5_REPORTER_TX_GRACEFUL_PERIOD 500 >-- >2.17.1 >
Powered by blists - more mailing lists