[PATCH] app/testpmd:add bond type description

[PATCH] app/testpmd:add bond type description

[RongQiang Xie]

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 <xie.rongqiang@zte.com.cn>
---
 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");
 
 	if (bonding_mode == BONDING_MODE_BALANCE) {
 		int balance_xmit_policy;
@@ -4454,12 +4456,15 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 
 	}
 
-	primary_id = rte_eth_bond_primary_get(port_id);
-	if (primary_id < 0) {
-		printf("\tFailed to get primary slave for port = %d\n", port_id);
-		return;
-	} else
+	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
+		bonding_mode == BONDING_MODE_TLB) {
+		primary_id = rte_eth_bond_primary_get(port_id);
+		if (primary_id < 0) {
+			printf("\tFailed to get primary slave for port = %d\n", port_id);
+			return;
+		}
 		printf("\tPrimary: [%d]\n", primary_id);
+	}
 
 }
 
-- 
1.8.3.1

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

[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 <xie.rongqiang@zte.com.cn>
> ---
>   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");


>   	if (bonding_mode == BONDING_MODE_BALANCE) {
>   		int balance_xmit_policy;
> @@ -4454,12 +4456,15 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
>   
>   	}
>   
> -	primary_id = rte_eth_bond_primary_get(port_id);
> -	if (primary_id < 0) {
> -		printf("\tFailed to get primary slave for port = %d\n", port_id);
> -		return;
> -	} else
> +	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
> +		bonding_mode == BONDING_MODE_TLB) {
> +		primary_id = rte_eth_bond_primary_get(port_id);
> +		if (primary_id < 0) {
> +			printf("\tFailed to get primary slave for port = %d\n", port_id);
> +			return;
> +		}
>   		printf("\tPrimary: [%d]\n", primary_id);
> +	}
>   
>   }
>   
> 

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

[Thomas Monjalon]

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 <xie.rongqiang@zte.com.cn>
> > ---
> >   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?

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

[xie.rongqiang]

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

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

cmd_show_bonding_config_parsed().

Finally, which way do you agree more? 

Looking forward to your early reply,Thank your. 


Thomas Monjalon <thomas@monjalon.net>  2017/07/03 02:11:52:

> :  Thomas Monjalon <thomas@monjalon.net>
> :  Declan Doherty <declan.doherty@intel.com>, 
> : dev@dpdk.org, RongQiang Xie <xie.rongqiang@zte.com.cn>, 
> 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 <xie.rongqiang@zte.com.cn>
> > > ---
> > >   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?
> 
> 

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

[Thomas Monjalon]

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 <thomas@monjalon.net>  2017/07/03 02:11:52:
> 
> > :  Thomas Monjalon <thomas@monjalon.net>
> > :  Declan Doherty <declan.doherty@intel.com>, 
> > : dev@dpdk.org, RongQiang Xie <xie.rongqiang@zte.com.cn>, 
> > 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 <xie.rongqiang@zte.com.cn>
> > > > ---
> > > >   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?
> > 
> > 
> 

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

[xie.rongqiang]

Hi,
   I make a new patch for this issue becase the previous patch has delete 
when the version 17.08 release.
 
   The website is http://www.dpdk.org/dev/patchwork/patch/27851/,Thank 
you.


Thomas Monjalon <thomas@monjalon.net> 写于 2017/08/24 04:22:17:

> 发件人:  Thomas Monjalon <thomas@monjalon.net>
> 收件人:  xie.rongqiang@zte.com.cn, Declan Doherty 
> <declan.doherty@intel.com>, 
> 抄送: dev@dpdk.org, jingjing.wu@intel.com
> 日期:  2017/08/24 04:23
> 主题: Re: [dpdk-dev] 答复: 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 <thomas@monjalon.net>  2017/07/03 02:11:52:
> > 
> > > :  Thomas Monjalon <thomas@monjalon.net>
> > > :  Declan Doherty <declan.doherty@intel.com>, 
> > > : dev@dpdk.org, RongQiang Xie <xie.rongqiang@zte.com.cn>, 
> > > 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 <xie.rongqiang@zte.com.cn>
> > > > > ---
> > > > >   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?
> > > 
> > > 
> > 
> 
> 
> 
> 

[PATCH] app/testpmd:add bond type description

