From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: =?utf-8?b?562U5aSNOiBSZTogIFtQQVRDSF0gYXBwL3Rlc3RwbWQ6?= =?utf-8?q?add_bond_type_description?= Date: Wed, 23 Aug 2017 22:22:17 +0200 Message-ID: <1632315.BKOmf88xrI@xps> References: <201706300758.v5U7wu8t037841@mse01.zte.com.cn> <5305497.JiKmSDtoNP@xps> <201708160250.v7G2o6Nb040299@mse01.zte.com.cn> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, jingjing.wu@intel.com To: xie.rongqiang@zte.com.cn, Declan Doherty Return-path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id BB7417D4A for ; Wed, 23 Aug 2017 22:22:19 +0200 (CEST) In-Reply-To: <201708160250.v7G2o6Nb040299@mse01.zte.com.cn> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 2017/07/03 02:11:52: > > > : Thomas Monjalon > > : Declan Doherty , > > : dev@dpdk.org, RongQiang Xie , > > 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: > > > On 30/06/17 08:56, RongQiang Xie wrote: > > > > 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 > > > > --- > > > > 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, > > > > printf("\tFailed to get bonding mode for port = %d\n", > port_id); > > > > 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, > "); > > > > + printf("\n\t\t\t4:802.3AD, 5:Adaptive TLB, 6:Adaptive Load > > Balancing]\n"); > > > > > > > > > > Good idea, but it would be clearer if we just returned the actual mode > > > > 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, "); > > > - 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? > > > > >