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 850EEC43334 for ; Tue, 12 Jul 2022 18:08:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232913AbiGLSIa (ORCPT ); Tue, 12 Jul 2022 14:08:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37396 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230372AbiGLSI3 (ORCPT ); Tue, 12 Jul 2022 14:08:29 -0400 Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DBDA1AA803 for ; Tue, 12 Jul 2022 11:08:26 -0700 (PDT) Received: by mail-wr1-x431.google.com with SMTP id o4so12301233wrh.3 for ; Tue, 12 Jul 2022 11:08:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=DGz+BY3xnZELHM6ryZfTEhs4BFSh6HG4c9xaRhOdDBo=; b=eATREUwe5KadLfNjw8E2OW3DLp6+xaXH9pE7xPuScqZ/3UTqteOJA4331fRQnqqLyl oAM//6GD5o7X7rzo41Dhzgi//jvDgdDAKpaN6K/IXpbcpKZqoNClJIXZpeTsMfaOqiMz TopiZP54AuO0nG/77QILQysk/4jc75EA07J7SD1PyzpZXuw/p8mo/umz5Jb8o2Q4VKNw wre9k0na3PvIpnwuEQ78yb4LnMB8u6GB3fpwoGWG6PfDfPmprrhxFsSyouCH2uOzGG2E uDn9ts3DGWA7aREWeNVDwcVvNrYV25QjtZO9hgCJCl1Fvb28MUyhtojAqU+CzDcYULLm psPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=DGz+BY3xnZELHM6ryZfTEhs4BFSh6HG4c9xaRhOdDBo=; b=tB6lZ0qSdd10PF36AvIlZfMa2o4wq22rdjAdxOK6nmxIlRt4JGcRF7zCia+ekH3Uga If+Axx4Jd60wadtG8e1S/GI5DWEdCyfMoBbz0GK98AMLk42Lg2qgiV70Su48LgokuBOo q8L+MrWITVAoYKW9sZTjouQDYeofwEvBwDRhtiWo/zPQMJtMS1MIKeS2spcQAAYXVqCd ZTeiqL5dWZCJw+lR58GjXB7KTejYk6H5XizeMahyeJp1q7EupXKM3eXdq4WBctR94b/G Mx/UEyhOZDPVpSrdKOoxDMk3GWhIYrasxFEQiP9Kpswq+HxmaVC8j6pBiJEHNGQaQHSv +38Q== X-Gm-Message-State: AJIora/ymlg/V8xIlUUHKmg5bxo0FrIVHqH7JmqVFoXn3UZx/9gQ+cVn lEHEVwjg0mnPfqsQsg34whSzpg== X-Google-Smtp-Source: AGRyM1v7/Ig14MY8t442WbNsWeoaSgcjtu4MSGmscQ8plX+daxEtm2CNix76Opc5SpGWbiGJHGXwTw== X-Received: by 2002:adf:fcca:0:b0:21d:68ff:2e5a with SMTP id f10-20020adffcca000000b0021d68ff2e5amr23300797wrs.453.1657649305307; Tue, 12 Jul 2022 11:08:25 -0700 (PDT) Received: from localhost (host-213-179-129-39.customer.m-online.net. [213.179.129.39]) by smtp.gmail.com with ESMTPSA id d20-20020a05600c4c1400b003a1980d55c4sm12883840wmp.47.2022.07.12.11.08.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Jul 2022 11:08:24 -0700 (PDT) Date: Tue, 12 Jul 2022 20:08:23 +0200 From: Jiri Pirko To: Vikas Gupta Cc: Jiri Pirko , Jakub Kicinski , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "David S. Miller" , dsahern@kernel.org, stephen@networkplumber.org, Eric Dumazet , pabeni@redhat.com, ast@kernel.org, leon@kernel.org, linux-doc@vger.kernel.org, corbet@lwn.net, Michael Chan , Andrew Gospodarek Subject: Re: [PATCH net-next v2 1/3] devlink: introduce framework for selftests Message-ID: References: <20220628164241.44360-1-vikas.gupta@broadcom.com> <20220707182950.29348-1-vikas.gupta@broadcom.com> <20220707182950.29348-2-vikas.gupta@broadcom.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org Tue, Jul 12, 2022 at 06:41:49PM CEST, vikas.gupta@broadcom.com wrote: >Hi Jiri, > >On Tue, Jul 12, 2022 at 11:58 AM Jiri Pirko wrote: >> >> Tue, Jul 12, 2022 at 08:16:11AM CEST, vikas.gupta@broadcom.com wrote: >> >Hi Jiri, >> > >> >On Mon, Jul 11, 2022 at 6:10 PM Jiri Pirko wrote: >> > >> >> Thu, Jul 07, 2022 at 08:29:48PM CEST, vikas.gupta@broadcom.com wrote: [...] >> >> > * enum devlink_trap_action - Packet trap action. >> >> > * @DEVLINK_TRAP_ACTION_DROP: Packet is dropped by the device and a copy >> >> is not >> >> >@@ -576,6 +598,10 @@ enum devlink_attr { >> >> > DEVLINK_ATTR_LINECARD_TYPE, /* string */ >> >> > DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES, /* nested */ >> >> > >> >> >+ DEVLINK_ATTR_SELFTESTS_MASK, /* u32 */ >> >> >> >> I don't see why this is u32 bitset. Just have one attr per test >> >> (NLA_FLAG) in a nested attr instead. >> >> >> > >> >As per your suggestion, for an example it should be like as below >> > >> > DEVLINK_ATTR_SELFTESTS, /* nested */ >> > >> > DEVLINK_ATTR_SELFTESTS_SOMETEST1 /* flag */ >> > >> > DEVLINK_ATTR_SELFTESTS_SOMETEST2 /* flag */ >> >> Yeah, but have the flags in separate enum, no need to pullute the >> devlink_attr enum by them. >> >> >> > >> >.... >> > >> >..... >> > >> > DEVLINK_ATTR_SLEFTESTS_RESULT_VAL, /* u8 */ >> > >> > >> > >> > If we have this way then we need to have a mapping (probably a function) >> >for drivers to tell them what tests need to be executed based on the flags >> >that are set. >> > Does this look OK? >> > The rationale behind choosing a mask is that we could directly pass the >> >mask-value to the drivers. >> >> If you have separate enum, you can use the attrs as bits internally in >> kernel. Add a helper that would help the driver to work with it. >> Pass a struct containing u32 (or u8) not to drivers. Once there are more >> tests than that, this structure can be easily extended and the helpers >> changed. This would make this scalable. No need for UAPI change or even >> internel driver api change. > >As per your suggestion, selftest attributes can be declared in separate >enum as below > >enum { > > DEVLINK_SELFTEST_SOMETEST, /* flag */ > > DEVLINK_SELFTEST_SOMETEST1, > > DEVLINK_SELFTEST_SOMETEST2, > >.... > >...... > > __DEVLINK_SELFTEST_MAX, > > DEVLINK_SELFTEST_MAX = __DEVLINK_SELFTEST_MAX - 1 > >}; >Below examples could be the flow of parameters/data from user to >kernel and vice-versa > > >Kernel to user for show command . Users can know what all tests are >supported by the driver. A return from kernel to user. >______ >|NEST | >|_____ |TEST1|TEST4|TEST7|... > > >User to kernel to execute test: If user wants to execute test4, test8, test1... >______ >|NEST | >|_____ |TEST4|TEST8|TEST1|... > > >Result Kernel to user execute test RES(u8) >______ >|NEST | >|_____ |RES4|RES8|RES1|... Hmm, I think it is not good idea to rely on the order, a netlink library can perhaps reorder it? Not sure here. > >Results are populated in the same order as the user passed the TESTs >flags. Does the above result format from kernel to user look OK ? >Else we need to have below way to form a result format, a nest should >be made for result> but since test flags are in different enum other than >devlink_attr and RES being part of devlink_attr, I believe it's not >good practice to make the below structure. Not a structure, no. Have it as another nest (could be the same attr as the parent nest: ______ |NEST | |_____ |NEST| |NEST| |NEST| TEST4,RES4 TEST8,RES8 TEST1, RES1 also, it is flexible to add another attr if needed (like maybe result message string containing error message? IDK). >______ >|NEST | >|_____ | TEST4, RES4|TEST8,RES8|TEST1,RES1|... > >Let me know if my understanding is correct. [...]