Re:答复: Re: [PATCH] app/testpmd:add bond type description

16/08/2017 04:31, xie.rongqiang@zte.com.cn:

I am sorry to reply so late for some reason.

And i figure out two ways to implement this kind of things inside the 
bonding code,

First,if can the function rte_eth_bond_mode_get() return string, so we can 
print

No it is better to use integers in API.

the bond mode straight, but in this way, we need fix the other c source 
where call the function. 

Second, we add an interface return bond mode string, in this way, we just 
call it in function

Yes a new function to convert integer to string seems better.

At the end, Declan should approve/decide.

cmd_show_bonding_config_parsed().

Finally, which way do you agree more? 

Looking forward to your early reply,Thank your. 


Thomas Monjalon [off-list ref]  2017/07/03 02:11:52:

quoted

:  Thomas Monjalon [off-list ref]
:  Declan Doherty [off-list ref], 
: dev@dpdk.org, RongQiang Xie [off-list ref], 
jingjing.wu@intel.com
:  2017/07/03 02:12
: Re: [dpdk-dev] [PATCH] app/testpmd:add bond type description

30/06/2017 17:39, Declan Doherty:

quoted

On 30/06/17 08:56, RongQiang Xie wrote:

quoted

In function cmd_show_bonding_config_parsed() used number represent
the bond type,in order more detailed,add bond type description
otherwise we may confused about the number type.
And also,the primary port just use in mode active backup and tlb,
so,when the mode is active backup or tlb show the primary port info
may be more appropriate.

Signed-off-by: RongQiang Xie <redacted>
---
  app/test-pmd/cmdline.c | 17 +++++++++++------
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index ff8ffd2..45845a4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c

@@ -4390,7 +4390,9 @@ static void cmd_show_bonding_config_parsed

(void *parsed_result,

quoted

quoted

        printf("\tFailed to get bonding mode for port = %d\n", 

port_id);

quoted

quoted

quoted

        return;
     } else
-      printf("\tBonding mode: %d\n", bonding_mode);
+      printf("\tBonding mode: %d ", bonding_mode);
+   printf("[0:Round Robin, 1:Active Backup, 2:Balance, 3:Broadcast, 

");

quoted

quoted

quoted

+   printf("\n\t\t\t4:802.3AD, 5:Adaptive TLB, 6:Adaptive Load 

Balancing]\n");

quoted

quoted

Good idea, but it would be clearer if we just returned the actual mode 

quoted

quoted

string so the user doesn't need to parse it themselves, like below.

-       } else
-               printf("\tBonding mode: %d ", bonding_mode);
-       printf("[0:Round Robin, 1:Active Backup, 2:Balance, 

3:Broadcast, ");

quoted

quoted

-       printf("\n\t\t\t4:802.3AD, 5:Adaptive TLB, 6:Adaptive Load 
Balancing]\n");
+       }
+
+       printf("\tBonding mode: %d (", bonding_mode);
+       switch (bonding_mode) {
+       case BONDING_MODE_ROUND_ROBIN:
+               printf("round-robin");
+               break;
+       case BONDING_MODE_ACTIVE_BACKUP:
+               printf("active-backup");
+               break;
+       case BONDING_MODE_BALANCE:
+               printf("link-aggregation");
+               break;
+       case BONDING_MODE_BROADCAST:
+               printf("broadcast");
+               break;
+       case BONDING_MODE_8023AD:
+               printf("link-aggregation-802.3ad");
+               break;
+       case BONDING_MODE_TLB:
+               printf("transmit-load-balancing");
+               break;
+       case BONDING_MODE_ALB:
+               printf("adaptive-load-balancing");
+               break;
+       default:
+               printf("unknown-mode");
+       }
+       printf(")\n");

I would say no.
Can we think how to implement this kind of things inside the bonding 

code?

quoted