DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Chaoyong He <chaoyong.he@corigine.com>
Cc: dev@dpdk.org, oss-drivers@corigine.com,
	Long Wu <long.wu@corigine.com>,
	Peng Zhang <peng.zhang@corigine.com>
Subject: Re: [PATCH 1/2] net/bonding: standard the log message
Date: Thu, 10 Oct 2024 11:10:21 -0700	[thread overview]
Message-ID: <20241010111021.1cb1b520@hermes.local> (raw)
In-Reply-To: <20240624020355.3712965-2-chaoyong.he@corigine.com>

On Mon, 24 Jun 2024 10:03:54 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> From: Long Wu <long.wu@corigine.com>
> 
> According to the check rules in the patch check script,
> drivers and libraries must use the logging framework.
> 
> So standard the log message of bonding driver by using
> the logging framework.
> 
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> ---
>  drivers/net/bonding/bonding_testpmd.c | 42 ++++++++++++---------------
>  1 file changed, 19 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
> index 8fcd6cadd0..45b636fea7 100644
> --- a/drivers/net/bonding/bonding_testpmd.c
> +++ b/drivers/net/bonding/bonding_testpmd.c
> @@ -34,15 +34,14 @@ static void cmd_set_bonding_mode_parsed(void *parsed_result,
>  	 * of device changed.
>  	 */
>  	if (port->port_status != RTE_PORT_STOPPED) {
> -		fprintf(stderr,
> -			"\t Error: Can't set bonding mode when port %d is not stopped\n",
> +		TESTPMD_LOG(ERR, "\t Error: Can't set bonding mode when port %d is not stopped\n",
>  			port_id);

Since the message may now be directed to syslog etc, avoid putting tab characters in.
Please remove the '\t ' from the message. 

>  		return;
>  	}
>  
>  	/* Set the bonding mode for the relevant port. */
>  	if (rte_eth_bond_mode_set(port_id, res->value) != 0)
> -		fprintf(stderr, "\t Failed to set bonding mode for port = %d.\n",
> +		TESTPMD_LOG(ERR, "\t Failed to set bonding mode for port = %d.\n",
>  			port_id);
>  }
>  
> @@ -98,23 +97,23 @@ static void cmd_set_bonding_lacp_dedicated_queues_parsed(void *parsed_result,
>  
>  	/** Check if the port is not started **/
>  	if (port->port_status != RTE_PORT_STOPPED) {
> -		fprintf(stderr, "Please stop port %d first\n", port_id);
> +		TESTPMD_LOG(ERR, "Please stop port %d first\n", port_id);
>  		return;
>  	}
>  
>  	if (!strcmp(res->mode, "enable")) {
>  		if (rte_eth_bond_8023ad_dedicated_queues_enable(port_id) == 0)
> -			printf("Dedicate queues for LACP control packets"
> +			TESTPMD_LOG(INFO, "Dedicate queues for LACP control packets"
>  					" enabled\n");

Don't split messages across source lines, should be able to fit this in 100 characters
or break line after INFO,

>  		else
> -			printf("Enabling dedicate queues for LACP control "
> +			TESTPMD_LOG(ERR, "Enabling dedicate queues for LACP control "
>  					"packets on port %d failed\n", port_id);
>  	} else if (!strcmp(res->mode, "disable")) {
>  		if (rte_eth_bond_8023ad_dedicated_queues_disable(port_id) == 0)
> -			printf("Dedicated queues for LACP control packets "
> +			TESTPMD_LOG(INFO, "Dedicated queues for LACP control packets "
>  					"disabled\n");
>  		else
> -			printf("Disabling dedicated queues for LACP control "
> +			TESTPMD_LOG(ERR, "Disabling dedicated queues for LACP control "
>  					"traffic on port %d failed\n", port_id);
>  	}
>  }
> @@ -178,14 +177,13 @@ static void cmd_set_bonding_balance_xmit_policy_parsed(void *parsed_result,
>  	} else if (!strcmp(res->policy, "l34")) {
>  		policy = BALANCE_XMIT_POLICY_LAYER34;
>  	} else {
> -		fprintf(stderr, "\t Invalid xmit policy selection");
> +		TESTPMD_LOG(ERR, "\t Invalid xmit policy selection");
>  		return;
>  	}
>  
>  	/* Set the bonding mode for the relevant port. */
>  	if (rte_eth_bond_xmit_policy_set(port_id, policy) != 0) {
> -		fprintf(stderr,
> -			"\t Failed to set bonding balance xmit policy for port = %d.\n",
> +		TESTPMD_LOG(ERR, "\t Failed to set bonding balance xmit policy for port = %d.\n",
>  			port_id);
>  	}
>  }
> @@ -239,7 +237,7 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
>  
>  	bonding_mode = rte_eth_bond_mode_get(port_id);
>  	if (bonding_mode < 0) {
> -		fprintf(stderr, "\tFailed to get bonding mode for port = %d\n",
> +		TESTPMD_LOG(ERR, "\tFailed to get bonding mode for port = %d\n",
>  			port_id);
>  		return;
>  	}
> @@ -292,7 +290,7 @@ static void cmd_set_bonding_primary_parsed(void *parsed_result,
>  
>  	/* Set the primary member for a bonding device. */
>  	if (rte_eth_bond_primary_set(main_port_id, member_port_id) != 0) {
> -		fprintf(stderr, "\t Failed to set primary member for port = %d.\n",
> +		TESTPMD_LOG(ERR, "\t Failed to set primary member for port = %d.\n",
>  			main_port_id);
>  		return;
>  	}
> @@ -348,8 +346,7 @@ static void cmd_add_bonding_member_parsed(void *parsed_result,
>  
>  	/* add the member for a bonding device. */
>  	if (rte_eth_bond_member_add(main_port_id, member_port_id) != 0) {
> -		fprintf(stderr,
> -			"\t Failed to add member %d to main port = %d.\n",
> +		TESTPMD_LOG(ERR, "\t Failed to add member %d to main port = %d.\n",
>  			member_port_id, main_port_id);
>  		return;
>  	}
> @@ -407,8 +404,7 @@ static void cmd_remove_bonding_member_parsed(void *parsed_result,
>  
>  	/* remove the member from a bonding device. */
>  	if (rte_eth_bond_member_remove(main_port_id, member_port_id) != 0) {
> -		fprintf(stderr,
> -			"\t Failed to remove member %d from main port = %d.\n",
> +		TESTPMD_LOG(ERR, "\t Failed to remove member %d from main port = %d.\n",
>  			member_port_id, main_port_id);
>  		return;
>  	}
> @@ -467,7 +463,7 @@ static void cmd_create_bonding_device_parsed(void *parsed_result,
>  	int ret;
>  
>  	if (test_done == 0) {
> -		fprintf(stderr, "Please stop forwarding first\n");
> +		TESTPMD_LOG(ERR, "Please stop forwarding first\n");
>  		return;
>  	}
>  
> @@ -477,10 +473,10 @@ static void cmd_create_bonding_device_parsed(void *parsed_result,
>  	/* Create a new bonding device. */
>  	port_id = rte_eth_bond_create(ethdev_name, res->mode, res->socket);
>  	if (port_id < 0) {
> -		fprintf(stderr, "\t Failed to create bonding device.\n");
> +		TESTPMD_LOG(ERR, "\t Failed to create bonding device.\n");
>  		return;
>  	}
> -	printf("Created new bonding device %s on (port %d).\n", ethdev_name,
> +	TESTPMD_LOG(INFO, "Created new bonding device %s on (port %d).\n", ethdev_name,
>  		port_id);
>  
>  	/* Update number of ports */
> @@ -488,7 +484,7 @@ static void cmd_create_bonding_device_parsed(void *parsed_result,
>  	reconfig(port_id, res->socket);
>  	ret = rte_eth_promiscuous_enable(port_id);
>  	if (ret != 0)
> -		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
> +		TESTPMD_LOG(ERR, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
>  			port_id, rte_strerror(-ret));

This could be NOTICE level since it keeps going.

>  
>  	ports[port_id].update_conf = 1;
> @@ -550,7 +546,7 @@ static void cmd_set_bond_mac_addr_parsed(void *parsed_result,
>  
>  	/* check the return value and print it if is < 0 */
>  	if (ret < 0)
> -		fprintf(stderr, "set_bond_mac_addr error: (%s)\n",
> +		TESTPMD_LOG(ERR, "set_bond_mac_addr error: (%s)\n",
>  			strerror(-ret));
>  }
>  
> @@ -603,7 +599,7 @@ static void cmd_set_bond_mon_period_parsed(void *parsed_result,
>  
>  	/* check the return value and print it if is < 0 */
>  	if (ret < 0)
> -		fprintf(stderr, "set_bond_mac_addr error: (%s)\n",
> +		TESTPMD_LOG(ERR, "set_bond_mac_addr error: (%s)\n",
>  			strerror(-ret));
>  }
>  


  reply	other threads:[~2024-10-10 18:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-24  2:03 [PATCH 0/2] add function to set dedicated queue size Chaoyong He
2024-06-24  2:03 ` [PATCH 1/2] net/bonding: standard the log message Chaoyong He
2024-10-10 18:10   ` Stephen Hemminger [this message]
2024-10-11  3:02     ` Chaoyong He
2024-06-24  2:03 ` [PATCH 2/2] net/bonding: add command to set dedicated queue size Chaoyong He
2024-10-10 18:13   ` Stephen Hemminger
2024-10-11  3:00     ` Chaoyong He
2024-10-11  3:24 ` [PATCH v2 0/2] add function " Chaoyong He
2024-10-11  3:24   ` [PATCH v2 1/2] net/bonding: standard the log message Chaoyong He
2024-10-11  5:10     ` Stephen Hemminger
2024-10-11  3:24   ` [PATCH v2 2/2] net/bonding: add command to set dedicated queue size Chaoyong He
2024-10-17 16:05     ` Thomas Monjalon
  -- strict thread matches above, loose matches on Subject: below --
2024-06-05  5:55 [PATCH 0/2] add function " Chaoyong He
2024-06-05  5:55 ` [PATCH 1/2] net/bonding: standard the log message Chaoyong He

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241010111021.1cb1b520@hermes.local \
    --to=stephen@networkplumber.org \
    --cc=chaoyong.he@corigine.com \
    --cc=dev@dpdk.org \
    --cc=long.wu@corigine.com \
    --cc=oss-drivers@corigine.com \
    --cc=peng.zhang@corigine.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).