netdev - Re: [PATCH take 2] pkt_sched: Fix qdisc_watchdog() vs. dev

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: <20080912080249.GA3971@ff.dom.local>
Date:	Fri, 12 Sep 2008 08:02:49 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	herbert@...dor.apana.org.au, netdev@...r.kernel.org,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCH take 2] pkt_sched: Fix qdisc_watchdog() vs.
	dev_deactivate() race

On Thu, Sep 11, 2008 at 09:49:29PM -0700, David Miller wrote:
> From: David Miller <davem@...emloft.net>
> Date: Thu, 11 Sep 2008 04:47:17 -0700 (PDT)
> 
> > Ok, so implementing ->peek() is the first step in dealing
> > with this.
> 
> Ok, here's a first cut at this.
> 
> Most of it is simple and straightforward.
> 
> As usual, though, CBQ, HFSC, and HTB are complicated.

I guess not for Patrick... (so CCed)

> 
> Most of the peek implementations just skb_peek() in their
> downstream queue or iterate over their prio array doing
> the same looking for a non-empty list.
> 
> But CBQ, HFSC, and HTB have complicated class iterators and
> internal time state machine things.  I tried to do my best
> in these cases.
> 
> I didn't want these things firing off class watchdog timers and
> stuff like this.  Just see if there is any packet ready now
> and return it.
> 
> The one exception is that I allow CBQ to advance it's time
> state machine.

Alas I'm still not sure how this whole peek idea is going to be
implemented (dequeuing after peeking doesn't have to give us the
same skb since in the meantime e.g. in HTB some other class with
higher prio can get enough tokens etc., and if there is a break
for transmit in the meantime with possible enqueuing, or we can
deal with something like sch_multiq, which depends on the current
state of the tx_queues, this all looks even more interesting).

So, until there is some example, even in pseudocode, for qdisc_run()
vs. some_classful_sched interaction, I think, I'm not able to give
more feedback now, but mabe only one doubt if this wrapper below is
really needed, since skb_peek() is more readable and it's used
directly in a few places anyway.

> +static inline struct sk_buff *__qdisc_peek_head(struct Qdisc *sch,
> +						struct sk_buff_head *list)
> +{
> +	return skb_peek(list);
> +}

Thanks,
Jarek P.
--
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