[Rongqiang XIE]

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 <xie.rongqiang@zte.com.cn>
---
 app/test-pmd/cmdline.c                       | 29 +++++++++++++--------
 drivers/net/bonding/rte_eth_bond.h           | 15 +++++++++++
 drivers/net/bonding/rte_eth_bond_api.c       | 39 ++++++++++++++++++++++++++++
 drivers/net/bonding/rte_eth_bond_version.map |  1 +
 4 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index cd8c358..c386a63 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -4593,6 +4593,7 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 {
 	struct cmd_show_bonding_config_result *res = parsed_result;
 	int bonding_mode, agg_mode;
+	char bonding_str[BONDING_MODE_STRING_LEN];
 	uint8_t slaves[RTE_MAX_ETHPORTS];
 	int num_slaves, num_active_slaves;
 	int primary_id;
@@ -4600,13 +4601,17 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 	portid_t port_id = res->port_id;
 
 	/* Display the bonding mode.*/
-	bonding_mode = rte_eth_bond_mode_get(port_id);
-	if (bonding_mode < 0) {
-		printf("\tFailed to get bonding mode for port = %d\n", port_id);
+	if (!rte_eth_bond_mode_string_get(port_id, bonding_str)) {
+		printf("\tFailed to get bonding mode string for port = %d\n", port_id);
 		return;
 	} else
-		printf("\tBonding mode: %d\n", bonding_mode);
+		printf("\tBonding mode: %s\n", bonding_str);
 
+	bonding_mode = rte_eth_bond_mode_get(port_id);
+	if (bonding_mode < 0) {
+		printf("\tFailed to get bonding mode for port = %d\n", port_id);
+		return;	
+	}
 	if (bonding_mode == BONDING_MODE_BALANCE) {
 		int balance_xmit_policy;
 
@@ -4685,13 +4690,15 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 		printf("\tActive Slaves: []\n");
 
 	}
-
-	primary_id = rte_eth_bond_primary_get(port_id);
-	if (primary_id < 0) {
-		printf("\tFailed to get primary slave for port = %d\n", port_id);
-		return;
-	} else
-		printf("\tPrimary: [%d]\n", primary_id);
+	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
+		bonding_mode == BONDING_MODE_TLB){
+		primary_id = rte_eth_bond_primary_get(port_id);
+		if (primary_id < 0) {
+			printf("\tFailed to get primary slave for port = %d\n", port_id);
+			return;
+		} else
+			printf("\tPrimary: [%d]\n", primary_id);
+	}
 
 }
 
diff --git a/drivers/net/bonding/rte_eth_bond.h b/drivers/net/bonding/rte_eth_bond.h
index 8efbf07..c25293a 100644
--- a/drivers/net/bonding/rte_eth_bond.h
+++ b/drivers/net/bonding/rte_eth_bond.h
@@ -117,6 +117,9 @@
 #define BALANCE_XMIT_POLICY_LAYER34		(2)
 /**< Layer 3+4 (IP Addresses + UDP Ports) transmit load balancing */
 
