From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B8126C282C0 for ; Wed, 23 Jan 2019 14:48:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7A9272133D for ; Wed, 23 Jan 2019 14:48:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=resnulli-us.20150623.gappssmtp.com header.i=@resnulli-us.20150623.gappssmtp.com header.b="psdk5CKt" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726157AbfAWOsW (ORCPT ); Wed, 23 Jan 2019 09:48:22 -0500 Received: from mail-wr1-f66.google.com ([209.85.221.66]:40306 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725871AbfAWOsV (ORCPT ); Wed, 23 Jan 2019 09:48:21 -0500 Received: by mail-wr1-f66.google.com with SMTP id p4so2766628wrt.7 for ; Wed, 23 Jan 2019 06:48:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=1xrKbEfBFPw/zivwiuxc3c94CvNrYh/iSdfTDv9x1Z4=; b=psdk5CKtYJi11K69rHyCpc0kJXQxqlehErni9VJv8mN0K+gCjHQeRCpy4o5J8Z4u2r MKl5hUmENoWuSKWYVGH//PG+Mu5BH88Xd1e41Nfh9vnlV1JNYnMQ1uuzYevPzxMOaZXB BjywzH0opdDZPpQtbvUKoxEFmf3lLdwZ66CSLW+/iBHDkIXQZxbDaRfyntxepEsGpE9W FozSIAz/nN96dduVw0/OuFh4jN+vphfdKglm79dKxeNZ9imNRrgD8r3WMGukzuBXh+U4 ghSJctW9AxyHTQomgVSE/fCCqBG4Me2u5zZV2Vtm+6n+pcKtmy1h3kDWHw92v8d6ydge 4TbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=1xrKbEfBFPw/zivwiuxc3c94CvNrYh/iSdfTDv9x1Z4=; b=TX0aMoQorJwX1bg+ntVr74LX1cxjwqLU5i0ik58sIinz+B9QjG0mwJTjt+BJlePqD7 wfMdBgJL6p97b5sLsmUDLGdo9frIz7SFXVQXassdnsi8JgQRmX+yW1qCWiVBh9rymlbr 1vcRKU8uP4kr4lSMWHQDcfFpyj+flgsNvAAqZEviNlVE7oswVZkZn/oeUd51O5I8mMRK qblSW16EAGbt5eUn3Lq/xBetExzq+8P+0WWd6Er9vS5yDvCe8rwlkeL7ePdeg16foV91 ElNDfjwKAqSmBMThDiyuBP143OA42Vv4fVRFznSmgnBX4EVzaZn86dP9cUGDQvzCqKUd 8wFw== X-Gm-Message-State: AJcUukcKKJBSHEIfnJUOYOZZZuQsYiaceK6Y+1YHd6H83dQxMF77vlLl c8EJsmly0P3YOoXBxkjxpKGiWw== X-Google-Smtp-Source: ALg8bN6jj4Ap2pRlcevUjFUIys6uEYbkt17UiJL6/pOUncN5r00z+YL9labI/YPm1+tUASGiqes/cg== X-Received: by 2002:a5d:548d:: with SMTP id h13mr3005215wrv.80.1548254899303; Wed, 23 Jan 2019 06:48:19 -0800 (PST) Received: from localhost ([193.47.165.251]) by smtp.gmail.com with ESMTPSA id h17sm99930781wrt.59.2019.01.23.06.48.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 23 Jan 2019 06:48:18 -0800 (PST) Date: Wed, 23 Jan 2019 15:39:30 +0100 From: Jiri Pirko To: Eran Ben Elisha Cc: netdev@vger.kernel.org, Jiri Pirko , "David S. Miller" , Saeed Mahameed , Moshe Shemesh Subject: Re: [PATCH net-next 2/7] net/mlx5e: Move driver to use devlink msg API Message-ID: <20190123143930.GH2191@nanopsycho> References: <1548172644-30862-1-git-send-email-eranbe@mellanox.com> <1548172644-30862-3-git-send-email-eranbe@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1548172644-30862-3-git-send-email-eranbe@mellanox.com> User-Agent: Mutt/1.9.5 (2018-04-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Tue, Jan 22, 2019 at 04:57:19PM CET, eranbe@mellanox.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 >Reviewed-by: Moshe Shemesh >--- > .../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 >