Subject: Re: [RFC iproute2-next v1 5/5] tc: Add support for configuring the taprio scheduler

From: Vinicius Costa Gomes <[email-masked]>
Date: Tue, 17 Jul 2018 09:44:02 -0700

Hi,

Stephen Hemminger <[email-masked]> writes:

> On Fri, 13 Jul 2018 17:06:11 -0700
> Vinicius Costa Gomes <[email-masked]> wrote:
>
>> +	while (fscanf(f, "%ms %x %" PRIu32 "\n", &cmd_str, &gatemask, &interval) != EOF)  {
>> +		struct rtattr *entry;
>> +
>> +		err = str_to_entry_cmd(cmd_str);
>> +		free(cmd_str);
>> +
>> +		if (err < 0)
>> +			return err;
>> +
>> +		cmd = err;
>> +
>> +		entry = addattr_nest(n, 1024, TCA_TAPRIO_SCHED_ENTRY);
>> +
>> +		addattr_l(n, 1024, TCA_TAPRIO_SCHED_ENTRY_INDEX, &index, sizeof(index));
>> +		addattr_l(n, 1024, TCA_TAPRIO_SCHED_ENTRY_CMD, &cmd, sizeof(cmd));
>> +		addattr_l(n, 1024, TCA_TAPRIO_SCHED_ENTRY_GATE_MASK, &gatemask, sizeof(gatemask));
>> +		addattr_l(n, 1024, TCA_TAPRIO_SCHED_ENTRY_INTERVAL, &interval, sizeof(interval));
>> +
>> +		addattr_nest_end(n, entry);
>> +	}
>> +
>
> Why not just use batch mode? Introducing another input mode in tc that is
> only in one qdisc seems like a bad idea.

Seems that I have missed batch mode. I am going to play with it a little
and see how things would look.

>
>> +
>> +static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
>> +{
>> +	struct rtattr *tb[TCA_TAPRIO_ATTR_MAX + 1];
>> +	struct tc_mqprio_qopt *qopt = 0;
>> +	__s64 cycle_time = 0, extension_time = 0, base_time = 0;
>> +	__s32 clockid = CLOCKID_INVALID;
>> +	__u32 preempt_mask = 0;
>> +	int i;
>> +
>> +	if (opt == NULL)
>> +		return 0;
>> +
>> +	parse_rtattr_nested(tb, TCA_TAPRIO_ATTR_MAX, opt);
>> +
>> +	if (tb[TCA_TAPRIO_ATTR_PRIOMAP] == NULL)
>> +		return -1;
>> +
>> +	qopt = RTA_DATA(tb[TCA_TAPRIO_ATTR_PRIOMAP]);
>> +
>> +	fprintf(f, "tc %u map ", qopt->num_tc);
>> +	for (i = 0; i <= TC_PRIO_MAX; i++)
>> +		fprintf(f, "%u ", qopt->prio_tc_map[i]);
>> +	fprintf(f, "\n	queues:");
>> +	for (i = 0; i < qopt->num_tc; i++)
>> +		fprintf(f, "(%u:%u) ", qopt->offset[i],
>> +			qopt->offset[i] + qopt->count[i] - 1);
>> +
>> +	if (tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME])
>> +		cycle_time = rta_getattr_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
>> +
>> +	if (tb[TCA_TAPRIO_ATTR_SCHED_EXTENSION_TIME])
>> +		extension_time = rta_getattr_s64(tb[TCA_TAPRIO_ATTR_SCHED_EXTENSION_TIME]);
>> +
>> +	if (tb[TCA_TAPRIO_ATTR_SCHED_BASE_TIME])
>> +		base_time = rta_getattr_s64(tb[TCA_TAPRIO_ATTR_SCHED_BASE_TIME]);
>> +
>> +	if (tb[TCA_TAPRIO_ATTR_PREEMPT_MASK])
>> +		preempt_mask = rta_getattr_s64(tb[TCA_TAPRIO_ATTR_PREEMPT_MASK]);
>> +
>> +	if (tb[TCA_TAPRIO_ATTR_SCHED_CLOCKID])
>> +		clockid = rta_getattr_s32(tb[TCA_TAPRIO_ATTR_SCHED_CLOCKID]);
>> +
>> +	fprintf(f, "\n	clockid %s ", get_clock_name(clockid));
>> +
>> +	fprintf(f, "\n	base-time %lld cycle-time %lld extension-time %lld ",
>> +		base_time, cycle_time, extension_time);
>> +
>> +	fprintf(f, "\n	preempt-mask 0x%x ", preempt_mask);
>> +
>> +	return print_sched_list(f, tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST]);
>
>
> Please implement JSON output using json_print functions.

Sure. Will do.

Cheers,