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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 94EA1C32772 for ; Tue, 23 Aug 2022 18:16:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233574AbiHWSQM (ORCPT ); Tue, 23 Aug 2022 14:16:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52158 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234156AbiHWSPj (ORCPT ); Tue, 23 Aug 2022 14:15:39 -0400 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EB6561403D for ; Tue, 23 Aug 2022 09:30:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1661272212; x=1692808212; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=EK2Oaz69LUz1QO47DkBl+rAu+2pRBCD7WrThBIW/by4=; b=FFQXQTwx64up+UVsLgOt04l5IexVoMU4j+jjwrH2qJ3pzF9qlDxBMIUS LKDXUn8N92OULw1RbAvHQo5lt2sO0scZJhgDrjGl4S8jhIe8LuujzDg0g ILhKQ79nRTbakUAAYchjNziWr48vUgBdkI9UWajWAkf58zUFWyv3PkY+L 12ELi56In1u1Eyo75mjlrdcjIHUcyoQRFA9Vj0T78nOvCYJOairDmC5sW zZS8QYUXY+KU0ed2XKEoEJgL9BE8ctwShN0LO4KtU5cVfqOIPUW0lG/25 1/kwcAwTrGrACQdCbAOXWSQ6JSo/Oums23tna+IPbDwphyjIU9QmrtNY5 g==; X-IronPort-AV: E=McAfee;i="6500,9779,10448"; a="357713687" X-IronPort-AV: E=Sophos;i="5.93,258,1654585200"; d="scan'208";a="357713687" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Aug 2022 09:29:54 -0700 X-IronPort-AV: E=Sophos;i="5.93,258,1654585200"; d="scan'208";a="638724515" Received: from mckumar-mobl2.gar.corp.intel.com (HELO [10.215.196.186]) ([10.215.196.186]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Aug 2022 09:29:50 -0700 Message-ID: Date: Tue, 23 Aug 2022 21:59:48 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [patch net-next 0/4] net: devlink: sync flash and dev info command Content-Language: en-US To: Jiri Pirko Cc: Jakub Kicinski , netdev@vger.kernel.org, davem@davemloft.net, idosch@nvidia.com, pabeni@redhat.com, edumazet@google.com, saeedm@nvidia.com, jacob.e.keller@intel.com, vikas.gupta@broadcom.com, gospo@broadcom.com, chandrashekar.devegowda@intel.com, soumya.prakash.mishra@intel.com, linuxwwan@intel.com References: <20220818130042.535762-1-jiri@resnulli.us> <20220818194940.30fd725e@kernel.org> From: "Kumar, M Chetan" In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 8/23/2022 5:50 PM, Jiri Pirko wrote: > Tue, Aug 23, 2022 at 12:09:06PM CEST, m.chetan.kumar@linux.intel.com wrote: >> On 8/19/2022 1:55 PM, Jiri Pirko wrote: >>> Fri, Aug 19, 2022 at 04:49:40AM CEST, kuba@kernel.org wrote: >>>> On Thu, 18 Aug 2022 15:00:38 +0200 Jiri Pirko wrote: >>>>> Currently it is up to the driver what versions to expose and what flash >>>>> update component names to accept. This is inconsistent. Thankfully, only >>>>> netdevsim is currently using components, so it is a good time >>>>> to sanitize this. >>>> >>>> Please take a look at recently merged code - 5417197dd516 ("Merge branch >>>> 'wwan-t7xx-fw-flashing-and-coredump-support'"), I don't see any versions >>>> there so I think you're gonna break them? >>> >>> Ah, crap. Too late :/ They are passing the string to FW (cmd is >>> the component name here): >>> static int t7xx_devlink_fb_flash(const char *cmd, struct t7xx_port *port) >>> { >>> char flash_command[T7XX_FB_COMMAND_SIZE]; >>> >>> snprintf(flash_command, sizeof(flash_command), "%s:%s", T7XX_FB_CMD_FLASH, cmd); >>> return t7xx_devlink_fb_raw_command(flash_command, port, NULL); >>> } >>> >>> This breaks the pairing with info.versions assumption. Any possibility >>> to revert this and let them redo? >>> >>> Ccing m.chetan.kumar@linux.intel.com, chandrashekar.devegowda@intel.com, >>> soumya.prakash.mishra@intel.com >>> >>> Guys, could you expose one version for component you are flashing? We >>> need 1:1 mapping here. >> >> Thanks for the heads-up. >> I had a look at the patch & my understanding is driver is supposed >> to expose flash update component name & version details via >> devlink_info_version_running_put_ext(). > > Yes. > >> >> Is version value a must ? Internally version value is not used for making any >> decision so in case driver/device doesn't support it should be ok to pass >> empty string ? > > No. > >> >> Ex: >> devlink_info_version_running_put_ext(req, "fw", "", >> DEVLINK_INFO_VERSION_TYPE_COMPONENT); >> >> One observation:- >> While testing I noticed "flash_components:" is not getting displayed as >> mentioned in cover note. > > You need iproute2 patch for that which is still in my queue: > https://github.com/jpirko/iproute2_mlxsw/commit/e1d36409362257cc42a435f6695d2058ab7ab683 Thanks. After applying this patch "flash_components" details are getting displayed. Another observation is if NULL is passed for version_value there is a crash. Below is the backtrace. 3187.556637] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 3187.556659] #PF: supervisor read access in kernel mode [ 3187.556666] #PF: error_code(0x0000) - not-present page 3187.556791] Call Trace: [ 3187.556796] [ 3187.556801] ? devlink_info_version_put+0x112/0x1d0 [ 3187.556823] ? __nla_put+0x20/0x30 [ 3187.556833] devlink_info_version_running_put_ext+0x1c/0x30 [ 3187.556851] t7xx_devlink_info_get+0x37/0x40 [mtk_t7xx] [ 3187.556880] devlink_nl_info_fill.constprop.0+0xa1/0x120 [ 3187.556892] devlink_nl_cmd_info_get_dumpit+0xa8/0x140 [ 3187.556901] netlink_dump+0x1a3/0x340 [ 3187.556913] __netlink_dump_start+0x1d0/0x290 Is driver expected to set version number along with component name ? mtk_t7xx WWAN driver is using the devlink interface for flashing the fw to WWAN device. If WWAN device is not capable of supporting the versioning for each component how should we handle ? Please suggest. -- Chetan