DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: <dev@dpdk.org>
Subject: Re: [RFC 1/4] telemetry: allow commands to receive file descriptors
Date: Tue, 16 Jun 2026 13:32:28 +0100	[thread overview]
Message-ID: <ajFCXCZfikPJTrLH@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <20260609210540.768074-2-stephen@networkplumber.org>

On Tue, Jun 09, 2026 at 02:02:02PM -0700, Stephen Hemminger wrote:
> Add rte_telemetry_register_cmd_fd_arg() to register a command whose
> callback also receives file descriptors passed by the client as
> SCM_RIGHTS ancillary data. The callback owns the descriptors and must
> close them.
> 
> This lets a client open a file itself and hand the descriptor to the
> primary process, so DPDK never opens the path. That avoids path and
> permission problems and works across container filesystem namespaces.
> 
> Existing commands and clients are unaffected. If unsolicited file
> descriptor is passed, it is closed.
> 

This scheme seems reasonable in general. My only concern is whether the
lack of potential windows support is an issue? For regular telemetry, there
was always the option of a windows implementation using regular
TCP/UDP/SCTP sockets bound to localhost. However, AFAIK there is no windows
implementation of anything that supports file descriptors or handles
between processes.

Some other pieces of feedback inline below.

/Bruce

> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  doc/guides/rel_notes/release_26_07.rst |   5 ++
>  lib/telemetry/rte_telemetry.h          |  66 ++++++++++++++
>  lib/telemetry/telemetry.c              | 115 ++++++++++++++++++++++---
>  3 files changed, 174 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
> index b5285af5fe..d7a2df88c1 100644
> --- a/doc/guides/rel_notes/release_26_07.rst
> +++ b/doc/guides/rel_notes/release_26_07.rst
> @@ -141,6 +141,11 @@ New Features
>    Added AGENTS.md file for AI review
>    and supporting scripts to review patches and documentation.
>  
> +* **Added telemetry support for passing file descriptors.**
> +
> +  Add experimental telemetry callback ``rte_telemetry_register_cmd_fd_arg()``
> +  to allow command to receive file descriptors passed by client.
> +
>  
>  Removed Items
>  -------------
> diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> index 0a58e518f7..3e32d2902b 100644
> --- a/lib/telemetry/rte_telemetry.h
> +++ b/lib/telemetry/rte_telemetry.h
> @@ -325,6 +325,37 @@ typedef int (*telemetry_cb)(const char *cmd, const char *params,
>  typedef int (*telemetry_arg_cb)(const char *cmd, const char *params, void *arg,
>  		struct rte_tel_data *info);
>  
> +/**
> + * This telemetry callback is used when registering a telemetry command with
> + * rte_telemetry_register_cmd_fd_arg().
> + *
> + * It behaves like telemetry_arg_cb, but additionally receives any file
> + * descriptors the client passed alongside the command as SCM_RIGHTS ancillary
> + * data. The callback takes ownership of these descriptors and is responsible
> + * for closing them.
> + *
> + * @param cmd
> + *   The cmd that was requested by the client.
> + * @param params
> + *   Contains data required by the callback function.
> + * @param arg
> + *   The opaque value that was passed to rte_telemetry_register_cmd_fd_arg().
> + * @param fds
> + *   Array of file descriptors received from the client. May be NULL when
> + *   n_fds is zero.
> + * @param n_fds
> + *   Number of file descriptors in the fds array.
> + * @param info
> + *   The information to be returned to the caller.
> + *
> + * @return
> + *   Length of buffer used on success.
> + * @return
> + *   Negative integer on error.
> + */
> +typedef int (*telemetry_fd_cb)(const char *cmd, const char *params, void *arg,
> +		const int *fds, unsigned int n_fds, struct rte_tel_data *info);
> +

Do we anticipate in future having callbacks taking more than one FD? Would
it not be simpler just to a single fd parameter (which is -1 on no fd
passed).

>  /**
>   * Used when registering a command and callback function with telemetry.
>   *
> @@ -368,6 +399,41 @@ __rte_experimental
>  int
>  rte_telemetry_register_cmd_arg(const char *cmd, telemetry_arg_cb fn, void *arg, const char *help);
>  
> +/**
> + * Register a command and a file-descriptor-aware callback with telemetry.
> + *
> + * The callback is invoked like rte_telemetry_register_cmd_arg(), but also
> + * receives any file descriptors the client passed alongside the command as
> + * SCM_RIGHTS ancillary data. This lets a client open a file (for example a
> + * capture output file) itself and hand the descriptor to the DPDK process,
> + * which never opens the path - avoiding path and permission concerns and
> + * working across container filesystem namespaces.
> + *
> + * Descriptors sent to a command registered with rte_telemetry_register_cmd()
> + * or rte_telemetry_register_cmd_arg() are rejected and the connection is
> + * closed.
> + *
> + * @param cmd
> + *   The command to register with telemetry.
> + * @param fn
> + *   Callback function to be called when the command is requested.
> + * @param arg
> + *   An opaque value that will be passed to the callback function.
> + * @param help
> + *   Help text for the command.
> + *
> + * @return
> + *   0 on success.
> + * @return
> + *   -EINVAL for invalid parameters failure.
> + * @return
> + *   -ENOMEM for mem allocation failure.
> + */
> +__rte_experimental
> +int
> +rte_telemetry_register_cmd_fd_arg(const char *cmd, telemetry_fd_cb fn, void *arg,
> +		const char *help);
> +

Do we want to make this experimental for later stabilization, or is this an
API that is best kept as internal-only? I'd tend towards keeping it
internal-only, rather than allowing apps to define callbacks which take
FDs.

>  /**
>   * @internal
>   * Free a container that has memory allocated.
> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
> index b109d076d4..30d3ae3a13 100644
> --- a/lib/telemetry/telemetry.c
> +++ b/lib/telemetry/telemetry.c
> @@ -29,6 +29,8 @@
>  #define MAX_CMD_LEN 56
>  #define MAX_OUTPUT_LEN (1024 * 16)
>  #define MAX_CONNECTIONS 10
> +/* Maximum number of file descriptors a client may pass with one command. */
> +#define MAX_FDS 8
>  

As above, do we really need multiple FDs?

>  #ifndef RTE_EXEC_ENV_WINDOWS
>  static void *
> @@ -39,6 +41,7 @@ struct cmd_callback {
>  	char cmd[MAX_CMD_LEN];
>  	telemetry_cb fn;
>  	telemetry_arg_cb fn_arg;
> +	telemetry_fd_cb fn_fd;
>  	void *arg;
>  	char help[RTE_TEL_MAX_STRING_LEN];
>  };
> @@ -72,15 +75,15 @@ static RTE_ATOMIC(uint16_t) v2_clients;
>  #endif /* !RTE_EXEC_ENV_WINDOWS */
>  
>  static int
> -register_cmd(const char *cmd, const char *help,
> -	     telemetry_cb fn, telemetry_arg_cb fn_arg, void *arg)
> +register_cmd(const char *cmd, const char *help, telemetry_cb fn,
> +	     telemetry_arg_cb fn_arg, telemetry_fd_cb fn_fd, void *arg)
>  {
>  	struct cmd_callback *new_callbacks;
>  	const char *cmdp = cmd;
>  	int i = 0;
>  
> -	if (strlen(cmd) >= MAX_CMD_LEN || (fn == NULL && fn_arg == NULL) || cmd[0] != '/'
> -			|| strlen(help) >= RTE_TEL_MAX_STRING_LEN)
> +	if (strlen(cmd) >= MAX_CMD_LEN || (fn == NULL && fn_arg == NULL && fn_fd == NULL)
> +			|| cmd[0] != '/' || strlen(help) >= RTE_TEL_MAX_STRING_LEN)
>  		return -EINVAL;
>  
>  	while (*cmdp != '\0') {
> @@ -107,6 +110,7 @@ register_cmd(const char *cmd, const char *help,
>  	strlcpy(callbacks[i].cmd, cmd, MAX_CMD_LEN);
>  	callbacks[i].fn = fn;
>  	callbacks[i].fn_arg = fn_arg;
> +	callbacks[i].fn_fd = fn_fd;
>  	callbacks[i].arg = arg;
>  	strlcpy(callbacks[i].help, help, RTE_TEL_MAX_STRING_LEN);
>  	num_callbacks++;
> @@ -119,14 +123,22 @@ RTE_EXPORT_SYMBOL(rte_telemetry_register_cmd)
>  int
>  rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help)
>  {
> -	return register_cmd(cmd, help, fn, NULL, NULL);
> +	return register_cmd(cmd, help, fn, NULL, NULL, NULL);
>  }
>  
>  RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_telemetry_register_cmd_arg, 24.11)
>  int
>  rte_telemetry_register_cmd_arg(const char *cmd, telemetry_arg_cb fn, void *arg, const char *help)
>  {
> -	return register_cmd(cmd, help, NULL, fn, arg);
> +	return register_cmd(cmd, help, NULL, fn, NULL, arg);
> +}
> +
> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_telemetry_register_cmd_fd_arg, 26.07)
> +int
> +rte_telemetry_register_cmd_fd_arg(const char *cmd, telemetry_fd_cb fn, void *arg,
> +		const char *help)
> +{
> +	return register_cmd(cmd, help, NULL, NULL, fn, arg);
>  }
>  
>  #ifndef RTE_EXEC_ENV_WINDOWS
> @@ -368,13 +380,70 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>  		TMTY_LOG_LINE(ERR, "Error writing to socket: %s", strerror(errno));
>  }
>  
> +/*
> + * Receive a command and any file descriptors the client passed alongside it
> + * as SCM_RIGHTS ancillary data. The payload length is returned (0 if the
> + * client sent an empty message or closed the connection, negative on error).
> + * Descriptors that arrive are returned in fds[]/n_fds and are owned by the
> + * caller. MSG_CTRUNC means more descriptors were sent than the control buffer
> + * could hold; *ctrunc is set so the caller can reject the command, but the
> + * descriptors that did fit are still returned so they can be closed rather
> + * than leaked.
> + */
> +static int
> +recv_with_fds(int s, char *buf, size_t buf_len, int *fds, unsigned int *n_fds,
> +	      bool *ctrunc)
> +{
> +	char cmsgbuf[CMSG_SPACE(sizeof(int) * MAX_FDS)];
> +	struct iovec iov = { .iov_base = buf, .iov_len = buf_len };
> +	struct msghdr msg = {
> +		.msg_iov = &iov,
> +		.msg_iovlen = 1,
> +		.msg_control = cmsgbuf,
> +		.msg_controllen = sizeof(cmsgbuf),
> +	};
> +	struct cmsghdr *cmsg;
> +	int bytes;
> +
> +	*n_fds = 0;
> +	*ctrunc = false;
> +
> +	bytes = recvmsg(s, &msg, 0);
> +	if (bytes < 0)
> +		return bytes;
> +
> +	if (msg.msg_flags & MSG_CTRUNC)
> +		*ctrunc = true;
> +
> +	for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
> +		if (cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS)
> +			continue;
> +		*n_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int);
> +		memcpy(fds, CMSG_DATA(cmsg), *n_fds * sizeof(int));
> +		break;
> +	}
> +	return bytes;
> +}
> +
>  static void
> -perform_command(const struct cmd_callback *cb, const char *cmd, const char *param, int s)
> +close_fds(const int *fds, unsigned int n_fds)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < n_fds; i++)
> +		close(fds[i]);
> +}
> +
> +static void
> +perform_command(const struct cmd_callback *cb, const char *cmd, const char *param,
> +		const int *fds, unsigned int n_fds, int s)
>  {
>  	struct rte_tel_data data = {0};
>  	int ret;
>  
> -	if (cb->fn_arg != NULL)
> +	if (cb->fn_fd != NULL)
> +		ret = cb->fn_fd(cmd, param, cb->arg, fds, n_fds, &data);
> +	else if (cb->fn_arg != NULL)
>  		ret = cb->fn_arg(cmd, param, cb->arg, &data);
>  	else
>  		ret = cb->fn(cmd, param, &data);
> @@ -412,8 +481,11 @@ client_handler(void *sock_id)
>  	}
>  
>  	/* receive data is not null terminated */
> -	int bytes = read(s, buffer, sizeof(buffer) - 1);
> -	while (bytes > 0) {
> +	int fds[MAX_FDS];
> +	unsigned int n_fds = 0;
> +	bool ctrunc = false;
> +	int bytes = recv_with_fds(s, buffer, sizeof(buffer) - 1, fds, &n_fds, &ctrunc);
> +	while (bytes > 0 || (bytes == 0 && n_fds > 0)) {
>  		buffer[bytes] = 0;
>  		const char *cmd = strtok(buffer, ",");
>  		const char *param = strtok(NULL, "\0");
> @@ -429,9 +501,28 @@ client_handler(void *sock_id)
>  				}
>  			rte_spinlock_unlock(&callback_sl);
>  		}
> -		perform_command(&cb, cmd, param, s);
>  
> -		bytes = read(s, buffer, sizeof(buffer) - 1);
> +		/*
> +		 * File descriptors go only to a command that registered to
> +		 * receive them. A command that did not, or a truncated control
> +		 * message, is a client error: close the descriptors and drop the
> +		 * connection rather than silently discarding them.
> +		 */
> +		if (n_fds > 0 && (cb.fn_fd == NULL || ctrunc)) {
> +			TMTY_LOG_LINE(ERR,
> +				"Closing connection: %u file descriptor(s) passed to '%s'%s",
> +				n_fds, cmd ? cmd : "(none)",
> +				ctrunc ? " (truncated)" : " which does not accept them");
> +			close_fds(fds, n_fds);
> +			break;
> +		}
> +
> +		/* an fd-aware callback takes ownership of the descriptors */
> +		perform_command(&cb, cmd, param, fds, n_fds, s);
> +
> +		n_fds = 0;
> +		ctrunc = false;

the receive function always resets this to false anyway, so you may be able
to omit this line (assuming compiler doesn't complain).

> +		bytes = recv_with_fds(s, buffer, sizeof(buffer) - 1, fds, &n_fds, &ctrunc);
>  	}
>  exit:
>  	close(s);
> -- 
> 2.53.0
> 

  reply	other threads:[~2026-06-16 12:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09 21:02 [RFC 0/4] alternative capture mechanism Stephen Hemminger
2026-06-09 21:02 ` [RFC 1/4] telemetry: allow commands to receive file descriptors Stephen Hemminger
2026-06-16 12:32   ` Bruce Richardson [this message]
2026-06-16 14:26     ` Stephen Hemminger
2026-06-09 21:02 ` [RFC 2/4] capture: infrastructure wireshark packet capture Stephen Hemminger
2026-06-09 21:02 ` [RFC 3/4] test: add test for capture hooks Stephen Hemminger
2026-06-09 21:02 ` [RFC 4/4] usertools/dpdk-wireshark-extcap.py: script for external capture Stephen Hemminger
2026-06-16 12:37 ` [RFC 0/4] alternative capture mechanism Bruce Richardson
2026-06-16 14:28   ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajFCXCZfikPJTrLH@bricha3-mobl1.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox