[v3,4/5] telemetry: implement empty stubs for Windows

Message ID 20200803103843.6928-5-fady@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series compiling ethdev lib under windows |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Fady Bader Aug. 3, 2020, 10:38 a.m. UTC
  Telemetry didn't compile under Windows.
Empty stubs implementation was added for Windows.

Signed-off-by: Fady Bader <fady@mellanox.com>
---
 lib/librte_telemetry/rte_telemetry.h    |  4 +++
 lib/librte_telemetry/telemetry.c        | 52 ++++++++++++++++++++++++++++++++-
 lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
 3 files changed, 80 insertions(+), 2 deletions(-)
  

Comments

Narcisa Ana Maria Vasile Aug. 4, 2020, 1:26 a.m. UTC | #1
On Mon, Aug 03, 2020 at 01:38:42PM +0300, Fady Bader wrote:
> Telemetry didn't compile under Windows.
> Empty stubs implementation was added for Windows.
> 
> Signed-off-by: Fady Bader <fady@mellanox.com>
> ---
>  lib/librte_telemetry/rte_telemetry.h    |  4 +++
>  lib/librte_telemetry/telemetry.c        | 52 ++++++++++++++++++++++++++++++++-
>  lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
>  3 files changed, 80 insertions(+), 2 deletions(-)
> 
> +
> +#ifndef RTE_EXEC_ENV_WINDOWS
>  /* list of command callbacks, with one command registered by default */
>  static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS];
>  static int num_callbacks; /* How many commands are registered */
>  /* Used when accessing or modifying list of command callbacks */
>  static rte_spinlock_t callback_sl = RTE_SPINLOCK_INITIALIZER;
>  static uint16_t v2_clients;
> +int
Remove 'int'

> +#endif
>  
>  int
>  rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help)
>  {
> +#ifndef RTE_EXEC_ENV_WINDOWS
> +
>  	int i = 0;
>  
>  	if (strlen(cmd) >= MAX_CMD_LEN || fn == NULL || cmd[0] != '/'
> @@ -76,8 +98,19 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help)
>  	rte_spinlock_unlock(&callback_sl);
>  
>  	return 0;
> +
> +#else
> +
> +	RTE_SET_USED(cmd);
> +	RTE_SET_USED(fn);
> +	RTE_SET_USED(help);
> +
> +	return 0;
> +
> +#endif
Maybe add a comment like /* RTE_EXEC_ENV_WINDOWS */ next to each #else/#endif, to be easier to follow what's being guarded out. Your guards are rather small,
so it's easy to follow, but sometimes when big chunks of code are being guarded, it's harder to tell what the guard was for (e.g. line 446 it's the end of a big #ifdef/#endif). 
>  }
>  
> +#ifndef RTE_EXEC_ENV_WINDOWS
  
Fady Bader Aug. 4, 2020, 6:05 a.m. UTC | #2
> -----Original Message-----
> From: Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>
> Sent: Tuesday, August 4, 2020 4:27 AM
> To: Fady Bader <fady@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Tasnim Bashar
> <tbashar@mellanox.com>; Tal Shnaiderman <talshn@mellanox.com>; Yohad Tor
> <yohadt@mellanox.com>; dmitry.kozliuk@gmail.com;
> harini.ramakrishnan@microsoft.com; ocardona@microsoft.com;
> pallavi.kadam@intel.com; ranjit.menon@intel.com; kevin.laatz@intel.com;
> ferruh.yigit@intel.com; arybchenko@solarflare.com; ciara.power@intel.com
> Subject: Re: [PATCH v3 4/5] telemetry: implement empty stubs for Windows
> 
> On Mon, Aug 03, 2020 at 01:38:42PM +0300, Fady Bader wrote:
> > Telemetry didn't compile under Windows.
> > Empty stubs implementation was added for Windows.
> >
> > Signed-off-by: Fady Bader <fady@mellanox.com>
> > ---
> >  lib/librte_telemetry/rte_telemetry.h    |  4 +++
> >  lib/librte_telemetry/telemetry.c        | 52
> ++++++++++++++++++++++++++++++++-
> >  lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> >  3 files changed, 80 insertions(+), 2 deletions(-)
> >
> > +
> > +#ifndef RTE_EXEC_ENV_WINDOWS
> >  /* list of command callbacks, with one command registered by default
> > */  static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS];
> >  static int num_callbacks; /* How many commands are registered */
> >  /* Used when accessing or modifying list of command callbacks */
> > static rte_spinlock_t callback_sl = RTE_SPINLOCK_INITIALIZER;  static
> > uint16_t v2_clients;
> > +int
> Remove 'int'
> 

OK.

