[RFC PATCH v2 3/5] net/bonding: move testpmd commands

[Andrew Rybchenko]

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 <david.marchand at redhat.com>

LGTM, just a couple of nits below. Anyway

Acked-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>

[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 <mode_value> <port_id>: "
> +			"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]