Re: [patch iproute2 net-next v11 4/4] tc: implement ingress/egress block index attributes for qdiscs

[David Ahern <dsahern@gmail.com>]

On 1/17/18 2:48 AM, Jiri Pirko wrote:
> diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
> index 3f91558..9f06d37 100644
> --- a/tc/tc_qdisc.c
> +++ b/tc/tc_qdisc.c
> @@ -31,6 +31,7 @@ static int usage(void)
>  	fprintf(stderr, "       [ handle QHANDLE ] [ root | ingress | clsact | parent CLASSID ]\n");
>  	fprintf(stderr, "       [ estimator INTERVAL TIME_CONSTANT ]\n");
>  	fprintf(stderr, "       [ stab [ help | STAB_OPTIONS] ]\n");
> +	fprintf(stderr, "       [ ingress_block BLOCK_INDEX ] [ egress_block BLOCK_INDEX ]\n");
>  	fprintf(stderr, "       [ [ QDISC_KIND ] [ help | OPTIONS ] ]\n");
>  	fprintf(stderr, "\n");
>  	fprintf(stderr, "       tc qdisc show [ dev STRING ] [ ingress | clsact ] [ invisible ]\n");
> @@ -61,6 +62,8 @@ static int tc_qdisc_modify(int cmd, unsigned int flags, int argc, char **argv)
>  		.n.nlmsg_type = cmd,
>  		.t.tcm_family = AF_UNSPEC,
>  	};
> +	__u32 ingress_block = 0;
> +	__u32 egress_block = 0;
>  
>  	while (argc > 0) {
>  		if (strcmp(*argv, "dev") == 0) {
> @@ -121,6 +124,14 @@ static int tc_qdisc_modify(int cmd, unsigned int flags, int argc, char **argv)
>  			if (parse_size_table(&argc, &argv, &stab.szopts) < 0)
>  				return -1;
>  			continue;
> +		} else if (matches(*argv, "ingress_block") == 0) {
> +			NEXT_ARG();
> +			if (get_u32(&ingress_block, *argv, 0) || !ingress_block)
> +				invarg("invalid ingress block index value", *argv);
> +		} else if (matches(*argv, "egress_block") == 0) {
> +			NEXT_ARG();
> +			if (get_u32(&egress_block, *argv, 0) || !egress_block)
> +				invarg("invalid egress block index value", *argv);
>  		} else if (matches(*argv, "help") == 0) {
>  			usage();
>  		} else {
> @@ -138,6 +149,13 @@ static int tc_qdisc_modify(int cmd, unsigned int flags, int argc, char **argv)
>  	if (est.ewma_log)
>  		addattr_l(&req.n, sizeof(req), TCA_RATE, &est, sizeof(est));
>  
> +	if (ingress_block)
> +		addattr32(&req.n, sizeof(req),
> +			  TCA_INGRESS_BLOCK, ingress_block);
> +	if (egress_block)
> +		addattr32(&req.n, sizeof(req),
> +			  TCA_EGRESS_BLOCK, egress_block);
> +
>  	if (q) {
>  		if (q->parse_qopt) {
>  			if (q->parse_qopt(q, argc, argv, &req.n, d))
> @@ -270,6 +288,24 @@ int print_qdisc(const struct sockaddr_nl *who,
>  	    (rta_getattr_u8(tb[TCA_HW_OFFLOAD])))
>  		print_bool(PRINT_ANY, "offloaded", "offloaded ", true);
>  
> +	if (tb[TCA_INGRESS_BLOCK] &&
> +	    RTA_PAYLOAD(tb[TCA_INGRESS_BLOCK]) >= sizeof(__u32)) {

same comment here and below about >=. you expect a u32 so require the
size to be sizeof u32.

> +		__u32 block = rta_getattr_u32(tb[TCA_INGRESS_BLOCK]);
> +
> +		if (block)
> +			print_uint(PRINT_ANY, "ingress_block",
> +				   "ingress_block %u ", block);
> +	}
> +
> +	if (tb[TCA_EGRESS_BLOCK] &&
> +	    RTA_PAYLOAD(tb[TCA_EGRESS_BLOCK]) >= sizeof(__u32)) {

> +		__u32 block = rta_getattr_u32(tb[TCA_EGRESS_BLOCK]);
> +
> +		if (block)
> +			print_uint(PRINT_ANY, "egress_block",
> +				   "egress_block %u ", block);
> +	}
> +
>  	/* pfifo_fast is generic enough to warrant the hardcoding --JHS */
>  	if (strcmp("pfifo_fast", RTA_DATA(tb[TCA_KIND])) == 0)
>  		q = get_qdisc_kind("prio");
>