+/* Max length size for bond mode string */
+#define BONDING_MODE_STRING_LEN      (30)
+
 /**
  * Create a bonded rte_eth_dev device
  *
@@ -189,6 +192,18 @@
 rte_eth_bond_mode_get(uint8_t bonded_port_id);
 
 /**
+ * Get link bonding mode string of bonded device
+ *
+ * @param bonded_port_id	Port ID of bonded device.
+ *
+ * @param mode          mode string
+ * @return
+ *	link bonding mode on success, negative value otherwise
+ */
+int
+rte_eth_bond_mode_string_get(uint8_t bonded_port_id, char *mode);
+
+/**
  * Set slave rte_eth_dev as primary slave of bonded device
  *
  * @param bonded_port_id	Port ID of bonded device.
diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index de1d9e0..5ba097c 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -510,6 +510,45 @@
 }
 
 int
+rte_eth_bond_mode_string_get(uint8_t bonded_port_id, char *mode)
+{
+	struct bond_dev_private *internals;
+
+	if (valid_bonded_port_id(bonded_port_id) != 0)
+		return -1;
+
+	internals = rte_eth_devices[bonded_port_id].data->dev_private;
+	
+	switch (internals->mode) {
+		case BONDING_MODE_ROUND_ROBIN:
+			memcpy(mode, "round-robin", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_ACTIVE_BACKUP:
+			memcpy(mode, "active-backup", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_BALANCE:
+			memcpy(mode, "link-aggregation", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_BROADCAST:
+			memcpy(mode, "broadcast", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_8023AD:
+			memcpy(mode, "link-aggregation-802.3ad", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_TLB:
+			memcpy(mode, "transmit-load-balancing", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_ALB:
+			memcpy(mode, "adaptive-load-balancing", BONDING_MODE_STRING_LEN);
+			break;
+		default:
+			memcpy(mode, "unknown-mode", BONDING_MODE_STRING_LEN);
+	}
+	
+	return 0;
+}
+
+int
 rte_eth_bond_primary_set(uint8_t bonded_port_id, uint8_t slave_port_id)
 {
 	struct bond_dev_private *internals;
diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
index 0f4e847..c08f60e 100644
--- a/drivers/net/bonding/rte_eth_bond_version.map
+++ b/drivers/net/bonding/rte_eth_bond_version.map
@@ -53,6 +53,7 @@ DPDK_17.08 {
         rte_eth_bond_8023ad_agg_selection_set;
         rte_eth_bond_8023ad_conf_get;
         rte_eth_bond_8023ad_setup;
+        rte_eth_bond_mode_string_get;
 
 
 } DPDK_16.07;
-- 
1.8.3.1

[PATCH] app/testpmd:add bond type description

[Rongqiang XIE]

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 <xie.rongqiang@zte.com.cn>
---
 app/test-pmd/cmdline.c                 | 29 +++++++++++++++----------
 drivers/net/bonding/rte_eth_bond.h     | 15 +++++++++++++
 drivers/net/bonding/rte_eth_bond_api.c | 39 ++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index cd8c358..c386a63 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -4593,6 +4593,7 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 {
 	struct cmd_show_bonding_config_result *res = parsed_result;
 	int bonding_mode, agg_mode;
+	char bonding_str[BONDING_MODE_STRING_LEN];
 	uint8_t slaves[RTE_MAX_ETHPORTS];
 	int num_slaves, num_active_slaves;
 	int primary_id;
@@ -4600,13 +4601,17 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 	portid_t port_id = res->port_id;
 
 	/* Display the bonding mode.*/
-	bonding_mode = rte_eth_bond_mode_get(port_id);
-	if (bonding_mode < 0) {
-		printf("\tFailed to get bonding mode for port = %d\n", port_id);
+	if (!rte_eth_bond_mode_string_get(port_id, bonding_str)) {
+		printf("\tFailed to get bonding mode string for port = %d\n", port_id);
 		return;
 	} else
-		printf("\tBonding mode: %d\n", bonding_mode);
+		printf("\tBonding mode: %s\n", bonding_str);
 
+	bonding_mode = rte_eth_bond_mode_get(port_id);
+	if (bonding_mode < 0) {
+		printf("\tFailed to get bonding mode for port = %d\n", port_id);
+		return;	
+	}
 	if (bonding_mode == BONDING_MODE_BALANCE) {
 		int balance_xmit_policy;
 
@@ -4685,13 +4690,15 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 		printf("\tActive Slaves: []\n");
 
 	}
-
-	primary_id = rte_eth_bond_primary_get(port_id);
-	if (primary_id < 0) {
-		printf("\tFailed to get primary slave for port = %d\n", port_id);
-		return;
-	} else
-		printf("\tPrimary: [%d]\n", primary_id);
+	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
+		bonding_mode == BONDING_MODE_TLB){
+		primary_id = rte_eth_bond_primary_get(port_id);
+		if (primary_id < 0) {
+			printf("\tFailed to get primary slave for port = %d\n", port_id);
+			return;
+		} else
+			printf("\tPrimary: [%d]\n", primary_id);
+	}
 
 }
 
diff --git a/drivers/net/bonding/rte_eth_bond.h b/drivers/net/bonding/rte_eth_bond.h
index 8efbf07..c25293a 100644
--- a/drivers/net/bonding/rte_eth_bond.h
+++ b/drivers/net/bonding/rte_eth_bond.h
@@ -117,6 +117,9 @@
 #define BALANCE_XMIT_POLICY_LAYER34		(2)
 /**< Layer 3+4 (IP Addresses + UDP Ports) transmit load balancing */
 
+/* Max length size for bond mode string */
+#define BONDING_MODE_STRING_LEN      (30)
+
 /**
  * Create a bonded rte_eth_dev device
  *
@@ -189,6 +192,18 @@
 rte_eth_bond_mode_get(uint8_t bonded_port_id);
 
 /**
+ * Get link bonding mode string of bonded device
+ *
+ * @param bonded_port_id	Port ID of bonded device.
+ *
+ * @param mode          mode string
+ * @return
+ *	link bonding mode on success, negative value otherwise
+ */
+int
+rte_eth_bond_mode_string_get(uint8_t bonded_port_id, char *mode);
+
+/**
  * Set slave rte_eth_dev as primary slave of bonded device
  *
  * @param bonded_port_id	Port ID of bonded device.
diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index de1d9e0..5ba097c 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -510,6 +510,45 @@
 }
 
 int
+rte_eth_bond_mode_string_get(uint8_t bonded_port_id, char *mode)
+{
+	struct bond_dev_private *internals;
+
+	if (valid_bonded_port_id(bonded_port_id) != 0)
+		return -1;
+
+	internals = rte_eth_devices[bonded_port_id].data->dev_private;
+	
+	switch (internals->mode) {
+		case BONDING_MODE_ROUND_ROBIN:
+			memcpy(mode, "round-robin", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_ACTIVE_BACKUP:
+			memcpy(mode, "active-backup", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_BALANCE:
+			memcpy(mode, "link-aggregation", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_BROADCAST:
+			memcpy(mode, "broadcast", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_8023AD:
+			memcpy(mode, "link-aggregation-802.3ad", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_TLB:
+			memcpy(mode, "transmit-load-balancing", BONDING_MODE_STRING_LEN);
+			break;
+		case BONDING_MODE_ALB:
+			memcpy(mode, "adaptive-load-balancing", BONDING_MODE_STRING_LEN);
+			break;
+		default:
+			memcpy(mode, "unknown-mode", BONDING_MODE_STRING_LEN);
+	}
+	
+	return 0;
+}
+
+int
 rte_eth_bond_primary_set(uint8_t bonded_port_id, uint8_t slave_port_id)
 {
 	struct bond_dev_private *internals;
-- 
1.8.3.1

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

[Declan Doherty]

On 30/06/2017 2:47 PM, Declan Doherty wrote:
> On 30/06/2017 2:52 AM, RongQiang Xie wrote:
>> In function  cmd_show_bonding_config_parsed() use 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 the active backup or tlb we
>> show the primary port info.
>>
>> Signed-off-by: RongQiang Xie <xie.rongqiang@zte.com.cn>
>> ---
>>  app/test-pmd/cmdline.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 4eac498..1ae5fc0 100755
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -4455,6 +4455,7 @@ static void cmd_show_bonding_config_parsed(void
>> *parsed_result,
>>          printf("\tActive Slaves: []\n");
>>
>>      }
>> +
>>      if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
>>          bonding_mode == BONDING_MODE_TLB) {
>>          primary_id = rte_eth_bond_primary_get(port_id);
>>
>
> Hey, I think you are missing some of your intended changes in this
> patch? I'm only seeing a white space addition. Also the body of your
> email seems to be merged with the emails subject line.
>
> Thanks
> Declan

Sorry you can disregard above. It seems that my email client (Mozilla 
Thunderbird) is having difficultly with some of the characters in the 
email and isn't rendering it correctly for me, your patches look fine in 
patchwork, so I'll review from there.

Declan

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

[Declan Doherty]

On 30/06/2017 2:52 AM, RongQiang Xie wrote:
> In function  cmd_show_bonding_config_parsed() use 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 the active backup or tlb we
> show the primary port info.
>
> Signed-off-by: RongQiang Xie <xie.rongqiang@zte.com.cn>
> ---
>  app/test-pmd/cmdline.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 4eac498..1ae5fc0 100755
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -4455,6 +4455,7 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
>  		printf("\tActive Slaves: []\n");
>
>  	}
> +	
>  	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
>  		bonding_mode == BONDING_MODE_TLB) {
>  		primary_id = rte_eth_bond_primary_get(port_id);
>

Hey, I think you are missing some of your intended changes in this 
patch? I'm only seeing a white space addition. Also the body of your 
email seems to be merged with the emails subject line.

Thanks
Declan

[PATCH] app/testpmd:add bond type description

[RongQiang Xie]

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 <xie.rongqiang@zte.com.cn>
---
 0001-app-testpmd-add-bond-type-description.patch | 60 ++++++++++++++++++++++++
 app/test-pmd/cmdline.c                           | 15 ++++--
 2 files changed, 70 insertions(+), 5 deletions(-)
 create mode 100644 0001-app-testpmd-add-bond-type-description.patch

diff --git a/0001-app-testpmd-add-bond-type-description.patch b/0001-app-testpmd-add-bond-type-description.patch
new file mode 100644
index 0000000..20252af
--- /dev/null
+++ b/0001-app-testpmd-add-bond-type-description.patch
@@ -0,0 +1,60 @@
+From 74563db80f4bfbf91e949a9a575cd67e84a9b4a3 Mon Sep 17 00:00:00 2001
+From: RongQiang Xie <xie.rongqiang@zte.com.cn>
+Date: Fri, 30 Jun 2017 10:26:39 +0800
+Subject: [PATCH] app/testpmd:add bond type description
+
+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 <xie.rongqiang@zte.com.cn>
+---
+ app/test-pmd/cmdline.c | 19 ++++++++++++-------
+ 1 file changed, 12 insertions(+), 7 deletions(-)
+ mode change 100644 => 100755 app/test-pmd/cmdline.c
+
+diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
+old mode 100644
+new mode 100755
+index ff8ffd2..3878934
+--- 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");
+ 
+ 	if (bonding_mode == BONDING_MODE_BALANCE) {
+ 		int balance_xmit_policy;
+@@ -4454,12 +4456,15 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
+ 
+ 	}
+ 
+-	primary_id = rte_eth_bond_primary_get(port_id);
+-	if (primary_id < 0) {
+-		printf("\tFailed to get primary slave for port = %d\n", port_id);
+-		return;
+-	} else
+-		printf("\tPrimary: [%d]\n", primary_id);
++	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
++			bonding_mode == BONDING_MODE_TLB) {
++		primary_id = rte_eth_bond_primary_get(port_id);
++		if (primary_id < 0) {
++			printf("\tFailed to get primary slave for port = %d\n", port_id);
++			return;
++		} else
++			printf("\tPrimary: [%d]\n", primary_id);
++	}
+ 
+ }
+ 
+-- 
+1.8.3.1
+
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index ff8ffd2..be4b80b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -4391,6 +4391,8 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 		return;
 	} else
 		printf("\tBonding mode: %d\n", 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");
 
 	if (bonding_mode == BONDING_MODE_BALANCE) {
 		int balance_xmit_policy;
@@ -4454,12 +4456,15 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 
 	}
 
-	primary_id = rte_eth_bond_primary_get(port_id);
-	if (primary_id < 0) {
-		printf("\tFailed to get primary slave for port = %d\n", port_id);
-		return;
-	} else
+	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP || 
+			bonding_mode == BONDING_MODE_TLB) {
+		primary_id = rte_eth_bond_primary_get(port_id);
+		if (primary_id < 0) {
+			printf("\tFailed to get primary slave for port = %d\n", port_id);
+			return;
+		}
 		printf("\tPrimary: [%d]\n", primary_id);
+	}
 
 }
 
-- 
1.8.3.1

[PATCH] app/testpmd:add bond type description

[RongQiang Xie]

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 <xie.rongqiang@zte.com.cn>
---
 app/test-pmd/cmdline.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)
 mode change 100644 => 100755 app/test-pmd/cmdline.c

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
old mode 100644
new mode 100755
index ff8ffd2..3878934
--- 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");
 
 	if (bonding_mode == BONDING_MODE_BALANCE) {
 		int balance_xmit_policy;
@@ -4454,12 +4456,15 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 
 	}
 
-	primary_id = rte_eth_bond_primary_get(port_id);
-	if (primary_id < 0) {
-		printf("\tFailed to get primary slave for port = %d\n", port_id);
-		return;
-	} else
-		printf("\tPrimary: [%d]\n", primary_id);
+	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
+			bonding_mode == BONDING_MODE_TLB) {
+		primary_id = rte_eth_bond_primary_get(port_id);
+		if (primary_id < 0) {
+			printf("\tFailed to get primary slave for port = %d\n", port_id);
+			return;
+		} else
+			printf("\tPrimary: [%d]\n", primary_id);
+	}
 
 }
 
-- 
1.8.3.1

[PATCH] app/testpmd:add bond type description

[RongQiang Xie]

In function  cmd_show_bonding_config_parsed() use 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 the active backup or tlb we
show the primary port info.

Signed-off-by: RongQiang Xie <xie.rongqiang@zte.com.cn>
---
 app/test-pmd/cmdline.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 4eac498..1ae5fc0 100755
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -4455,6 +4455,7 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
 		printf("\tActive Slaves: []\n");
 
 	}
+	
 	if (bonding_mode == BONDING_MODE_ACTIVE_BACKUP ||
 		bonding_mode == BONDING_MODE_TLB) {
 		primary_id = rte_eth_bond_primary_get(port_id);
-- 
1.8.3.1