diff mbox

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

Message ID 1465907296-27117-5-git-send-email-bernard.iremonger@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Iremonger, Bernard June 14, 2016, 12:28 p.m. UTC 
  Add call to fwd_config_setup to init_config.
Remove fwd_config_setup from fwd_config_display.
Add call to fwd_config_setup for corelist, coremask and nbcore setup.
Add call to fwd_config_setup for portlist, portmask and nbport setup.
Add call to fwd_config_setup for rxq, txq, rxd and txd setup.

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

Comments

De Lara Guarch, Pablo June 14, 2016, 1 p.m. UTC | #1 
Hi Bernard,

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, June 14, 2016 1:28 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Iremonger, Bernard
> Subject: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> fwd_config_display
> 
> Add call to fwd_config_setup to init_config.
> Remove fwd_config_setup from fwd_config_display.
> Add call to fwd_config_setup for corelist, coremask and nbcore setup.
> Add call to fwd_config_setup for portlist, portmask and nbport setup.
> Add call to fwd_config_setup for rxq, txq, rxd and txd setup.
> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>

Apologies for misleading here, but I think we should drop this patch for the moment.
It is conflicting with another patch and it is not really necessary for your changes (am I right here?),
so I would say best thing to do is to drop it and discuss if we want this change separately.
Iremonger, Bernard June 14, 2016, 2:14 p.m. UTC | #2 
Hi Pablo,

<snip>

> > Subject: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> > fwd_config_display
> >
> > Add call to fwd_config_setup to init_config.
> > Remove fwd_config_setup from fwd_config_display.
> > Add call to fwd_config_setup for corelist, coremask and nbcore setup.
> > Add call to fwd_config_setup for portlist, portmask and nbport setup.
> > Add call to fwd_config_setup for rxq, txq, rxd and txd setup.
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
> Apologies for misleading here, but I think we should drop this patch for the
> moment.
> It is conflicting with another patch and it is not really necessary for your
> changes (am I right here?), so I would say best thing to do is to drop it and
> discuss if we want this change separately.

This patch is needed for the patchset and should not be dropped.

Is it conflicting with patch the following patch?

http://dpdk.org/dev/patchwork/patch/13132/

This patch is renaming fwd_config_display to fwd_config_setup_display.
It clarifies what the function is doing, but does not address the issue of separating the setup from the display.

Separating the setup from the display is resolved in my patch.

Regards,

Bernard.
De Lara Guarch, Pablo June 14, 2016, 2:30 p.m. UTC | #3 
> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, June 14, 2016 3:14 PM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Subject: RE: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> fwd_config_display
> 
> Hi Pablo,
> 
> <snip>
> 
> > > Subject: [PATCH v4 4/9] app/testpmd: remove fwd_config_setup from
> > > fwd_config_display
> > >
> > > Add call to fwd_config_setup to init_config.
> > > Remove fwd_config_setup from fwd_config_display.
> > > Add call to fwd_config_setup for corelist, coremask and nbcore setup.
> > > Add call to fwd_config_setup for portlist, portmask and nbport setup.
> > > Add call to fwd_config_setup for rxq, txq, rxd and txd setup.
> > >
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> >
> > Apologies for misleading here, but I think we should drop this patch for the
> > moment.
> > It is conflicting with another patch and it is not really necessary for your
> > changes (am I right here?), so I would say best thing to do is to drop it and
> > discuss if we want this change separately.
> 
> This patch is needed for the patchset and should not be dropped.
> 
> Is it conflicting with patch the following patch?
> 
> http://dpdk.org/dev/patchwork/patch/13132/
> 
> This patch is renaming fwd_config_display to fwd_config_setup_display.
> It clarifies what the function is doing, but does not address the issue of
> separating the setup from the display.
> 
> Separating the setup from the display is resolved in my patch.

Right, I thought this was only refactoring. Sorry about that!
Anyway, since fwd_config_display() is going to call only pkt_fwd_config_display() now,
it makes sense to move the code from that function to fwd_config_display().
You can send a separate patch for that, as it is only refactoring.

Thanks,
Pablo

> 
> Regards,
> 
> Bernard.
> 
>
diff mbox

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index fd389ac..44a56f0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1223,6 +1223,8 @@  cmd_config_rx_tx_parsed(void *parsed_result,
 		return;
 	}
 
+	fwd_config_setup();
+
 	init_port_config();
 
 	cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
@@ -2520,16 +2522,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 +2579,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 +2622,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..47acbf6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1424,7 +1424,6 @@  pkt_fwd_config_display(struct fwd_config *cfg)
 void
 fwd_config_display(void)
 {
-	fwd_config_setup();
 	pkt_fwd_config_display(&cur_fwd_config);
 }
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f22d1b6..e8698cf 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -591,6 +591,8 @@  init_config(void)
 	/* Configuration of packet forwarding streams. */
 	if (init_fwd_streams() < 0)
 		rte_exit(EXIT_FAILURE, "FAIL from init_fwd_streams()\n");
+
+	fwd_config_setup();
 }