Re: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue settings

[Michal Kubecek <mkubecek@suse.cz>]

On Tue, Feb 05, 2019 at 04:01:03PM -0800, Jeff Kirsher wrote:
> +static int do_perqueue(struct cmd_context *ctx)
> +{
> +	__u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)] = {0};
> +	int i, n_queues = 0;
> +
> +	if (ctx->argc == 0)
> +		exit_bad_args();
> +
> +	/*
> +	 * The sub commands will be applied to
> +	 * all queues if no queue_mask set
> +	 */
> +	if (strncmp(*ctx->argp, "queue_mask", 10)) {

This would match any string starting with "queue_mask", is it intended? 

> +		n_queues = find_max_num_queues(ctx);
> +		if (n_queues < 0) {
> +			perror("Cannot get number of queues");
> +			return -EFAULT;
> +		}
> +		for (i = 0; i < n_queues / 32; i++)
> +			queue_mask[i] = ~0;
> +		queue_mask[i] = (1 << (n_queues - i * 32)) - 1;

It's unlikely today, I guess, but in theory, this would overflow if
n_queues == MAX_NUM_QUEUE

> +		fprintf(stdout,
> +			"The sub commands will be applied to all %d queues\n",
> +			n_queues);
> +	} else {
> +		ctx->argc--;
> +		ctx->argp++;
> +		n_queues = set_queue_mask(queue_mask, *ctx->argp);
> +		if (n_queues < 0) {
> +			perror("Invalid queue mask");
> +			return n_queues;
> +		}
> +		ctx->argc--;
> +		ctx->argp++;
> +	}
> +
> +	i = find_option(ctx->argc, ctx->argp);
> +	if (i < 0)
> +		exit_bad_args();
> +
> +	/* no sub_command support yet */
> +
> +	return 0;
> +}

Michal Kubecek