[04/10] test: l2fwd: add pktio driver print out

Message ID 1486384684-14761-5-git-send-email-petri.savolainen@linaro.org
State Superseded
Headers show
Series
  • Packet function inline
Related show

Commit Message

Petri Savolainen Feb. 6, 2017, 12:37 p.m.
Print out pktio driver name in start up. Driver name (e.g. dpdk or
netmap) helps in checking that correct pktio device started.

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

---
 test/common_plat/performance/odp_l2fwd.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.8.1

Comments

Bill Fischofer Feb. 7, 2017, 12:10 a.m. | #1
It's not clear why this unrelated patch is part of this series. The
only parts that seem relevant to this series are Parts 1, 2, 3, 9, and
10. The rest should either be their own patches or else be part of a
general "miscellaneous cleanups" series.

On Mon, Feb 6, 2017 at 6:37 AM, Petri Savolainen
<petri.savolainen@linaro.org> wrote:
> Print out pktio driver name in start up. Driver name (e.g. dpdk or

> netmap) helps in checking that correct pktio device started.

>

> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> ---

>  test/common_plat/performance/odp_l2fwd.c | 10 ++++++++--

>  1 file changed, 8 insertions(+), 2 deletions(-)

>

> diff --git a/test/common_plat/performance/odp_l2fwd.c b/test/common_plat/performance/odp_l2fwd.c

> index 54dc4cf..9864c64 100644

> --- a/test/common_plat/performance/odp_l2fwd.c

> +++ b/test/common_plat/performance/odp_l2fwd.c

> @@ -603,6 +603,7 @@ static int create_pktio(const char *dev, int idx, int num_rx, int num_tx,

>         odp_pktio_op_mode_t mode_rx;

>         odp_pktio_op_mode_t mode_tx;

>         pktin_mode_t in_mode = gbl_args->appl.in_mode;

> +       odp_pktio_info_t info;

>

>         odp_pktio_param_init(&pktio_param);

>

> @@ -620,8 +621,13 @@ static int create_pktio(const char *dev, int idx, int num_rx, int num_tx,

>                 return -1;

>         }

>

> -       printf("created pktio %" PRIu64 " (%s)\n",

> -              odp_pktio_to_u64(pktio), dev);

> +       if (odp_pktio_info(pktio, &info)) {

> +               LOG_ERR("Error: pktio info failed %s\n", dev);

> +               return -1;

> +       }

> +

> +       printf("created pktio %" PRIu64 ", dev: %s, drv: %s\n",

> +              odp_pktio_to_u64(pktio), dev, info.drv_name);

>

>         if (odp_pktio_capability(pktio, &capa)) {

>                 LOG_ERR("Error: capability query failed %s\n", dev);

> --

> 2.8.1

>
Savolainen, Petri (Nokia - FI/Espoo) Feb. 7, 2017, 8:03 a.m. | #2
> -----Original Message-----

> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

> Sent: Tuesday, February 07, 2017 2:10 AM

> To: Petri Savolainen <petri.savolainen@linaro.org>

> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [PATCH 04/10] test: l2fwd: add pktio driver print

> out

> 

> It's not clear why this unrelated patch is part of this series. The

> only parts that seem relevant to this series are Parts 1, 2, 3, 9, and

> 10. The rest should either be their own patches or else be part of a

> general "miscellaneous cleanups" series.


All those "extra" patches are fixing issues in test apps, etc and make debugging (packet implementation changes) easier.

E.g. this print out of driver name is useful when you want to be sure that you are testing with particular pktio (e.g. netmap vs socket). Current implementation of pktio moves from one failing pktio_open() to next one until an open succeeds or we are out of pktio drivers. Now e.g. if pktio_open() fails for netmap during development (for some reason), a following pktio_open() for socket may continue from there and you end up running socket code while you think you are running netmap code.

-Petri
Bill Fischofer Feb. 7, 2017, 12:47 p.m. | #3
On Tue, Feb 7, 2017 at 2:03 AM, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia-bell-labs.com> wrote:
>

>

>> -----Original Message-----

>> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

>> Sent: Tuesday, February 07, 2017 2:10 AM

>> To: Petri Savolainen <petri.savolainen@linaro.org>

>> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [PATCH 04/10] test: l2fwd: add pktio driver print

>> out

>>

>> It's not clear why this unrelated patch is part of this series. The

>> only parts that seem relevant to this series are Parts 1, 2, 3, 9, and

>> 10. The rest should either be their own patches or else be part of a

>> general "miscellaneous cleanups" series.

>

> All those "extra" patches are fixing issues in test apps, etc and make debugging (packet implementation changes) easier.

>

> E.g. this print out of driver name is useful when you want to be sure that you are testing with particular pktio (e.g. netmap vs socket). Current implementation of pktio moves from one failing pktio_open() to next one until an open succeeds or we are out of pktio drivers. Now e.g. if pktio_open() fails for netmap during development (for some reason), a following pktio_open() for socket may continue from there and you end up running socket code while you think you are running netmap code.


I don't dispute that these are useful changes, but they don't belong
in this series. A patch series is supposed to be a single "thought".
These cleanups have nothing to do with the thought behind the main
patch series, which is why they should be in a separate patch set.

>

> -Petri

Patch

diff --git a/test/common_plat/performance/odp_l2fwd.c b/test/common_plat/performance/odp_l2fwd.c
index 54dc4cf..9864c64 100644
--- a/test/common_plat/performance/odp_l2fwd.c
+++ b/test/common_plat/performance/odp_l2fwd.c
@@ -603,6 +603,7 @@  static int create_pktio(const char *dev, int idx, int num_rx, int num_tx,
 	odp_pktio_op_mode_t mode_rx;
 	odp_pktio_op_mode_t mode_tx;
 	pktin_mode_t in_mode = gbl_args->appl.in_mode;
+	odp_pktio_info_t info;
 
 	odp_pktio_param_init(&pktio_param);
 
@@ -620,8 +621,13 @@  static int create_pktio(const char *dev, int idx, int num_rx, int num_tx,
 		return -1;
 	}
 
-	printf("created pktio %" PRIu64 " (%s)\n",
-	       odp_pktio_to_u64(pktio), dev);
+	if (odp_pktio_info(pktio, &info)) {
+		LOG_ERR("Error: pktio info failed %s\n", dev);
+		return -1;
+	}
+
+	printf("created pktio %" PRIu64 ", dev: %s, drv: %s\n",
+	       odp_pktio_to_u64(pktio), dev, info.drv_name);
 
 	if (odp_pktio_capability(pktio, &capa)) {
 		LOG_ERR("Error: capability query failed %s\n", dev);