On Sat, Jun 23, 2018 at 1:47 PM, Nishanth Devarajan <ndev2...@gmail.com> wrote: > diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h > index 37b5096..6fd07e8 100644 > --- a/include/uapi/linux/pkt_sched.h > +++ b/include/uapi/linux/pkt_sched.h ... > +#define SKBPRIO_MAX_PRIORITY 64 > + > +struct tc_skbprio_qopt { > + __u32 limit; /* Queue length in packets. */ > +};
Since this is just an integer, you can just make it NLA_U32 instead of a struct? > +static int skbprio_change(struct Qdisc *sch, struct nlattr *opt, > + struct netlink_ext_ack *extack) > +{ > + struct skbprio_sched_data *q = qdisc_priv(sch); > + struct tc_skbprio_qopt *ctl = nla_data(opt); > + const unsigned int min_limit = 1; > + > + if (ctl->limit == (typeof(ctl->limit))-1) > + q->max_limit = max(qdisc_dev(sch)->tx_queue_len, min_limit); > + else if (ctl->limit < min_limit || > + ctl->limit > qdisc_dev(sch)->tx_queue_len) > + return -EINVAL; > + else > + q->max_limit = ctl->limit; > + > + return 0; > +} Isn't q->max_limit same with sch->limit? Also, please avoid dev->tx_queue_len here, it may change independently of your qdisc change, unless you want to implement ops->change_tx_queue_len().