> > +#endif
> >
> >  int
> >  rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const
> > char *help)  {
> > +#ifndef RTE_EXEC_ENV_WINDOWS
> > +
> >  	int i = 0;
> >
> >  	if (strlen(cmd) >= MAX_CMD_LEN || fn == NULL || cmd[0] != '/'
> > @@ -76,8 +98,19 @@ rte_telemetry_register_cmd(const char *cmd,
> telemetry_cb fn, const char *help)
> >  	rte_spinlock_unlock(&callback_sl);
> >
> >  	return 0;
> > +
> > +#else
> > +
> > +	RTE_SET_USED(cmd);
> > +	RTE_SET_USED(fn);
> > +	RTE_SET_USED(help);
> > +
> > +	return 0;
> > +
> > +#endif
> Maybe add a comment like /* RTE_EXEC_ENV_WINDOWS */ next to each
> #else/#endif, to be easier to follow what's being guarded out. Your guards are
> rather small, so it's easy to follow, but sometimes when big chunks of code are
> being guarded, it's harder to tell what the guard was for (e.g. line 446 it's the end
> of a big #ifdef/#endif).

Ok, I'll send a new patch soon.

> >  }
> >
> > +#ifndef RTE_EXEC_ENV_WINDOWS
  

Patch

diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index d13010b8fb..c3c0e44599 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -5,6 +5,10 @@ 
 #include <stdint.h>
 #include <rte_compat.h>
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <sched.h>
+#endif
+
 #ifndef _RTE_TELEMETRY_H_
 #define _RTE_TELEMETRY_H_
 
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 0252282735..51fc0de6ae 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -1,7 +1,7 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2020 Intel Corporation
  */
-
+#ifndef RTE_EXEC_ENV_WINDOWS
 #include <unistd.h>
 #include <pthread.h>
 #include <sys/socket.h>
@@ -20,6 +20,20 @@ 
 #include "telemetry_data.h"
 #include "rte_telemetry_legacy.h"
 
+#else
+
+/* includes for Windows */
+#include <rte_common.h>
+#include <rte_log.h>
+
+#include "rte_telemetry.h"
+#include "telemetry_json.h"
+#include "telemetry_data.h"
+#include "rte_telemetry_legacy.h"
+
+#endif
+
+#ifndef RTE_EXEC_ENV_WINDOWS
 #define MAX_CMD_LEN 56
 #define MAX_HELP_LEN 64
 #define MAX_OUTPUT_LEN (1024 * 16)
@@ -42,17 +56,25 @@  struct socket {
 };
 static struct socket v2_socket; /* socket for v2 telemetry */
 static struct socket v1_socket; /* socket for v1 telemetry */
+#endif
+
 static char telemetry_log_error[1024]; /* Will contain error on init failure */
+
+#ifndef RTE_EXEC_ENV_WINDOWS
 /* list of command callbacks, with one command registered by default */
 static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS];
 static int num_callbacks; /* How many commands are registered */
 /* Used when accessing or modifying list of command callbacks */
 static rte_spinlock_t callback_sl = RTE_SPINLOCK_INITIALIZER;
 static uint16_t v2_clients;
+int
+#endif
 
 int
 rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help)
 {
+#ifndef RTE_EXEC_ENV_WINDOWS
+
 	int i = 0;
 
 	if (strlen(cmd) >= MAX_CMD_LEN || fn == NULL || cmd[0] != '/'
@@ -76,8 +98,19 @@  rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help)
 	rte_spinlock_unlock(&callback_sl);
 
 	return 0;
+
+#else
+
+	RTE_SET_USED(cmd);
+	RTE_SET_USED(fn);
+	RTE_SET_USED(help);
+
+	return 0;
+
+#endif
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static int
 list_commands(const char *cmd __rte_unused, const char *params __rte_unused,
 		struct rte_tel_data *d)
@@ -411,11 +444,14 @@  telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset)
 
 	return 0;
 }
+#endif
 
 int32_t
 rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
 		const char **err_str)
 {
+#ifndef RTE_EXEC_ENV_WINDOWS
+
 	if (telemetry_v2_init(runtime_dir, cpuset) != 0) {
 		*err_str = telemetry_log_error;
 		return -1;
@@ -424,4 +460,18 @@  rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
 		*err_str = telemetry_log_error;
 	}
 	return 0;
+
+#else
+
+	RTE_SET_USED(runtime_dir);
+	RTE_SET_USED(cpuset);
+	RTE_SET_USED(err_str);
+
+	snprintf(telemetry_log_error, sizeof(telemetry_log_error),
+		"Telemetry is not supported on Windows.");
+
+	return 0;
+
+#endif
 }
+
diff --git a/lib/librte_telemetry/telemetry_legacy.c b/lib/librte_telemetry/telemetry_legacy.c
index a341fe4ebd..674f9c40ef 100644
--- a/lib/librte_telemetry/telemetry_legacy.c
+++ b/lib/librte_telemetry/telemetry_legacy.c
@@ -1,7 +1,7 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2020 Intel Corporation
  */
-
+#ifndef RTE_EXEC_ENV_WINDOWS
 #include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
@@ -15,6 +15,15 @@ 
 
 #include "rte_telemetry_legacy.h"
 
+#else
+
+#include <rte_common.h>
+
+#include "rte_telemetry_legacy.h"
+
+#endif
+#ifndef RTE_EXEC_ENV_WINDOWS
+
 #define MAX_LEN 128
 #define BUF_SIZE 1024
 #define CLIENTS_UNREG_ACTION "\"action\":2"
@@ -48,12 +57,15 @@  struct json_command callbacks[TELEMETRY_LEGACY_MAX_CALLBACKS] = {
 };
 int num_legacy_callbacks = 1;
 static rte_spinlock_t callback_sl = RTE_SPINLOCK_INITIALIZER;
+#endif
 
 int
 rte_telemetry_legacy_register(const char *cmd,
 		enum rte_telemetry_legacy_data_req data_req,
 		telemetry_legacy_cb fn)
 {
+#ifndef RTE_EXEC_ENV_WINDOWS
+
 	if (fn == NULL)
 		return -EINVAL;
 	if (num_legacy_callbacks >= (int) RTE_DIM(callbacks))
@@ -71,8 +83,19 @@  rte_telemetry_legacy_register(const char *cmd,
 	rte_spinlock_unlock(&callback_sl);
 
 	return 0;
+
+#else
+
+	RTE_SET_USED(cmd);
+	RTE_SET_USED(data_req);
+	RTE_SET_USED(fn);
+
+	return 0;
+
+#endif
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static int
 register_client(const char *cmd __rte_unused, const char *params,
 		char *buffer __rte_unused, int buf_len __rte_unused)
@@ -239,3 +262,4 @@  legacy_client_handler(void *sock_id)
 	close(s);
 	return NULL;
 }
+#endif