[v4,4/5] telemetry: implement empty stubs for Windows
Checks
Commit Message
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 | 51 ++++++++++++++++++++++++++++++++-
lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
3 files changed, 79 insertions(+), 2 deletions(-)
Comments
On Tue, Aug 04, 2020 at 09:29:46AM +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 | 51 ++++++++++++++++++++++++++++++++-
> lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> 3 files changed, 79 insertions(+), 2 deletions(-)
>
Acked-by: Narcisa Vasile <navasile@linux.microsoft.com>
On Tue, 4 Aug 2020 09:29:46 +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 | 51 ++++++++++++++++++++++++++++++++-
> lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> 3 files changed, 79 insertions(+), 2 deletions(-)
You could #ifdef code in librte_ethdev that uses librte_telemetry and not
build librte_telemetry at all. This approach is already taken in
eal_common_options.c and it avoids spreading #ifdef throughout telemetry code.
05/08/2020 01:39, Dmitry Kozlyuk:
> On Tue, 4 Aug 2020 09:29:46 +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 | 51 ++++++++++++++++++++++++++++++++-
> > lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> > 3 files changed, 79 insertions(+), 2 deletions(-)
>
> You could #ifdef code in librte_ethdev that uses librte_telemetry and not
> build librte_telemetry at all. This approach is already taken in
> eal_common_options.c and it avoids spreading #ifdef throughout telemetry code.
The problem is that telemetry can be used anywhere, not only in ethdev.
I feel it is better to #ifdef telemetry than every telemetry calls.
04/08/2020 08:29, Fady Bader:
> 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 | 51 ++++++++++++++++++++++++++++++++-
> lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> 3 files changed, 79 insertions(+), 2 deletions(-)
The removed lines are blank lines which should remain.
On Wed, Aug 05, 2020 at 10:27:27AM +0200, Thomas Monjalon wrote:
> 05/08/2020 01:39, Dmitry Kozlyuk:
> > On Tue, 4 Aug 2020 09:29:46 +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 | 51 ++++++++++++++++++++++++++++++++-
> > > lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> > > 3 files changed, 79 insertions(+), 2 deletions(-)
> >
> > You could #ifdef code in librte_ethdev that uses librte_telemetry and not
> > build librte_telemetry at all. This approach is already taken in
> > eal_common_options.c and it avoids spreading #ifdef throughout telemetry code.
>
> The problem is that telemetry can be used anywhere, not only in ethdev.
> I feel it is better to #ifdef telemetry than every telemetry calls.
>
Given that the majority of telemetry has no external dependencies (jansson
is only used for the older compatibility part), and it uses sockets for
communication, is there a reason why it can't just be made to build and
work on windows? The unix domain socket could be converted to a standard
UDP socket on localhost, perhaps. Is there anything unix-specific beyond
that?
/Bruce
05/08/2020 10:40, Bruce Richardson:
> On Wed, Aug 05, 2020 at 10:27:27AM +0200, Thomas Monjalon wrote:
> > 05/08/2020 01:39, Dmitry Kozlyuk:
> > > On Tue, 4 Aug 2020 09:29:46 +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 | 51 ++++++++++++++++++++++++++++++++-
> > > > lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> > > > 3 files changed, 79 insertions(+), 2 deletions(-)
> > >
> > > You could #ifdef code in librte_ethdev that uses librte_telemetry and not
> > > build librte_telemetry at all. This approach is already taken in
> > > eal_common_options.c and it avoids spreading #ifdef throughout telemetry code.
> >
> > The problem is that telemetry can be used anywhere, not only in ethdev.
> > I feel it is better to #ifdef telemetry than every telemetry calls.
> >
> Given that the majority of telemetry has no external dependencies (jansson
> is only used for the older compatibility part), and it uses sockets for
> communication, is there a reason why it can't just be made to build and
> work on windows? The unix domain socket could be converted to a standard
> UDP socket on localhost, perhaps. Is there anything unix-specific beyond
> that?
Yes of course telemetry should build on Windows.
But it seems nobody committed yet to work on it.
Feel free :)
On Wed, Aug 05, 2020 at 11:06:04AM +0200, Thomas Monjalon wrote:
> 05/08/2020 10:40, Bruce Richardson:
> > On Wed, Aug 05, 2020 at 10:27:27AM +0200, Thomas Monjalon wrote:
> > > 05/08/2020 01:39, Dmitry Kozlyuk:
> > > > On Tue, 4 Aug 2020 09:29:46 +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 | 51 ++++++++++++++++++++++++++++++++-
> > > > > lib/librte_telemetry/telemetry_legacy.c | 26 ++++++++++++++++-
> > > > > 3 files changed, 79 insertions(+), 2 deletions(-)
> > > >
> > > > You could #ifdef code in librte_ethdev that uses librte_telemetry and not
> > > > build librte_telemetry at all. This approach is already taken in
> > > > eal_common_options.c and it avoids spreading #ifdef throughout telemetry code.
> > >
> > > The problem is that telemetry can be used anywhere, not only in ethdev.
> > > I feel it is better to #ifdef telemetry than every telemetry calls.
> > >
> > Given that the majority of telemetry has no external dependencies (jansson
> > is only used for the older compatibility part), and it uses sockets for
> > communication, is there a reason why it can't just be made to build and
> > work on windows? The unix domain socket could be converted to a standard
> > UDP socket on localhost, perhaps. Is there anything unix-specific beyond
> > that?
>
> Yes of course telemetry should build on Windows.
> But it seems nobody committed yet to work on it.
> Feel free :)
Thanks, I might give it a go sometime if I have the chance. However, I was
mainly just checking that there was no known serious impediment to having
it work.
@@ -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_
@@ -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 /* RTE_EXEC_ENV_WINDOWS */
+
+/* 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,24 @@ 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;
+#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 +97,19 @@ rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help)
rte_spinlock_unlock(&callback_sl);
return 0;
+
+#else /* RTE_EXEC_ENV_WINDOWS */
+
+ 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 +443,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 +459,18 @@ rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
*err_str = telemetry_log_error;
}
return 0;
+
+#else /* RTE_EXEC_ENV_WINDOWS */
+
+ 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
}
+
@@ -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 /* RTE_EXEC_ENV_WINDOWS */
+
+#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_EXEC_ENV_WINDOWS */
+
+ 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