[dpdk-dev,v3,4/9] app/testpmd: remove fwd_config_setup from fwd_config_display

Message ID 1465744997-15487-5-git-send-email-bernard.iremonger@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Iremonger, Bernard June 12, 2016, 3:23 p.m. UTC
  Remove fwd_config_setup from fwd_config_display and check that
forwarding has been setup before displaying forwarding configuration.
Add call to fwd_config_setup for corelist, coremask, nbcore setup.
Add call to fwd_config_setup for portlist, portmask, nbport setup.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
 app/test-pmd/config.c  |  6 ++++--
 2 files changed, 20 insertions(+), 9 deletions(-)
  

Comments

De Lara Guarch, Pablo June 13, 2016, 3:57 p.m. UTC | #1
Hi Bernard,

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Sunday, June 12, 2016 4:23 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Iremonger, Bernard
> Subject: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from
> fwd_config_display
> 
> Remove fwd_config_setup from fwd_config_display and check that
> forwarding has been setup before displaying forwarding configuration.
> Add call to fwd_config_setup for corelist, coremask, nbcore setup.
> Add call to fwd_config_setup for portlist, portmask, nbport setup.

Probably we need to add that call to the commands that change number of RX/TX queues, as well.

> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
>  app/test-pmd/config.c  |  6 ++++--
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 

 [...]

> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
>  void
>  fwd_config_display(void)
>  {
> -	fwd_config_setup();
> -	pkt_fwd_config_display(&cur_fwd_config);
> +	if (cur_fwd_config.nb_fwd_ports)
> +		pkt_fwd_config_display(&cur_fwd_config);
> +	else
> +		printf("Please set portlist first\n");

I think this is not necessary. I would call fwd_config_setup() at startup,
so everything is setup when this is called (even if it is the first command when the prompt comes up).
I think at the end of init_config() is a good place.


>  }
> 
>  int
> --
> 2.6.3
  
Iremonger, Bernard June 13, 2016, 4:07 p.m. UTC | #2
Hi Pablo,

<snip>

> > Subject: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from
> > fwd_config_display
> >
> > Remove fwd_config_setup from fwd_config_display and check that
> > forwarding has been setup before displaying forwarding configuration.
> > Add call to fwd_config_setup for corelist, coremask, nbcore setup.
> > Add call to fwd_config_setup for portlist, portmask, nbport setup.
> 
> Probably we need to add that call to the commands that change number of
> RX/TX queues, as well.

I will check this.

> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> >  app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
> > app/test-pmd/config.c  |  6 ++++--
> >  2 files changed, 20 insertions(+), 9 deletions(-)
> >
> 
>  [...]
> 
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
> > void
> >  fwd_config_display(void)
> >  {
> > -	fwd_config_setup();
> > -	pkt_fwd_config_display(&cur_fwd_config);
> > +	if (cur_fwd_config.nb_fwd_ports)
> > +		pkt_fwd_config_display(&cur_fwd_config);
> > +	else
> > +		printf("Please set portlist first\n");
> 
> I think this is not necessary. I would call fwd_config_setup() at startup, so
> everything is setup when this is called (even if it is the first command when
> the prompt comes up).
> I think at the end of init_config() is a good place.

Ok, I will try this. 

<snip>

Regards,

Bernard.
  
Iremonger, Bernard June 14, 2016, 9:18 a.m. UTC | #3
Hi Pablo,

 <snip>
> > > Subject: [PATCH v3 4/9] app/testpmd: remove fwd_config_setup from
> > > fwd_config_display
> > >
> > > Remove fwd_config_setup from fwd_config_display and check that
> > > forwarding has been setup before displaying forwarding configuration.
> > > Add call to fwd_config_setup for corelist, coremask, nbcore setup.
> > > Add call to fwd_config_setup for portlist, portmask, nbport setup.
> >
> > Probably we need to add that call to the commands that change number
> > of RX/TX queues, as well.
> 
> I will check this.

Should fwd_config_setup  be added for the  --rxd and --txd  options as well?
 
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > ---
> > >  app/test-pmd/cmdline.c | 23 ++++++++++++++++-------
> > > app/test-pmd/config.c  |  6 ++++--
> > >  2 files changed, 20 insertions(+), 9 deletions(-)
> > >
> >
> >  [...]
> >
> > > --- a/app/test-pmd/config.c
> > > +++ b/app/test-pmd/config.c
> > > @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config
> > > *cfg) void
> > >  fwd_config_display(void)
> > >  {
> > > -	fwd_config_setup();
> > > -	pkt_fwd_config_display(&cur_fwd_config);
> > > +	if (cur_fwd_config.nb_fwd_ports)
> > > +		pkt_fwd_config_display(&cur_fwd_config);
> > > +	else
> > > +		printf("Please set portlist first\n");
> >
> > I think this is not necessary. I would call fwd_config_setup() at
> > startup, so everything is setup when this is called (even if it is the
> > first command when the prompt comes up).
> > I think at the end of init_config() is a good place.
> 
> Ok, I will try this.

Yes, this works.

<snip>

Regards,

Bernard.
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index fd389ac..2c2a0c7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2520,16 +2520,20 @@  static void cmd_set_list_parsed(void *parsed_result,
 		nb_item = parse_item_list(res->list_of_items, "core",
 					  RTE_MAX_LCORE,
 					  parsed_items.lcorelist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_lcores_list(parsed_items.lcorelist, nb_item);
+			fwd_config_setup();
+		}
 		return;
 	}
 	if (!strcmp(res->list_name, "portlist")) {
 		nb_item = parse_item_list(res->list_of_items, "port",
 					  RTE_MAX_ETHPORTS,
 					  parsed_items.portlist, 1);
-		if (nb_item > 0)
+		if (nb_item > 0) {
 			set_fwd_ports_list(parsed_items.portlist, nb_item);
+			fwd_config_setup();
+		}
 	}
 }
 
@@ -2573,10 +2577,13 @@  static void cmd_set_mask_parsed(void *parsed_result,
 		printf("Please stop forwarding first\n");
 		return;
 	}
-	if (!strcmp(res->mask, "coremask"))
+	if (!strcmp(res->mask, "coremask")) {
 		set_fwd_lcores_mask(res->hexavalue);
-	else if (!strcmp(res->mask, "portmask"))
+		fwd_config_setup();
+	} else if (!strcmp(res->mask, "portmask")) {
 		set_fwd_ports_mask(res->hexavalue);
+		fwd_config_setup();
+	}
 }
 
 cmdline_parse_token_string_t cmd_setmask_set =
@@ -2613,11 +2620,13 @@  static void cmd_set_parsed(void *parsed_result,
 			   __attribute__((unused)) void *data)
 {
 	struct cmd_set_result *res = parsed_result;
-	if (!strcmp(res->what, "nbport"))
+	if (!strcmp(res->what, "nbport")) {
 		set_fwd_ports_number(res->value);
-	else if (!strcmp(res->what, "nbcore"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "nbcore")) {
 		set_fwd_lcores_number(res->value);
-	else if (!strcmp(res->what, "burst"))
+		fwd_config_setup();
+	} else if (!strcmp(res->what, "burst"))
 		set_nb_pkt_per_burst(res->value);
 	else if (!strcmp(res->what, "verbose"))
 		set_verbose_level(res->value);
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f434999..8ef9c85 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1424,8 +1424,10 @@  pkt_fwd_config_display(struct fwd_config *cfg)
 void
 fwd_config_display(void)
 {
-	fwd_config_setup();
-	pkt_fwd_config_display(&cur_fwd_config);
+	if (cur_fwd_config.nb_fwd_ports)
+		pkt_fwd_config_display(&cur_fwd_config);
+	else
+		printf("Please set portlist first\n");
 }
 
 int