From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 54017A04E6; Wed, 9 Dec 2020 14:48:02 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A8FB8C98C; Wed, 9 Dec 2020 14:48:00 +0100 (CET) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 024ABC974 for ; Wed, 9 Dec 2020 14:47:57 +0100 (CET) IronPort-SDR: +obCtAnBku8814hysLUlD5lAVT4laI73Xzw4FY9O4Pz0JacmVz1tIbsA01gg0GCARTVDaWSw01 LEKGho2u1YMw== X-IronPort-AV: E=McAfee;i="6000,8403,9829"; a="153884980" X-IronPort-AV: E=Sophos;i="5.78,405,1599548400"; d="scan'208";a="153884980" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Dec 2020 05:47:56 -0800 IronPort-SDR: WqNAh4ZIkuinUBRT/TxRXqhxOrl7KA8AscZF9dulGYuOJiajXoWreUu4gstyZgmlFZAgEW9Yy+ QiipfDtlG07w== X-IronPort-AV: E=Sophos;i="5.78,405,1599548400"; d="scan'208";a="318288832" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.215.89]) ([10.213.215.89]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Dec 2020 05:47:55 -0800 To: Andrew Boyer , dev@dpdk.org Cc: Alfredo Cardigliano References: <20201203203418.15064-1-aboyer@pensando.io> <20201204201646.51746-10-aboyer@pensando.io> From: Ferruh Yigit Message-ID: <5febeda3-f733-303c-0a13-6e9f1dfe4306@intel.com> Date: Wed, 9 Dec 2020 13:47:51 +0000 MIME-Version: 1.0 In-Reply-To: <20201204201646.51746-10-aboyer@pensando.io> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 9/9] net/ionic: minor logging fixups X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 12/4/2020 8:16 PM, Andrew Boyer wrote: > Expose ionic_opcode_to_str() so it can be used for dev cmds, too. > Store the device name in struct adapter. > > Switch to memcpy() to work around gcc false positives. > > Signed-off-by: Andrew Boyer > --- > drivers/net/ionic/ionic.h | 1 + > drivers/net/ionic/ionic_dev.c | 5 +++ > drivers/net/ionic/ionic_dev.h | 2 + > drivers/net/ionic/ionic_ethdev.c | 4 +- > drivers/net/ionic/ionic_lif.c | 68 ++++++++++++++++--------------- > drivers/net/ionic/ionic_mac_api.c | 4 +- > drivers/net/ionic/ionic_main.c | 32 ++++++++------- > drivers/net/ionic/ionic_rxtx.c | 41 ++++++++----------- > 8 files changed, 84 insertions(+), 73 deletions(-) > <...> > @@ -1217,12 +1221,11 @@ ionic_lif_notifyq_init(struct ionic_lif *lif) > } > }; > > - IONIC_PRINT(DEBUG, "notifyq_init.index %d", > - ctx.cmd.q_init.index); > - IONIC_PRINT(DEBUG, "notifyq_init.ring_base 0x%" PRIx64 "", > - ctx.cmd.q_init.ring_base); > + IONIC_PRINT(DEBUG, "notifyq_init.index %d", q->index); > + IONIC_PRINT(DEBUG, "notifyq_init.ring_base %#jx", q->base_pa); There are lots of similar PRIx64 -> %j change in this patch, '%j' specifier is for 'intmax_t' and which seems 64bit storage, so this should work with 64 bit variable 'q->base_pa', but the variable is explicitly uint64_t why replacing 'PRIx64' usage which is correct and more common usage in the DPDK? Why ionic is want to do this in its own way, I am not clear of the motivation of these changes really, can you please clarify? <...> > @@ -1448,8 +1450,9 @@ ionic_lif_set_name(struct ionic_lif *lif) > }, > }; > > - snprintf(ctx.cmd.lif_setattr.name, sizeof(ctx.cmd.lif_setattr.name), > - "%d", lif->port_id); > + /* FW is responsible for NULL terminating this field */ > + memcpy(ctx.cmd.lif_setattr.name, lif->name, > + sizeof(ctx.cmd.lif_setattr.name)); Even though FW may be guaranting the string will be null terminated, won't it be nice to provide input as null terminated if this is the expectation?