Re: [RFC PATCH v8 05/13] trace-cmd: Add TRACE_REQ and TRACE_RESP messages

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 22 Feb 2019 20:05:31 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:

> +static int make_trace_resp(struct tracecmd_msg *msg,
> +			   int page_size, int nr_cpus, unsigned int *ports)

I wonder if we should just have a generic "argc"/"argv" protocol to
send dynamic amounts of data, and not have specific dynamic messages?

Perhaps pass the numbers as ASCII? And then converted them at the other
end. We need to handle ntoh anyway, so a conversion (albeit a fast one)
is probably done anyway.

We could send:
 int argc =  6;
 char **argv = { "4096", "16", "33445", "33446", "33448", "33449" };

Or, if you are not comfortable with sending ASCII and converting, what
about have a specific "sending an array of numbers"?

 int argc = 6;
 int *argv = { 4096, 16, 33445, 33446, 33448, 33449 };

Thoughts?

-- Steve

> +{
> +	int ports_size = nr_cpus * sizeof(*msg->port_array);
> +	int i;
> +
> +	msg->hdr.size = htonl(ntohl(msg->hdr.size) + ports_size);
> +	msg->trace_resp.cpus = htonl(nr_cpus);
> +	msg->trace_resp.page_size = htonl(page_size);
> +
> +	msg->port_array = malloc(ports_size);
> +	if (!msg->port_array)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < nr_cpus; i++)
> +		msg->port_array[i] = htonl(ports[i]);
> +
> +	return 0;
> +}
> +
> +int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
> +				 int nr_cpus, int page_size,
> +				 unsigned int *ports)
> +{
> +	struct tracecmd_msg msg;
> +	int ret;
> +
> +	tracecmd_msg_init(MSG_TRACE_RESP, &msg);
> +	ret = make_trace_resp(&msg, page_size, nr_cpus, ports);
> +	if (ret < 0)
> +		return ret;
> +
> +	return tracecmd_msg_send(msg_handle->fd, &msg);
> +}
> +
> +int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
> +				 int *nr_cpus, int *page_size,
> +				 unsigned int **ports)
> +{
> +	struct tracecmd_msg msg;
> +	ssize_t buf_len;
> +	int i, ret;
> +
> +	ret = tracecmd_msg_recv(msg_handle->fd, &msg);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ntohl(msg.hdr.cmd) != MSG_TRACE_RESP) {
> +		ret = -ENOTSUP;
> +		goto out;
> +	}
> +
> +	buf_len = ntohl(msg.hdr.size) - MSG_HDR_LEN - ntohl(msg.hdr.cmd_size);
> +	if (buf_len <= 0 ||
> +	    buf_len != sizeof(*msg.port_array) * ntohl(msg.trace_resp.cpus)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	*nr_cpus = ntohl(msg.trace_resp.cpus);
> +	*page_size = ntohl(msg.trace_resp.page_size);
> +	*ports = calloc(*nr_cpus, sizeof(**ports));
> +	if (!*ports) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	for (i = 0; i < *nr_cpus; i++)
> +		(*ports)[i] = ntohl(msg.port_array[i]);
> +
> +	msg_free(&msg);
> +	return 0;
> +
> +out:
> +	error_operation(&msg);
> +	if (ret == -EOPNOTSUPP)
> +		handle_unexpected_msg(msg_handle, &msg);
> +	msg_free(&msg);
> +	return ret;
> +}