[Intel-wired-lan] [next-queue PATCH 2/3] net/sched: Introduce Credit Based Shaper (CBS) qdisc

Cong Wang xiyou.wangcong at gmail.com
Thu Sep 28 22:34:47 UTC 2017


On Wed, Sep 27, 2017 at 2:14 PM, Vinicius Costa Gomes
<vinicius.gomes at intel.com> wrote:
> Hi,
>
> Cong Wang <xiyou.wangcong at gmail.com> writes:
>
>> On Tue, Sep 26, 2017 at 4:39 PM, Vinicius Costa Gomes
>> <vinicius.gomes at intel.com> wrote:
>>> +static int cbs_init(struct Qdisc *sch, struct nlattr *opt)
>>> +{
>>> +       struct cbs_sched_data *q = qdisc_priv(sch);
>>> +       struct net_device *dev = qdisc_dev(sch);
>>> +
>>> +       if (!opt)
>>> +               return -EINVAL;
>>> +
>>> +       /* FIXME: this means that we can only install this qdisc
>>> +        * "under" mqprio. Do we need a more generic way to retrieve
>>> +        * the queue, or do we pass the netdev_queue to the driver?
>>> +        */
>>> +       q->queue = TC_H_MIN(sch->parent) - 1 - netdev_get_num_tc(dev);
>>> +
>>> +       return cbs_change(sch, opt);
>>> +}
>>
>> Yeah it is ugly to assume its parent is mqprio, at least you should
>> error out if it is not the case.
>
> Will add an error for this, for now.
>
>>
>> I am not sure how we can solve this elegantly, perhaps you should
>> extend mqprio rather than add a new one?
>
> Is the alternative hinted in the FIXME worse? Instead of passing the
> index of the hardware queue to the driver we pass the pointer to a
> netdev_queue to the driver and it "discovers" the HW queue from that.

Does this way solve the dependency on mqprio? If yes then it is good.
And you have to fix it before merge, we don't have any qdisc depending
a specific type of qdisc to be its parent.


More information about the Intel-wired-lan mailing list