> On Mon, 24 Jun 2024 10:03:54 +0800
> Chaoyong He <chaoyong...@corigine.com> wrote:
> 
> > From: Long Wu <long...@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...@corigine.com>
> > Reviewed-by: Peng Zhang <peng.zh...@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong...@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.

Okay, will do in next version patch.

> 
> >             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,
>

Okay, will do in next version patch.
 
> >             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.

Okay, will do in next version patch.

> 
> >
> >     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 via email to