netdev - Re: [UPDATED] [NET-NEXT PATCH 1/2] pkt_sched: Add multiqueue scheduler support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1220309354.14337.34.camel@ahduyck-laptop>
Date:	Mon, 01 Sep 2008 15:49:14 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Jarek Poplawski <jarkao2@...il.com>
Cc:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>, jeff@...zik.org,
	netdev@...r.kernel.org, davem@...emloft.net,
	Alexander Duyck <alexander.h.duyck@...el.com>
Subject: Re: [UPDATED] [NET-NEXT PATCH 1/2] pkt_sched: Add multiqueue
	scheduler support

On Mon, 2008-09-01 at 23:05 +0200, Jarek Poplawski wrote:

> Mostly looks OK to me, but a few (late) doubts below:

Most of your suggestions I agree with, with the following exceptions.
> ...
> > +static int multiq_tune(struct Qdisc *sch, struct nlattr *opt)
> > +{
> > +	struct multiq_sched_data *q = qdisc_priv(sch);
> > +	struct tc_multiq_qopt *qopt;
> > +	struct Qdisc **queues;
> > +	int i;
> > +
> > +	if (sch->parent != TC_H_ROOT)
> > +		return -EINVAL;
> 
> Is it necessary?
> 
I think so.  Basically I want to have this qdisc as the root for all
other qdiscs because the hardware queue decision needs to be made as
soon as possible in order to avoid any head of line blocking issues.
This way you don't end up with multiple qdiscs fighting over hardware
queues.

> > +static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb)
> > +{
> > +	struct multiq_sched_data *q = qdisc_priv(sch);
> > +	unsigned char *b = skb_tail_pointer(skb);
> > +	struct nlattr *nest;
> > +	struct tc_multiq_qopt opt;
> > +
> > +	opt.bands = q->bands;
> > +
> > +	nest = nla_nest_compat_start(skb, TCA_OPTIONS, sizeof(opt), &opt);
> 
> http://marc.info/?l=linux-netdev&m=121993231608269&w=2

I can dump the whole nested_compat setup and replace it all with an
nla_put, because it is inefficient to waste the space on an empty nested
attribute that isn't needed.  I think this was just a holdover from
prio/rr anyway.

> 
> > +	if (nest == NULL)
> > +		goto nla_put_failure;
> > +	nla_nest_compat_end(skb, nest);
> ...
> 
> Jarek P.

Thanks,

Alex


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html