From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B691AA0503; Fri, 20 May 2022 08:55:51 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5354942823; Fri, 20 May 2022 08:55:49 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 1FDE040151 for ; Fri, 20 May 2022 08:55:48 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id CC83A8B; Fri, 20 May 2022 09:55:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru CC83A8B DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1653029747; bh=9wS4mcondA5Rb68VWnKH4Dy6e878Shz5C/4Jh/UwsnQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=heDky7sa9BzNuvt5jS9rrc+6+WncZciK7/3hd2N47XfW7ugYqSj3cr18w8RicYn4G OY7xui14lloqXp13+RWrENAcYmGAjvwOGhAbeYPrRfv+w7f6xwkedugGRJbAnkKUpW KV4I1RLpsA8V9vF8Kwn7Jja6cMEeRisY7IvbsF84= Message-ID: <103050ec-70aa-9d28-fc42-7c40072196ff@oktetlabs.ru> Date: Fri, 20 May 2022 09:55:47 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [RFC PATCH v2 3/5] net/bonding: move testpmd commands Content-Language: en-US To: David Marchand , dev@dpdk.org Cc: thomas@monjalon.net, Xiaoyun Li , Aman Singh , Yuying Zhang , Chas Williams , "Min Hu (Connor)" References: <20220513075718.18674-1-david.marchand@redhat.com> <20220518194649.1868574-1-david.marchand@redhat.com> <20220518194649.1868574-4-david.marchand@redhat.com> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: <20220518194649.1868574-4-david.marchand@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 5/18/22 22:46, David Marchand wrote: > Move related specific testpmd commands into this driver directory. > While at it, fix checkpatch warnings. > > Signed-off-by: David Marchand LGTM, just a couple of nits below. Anyway Acked-by: Andrew Rybchenko [snip] > diff --git a/drivers/net/bonding/meson.build b/drivers/net/bonding/meson.build > index 402b44be1a..faea892295 100644 > --- a/drivers/net/bonding/meson.build > +++ b/drivers/net/bonding/meson.build > @@ -16,6 +16,7 @@ sources = files( > 'rte_eth_bond_flow.c', > 'rte_eth_bond_pmd.c', > ) > +testpmd_sources = files('rte_eth_bond_testpmd.c') I'd not follow bonding naming conventions above and name the file as you do for i40e and ixgbe: bond_testpmd.c I think naming scheme used in bonding and some other PMDs is legacy. > > deps += 'sched' # needed for rte_bitmap.h > deps += ['ip_frag'] > diff --git a/drivers/net/bonding/rte_eth_bond_testpmd.c b/drivers/net/bonding/rte_eth_bond_testpmd.c > new file mode 100644 [snip] > +static cmdline_parse_token_string_t cmd_setbonding_mode_set = > +TOKEN_STRING_INITIALIZER(struct cmd_set_bonding_mode_result, > + set, "set"); I think TOKEN should be one TAB aligned above as it is done for the most of cases in the code. > +static cmdline_parse_token_string_t cmd_setbonding_mode_bonding = > +TOKEN_STRING_INITIALIZER(struct cmd_set_bonding_mode_result, > + bonding, "bonding"); > +static cmdline_parse_token_string_t cmd_setbonding_mode_mode = > +TOKEN_STRING_INITIALIZER(struct cmd_set_bonding_mode_result, > + mode, "mode"); > +static cmdline_parse_token_num_t cmd_setbonding_mode_value = > +TOKEN_NUM_INITIALIZER(struct cmd_set_bonding_mode_result, > + value, RTE_UINT8); > +static cmdline_parse_token_num_t cmd_setbonding_mode_port = > +TOKEN_NUM_INITIALIZER(struct cmd_set_bonding_mode_result, > + port_id, RTE_UINT16); > + > +static cmdline_parse_inst_t cmd_set_bonding_mode = { > + .f = cmd_set_bonding_mode_parsed, > + .help_str = "set bonding mode : " > + "Set the bonding mode for port_id", > + .data = NULL, > + .tokens = { > + (void *)&cmd_setbonding_mode_set, > + (void *)&cmd_setbonding_mode_bonding, > + (void *)&cmd_setbonding_mode_mode, > + (void *)&cmd_setbonding_mode_value, > + (void *)&cmd_setbonding_mode_port, > + NULL > + } While on it, I'd fix above alignments to be just one TAB per level. I don't understand why 2 TABs per level is used above. [snip]