[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-net
Subject:    Re: list_for_each_entry_rcu in net/core/dev.c
From:       Stephen Hemminger <shemminger () osdl ! org>
Date:       2003-10-27 18:33:36
[Download RAW message or body]

On 25 Oct 2003 12:42:20 +0200
"Michele 'mydecay' Marchetto" <smarchetto1@tin.it> wrote:

> hi all.
> i have noticed that in list_for_each_entry_rcu the checks
> (!ptype-&gt;dev || ptype-&gt;dev == skb-&gt;dev) are made on the current
> packet but the function (delived_skb) is called on the previous one.
> 
> i can't understand why this make sense.. anyone can explain me?
> 
> Moreover i created a patch that removes pt_prev and uses only ptype and
> all seems to work fine..
> 

Do you understand the purpose of pt_prev?

This loop is written so that the original usage count on the socket buffer
is incremented if there are more than one protocol registered for the same
packet type.  The code as it is confusing, (maybe sub-optimal) but correct.

Also in a list_for_each_entry_rcu loop, ptype will always be non-NULL.
So your code is wrong, for the case of two protocols on the same type.

Let's walk through, your code and see why it leaks.

	list_for_each_entry_rcu(ptype, &ptype_all, list) {
						// ptype = first registered protocol
	if (ptype)				// always non-null
		// inline of deliver_skb
		atomic_inc(&skb->users)		// was 1 now 2
		ret = ptype->func(skb, skb->dev, ptype);  // upcall for first protocol.

	list_for_each_entry_rcu(ptype, &ptype_all, list) {
						// ptype = second protocol
	if (ptype)
		atomic_inc(&skb->users)		// was 2 now 3
		ret = ptype->func(skb, skb->dev, ptype);   // upcall for second protocol

 skb still has user count of 1 even after both protocols have processed it so it leaks!
-
To unsubscribe from this list: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic