[dpdk-dev] app/testpmd:add bond type description
Checks
Commit Message
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(-)
Comments
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);
> + }
>
> }
>
>
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?
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?
>
>
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?
> >
> >
>
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?
> > >
> > >
> >
>
>
>
>
@@ -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);
+ }
}