Netdev List
 help / color / mirror / Atom feed
* Re: [patch net-next-2.6 8/8 v2] net: get rid of orig_dev parameter of packet handlers
From: Nicolas de Pesloüan @ 2011-03-05 22:18 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110305220752.GJ8573@psychotron.redhat.com>

Le 05/03/2011 23:07, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 05:56:24PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 16:32, Jiri Pirko a écrit :
>>> Since orig_dev was used in af_packet (where it was replaced) and by
>>> bonding ptype handlers (which are no longer with us) only, orig_dev
>>> parameter is no longer needed ->   toss it out
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>>>
>> [snip]
>>
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 58daddb..c71bd18 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>> [snip]
>>> @@ -3216,13 +3214,13 @@ ncls:
>>>   		    (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
>>>   		     ptype->dev == orig_dev)) {
>>>   			if (pt_prev)
>>> -				ret = deliver_skb(skb, pt_prev, orig_dev);
>>> +				ret = deliver_skb(skb, pt_prev);
>>>   			pt_prev = ptype;
>>>   		}
>>>   	}
>>>
>>>   	if (pt_prev) {
>>> -		ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
>>> +		ret = pt_prev->func(skb, skb->dev, pt_prev);
>>>   	} else {
>>>   		atomic_long_inc(&skb->dev->rx_dropped);
>>>   		kfree_skb(skb);
>>
>> This part does not properly apply on top of current net-next-2.6.
>>
>> Line 3216 is currently
>>
>>                     (ptype->dev == null_or_dev || ptype->dev == skb->dev)) {
>>
>> 	Nicolas.
>
>
> depends on "net: allow handlers to be processed for orig_dev" which is a
> regression fixer
>

You are right. I reviewed this patch. :-)

^ permalink raw reply

* Re: Possible regression in __netif_receive_skb() between 2.6.38-rc7 and net-next-2.6
From: Jiri Pirko @ 2011-03-05 22:09 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: netdev@vger.kernel.org, David Miller, Stephen Hemminger,
	Jay Vosburgh, Patrick Mc Hardy, Eric Dumazet, Andy Gospodarek
In-Reply-To: <4D72AB79.3000605@gmail.com>

Sat, Mar 05, 2011 at 10:30:33PM CET, nicolas.2p.debian@gmail.com wrote:
>Hi,
>
>Comparing __netif_receive_skb() between 2.6.38-rc7 and net-next-2.6,
>I noticed an important difference: The ptype_base loop used to
>deliver to orig_dev and this is not true anymore.

I believe this is adressed by submitted patch " net: allow handlers to
be processed for orig_dev"


>
>[Note that this is unrelated to Jiri's today's patch that remove the
>orig_dev parameter to protocol handler].
>
>Imagine the following simple setup:
>
>eth0 -> bond0
>
>- A packet handler registered on eth0, with ptype->type == NULL will
>receive the packet, because it will be delivered in the ptype_all
>loop, which is inside the another_round loop.
>- The same packet handler, registered on eth0, but with ptype->type
>!= NULL won't receive the packet, because the ptype_base loop doesn't
>deliver to orig_dev anymore.
>
>I think this can lead to a regression for user space: an application
>using af_packet to listen to eth0 will receive the packet flow if the
>registered protocol is NULL, but won't receive anything if the
>registered protocol is not NULL.
>
>Can someone confirm?
>
>	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 8/8 v2] net: get rid of orig_dev parameter of packet handlers
From: Jiri Pirko @ 2011-03-05 22:07 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <4D726B38.4040403@gmail.com>

Sat, Mar 05, 2011 at 05:56:24PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 16:32, Jiri Pirko a écrit :
>>Since orig_dev was used in af_packet (where it was replaced) and by
>>bonding ptype handlers (which are no longer with us) only, orig_dev
>>parameter is no longer needed ->  toss it out
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>>
>[snip]
>
>>diff --git a/net/core/dev.c b/net/core/dev.c
>>index 58daddb..c71bd18 100644
>>--- a/net/core/dev.c
>>+++ b/net/core/dev.c
>[snip]
>>@@ -3216,13 +3214,13 @@ ncls:
>>  		    (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
>>  		     ptype->dev == orig_dev)) {
>>  			if (pt_prev)
>>-				ret = deliver_skb(skb, pt_prev, orig_dev);
>>+				ret = deliver_skb(skb, pt_prev);
>>  			pt_prev = ptype;
>>  		}
>>  	}
>>
>>  	if (pt_prev) {
>>-		ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
>>+		ret = pt_prev->func(skb, skb->dev, pt_prev);
>>  	} else {
>>  		atomic_long_inc(&skb->dev->rx_dropped);
>>  		kfree_skb(skb);
>
>This part does not properly apply on top of current net-next-2.6.
>
>Line 3216 is currently
>
>                    (ptype->dev == null_or_dev || ptype->dev == skb->dev)) {
>
>	Nicolas.


depends on "net: allow handlers to be processed for orig_dev" which is a
regression fixer

^ permalink raw reply

* Possible regression in __netif_receive_skb() between 2.6.38-rc7 and net-next-2.6
From: Nicolas de Pesloüan @ 2011-03-05 21:30 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: Jiri Pirko, David Miller, Stephen Hemminger, Jay Vosburgh,
	Patrick Mc Hardy, Eric Dumazet, Andy Gospodarek

Hi,

Comparing __netif_receive_skb() between 2.6.38-rc7 and net-next-2.6, I noticed an important 
difference: The ptype_base loop used to deliver to orig_dev and this is not true anymore.

[Note that this is unrelated to Jiri's today's patch that remove the orig_dev parameter to protocol 
handler].

Imagine the following simple setup:

eth0 -> bond0

- A packet handler registered on eth0, with ptype->type == NULL will receive the packet, because it 
will be delivered in the ptype_all loop, which is inside the another_round loop.
- The same packet handler, registered on eth0, but with ptype->type != NULL won't receive the 
packet, because the ptype_base loop doesn't deliver to orig_dev anymore.

I think this can lead to a regression for user space: an application using af_packet to listen to 
eth0 will receive the packet flow if the registered protocol is NULL, but won't receive anything if 
the registered protocol is not NULL.

Can someone confirm?

	Nicolas.

^ permalink raw reply

* Re: EPT: Misconfiguration
From: Ruben Kerkhof @ 2011-03-05 18:57 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, netdev
In-Reply-To: <4D6A2B91.5020100@redhat.com>

On Sun, Feb 27, 2011 at 11:46, Avi Kivity <avi@redhat.com> wrote:
>
> Copying netdev: looks like memory corruption in the networking stack.
>
> Archive link: http://www.spinics.net/lists/kvm/msg50651.html (for the
> attachment).

There's now only a single guest running on this host (Ubuntu Maverick).
I've also upgraded the host kernel to 2.6.38-rc6, and this just
happened (after a day or so):

2011-03-05T19:41:58.328866+01:00 phy005 kernel: [85271.656862] BUG
kmalloc-2048 (Not tainted): Object padding overwritten
2011-03-05T19:41:58.328870+01:00 phy005 kernel: [85271.656864]
-----------------------------------------------------------------------------
2011-03-05T19:41:58.328875+01:00 phy005 kernel: [85271.656866]
2011-03-05T19:41:58.328885+01:00 phy005 kernel: [85271.656870] INFO:
0xffff880c0d52a960-0xffff880c0d52a967. First byte 0x0 instead of 0x5a
2011-03-05T19:41:58.328890+01:00 phy005 kernel: [85271.656880] INFO:
Allocated in __netdev_alloc_skb+0x1f/0x3b age=16039 cpu=5 pid=0
2011-03-05T19:41:58.328894+01:00 phy005 kernel: [85271.656886] INFO:
Freed in skb_release_data+0xa5/0xaa age=0 cpu=5 pid=1766
2011-03-05T19:41:58.328898+01:00 phy005 kernel: [85271.656890] INFO:
Slab 0xffffea002a2ea0c0 objects=15 used=13 fp=0xffff880c0d52a120
flags=0xc00000000040c1
2011-03-05T19:41:58.328902+01:00 phy005 kernel: [85271.656894] INFO:
Object 0xffff880c0d52a120 @offset=8480 fp=0xffff880c0d52d2d0
2011-03-05T19:41:58.328905+01:00 phy005 kernel: [85271.656895]
2011-03-05T19:41:58.328909+01:00 phy005 kernel: [85271.656897] Bytes
b4 0xffff880c0d52a110:  14 89 12 05 01 00 00 00 5a 5a 5a 5a 5a 5a 5a
5a ........ZZZZZZZZ
2011-03-05T19:41:58.328913+01:00 phy005 kernel: [85271.656909]
Object 0xffff880c0d52a120:  6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b kkkkkkkkkkkkkkkk

We have a quite complex network stack, two interfaces (igb) attached
to bond0, with on top two bridges and on that two vlans.
The guest is running a vpn and an IPv6 tunnel.

Let me know if more info is needed.

Kind regards,

Ruben

^ permalink raw reply

* Re: [patch net-next-2.6 8/8 v2] net: get rid of orig_dev parameter of packet handlers
From: Nicolas de Pesloüan @ 2011-03-05 16:56 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110305153247.GH8573@psychotron.redhat.com>

Le 05/03/2011 16:32, Jiri Pirko a écrit :
> Since orig_dev was used in af_packet (where it was replaced) and by
> bonding ptype handlers (which are no longer with us) only, orig_dev
> parameter is no longer needed ->  toss it out
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>
[snip]

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 58daddb..c71bd18 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
[snip]
> @@ -3216,13 +3214,13 @@ ncls:
>   		    (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
>   		     ptype->dev == orig_dev)) {
>   			if (pt_prev)
> -				ret = deliver_skb(skb, pt_prev, orig_dev);
> +				ret = deliver_skb(skb, pt_prev);
>   			pt_prev = ptype;
>   		}
>   	}
>
>   	if (pt_prev) {
> -		ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
> +		ret = pt_prev->func(skb, skb->dev, pt_prev);
>   	} else {
>   		atomic_long_inc(&skb->dev->rx_dropped);
>   		kfree_skb(skb);

This part does not properly apply on top of current net-next-2.6.

Line 3216 is currently

                     (ptype->dev == null_or_dev || ptype->dev == skb->dev)) {

	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Changli Gao @ 2011-03-05 16:45 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	nicolas.2p.debian, andy
In-Reply-To: <1299313794-5218-8-git-send-email-jpirko@redhat.com>

On Sat, Mar 5, 2011 at 4:29 PM, Jiri Pirko <jpirko@redhat.com> wrote:
>
> -typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
> +enum rx_handler_result {
> +       RX_HANDLER_CONSUMED,
> +       RX_HANDLER_ANOTHER,
> +       RX_HANDLER_EXACT,
> +       RX_HANDLER_PASS,
> +};

Why not extend the macros NET_RX_SUCCESS and NET_RX_DROP?  Then the
callers of __netif_receive_skb() may get the right congestion info
from its return value.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [patch net-next-2.6 1/8] af_packet: use skb->skb_iif instead of orig_dev->ifindex
From: Changli Gao @ 2011-03-05 16:34 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	nicolas.2p.debian, andy
In-Reply-To: <1299313794-5218-2-git-send-email-jpirko@redhat.com>

On Sat, Mar 5, 2011 at 4:29 PM, Jiri Pirko <jpirko@redhat.com> wrote:
> Since skb_iif has the desired value (ifindex of physical device actually
> received the traffic) use that instead.
>

It isn't true for the packets returned from a ifb pseudo NIC.

f.e.:

eth0 -(redirect)-> ifb0 ->

skb->skb_iff is ifb0->ifindex.
orig_dev = skb->dev = eth0.

However, these packets aren't processed by af_packet, so there isn't
any problem.

Reviewed-by: Changli Gao <xiaosuo@gmail.com>

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] net: mac80211: fix compilation warning
From: Larry Finger @ 2011-03-05 16:30 UTC (permalink / raw)
  To: Jovi Zhang
  Cc: Ben Hutchings, John W. Linville, Johannes Berg, David S. Miller,
	open list:NETWORKING [WIREL..., open list:NETWORKING [GENERAL],
	open list
In-Reply-To: <AANLkTimnSvBKSL6_d3RxufxUL_C9wCM+Fz66_B5Sc0KE@mail.gmail.com>

On 03/05/2011 07:52 AM, Jovi Zhang wrote:
> On Sat, Mar 5, 2011 at 8:31 PM, Ben Hutchings<bhutchings@solarflare.com>  wrote:
>> On Wed, 2011-03-02 at 18:32 -0500, bookjovi@gmail.com wrote:
>>> From: Jovi Zhang<bookjovi@gmail.com>
>>>
>>> this commit fix compilation warning as following:
>>> net/mac80211/tx.c:1753: warning: unused variable mppath
>> [...]
>>
>> You clearly didn't try building this with CONFIG_MAC80211_MESH enabled.
>>
> Sorry, indeed, maybe should be like this:
>
> +#ifdef CONFIG_MAC80211_MESH
>         struct mesh_path *mppath = NULL;
> +#endif

Linville likes "struct mesh_path *mppath __maybe_unused = NULL;" over the ifdef 
form.

Larry

^ permalink raw reply

* Re: [patch net-next-2.6] net: comment rx_handler results
From: Nicolas de Pesloüan @ 2011-03-05 15:50 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <20110305153704.GI8573@psychotron.redhat.com>

Le 05/03/2011 16:37, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 04:27:52PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 16:13, Jiri Pirko a écrit :
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>> ---
>>>   include/linux/netdevice.h |   12 ++++++++----
>>>   1 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index 48a9638..26e03f9 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -391,10 +391,14 @@ enum gro_result {
>>>   typedef enum gro_result gro_result_t;
>>>
>>>   enum rx_handler_result {
>>> -	RX_HANDLER_CONSUMED,
>>> -	RX_HANDLER_ANOTHER,
>>> -	RX_HANDLER_EXACT,
>>> -	RX_HANDLER_PASS,
>>> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>>> +				   do not process it further. */
>>> +	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
>>> +				   This is indicated in case skb->dev
>>> +				   was changed by rx_handler */
>>> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>>> +	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
>>> +				   no rx_handler was called */
>>
>> Changing skb->dev should always cause another_round.
>
> Not if you reinject the skb.
>
>>
>> So, RX_HANDLER_ANOTHER could be removed and replaced with a test for
>> skb->dev change, with RX_HANDER_EXACT and RX_HANDLER_PASS.
>
> That was there before. Needs extra variable and I think this way, via
> results the code is much easier to read.
>
>>
>>> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>>> +				   do not process it further. */
>>> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>>> +	RX_HANDLER_PASS,	/* Do normal (wildcard and exact) delivery */
>>
>> And any rx_handler returning RX_HANDLER_EXACT should cause exact
>> delivery, even if possible next rx_handlers return RX_HANDLER_PASS.
>
> When rx_handler returns RX_HANDLER_EXACT, there will bo no other call to
> any rx_handler (that is the case only when RX_HANDLER_ANOTHER is
> returned)

I agree, but we clearly need a kernel-doc that explain in more detail the expected behavior of every 
return values.

Some examples, showing some normal and some stacking setups, would help, I think. One more follow-up 
patch...

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6] net: comment rx_handler results
From: Jiri Pirko @ 2011-03-05 15:37 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <4D725678.2020706@gmail.com>

Sat, Mar 05, 2011 at 04:27:52PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 16:13, Jiri Pirko a écrit :
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>---
>>  include/linux/netdevice.h |   12 ++++++++----
>>  1 files changed, 8 insertions(+), 4 deletions(-)
>>
>>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>index 48a9638..26e03f9 100644
>>--- a/include/linux/netdevice.h
>>+++ b/include/linux/netdevice.h
>>@@ -391,10 +391,14 @@ enum gro_result {
>>  typedef enum gro_result gro_result_t;
>>
>>  enum rx_handler_result {
>>-	RX_HANDLER_CONSUMED,
>>-	RX_HANDLER_ANOTHER,
>>-	RX_HANDLER_EXACT,
>>-	RX_HANDLER_PASS,
>>+	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>>+				   do not process it further. */
>>+	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
>>+				   This is indicated in case skb->dev
>>+				   was changed by rx_handler */
>>+	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>>+	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
>>+				   no rx_handler was called */
>
>Changing skb->dev should always cause another_round.

Not if you reinject the skb.

>
>So, RX_HANDLER_ANOTHER could be removed and replaced with a test for
>skb->dev change, with RX_HANDER_EXACT and RX_HANDLER_PASS.

That was there before. Needs extra variable and I think this way, via
results the code is much easier to read.

>
>> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>> +				   do not process it further. */
>> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>> +	RX_HANDLER_PASS,	/* Do normal (wildcard and exact) delivery */
>
>And any rx_handler returning RX_HANDLER_EXACT should cause exact
>delivery, even if possible next rx_handlers return RX_HANDLER_PASS.

When rx_handler returns RX_HANDLER_EXACT, there will bo no other call to
any rx_handler (that is the case only when RX_HANDLER_ANOTHER is
returned)
>
>	Nicolas.

^ permalink raw reply

* [patch net-next-2.6 8/8 v2] net: get rid of orig_dev parameter of packet handlers
From: Jiri Pirko @ 2011-03-05 15:32 UTC (permalink / raw)
  To: netdev
  Cc: davem, shemminger, kaber, fubar, eric.dumazet, nicolas.2p.debian,
	andy
In-Reply-To: <1299320969-7951-9-git-send-email-jpirko@redhat.com>

Since orig_dev was used in af_packet (where it was replaced) and by
bonding ptype handlers (which are no longer with us) only, orig_dev
parameter is no longer needed -> toss it out

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

v1->v2:
	moved __copy_skb_header hunk to 7/8

---
 drivers/block/aoe/aoenet.c      |    2 +-
 drivers/net/hamradio/bpqether.c |    4 ++--
 drivers/net/pppoe.c             |    4 ++--
 drivers/net/wan/hdlc.c          |    2 +-
 drivers/net/wan/lapbether.c     |    2 +-
 drivers/scsi/fcoe/fcoe.c        |   12 ++++--------
 include/linux/netdevice.h       |    3 +--
 include/net/ax25.h              |    3 ++-
 include/net/datalink.h          |    2 +-
 include/net/ip.h                |    2 +-
 include/net/ipv6.h              |    3 +--
 include/net/irda/irda.h         |    3 +--
 include/net/llc.h               |    8 +++-----
 include/net/p8022.h             |    3 +--
 include/net/psnap.h             |    3 +--
 include/net/x25.h               |    2 +-
 net/802/p8022.c                 |    3 +--
 net/802/psnap.c                 |    7 +++----
 net/802/stp.c                   |    2 +-
 net/8021q/vlan.h                |    2 +-
 net/8021q/vlan_dev.c            |    2 +-
 net/appletalk/aarp.c            |    2 +-
 net/appletalk/ddp.c             |    6 +++---
 net/ax25/ax25_in.c              |    2 +-
 net/batman-adv/hard-interface.c |    6 ++----
 net/caif/caif_dev.c             |    2 +-
 net/can/af_can.c                |    2 +-
 net/core/dev.c                  |   26 ++++++++++++--------------
 net/decnet/af_decnet.c          |    2 +-
 net/decnet/dn_route.c           |    2 +-
 net/dsa/tag_dsa.c               |    2 +-
 net/dsa/tag_edsa.c              |    2 +-
 net/dsa/tag_trailer.c           |    2 +-
 net/econet/af_econet.c          |    2 +-
 net/ieee802154/af_ieee802154.c  |    2 +-
 net/ipv4/arp.c                  |    2 +-
 net/ipv4/ip_input.c             |    2 +-
 net/ipv4/ipconfig.c             |   13 ++++++++-----
 net/ipv6/ip6_input.c            |    3 ++-
 net/ipx/af_ipx.c                |    3 ++-
 net/irda/irlap_frame.c          |    2 +-
 net/llc/llc_core.c              |    3 +--
 net/llc/llc_input.c             |    8 ++++----
 net/packet/af_packet.c          |    6 +++---
 net/phonet/af_phonet.c          |    3 +--
 net/tipc/eth_media.c            |    2 +-
 net/x25/x25_dev.c               |    2 +-
 47 files changed, 85 insertions(+), 98 deletions(-)

diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
index 4d3bc0d..c12e14b 100644
--- a/drivers/block/aoe/aoenet.c
+++ b/drivers/block/aoe/aoenet.c
@@ -99,7 +99,7 @@ aoenet_xmit(struct sk_buff_head *queue)
  * (1) len doesn't include the header by default.  I want this. 
  */
 static int
-aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt, struct net_device *orig_dev)
+aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt)
 {
 	struct aoe_hdr *h;
 	u32 n;
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index 8931168..f17ca33 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -95,7 +95,7 @@ static char bcast_addr[6]={0xFF,0xFF,0xFF,0xFF,0xFF,0xFF};
 
 static char bpq_eth_addr[6];
 
-static int bpq_rcv(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+static int bpq_rcv(struct sk_buff *, struct net_device *, struct packet_type *);
 static int bpq_device_event(struct notifier_block *, unsigned long, void *);
 
 static struct packet_type bpq_packet_type __read_mostly = {
@@ -177,7 +177,7 @@ static inline int dev_is_ethdev(struct net_device *dev)
 /*
  *	Receive an AX.25 frame via an ethernet interface.
  */
-static int bpq_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype, struct net_device *orig_dev)
+static int bpq_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype)
 {
 	int len;
 	char * ptr;
diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index 78c0e3c..b99cdaa 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -419,7 +419,7 @@ abort_kfree:
  *
  ***********************************************************************/
 static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *pt, struct net_device *orig_dev)
+		     struct packet_type *pt)
 {
 	struct pppoe_hdr *ph;
 	struct pppox_sock *po;
@@ -467,7 +467,7 @@ out:
  *
  ***********************************************************************/
 static int pppoe_disc_rcv(struct sk_buff *skb, struct net_device *dev,
-			  struct packet_type *pt, struct net_device *orig_dev)
+			  struct packet_type *pt)
 
 {
 	struct pppoe_hdr *ph;
diff --git a/drivers/net/wan/hdlc.c b/drivers/net/wan/hdlc.c
index 5d4bb61..0ac4f27 100644
--- a/drivers/net/wan/hdlc.c
+++ b/drivers/net/wan/hdlc.c
@@ -53,7 +53,7 @@ int hdlc_change_mtu(struct net_device *dev, int new_mtu)
 }
 
 static int hdlc_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *p, struct net_device *orig_dev)
+		    struct packet_type *p)
 {
 	struct hdlc_device *hdlc = dev_to_hdlc(dev);
 
diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c
index 7f5bb91..993d71a 100644
--- a/drivers/net/wan/lapbether.c
+++ b/drivers/net/wan/lapbether.c
@@ -86,7 +86,7 @@ static __inline__ int dev_is_ethdev(struct net_device *dev)
 /*
  *	Receive a LAPB frame via an ethernet interface.
  */
-static int lapbeth_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype, struct net_device *orig_dev)
+static int lapbeth_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype)
 {
 	int len, err;
 	struct lapbethdev *lapbeth;
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3becc6a..d7442b6 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -70,7 +70,7 @@ DEFINE_PER_CPU(struct fcoe_percpu_s, fcoe_percpu);
 static int fcoe_reset(struct Scsi_Host *);
 static int fcoe_xmit(struct fc_lport *, struct fc_frame *);
 static int fcoe_rcv(struct sk_buff *, struct net_device *,
-		    struct packet_type *, struct net_device *);
+		    struct packet_type *);
 static int fcoe_percpu_receive_thread(void *);
 static void fcoe_clean_pending_queue(struct fc_lport *);
 static void fcoe_percpu_clean(struct fc_lport *);
@@ -88,7 +88,7 @@ static struct fcoe_interface
 *fcoe_hostlist_lookup_port(const struct net_device *);
 
 static int fcoe_fip_recv(struct sk_buff *, struct net_device *,
-			 struct packet_type *, struct net_device *);
+			 struct packet_type *);
 
 static void fcoe_fip_send(struct fcoe_ctlr *, struct sk_buff *);
 static void fcoe_update_src_mac(struct fc_lport *, u8 *);
@@ -465,14 +465,11 @@ static inline void fcoe_interface_put(struct fcoe_interface *fcoe)
  * @skb:      The receive skb
  * @netdev:   The associated net device
  * @ptype:    The packet_type structure which was used to register this handler
- * @orig_dev: The original net_device the the skb was received on.
- *	      (in case dev is a bond)
  *
  * Returns: 0 for success
  */
 static int fcoe_fip_recv(struct sk_buff *skb, struct net_device *netdev,
-			 struct packet_type *ptype,
-			 struct net_device *orig_dev)
+			 struct packet_type *ptype)
 {
 	struct fcoe_interface *fcoe;
 
@@ -1228,7 +1225,6 @@ static int fcoe_cpu_callback(struct notifier_block *nfb,
  * @skb:    The received packet
  * @netdev: The net device that the packet was received on
  * @ptype:  The packet type context
- * @olddev: The last device net device
  *
  * This routine is called by NET_RX_SOFTIRQ. It receives a packet, builds a
  * FC frame and passes the frame to libfc.
@@ -1236,7 +1232,7 @@ static int fcoe_cpu_callback(struct notifier_block *nfb,
  * Returns: 0 for success
  */
 int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
-	     struct packet_type *ptype, struct net_device *olddev)
+	     struct packet_type *ptype)
 {
 	struct fc_lport *lport;
 	struct fcoe_rcv_info *fr;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ff386a4..48a9638 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1456,8 +1456,7 @@ struct packet_type {
 	struct net_device	*dev;	/* NULL is wildcarded here	     */
 	int			(*func) (struct sk_buff *,
 					 struct net_device *,
-					 struct packet_type *,
-					 struct net_device *);
+					 struct packet_type *);
 	struct sk_buff		*(*gso_segment)(struct sk_buff *skb,
 						u32 features);
 	int			(*gso_send_check)(struct sk_buff *skb);
diff --git a/include/net/ax25.h b/include/net/ax25.h
index 206d222..fcbe6d8 100644
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -362,7 +362,8 @@ extern int  ax25_protocol_is_registered(unsigned int);
 
 /* ax25_in.c */
 extern int  ax25_rx_iframe(ax25_cb *, struct sk_buff *);
-extern int  ax25_kiss_rcv(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+extern int  ax25_kiss_rcv(struct sk_buff *, struct net_device *,
+			  struct packet_type *);
 
 /* ax25_ip.c */
 extern int ax25_hard_header(struct sk_buff *, struct net_device *,
diff --git a/include/net/datalink.h b/include/net/datalink.h
index deb7ca7..5797ba3 100644
--- a/include/net/datalink.h
+++ b/include/net/datalink.h
@@ -9,7 +9,7 @@ struct datalink_proto {
         unsigned short  header_length;
 
         int     (*rcvfunc)(struct sk_buff *, struct net_device *,
-                                struct packet_type *, struct net_device *);
+                                struct packet_type *);
 	int     (*request)(struct datalink_proto *, struct sk_buff *,
                                         unsigned char *);
 	struct list_head node;
diff --git a/include/net/ip.h b/include/net/ip.h
index a4f6311..3b04c24 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -94,7 +94,7 @@ extern int		ip_build_and_send_pkt(struct sk_buff *skb, struct sock *sk,
 					      __be32 saddr, __be32 daddr,
 					      struct ip_options *opt);
 extern int		ip_rcv(struct sk_buff *skb, struct net_device *dev,
-			       struct packet_type *pt, struct net_device *orig_dev);
+			       struct packet_type *pt);
 extern int		ip_local_deliver(struct sk_buff *skb);
 extern int		ip_mr_input(struct sk_buff *skb);
 extern int		ip_output(struct sk_buff *skb);
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index d6d077d..990f90b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -470,8 +470,7 @@ static __inline__ void ipv6_select_ident(struct frag_hdr *fhdr)
 
 extern int			ipv6_rcv(struct sk_buff *skb, 
 					 struct net_device *dev, 
-					 struct packet_type *pt,
-					 struct net_device *orig_dev);
+					 struct packet_type *pt);
 
 extern int			ip6_rcv_finish(struct sk_buff *skb);
 
diff --git a/include/net/irda/irda.h b/include/net/irda/irda.h
index 3bed61d..2c6e25d 100644
--- a/include/net/irda/irda.h
+++ b/include/net/irda/irda.h
@@ -125,7 +125,6 @@ extern int irda_nl_register(void);
 extern void irda_nl_unregister(void);
 
 extern int irlap_driver_rcv(struct sk_buff *skb, struct net_device *dev,
-			    struct packet_type *ptype,
-			    struct net_device *orig_dev);
+			    struct packet_type *ptype);
 
 #endif /* NET_IRDA_H */
diff --git a/include/net/llc.h b/include/net/llc.h
index 5503b74..73bfea5 100644
--- a/include/net/llc.h
+++ b/include/net/llc.h
@@ -58,8 +58,7 @@ struct llc_sap {
 	atomic_t         refcnt;
 	int		 (*rcv_func)(struct sk_buff *skb,
 				     struct net_device *dev,
-				     struct packet_type *pt,
-				     struct net_device *orig_dev);
+				     struct packet_type *pt);
 	struct llc_addr	 laddr;
 	struct list_head node;
 	spinlock_t sk_lock;
@@ -96,7 +95,7 @@ extern struct list_head llc_sap_list;
 extern spinlock_t llc_sap_list_lock;
 
 extern int llc_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev);
+		   struct packet_type *pt);
 
 extern int llc_mac_hdr_init(struct sk_buff *skb,
 			    const unsigned char *sa, const unsigned char *da);
@@ -110,8 +109,7 @@ extern void llc_set_station_handler(void (*handler)(struct sk_buff *skb));
 extern struct llc_sap *llc_sap_open(unsigned char lsap,
 				    int (*rcv)(struct sk_buff *skb,
 					       struct net_device *dev,
-					       struct packet_type *pt,
-					       struct net_device *orig_dev));
+					       struct packet_type *pt));
 static inline void llc_sap_hold(struct llc_sap *sap)
 {
 	atomic_inc(&sap->refcnt);
diff --git a/include/net/p8022.h b/include/net/p8022.h
index 42e9fac..6d878ae 100644
--- a/include/net/p8022.h
+++ b/include/net/p8022.h
@@ -4,8 +4,7 @@ extern struct datalink_proto *
 	register_8022_client(unsigned char type,
 			     int (*func)(struct sk_buff *skb,
 					 struct net_device *dev,
-					 struct packet_type *pt,
-					 struct net_device *orig_dev));
+					 struct packet_type *pt));
 extern void unregister_8022_client(struct datalink_proto *proto);
 
 extern struct datalink_proto *make_8023_client(void);
diff --git a/include/net/psnap.h b/include/net/psnap.h
index fe456c2..604bf4b 100644
--- a/include/net/psnap.h
+++ b/include/net/psnap.h
@@ -4,8 +4,7 @@
 extern struct datalink_proto *
 register_snap_client(const unsigned char *desc,
 		     int (*rcvfunc)(struct sk_buff *, struct net_device *,
-				    struct packet_type *,
-				    struct net_device *orig_dev));
+				    struct packet_type *));
 extern void unregister_snap_client(struct datalink_proto *proto);
 
 #endif
diff --git a/include/net/x25.h b/include/net/x25.h
index a06119a..3f1c0df2 100644
--- a/include/net/x25.h
+++ b/include/net/x25.h
@@ -202,7 +202,7 @@ extern void x25_kill_by_neigh(struct x25_neigh *);
 
 /* x25_dev.c */
 extern void x25_send_frame(struct sk_buff *, struct x25_neigh *);
-extern int  x25_lapb_receive_frame(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+extern int  x25_lapb_receive_frame(struct sk_buff *, struct net_device *, struct packet_type *);
 extern void x25_establish_link(struct x25_neigh *);
 extern void x25_terminate_link(struct x25_neigh *);
 
diff --git a/net/802/p8022.c b/net/802/p8022.c
index 7f353c4..4ab1339 100644
--- a/net/802/p8022.c
+++ b/net/802/p8022.c
@@ -36,8 +36,7 @@ static int p8022_request(struct datalink_proto *dl, struct sk_buff *skb,
 struct datalink_proto *register_8022_client(unsigned char type,
 					    int (*func)(struct sk_buff *skb,
 							struct net_device *dev,
-							struct packet_type *pt,
-							struct net_device *orig_dev))
+							struct packet_type *pt))
 {
 	struct datalink_proto *proto;
 
diff --git a/net/802/psnap.c b/net/802/psnap.c
index 21cde8f..e74b48e 100644
--- a/net/802/psnap.c
+++ b/net/802/psnap.c
@@ -47,7 +47,7 @@ static struct datalink_proto *find_snap_client(const unsigned char *desc)
  *	A SNAP packet has arrived
  */
 static int snap_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	int rc = 1;
 	struct datalink_proto *proto;
@@ -64,7 +64,7 @@ static int snap_rcv(struct sk_buff *skb, struct net_device *dev,
 		/* Pass the frame on. */
 		skb->transport_header += 5;
 		skb_pull_rcsum(skb, 5);
-		rc = proto->rcvfunc(skb, dev, &snap_packet_type, orig_dev);
+		rc = proto->rcvfunc(skb, dev, &snap_packet_type);
 	}
 	rcu_read_unlock();
 
@@ -126,8 +126,7 @@ module_exit(snap_exit);
 struct datalink_proto *register_snap_client(const unsigned char *desc,
 					    int (*rcvfunc)(struct sk_buff *,
 							   struct net_device *,
-							   struct packet_type *,
-							   struct net_device *))
+							   struct packet_type *))
 {
 	struct datalink_proto *proto = NULL;
 
diff --git a/net/802/stp.c b/net/802/stp.c
index 978c30b..dec1f88 100644
--- a/net/802/stp.c
+++ b/net/802/stp.c
@@ -30,7 +30,7 @@ static DEFINE_MUTEX(stp_proto_mutex);
 
 /* Called under rcu_read_lock from LLC */
 static int stp_pdu_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+		       struct packet_type *pt)
 {
 	const struct ethhdr *eh = eth_hdr(skb);
 	const struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 5687c9b..6f8941e 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -76,7 +76,7 @@ static inline struct vlan_dev_info *vlan_dev_info(const struct net_device *dev)
 
 /* found in vlan_dev.c */
 int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
-		  struct packet_type *ptype, struct net_device *orig_dev);
+		  struct packet_type *ptype);
 void vlan_dev_set_ingress_priority(const struct net_device *dev,
 				   u32 skb_prio, u16 vlan_prio);
 int vlan_dev_set_egress_priority(const struct net_device *dev,
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index be73753..6e396b9 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -138,7 +138,7 @@ static inline void vlan_set_encap_proto(struct sk_buff *skb,
  *
  */
 int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
-		  struct packet_type *ptype, struct net_device *orig_dev)
+		  struct packet_type *ptype)
 {
 	struct vlan_hdr *vhdr;
 	struct vlan_pcpu_stats *rx_stats;
diff --git a/net/appletalk/aarp.c b/net/appletalk/aarp.c
index 50dce79..996f366 100644
--- a/net/appletalk/aarp.c
+++ b/net/appletalk/aarp.c
@@ -715,7 +715,7 @@ static void __aarp_resolved(struct aarp_entry **list, struct aarp_entry *a,
  *	frame. We currently only support Ethernet.
  */
 static int aarp_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	struct elapaarp *ea = aarp_hdr(skb);
 	int hash, ret = 0;
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index c410b93..61b4983 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1425,7 +1425,7 @@ drop:
  *	layer.  [ie ARPHRD_ETHERTALK]
  */
 static int atalk_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *pt, struct net_device *orig_dev)
+		     struct packet_type *pt)
 {
 	struct ddpehdr *ddp;
 	struct sock *sock;
@@ -1522,7 +1522,7 @@ out:
  * header and append a long one.
  */
 static int ltalk_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *pt, struct net_device *orig_dev)
+		     struct packet_type *pt)
 {
 	if (!net_eq(dev_net(dev), &init_net))
 		goto freeit;
@@ -1568,7 +1568,7 @@ static int ltalk_rcv(struct sk_buff *skb, struct net_device *dev,
 	}
 	skb_reset_transport_header(skb);
 
-	return atalk_rcv(skb, dev, pt, orig_dev);
+	return atalk_rcv(skb, dev, pt);
 freeit:
 	kfree_skb(skb);
 	return 0;
diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
index 9bb7765..f6f94e6 100644
--- a/net/ax25/ax25_in.c
+++ b/net/ax25/ax25_in.c
@@ -436,7 +436,7 @@ free:
  *	Receive an AX.25 frame via a SLIP interface.
  */
 int ax25_kiss_rcv(struct sk_buff *skb, struct net_device *dev,
-		  struct packet_type *ptype, struct net_device *orig_dev)
+		  struct packet_type *ptype)
 {
 	skb_orphan(skb);
 
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index f2131f4..08579b0 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -37,8 +37,7 @@ static DEFINE_SPINLOCK(if_list_lock);
 
 static int batman_skb_recv(struct sk_buff *skb,
 			   struct net_device *dev,
-			   struct packet_type *ptype,
-			   struct net_device *orig_dev);
+			   struct packet_type *ptype);
 
 static void hardif_free_rcu(struct rcu_head *rcu)
 {
@@ -556,8 +555,7 @@ out:
 /* receive a packet with the batman ethertype coming on a hard
  * interface */
 static int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
-			   struct packet_type *ptype,
-			   struct net_device *orig_dev)
+			   struct packet_type *ptype)
 {
 	struct bat_priv *bat_priv;
 	struct batman_packet *batman_packet;
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index a42a408..9f016f0 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -165,7 +165,7 @@ static int modemcmd(struct cflayer *layr, enum caif_modemcmd ctrl)
  * On error, returns non-zero and releases the skb.
  */
 static int receive(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pkttype, struct net_device *orig_dev)
+		   struct packet_type *pkttype)
 {
 	struct net *net;
 	struct cfpkt *pkt;
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 702be5a..ad32de5 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -616,7 +616,7 @@ static int can_rcv_filter(struct dev_rcv_lists *d, struct sk_buff *skb)
 }
 
 static int can_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev)
+		   struct packet_type *pt)
 {
 	struct dev_rcv_lists *d;
 	struct can_frame *cf = (struct can_frame *)skb->data;
diff --git a/net/core/dev.c b/net/core/dev.c
index 58daddb..c71bd18 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1534,11 +1534,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
 EXPORT_SYMBOL_GPL(dev_forward_skb);
 
 static inline int deliver_skb(struct sk_buff *skb,
-			      struct packet_type *pt_prev,
-			      struct net_device *orig_dev)
+			      struct packet_type *pt_prev)
 {
 	atomic_inc(&skb->users);
-	return pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
+	return pt_prev->func(skb, skb->dev, pt_prev);
 }
 
 /*
@@ -1561,7 +1560,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 		    (ptype->af_packet_priv == NULL ||
 		     (struct sock *)ptype->af_packet_priv != skb->sk)) {
 			if (pt_prev) {
-				deliver_skb(skb2, pt_prev, skb->dev);
+				deliver_skb(skb2, pt_prev);
 				pt_prev = ptype;
 				continue;
 			}
@@ -1594,7 +1593,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 	if (pt_prev)
-		pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
+		pt_prev->func(skb2, skb->dev, pt_prev);
 	rcu_read_unlock();
 }
 
@@ -3025,8 +3024,7 @@ static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
 }
 
 static inline struct sk_buff *handle_ing(struct sk_buff *skb,
-					 struct packet_type **pt_prev,
-					 int *ret, struct net_device *orig_dev)
+					 struct packet_type **pt_prev, int *ret)
 {
 	struct netdev_queue *rxq = rcu_dereference(skb->dev->ingress_queue);
 
@@ -3034,7 +3032,7 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb,
 		goto out;
 
 	if (*pt_prev) {
-		*ret = deliver_skb(skb, *pt_prev, orig_dev);
+		*ret = deliver_skb(skb, *pt_prev);
 		*pt_prev = NULL;
 	}
 
@@ -3160,13 +3158,13 @@ another_round:
 	list_for_each_entry_rcu(ptype, &ptype_all, list) {
 		if (!ptype->dev || ptype->dev == skb->dev) {
 			if (pt_prev)
-				ret = deliver_skb(skb, pt_prev, orig_dev);
+				ret = deliver_skb(skb, pt_prev);
 			pt_prev = ptype;
 		}
 	}
 
 #ifdef CONFIG_NET_CLS_ACT
-	skb = handle_ing(skb, &pt_prev, &ret, orig_dev);
+	skb = handle_ing(skb, &pt_prev, &ret);
 	if (!skb)
 		goto out;
 ncls:
@@ -3175,7 +3173,7 @@ ncls:
 	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (rx_handler) {
 		if (pt_prev) {
-			ret = deliver_skb(skb, pt_prev, orig_dev);
+			ret = deliver_skb(skb, pt_prev);
 			pt_prev = NULL;
 		}
 		switch (rx_handler(&skb)) {
@@ -3194,7 +3192,7 @@ ncls:
 
 	if (vlan_tx_tag_present(skb)) {
 		if (pt_prev) {
-			ret = deliver_skb(skb, pt_prev, orig_dev);
+			ret = deliver_skb(skb, pt_prev);
 			pt_prev = NULL;
 		}
 		if (vlan_hwaccel_do_receive(&skb)) {
@@ -3216,13 +3214,13 @@ ncls:
 		    (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
 		     ptype->dev == orig_dev)) {
 			if (pt_prev)
-				ret = deliver_skb(skb, pt_prev, orig_dev);
+				ret = deliver_skb(skb, pt_prev);
 			pt_prev = ptype;
 		}
 	}
 
 	if (pt_prev) {
-		ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
+		ret = pt_prev->func(skb, skb->dev, pt_prev);
 	} else {
 		atomic_long_inc(&skb->dev->rx_dropped);
 		kfree_skb(skb);
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 2af15b1..1421721 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -2106,7 +2106,7 @@ static struct notifier_block dn_dev_notifier = {
 	.notifier_call = dn_device_event,
 };
 
-extern int dn_route_rcv(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+extern int dn_route_rcv(struct sk_buff *, struct net_device *, struct packet_type *);
 
 static struct packet_type dn_dix_packet_type __read_mostly = {
 	.type =		cpu_to_be16(ETH_P_DNA_RT),
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 484fdbf..20abc79 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -588,7 +588,7 @@ static int dn_route_ptp_hello(struct sk_buff *skb)
 	return NET_RX_SUCCESS;
 }
 
-int dn_route_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+int dn_route_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt)
 {
 	struct dn_skb_cb *cb;
 	unsigned char flags = 0;
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 98dfe80..56b686e 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -77,7 +77,7 @@ out_free:
 }
 
 static int dsa_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev)
+		   struct packet_type *pt)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index 6f38332..474a955 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -90,7 +90,7 @@ out_free:
 }
 
 static int edsa_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index d6d7d0a..f8f36ad 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -67,7 +67,7 @@ netdev_tx_t trailer_xmit(struct sk_buff *skb, struct net_device *dev)
 }
 
 static int trailer_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+		       struct packet_type *pt)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
index 0c28263..edeea09 100644
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -1052,7 +1052,7 @@ release:
  *	Receive an Econet frame from a device.
  */
 
-static int econet_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int econet_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt)
 {
 	struct ec_framehdr *hdr;
 	struct sock *sk = NULL;
diff --git a/net/ieee802154/af_ieee802154.c b/net/ieee802154/af_ieee802154.c
index 6df6ecf..68d5bce 100644
--- a/net/ieee802154/af_ieee802154.c
+++ b/net/ieee802154/af_ieee802154.c
@@ -299,7 +299,7 @@ static const struct net_proto_family ieee802154_family_ops = {
 };
 
 static int ieee802154_rcv(struct sk_buff *skb, struct net_device *dev,
-	struct packet_type *pt, struct net_device *orig_dev)
+			  struct packet_type *pt)
 {
 	if (!netif_running(dev))
 		return -ENODEV;
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index fa9988d..53a8f14 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -974,7 +974,7 @@ static void parp_redo(struct sk_buff *skb)
  */
 
 static int arp_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev)
+		   struct packet_type *pt)
 {
 	struct arphdr *arp;
 
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index d7b2b09..639b69b 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -372,7 +372,7 @@ drop:
 /*
  * 	Main IP Receive routine.
  */
-int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt)
 {
 	struct iphdr *iph;
 	u32 len;
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..5669509 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -438,7 +438,8 @@ static int __init ic_defaults(void)
 
 #ifdef IPCONFIG_RARP
 
-static int ic_rarp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev);
+static int ic_rarp_recv(struct sk_buff *skb, struct net_device *dev,
+			struct packet_type *pt);
 
 static struct packet_type rarp_packet_type __initdata = {
 	.type =	cpu_to_be16(ETH_P_RARP),
@@ -458,8 +459,8 @@ static inline void __init ic_rarp_cleanup(void)
 /*
  *  Process received RARP packet.
  */
-static int __init
-ic_rarp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int __init ic_rarp_recv(struct sk_buff *skb, struct net_device *dev,
+			       struct packet_type *pt)
 {
 	struct arphdr *rarp;
 	unsigned char *rarp_ptr;
@@ -600,7 +601,8 @@ struct bootp_pkt {		/* BOOTP packet format */
 #define DHCPRELEASE	7
 #define DHCPINFORM	8
 
-static int ic_bootp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev);
+static int ic_bootp_recv(struct sk_buff *skb, struct net_device *dev
+			 struct packet_type *pt);
 
 static struct packet_type bootp_packet_type __initdata = {
 	.type =	cpu_to_be16(ETH_P_IP),
@@ -893,7 +895,8 @@ static void __init ic_do_bootp_ext(u8 *ext)
 /*
  *  Receive BOOTP reply.
  */
-static int __init ic_bootp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int __init ic_bootp_recv(struct sk_buff *skb, struct net_device *dev,
+				struct packet_type *pt)
 {
 	struct bootp_pkt *b;
 	struct iphdr *h;
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a83e920..dd72591 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -55,7 +55,8 @@ inline int ip6_rcv_finish( struct sk_buff *skb)
 	return dst_input(skb);
 }
 
-int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+int ipv6_rcv(struct sk_buff *skb, struct net_device *dev,
+	     struct packet_type *pt)
 {
 	struct ipv6hdr *hdr;
 	u32 		pkt_len;
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index da3d21c..614e207 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1665,7 +1665,8 @@ static unsigned int ipx_datagram_poll(struct file *file, struct socket *sock,
 	return rc;
 }
 
-static int ipx_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int ipx_rcv(struct sk_buff *skb, struct net_device *dev,
+		   struct packet_type *pt)
 {
 	/* NULL here for pt means the packet was looped back */
 	struct ipx_interface *intrfc;
diff --git a/net/irda/irlap_frame.c b/net/irda/irlap_frame.c
index 688222c..7850321 100644
--- a/net/irda/irlap_frame.c
+++ b/net/irda/irlap_frame.c
@@ -1306,7 +1306,7 @@ static void irlap_recv_test_frame(struct irlap_cb *self, struct sk_buff *skb,
  * Jean II
  */
 int irlap_driver_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *ptype, struct net_device *orig_dev)
+		     struct packet_type *ptype)
 {
 	struct irlap_info info;
 	struct irlap_cb *self;
diff --git a/net/llc/llc_core.c b/net/llc/llc_core.c
index 2bb0ddf..dfeda38 100644
--- a/net/llc/llc_core.c
+++ b/net/llc/llc_core.c
@@ -91,8 +91,7 @@ struct llc_sap *llc_sap_find(unsigned char sap_value)
 struct llc_sap *llc_sap_open(unsigned char lsap,
 			     int (*func)(struct sk_buff *skb,
 					 struct net_device *dev,
-					 struct packet_type *pt,
-					 struct net_device *orig_dev))
+					 struct packet_type *pt))
 {
 	struct llc_sap *sap = NULL;
 
diff --git a/net/llc/llc_input.c b/net/llc/llc_input.c
index 058f1e9..d884e23 100644
--- a/net/llc/llc_input.c
+++ b/net/llc/llc_input.c
@@ -143,13 +143,13 @@ static inline int llc_fixup_skb(struct sk_buff *skb)
  *	data now), it queues this frame in the connection's backlog.
  */
 int llc_rcv(struct sk_buff *skb, struct net_device *dev,
-	    struct packet_type *pt, struct net_device *orig_dev)
+	    struct packet_type *pt)
 {
 	struct llc_sap *sap;
 	struct llc_pdu_sn *pdu;
 	int dest;
 	int (*rcv)(struct sk_buff *, struct net_device *,
-		   struct packet_type *, struct net_device *);
+		   struct packet_type *);
 
 	if (!net_eq(dev_net(dev), &init_net))
 		goto drop;
@@ -184,14 +184,14 @@ int llc_rcv(struct sk_buff *skb, struct net_device *dev,
 	dest = llc_pdu_type(skb);
 	if (unlikely(!dest || !llc_type_handlers[dest - 1])) {
 		if (rcv)
-			rcv(skb, dev, pt, orig_dev);
+			rcv(skb, dev, pt);
 		else
 			kfree_skb(skb);
 	} else {
 		if (rcv) {
 			struct sk_buff *cskb = skb_clone(skb, GFP_ATOMIC);
 			if (cskb)
-				rcv(cskb, dev, pt, orig_dev);
+				rcv(cskb, dev, pt);
 		}
 		llc_type_handlers[dest - 1](sap, skb);
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index b34294e..85e2493 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -350,7 +350,7 @@ static const struct proto_ops packet_ops;
 static const struct proto_ops packet_ops_spkt;
 
 static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
-			   struct packet_type *pt, struct net_device *orig_dev)
+			   struct packet_type *pt)
 {
 	struct sock *sk;
 	struct sockaddr_pkt *spkt;
@@ -557,7 +557,7 @@ static inline unsigned int run_filter(const struct sk_buff *skb,
  */
 
 static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
-		      struct packet_type *pt, struct net_device *orig_dev)
+		      struct packet_type *pt)
 {
 	struct sock *sk;
 	struct sockaddr_ll *sll;
@@ -667,7 +667,7 @@ drop:
 }
 
 static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+		       struct packet_type *pt)
 {
 	struct sock *sk;
 	struct packet_sock *po;
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 30cc676..2fb5bf2 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -372,8 +372,7 @@ static int send_reset_indications(struct sk_buff *rskb)
  * On error, returns non-zero and releases the skb.
  */
 static int phonet_rcv(struct sk_buff *skb, struct net_device *dev,
-			struct packet_type *pkttype,
-			struct net_device *orig_dev)
+			struct packet_type *pkttype)
 {
 	struct net *net = dev_net(dev);
 	struct phonethdr *ph;
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index b69092e..21c8fe4 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -100,7 +100,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
  */
 
 static int recv_msg(struct sk_buff *buf, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	struct eth_bearer *eb_ptr = (struct eth_bearer *)pt->af_packet_priv;
 
diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
index 9005f6d..4b049ec 100644
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -92,7 +92,7 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
 }
 
 int x25_lapb_receive_frame(struct sk_buff *skb, struct net_device *dev,
-			   struct packet_type *ptype, struct net_device *orig_dev)
+			   struct packet_type *ptype)
 {
 	struct sk_buff *nskb;
 	struct x25_neigh *nb;
-- 
1.7.4


^ permalink raw reply related

* [patch net-next-2.6 7/8 v2] net: introduce rx_handler results and logic around that
From: Jiri Pirko @ 2011-03-05 15:31 UTC (permalink / raw)
  To: netdev
  Cc: davem, shemminger, kaber, fubar, eric.dumazet, nicolas.2p.debian,
	andy
In-Reply-To: <1299320969-7951-8-git-send-email-jpirko@redhat.com>

This patch allows rx_handlers to better signalize what to do next to
it's caller. That makes skb->deliver_no_wcard no longer needed.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

v1->v2:
	moved __copy_skb_header hunk from 8/8 here

---
 drivers/net/bonding/bond_main.c |   20 +++++++++++---------
 drivers/net/macvlan.c           |   11 ++++++-----
 include/linux/netdevice.h       |    9 ++++++++-
 include/linux/skbuff.h          |    5 +----
 net/bridge/br_input.c           |   25 +++++++++++++++----------
 net/bridge/br_private.h         |    2 +-
 net/core/dev.c                  |   19 ++++++++++++-------
 net/core/skbuff.c               |    1 -
 8 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8ba7faa..ab56190 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1483,20 +1483,23 @@ static bool bond_should_deliver_exact_match(struct sk_buff *skb,
 	return false;
 }
 
-static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
+static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
 {
+	struct sk_buff *skb = *pskb;
 	struct slave *slave;
 	struct net_device *bond_dev;
 	struct bonding *bond;
 
-	skb = skb_share_check(skb, GFP_ATOMIC);
-	if (unlikely(!skb))
-		return NULL;
-
 	slave = bond_slave_get_rcu(skb->dev);
 	bond_dev = ACCESS_ONCE(slave->dev->master);
 	if (unlikely(!bond_dev))
-		return skb;
+		return RX_HANDLER_PASS;
+
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (unlikely(!skb))
+		return RX_HANDLER_CONSUMED;
+
+	*pskb = skb;
 
 	bond = netdev_priv(bond_dev);
 
@@ -1513,8 +1516,7 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
 	}
 
 	if (bond_should_deliver_exact_match(skb, slave, bond)) {
-		skb->deliver_no_wcard = 1;
-		return skb;
+		return RX_HANDLER_EXACT;
 	}
 
 	skb->dev = bond_dev;
@@ -1527,7 +1529,7 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
 		memcpy(dest, bond_dev->dev_addr, ETH_ALEN);
 	}
 
-	return skb;
+	return RX_HANDLER_ANOTHER;
 }
 
 /* enslave device <slave> to bond device <master> */
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 6ed577b..ead9a8f 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -152,9 +152,10 @@ static void macvlan_broadcast(struct sk_buff *skb,
 }
 
 /* called under rcu_read_lock() from netif_receive_skb */
-static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
+static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
 {
 	struct macvlan_port *port;
+	struct sk_buff *skb = *pskb;
 	const struct ethhdr *eth = eth_hdr(skb);
 	const struct macvlan_dev *vlan;
 	const struct macvlan_dev *src;
@@ -184,7 +185,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 			 */
 			macvlan_broadcast(skb, port, src->dev,
 					  MACVLAN_MODE_VEPA);
-		return skb;
+		return RX_HANDLER_PASS;
 	}
 
 	if (port->passthru)
@@ -192,12 +193,12 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 	else
 		vlan = macvlan_hash_lookup(port, eth->h_dest);
 	if (vlan == NULL)
-		return skb;
+		return RX_HANDLER_PASS;
 
 	dev = vlan->dev;
 	if (unlikely(!(dev->flags & IFF_UP))) {
 		kfree_skb(skb);
-		return NULL;
+		return RX_HANDLER_CONSUMED;
 	}
 	len = skb->len + ETH_HLEN;
 	skb = skb_share_check(skb, GFP_ATOMIC);
@@ -211,7 +212,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 
 out:
 	macvlan_count_rx(vlan, len, ret == NET_RX_SUCCESS, 0);
-	return NULL;
+	return RX_HANDLER_CONSUMED;
 }
 
 static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8be4056..ff386a4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -390,7 +390,14 @@ enum gro_result {
 };
 typedef enum gro_result gro_result_t;
 
-typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
+enum rx_handler_result {
+	RX_HANDLER_CONSUMED,
+	RX_HANDLER_ANOTHER,
+	RX_HANDLER_EXACT,
+	RX_HANDLER_PASS,
+};
+typedef enum rx_handler_result rx_handler_result_t;
+typedef rx_handler_result_t rx_handler_func_t(struct sk_buff **pskb);
 
 extern void __napi_schedule(struct napi_struct *n);
 
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 31f02d0..24cfa62 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -388,10 +388,7 @@ struct sk_buff {
 	kmemcheck_bitfield_begin(flags2);
 	__u16			queue_mapping:16;
 #ifdef CONFIG_IPV6_NDISC_NODETYPE
-	__u8			ndisc_nodetype:2,
-				deliver_no_wcard:1;
-#else
-	__u8			deliver_no_wcard:1;
+	__u8			ndisc_nodetype:2;
 #endif
 	__u8			ooo_okay:1;
 	kmemcheck_bitfield_end(flags2);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 88e4aa9..e216079 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -139,21 +139,22 @@ static inline int is_link_local(const unsigned char *dest)
  * Return NULL if skb is handled
  * note: already called with rcu_read_lock
  */
-struct sk_buff *br_handle_frame(struct sk_buff *skb)
+rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 {
 	struct net_bridge_port *p;
+	struct sk_buff *skb = *pskb;
 	const unsigned char *dest = eth_hdr(skb)->h_dest;
 	br_should_route_hook_t *rhook;
 
 	if (unlikely(skb->pkt_type == PACKET_LOOPBACK))
-		return skb;
+		return RX_HANDLER_PASS;
 
 	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
 		goto drop;
 
 	skb = skb_share_check(skb, GFP_ATOMIC);
 	if (!skb)
-		return NULL;
+		return RX_HANDLER_CONSUMED;
 
 	p = br_port_get_rcu(skb->dev);
 
@@ -167,10 +168,12 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb)
 			goto forward;
 
 		if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
-			    NULL, br_handle_local_finish))
-			return NULL;	/* frame consumed by filter */
-		else
-			return skb;	/* continue processing */
+			    NULL, br_handle_local_finish)) {
+			return RX_HANDLER_CONSUMED; /* consumed by filter */
+		} else {
+			*pskb = skb;
+			return RX_HANDLER_PASS;	/* continue processing */
+		}
 	}
 
 forward:
@@ -178,8 +181,10 @@ forward:
 	case BR_STATE_FORWARDING:
 		rhook = rcu_dereference(br_should_route_hook);
 		if (rhook) {
-			if ((*rhook)(skb))
-				return skb;
+			if ((*rhook)(skb)) {
+				*pskb = skb;
+				return RX_HANDLER_PASS;
+			}
 			dest = eth_hdr(skb)->h_dest;
 		}
 		/* fall through */
@@ -194,5 +199,5 @@ forward:
 drop:
 		kfree_skb(skb);
 	}
-	return NULL;
+	return RX_HANDLER_CONSUMED;
 }
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f7afc36..19e2f46 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -379,7 +379,7 @@ extern void br_features_recompute(struct net_bridge *br);
 
 /* br_input.c */
 extern int br_handle_frame_finish(struct sk_buff *skb);
-extern struct sk_buff *br_handle_frame(struct sk_buff *skb);
+extern rx_handler_result_t br_handle_frame(struct sk_buff **pskb);
 
 /* br_ioctl.c */
 extern int br_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
diff --git a/net/core/dev.c b/net/core/dev.c
index 9f66de9..58daddb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3121,6 +3121,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
 	rx_handler_func_t *rx_handler;
 	struct net_device *orig_dev;
 	struct net_device *null_or_dev;
+	bool deliver_exact = false;
 	int ret = NET_RX_DROP;
 	__be16 type;
 
@@ -3173,18 +3174,22 @@ ncls:
 
 	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (rx_handler) {
-		struct net_device *prev_dev;
-
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);
 			pt_prev = NULL;
 		}
-		prev_dev = skb->dev;
-		skb = rx_handler(skb);
-		if (!skb)
+		switch (rx_handler(&skb)) {
+		case RX_HANDLER_CONSUMED:
 			goto out;
-		if (skb->dev != prev_dev)
+		case RX_HANDLER_ANOTHER:
 			goto another_round;
+		case RX_HANDLER_EXACT:
+			deliver_exact = true;
+		case RX_HANDLER_PASS:
+			break;
+		default:
+			BUG();
+		}
 	}
 
 	if (vlan_tx_tag_present(skb)) {
@@ -3202,7 +3207,7 @@ ncls:
 	vlan_on_bond_hook(skb);
 
 	/* deliver only exact match when indicated */
-	null_or_dev = skb->deliver_no_wcard ? skb->dev : NULL;
+	null_or_dev = deliver_exact ? skb->dev : NULL;
 
 	type = skb->protocol;
 	list_for_each_entry_rcu(ptype,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 1eb526a..801dd08 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -523,7 +523,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	new->ip_summed		= old->ip_summed;
 	skb_copy_queue_mapping(new, old);
 	new->priority		= old->priority;
-	new->deliver_no_wcard	= old->deliver_no_wcard;
 #if defined(CONFIG_IP_VS) || defined(CONFIG_IP_VS_MODULE)
 	new->ipvs_property	= old->ipvs_property;
 #endif
-- 
1.7.4


^ permalink raw reply related

* Re: [patch net-next-2.6] net: comment rx_handler results
From: Nicolas de Pesloüan @ 2011-03-05 15:27 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <20110305151331.GE8573@psychotron.redhat.com>

Le 05/03/2011 16:13, Jiri Pirko a écrit :
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
> ---
>   include/linux/netdevice.h |   12 ++++++++----
>   1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 48a9638..26e03f9 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -391,10 +391,14 @@ enum gro_result {
>   typedef enum gro_result gro_result_t;
>
>   enum rx_handler_result {
> -	RX_HANDLER_CONSUMED,
> -	RX_HANDLER_ANOTHER,
> -	RX_HANDLER_EXACT,
> -	RX_HANDLER_PASS,
> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
> +				   do not process it further. */
> +	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
> +				   This is indicated in case skb->dev
> +				   was changed by rx_handler */
> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
> +	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
> +				   no rx_handler was called */

Changing skb->dev should always cause another_round.

So, RX_HANDLER_ANOTHER could be removed and replaced with a test for skb->dev change, with 
RX_HANDER_EXACT and RX_HANDLER_PASS.

 > +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
 > +				   do not process it further. */
 > +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
 > +	RX_HANDLER_PASS,	/* Do normal (wildcard and exact) delivery */

And any rx_handler returning RX_HANDLER_EXACT should cause exact delivery, even if possible next 
rx_handlers return RX_HANDLER_PASS.

	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 4/8] bonding: wrap slave state work
From: Nicolas de Pesloüan @ 2011-03-05 15:21 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <1299320969-7951-5-git-send-email-jpirko@redhat.com>

Le 05/03/2011 11:29, Jiri Pirko a écrit :
> transfers slave->state into slave->backup (that it's going to transfer
> into bitfield. Introduce wrapper inlines to do the work with it.
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
> ---
>   drivers/net/bonding/bond_3ad.c   |    2 +-
>   drivers/net/bonding/bond_main.c  |   36 ++++++++++++++++++------------------
>   drivers/net/bonding/bond_sysfs.c |    2 +-
>   drivers/net/bonding/bonding.h    |   31 ++++++++++++++++++++++++++-----
>   4 files changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index 1024ae1..047af0b 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -246,7 +246,7 @@ static inline void __enable_port(struct port *port)
>    */
>   static inline int __port_is_enabled(struct port *port)
>   {
> -	return port->slave->state == BOND_STATE_ACTIVE;
> +	return bond_is_active_slave(port->slave);
>   }
>
>   /**
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7923184..62020a7 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1872,7 +1872,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>   		break;
>   	case BOND_MODE_TLB:
>   	case BOND_MODE_ALB:
> -		new_slave->state = BOND_STATE_ACTIVE;
> +		bond_set_active_slave(new_slave);
>   		bond_set_slave_inactive_flags(new_slave);
>   		bond_select_active_slave(bond);
>   		break;
> @@ -1880,7 +1880,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>   		pr_debug("This slave is always active in trunk mode\n");
>
>   		/* always active in trunk mode */
> -		new_slave->state = BOND_STATE_ACTIVE;
> +		bond_set_active_slave(new_slave);
>
>   		/* In trunking mode there is little meaning to curr_active_slave
>   		 * anyway (it holds no special properties of the bond device),
> @@ -1918,7 +1918,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>
>   	pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
>   		bond_dev->name, slave_dev->name,
> -		new_slave->state == BOND_STATE_ACTIVE ? "n active" : " backup",
> +		bond_is_active_slave(new_slave) ? "n active" : " backup",
 >   		new_slave->link != BOND_LINK_DOWN ? "n up" : " down");

I would prefer the following, for the benefit of non native English readers:

    	pr_info("%s: enslaving %s as %s interface with %s link.\n",
  		bond_is_active_slave(new_slave) ? "an active" : "a backup",
    		new_slave->link != BOND_LINK_DOWN ? "an up" : "a down");

It can be in a follow-up patch.

>
>   	/* enslave is successful */
> @@ -2016,7 +2016,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>
>   	pr_info("%s: releasing %s interface %s\n",
>   		bond_dev->name,
> -		(slave->state == BOND_STATE_ACTIVE) ? "active" : "backup",
> +		bond_is_active_slave(slave) ? "active" : "backup",
>   		slave_dev->name);
>
>   	oldcurrent = bond->curr_active_slave;
> @@ -2357,7 +2357,7 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
>   			res = 0;
>   			strcpy(info->slave_name, slave->dev->name);
>   			info->link = slave->link;
> -			info->state = slave->state;
> +			info->state = bond_slave_state(slave);
>   			info->link_failure_count = slave->link_failure_count;
>   			break;
>   		}
> @@ -2396,7 +2396,7 @@ static int bond_miimon_inspect(struct bonding *bond)
>   					bond->dev->name,
>   					(bond->params.mode ==
>   					 BOND_MODE_ACTIVEBACKUP) ?
> -					((slave->state == BOND_STATE_ACTIVE) ?
> +					(bond_is_active_slave(slave) ?
>   					 "active " : "backup ") : "",
>   					slave->dev->name,
>   					bond->params.downdelay * bond->params.miimon);
> @@ -2487,13 +2487,13 @@ static void bond_miimon_commit(struct bonding *bond)
>
>   			if (bond->params.mode == BOND_MODE_8023AD) {
>   				/* prevent it from being the active one */
> -				slave->state = BOND_STATE_BACKUP;
> +				bond_set_backup_slave(slave);
>   			} else if (bond->params.mode != BOND_MODE_ACTIVEBACKUP) {
>   				/* make it immediately active */
> -				slave->state = BOND_STATE_ACTIVE;
> +				bond_set_active_slave(slave);
>   			} else if (slave != bond->primary_slave) {
>   				/* prevent it from being the active one */
> -				slave->state = BOND_STATE_BACKUP;
> +				bond_set_backup_slave(slave);
>   			}
>
>   			bond_update_speed_duplex(slave);
> @@ -2871,7 +2871,7 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>   	memcpy(&tip, arp_ptr, 4);
>
>   	pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
> -		 bond->dev->name, slave->dev->name, slave->state,
> +		 bond->dev->name, slave->dev->name, bond_slave_state(slave),
>   		 bond->params.arp_validate, slave_do_arp_validate(bond, slave),
>   		&sip,&tip);
>
> @@ -2883,7 +2883,7 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>   	 * the active, through one switch, the router, then the other
>   	 * switch before reaching the backup.
>   	 */
> -	if (slave->state == BOND_STATE_ACTIVE)
> +	if (bond_is_active_slave(slave))
>   		bond_validate_arp(bond, slave, sip, tip);
>   	else
>   		bond_validate_arp(bond, slave, tip, sip);
> @@ -2945,7 +2945,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
>   				slave->dev->last_rx + delta_in_ticks)) {
>
>   				slave->link  = BOND_LINK_UP;
> -				slave->state = BOND_STATE_ACTIVE;
> +				bond_set_active_slave(slave);
>
>   				/* primary_slave has no meaning in round-robin
>   				 * mode. the window of a slave being up and
> @@ -2978,7 +2978,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
>   				slave->dev->last_rx + 2 * delta_in_ticks)) {
>
>   				slave->link  = BOND_LINK_DOWN;
> -				slave->state = BOND_STATE_BACKUP;
> +				bond_set_backup_slave(slave);
>
>   				if (slave->link_failure_count<  UINT_MAX)
>   					slave->link_failure_count++;
> @@ -3072,7 +3072,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
>   		 * gives each slave a chance to tx/rx traffic
>   		 * before being taken out
>   		 */
> -		if (slave->state == BOND_STATE_BACKUP&&
> +		if (!bond_is_active_slave(slave)&&
>   		!bond->current_arp_slave&&
>   		!time_in_range(jiffies,
>   			slave_last_rx(bond, slave) - delta_in_ticks,
> @@ -3089,7 +3089,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
>   		 *    the bond has an IP address)
>   		 */
>   		trans_start = dev_trans_start(slave->dev);
> -		if ((slave->state == BOND_STATE_ACTIVE)&&
> +		if (bond_is_active_slave(slave)&&
>   		(!time_in_range(jiffies,
>   			trans_start - delta_in_ticks,
>   			trans_start + 2 * delta_in_ticks) ||
> @@ -4446,7 +4446,7 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
>   	bond_for_each_slave_from(bond, slave, i, start_at) {
>   		if (IS_UP(slave->dev)&&
>   		(slave->link == BOND_LINK_UP)&&
> -		    (slave->state == BOND_STATE_ACTIVE)) {
> +		    bond_is_active_slave(slave)) {
>   			res = bond_dev_queue_xmit(bond, skb, slave->dev);
>   			break;
>   		}
> @@ -4523,7 +4523,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
>   	bond_for_each_slave_from(bond, slave, i, start_at) {
>   		if (IS_UP(slave->dev)&&
>   		(slave->link == BOND_LINK_UP)&&
> -		    (slave->state == BOND_STATE_ACTIVE)) {
> +		    bond_is_active_slave(slave)) {
>   			res = bond_dev_queue_xmit(bond, skb, slave->dev);
>   			break;
>   		}
> @@ -4564,7 +4564,7 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
>   	bond_for_each_slave_from(bond, slave, i, start_at) {
>   		if (IS_UP(slave->dev)&&
>   		(slave->link == BOND_LINK_UP)&&
> -		    (slave->state == BOND_STATE_ACTIVE)) {
> +		    bond_is_active_slave(slave)) {
>   			if (tx_dev) {
>   				struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
>   				if (!skb2) {
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 05e0ae5..344d23f 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -1579,7 +1579,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
>   	}
>
>   	bond_for_each_slave(bond, slave, i) {
> -		if (slave->state == BOND_STATE_BACKUP) {
> +		if (!bond_is_active_slave(slave)) {
>   			if (new_value)
>   				slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
>   			else
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index ddee62f..8a3718b 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -53,7 +53,7 @@
>   		    (((slave)->dev->flags&  IFF_UP)&&  \
>   		     netif_running((slave)->dev)&&  \
>   		     ((slave)->link == BOND_LINK_UP)&&  \
> -		     ((slave)->state == BOND_STATE_ACTIVE))
> +		     bond_is_active_slave(slave))
>
>
>   #define USES_PRIMARY(mode)				\
> @@ -190,7 +190,8 @@ struct slave {
>   	unsigned long last_arp_rx;
>   	s8     link;    /* one of BOND_LINK_XXXX */
>   	s8     new_link;
> -	s8     state;   /* one of BOND_STATE_XXXX */
> +	u8     backup;	/* indicates backup slave. Value corresponds with
> +			   BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
>   	u32    original_mtu;
>   	u32    link_failure_count;
>   	u8     perm_hwaddr[ETH_ALEN];
> @@ -302,6 +303,26 @@ static inline bool bond_is_lb(const struct bonding *bond)
>   		bond->params.mode == BOND_MODE_ALB);
>   }
>
> +static inline void bond_set_active_slave(struct slave *slave)
> +{
> +	slave->backup = 0;

In the comment above, you said that the possible value for backup corresponds with BOND_STATE_ACTIVE 
and BOND_STATE_BACKUP.

So, should be:

slave->backup = BOND_STATE_ACTIVE;

> +}
> +
> +static inline void bond_set_backup_slave(struct slave *slave)
> +{
> +	slave->backup = 1;

slave->backup = BOND_STATE_BACKUP;

> +}
> +
> +static inline int bond_slave_state(struct slave *slave)
> +{
> +	return slave->backup;
> +}
> +
> +static inline bool bond_is_active_slave(struct slave *slave)
> +{
> +	return !bond_slave_state(slave);
> +}
> +
>   #define BOND_PRI_RESELECT_ALWAYS	0
>   #define BOND_PRI_RESELECT_BETTER	1
>   #define BOND_PRI_RESELECT_FAILURE	2
> @@ -319,7 +340,7 @@ static inline bool bond_is_lb(const struct bonding *bond)
>   static inline int slave_do_arp_validate(struct bonding *bond,
>   					struct slave *slave)
>   {
> -	return bond->params.arp_validate&  (1<<  slave->state);
> +	return bond->params.arp_validate&  (1<<  bond_slave_state(slave));
>   }
>
>   static inline unsigned long slave_last_rx(struct bonding *bond,
> @@ -351,14 +372,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave)
>   {
>   	struct bonding *bond = netdev_priv(slave->dev->master);
>   	if (!bond_is_lb(bond))
> -		slave->state = BOND_STATE_BACKUP;
> +		bond_set_backup_slave(slave);
>   	if (!bond->params.all_slaves_active)
>   		slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
>   }
>
>   static inline void bond_set_slave_active_flags(struct slave *slave)
>   {
> -	slave->state = BOND_STATE_ACTIVE;
> +	bond_set_active_slave(slave);
>   	slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
>   }
>


^ permalink raw reply

* Re: [patch net-next-2.6 8/8] net: get rid of orig_dev parameter of packet handlers
From: Jiri Pirko @ 2011-03-05 15:15 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <4D72512C.2060802@gmail.com>

Sat, Mar 05, 2011 at 04:05:16PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>Since orig_dev was used in af_packet (where it was replaced) and by
>>bonding ptype handlers (which are no longer with us) only, orig_dev
>>parameter is no longer needed ->  toss it out
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>
>[snip]
>>-	new->deliver_no_wcard	= old->deliver_no_wcard;
>
>deliver_no_wcard was removed from sk_buff in your #7 patch. Shouldn't
>the above line be part of the #7 patch?

Indeed, I wonder how if got here. Will repost to correct this.
>
>Except from this point,
>
>Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>
>	Nicolas.

^ permalink raw reply

* [patch net-next-2.6] net: comment rx_handler results
From: Jiri Pirko @ 2011-03-05 15:13 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	nicolas.2p.debian, andy
In-Reply-To: <1299329330.4277.97.camel@localhost>


Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 include/linux/netdevice.h |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 48a9638..26e03f9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -391,10 +391,14 @@ enum gro_result {
 typedef enum gro_result gro_result_t;
 
 enum rx_handler_result {
-	RX_HANDLER_CONSUMED,
-	RX_HANDLER_ANOTHER,
-	RX_HANDLER_EXACT,
-	RX_HANDLER_PASS,
+	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
+				   do not process it further. */
+	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
+				   This is indicated in case skb->dev
+				   was changed by rx_handler */
+	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
+	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
+				   no rx_handler was called */
 };
 typedef enum rx_handler_result rx_handler_result_t;
 typedef rx_handler_result_t rx_handler_func_t(struct sk_buff **pskb);
-- 
1.7.4


^ permalink raw reply related

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Nicolas de Pesloüan @ 2011-03-05 15:06 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <20110305145408.GD8573@psychotron.redhat.com>

Le 05/03/2011 15:54, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 03:52:19PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 13:48, Ben Hutchings a écrit :
>>> On Sat, 2011-03-05 at 11:29 +0100, Jiri Pirko wrote:
>>>> This patch allows rx_handlers to better signalize what to do next to
>>>> it's caller. That makes skb->deliver_no_wcard no longer needed.
>>> [...]
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -390,7 +390,14 @@ enum gro_result {
>>>>   };
>>>>   typedef enum gro_result gro_result_t;
>>>>
>>>> -typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
>>>> +enum rx_handler_result {
>>>> +	RX_HANDLER_CONSUMED,
>>>> +	RX_HANDLER_ANOTHER,
>>>> +	RX_HANDLER_EXACT,
>>>> +	RX_HANDLER_PASS,
>>>> +};
>>> [...]
>>>
>>> This should have a comment (preferably kernel-doc) clearly specifying
>>> the meaning of each code, as the differences between ANOTHER/EXACT/PASS
>>> are fairly subtle.
>>>
>>> Ben,
>>
>> Except from the lack of proper documentation, this patch looks very good.
>
> Okay guys, I'll write something about that and send it in another patch.

Agreed that it can be in a follow-up patch.

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

^ permalink raw reply

* Re: [patch net-next-2.6 8/8] net: get rid of orig_dev parameter of packet handlers
From: Nicolas de Pesloüan @ 2011-03-05 15:05 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <1299320969-7951-9-git-send-email-jpirko@redhat.com>

Le 05/03/2011 11:29, Jiri Pirko a écrit :
> Since orig_dev was used in af_packet (where it was replaced) and by
> bonding ptype handlers (which are no longer with us) only, orig_dev
> parameter is no longer needed ->  toss it out
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>

[snip]
> -	new->deliver_no_wcard	= old->deliver_no_wcard;

deliver_no_wcard was removed from sk_buff in your #7 patch. Shouldn't the above line be part of the 
#7 patch?

Except from this point,

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Jiri Pirko @ 2011-03-05 14:54 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <4D724E23.7020904@gmail.com>

Sat, Mar 05, 2011 at 03:52:19PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 13:48, Ben Hutchings a écrit :
>>On Sat, 2011-03-05 at 11:29 +0100, Jiri Pirko wrote:
>>>This patch allows rx_handlers to better signalize what to do next to
>>>it's caller. That makes skb->deliver_no_wcard no longer needed.
>>[...]
>>>--- a/include/linux/netdevice.h
>>>+++ b/include/linux/netdevice.h
>>>@@ -390,7 +390,14 @@ enum gro_result {
>>>  };
>>>  typedef enum gro_result gro_result_t;
>>>
>>>-typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
>>>+enum rx_handler_result {
>>>+	RX_HANDLER_CONSUMED,
>>>+	RX_HANDLER_ANOTHER,
>>>+	RX_HANDLER_EXACT,
>>>+	RX_HANDLER_PASS,
>>>+};
>>[...]
>>
>>This should have a comment (preferably kernel-doc) clearly specifying
>>the meaning of each code, as the differences between ANOTHER/EXACT/PASS
>>are fairly subtle.
>>
>>Ben,
>
>Except from the lack of proper documentation, this patch looks very good.

Okay guys, I'll write something about that and send it in another patch.

>
>	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Nicolas de Pesloüan @ 2011-03-05 14:52 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <1299329330.4277.97.camel@localhost>

Le 05/03/2011 13:48, Ben Hutchings a écrit :
> On Sat, 2011-03-05 at 11:29 +0100, Jiri Pirko wrote:
>> This patch allows rx_handlers to better signalize what to do next to
>> it's caller. That makes skb->deliver_no_wcard no longer needed.
> [...]
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -390,7 +390,14 @@ enum gro_result {
>>   };
>>   typedef enum gro_result gro_result_t;
>>
>> -typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
>> +enum rx_handler_result {
>> +	RX_HANDLER_CONSUMED,
>> +	RX_HANDLER_ANOTHER,
>> +	RX_HANDLER_EXACT,
>> +	RX_HANDLER_PASS,
>> +};
> [...]
>
> This should have a comment (preferably kernel-doc) clearly specifying
> the meaning of each code, as the differences between ANOTHER/EXACT/PASS
> are fairly subtle.
>
> Ben,

Except from the lack of proper documentation, this patch looks very good.

	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 6/8] bonding: move processing of recv handlers into handle_frame()
From: Nicolas de Pesloüan @ 2011-03-05 14:50 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110305144314.GC8573@psychotron.redhat.com>

Le 05/03/2011 15:43, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 03:33:30PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>> Since now when bonding uses rx_handler, all traffic going into bond
>>> device goes thru bond_handle_frame. So there's no need to go back into
>>> bonding code later via ptype handlers. This patch converts
>>> original ptype handlers into "bonding receive probes". These functions
>>> are called from bond_handle_frame and they are registered per-mode.
>>
>> Does this still support having the arp_ip_target on a vlan?
>>
>> (eth0 ->  bond0 ->  bond0.100, with arp_ip_target only reachable through bond0.100).
>
> This case is still covered with vlan_on_bond_hook
> eth0->
> 	bond_handle_frame
> bond0->
> 	vlan_hwaccel_do_receive
> bond0.5->
> 	vlan_on_bond_hook ->  reinject into bond0
> 				->  bond_handle_frame (here it is processed)

Sound good to me.

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

^ permalink raw reply

* Re: [patch net-next-2.6 3/8] net: get rid of multiple bond-related netdevice->priv_flags
From: Nicolas de Pesloüan @ 2011-03-05 14:46 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	andy
In-Reply-To: <1299335862.4277.99.camel@localhost>

Le 05/03/2011 15:37, Ben Hutchings a écrit :
> On Sat, 2011-03-05 at 15:14 +0100, Nicolas de Pesloüan wrote:
>> Le 05/03/2011 11:29, Jiri Pirko a écrit :
> [...]
>>> --- a/include/linux/if.h
>>> +++ b/include/linux/if.h
>>> @@ -60,21 +60,17 @@
>>>    #define IFF_802_1Q_VLAN 0x1             /* 802.1Q VLAN device.          */
>>>    #define IFF_EBRIDGE	0x2		/* Ethernet bridging device.	*/
>>>    #define IFF_SLAVE_INACTIVE	0x4	/* bonding slave not the curr. active */
>>> -#define IFF_MASTER_8023AD	0x8	/* bonding master, 802.3ad. 	*/
>>> -#define IFF_MASTER_ALB	0x10		/* bonding master, balance-alb.	*/
>>> -#define IFF_BONDING	0x20		/* bonding master or slave	*/
>>> -#define IFF_SLAVE_NEEDARP 0x40		/* need ARPs for validation	*/
>>> -#define IFF_ISATAP	0x80		/* ISATAP interface (RFC4214)	*/
>>> -#define IFF_MASTER_ARPMON 0x100		/* bonding master, ARP mon in use */
>>> -#define IFF_WAN_HDLC	0x200		/* WAN HDLC device		*/
>>> -#define IFF_XMIT_DST_RELEASE 0x400	/* dev_hard_start_xmit() is allowed to
>>> +#define IFF_BONDING	0x8		/* bonding master or slave	*/
>>> +#define IFF_ISATAP	0x10		/* ISATAP interface (RFC4214)	*/
>>> +#define IFF_WAN_HDLC	0x20		/* WAN HDLC device		*/
>>> +#define IFF_XMIT_DST_RELEASE 0x40	/* dev_hard_start_xmit() is allowed to
>>>    					 * release skb->dst
>>>    					 */
>>
>> Why did you changed those values? Aren't those exposed to userland? Just removing the unused one
>> sounds good to me.
> [...]
>
> Look just one line further up:
>
> /* Private (from user) interface flags (netdevice->priv_flags). */
>
> Maybe they should have a prefix IFPF_ instead of IFF_ though.
>
> Ben.

Yes, I noticed it while reviewing the next one. Sorry for the noise.

So,

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>



^ permalink raw reply

* Re: [patch net-next-2.6 6/8] bonding: move processing of recv handlers into handle_frame()
From: Jiri Pirko @ 2011-03-05 14:43 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <4D7249BA.8030401@gmail.com>

Sat, Mar 05, 2011 at 03:33:30PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>Since now when bonding uses rx_handler, all traffic going into bond
>>device goes thru bond_handle_frame. So there's no need to go back into
>>bonding code later via ptype handlers. This patch converts
>>original ptype handlers into "bonding receive probes". These functions
>>are called from bond_handle_frame and they are registered per-mode.
>
>Does this still support having the arp_ip_target on a vlan?
>
>(eth0 -> bond0 -> bond0.100, with arp_ip_target only reachable through bond0.100).

This case is still covered with vlan_on_bond_hook
eth0->
	bond_handle_frame
bond0->
	vlan_hwaccel_do_receive
bond0.5->
	vlan_on_bond_hook -> reinject into bond0
				-> bond_handle_frame (here it is processed)

>
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>---
>>  drivers/net/bonding/bond_3ad.c   |   29 ++--------
>>  drivers/net/bonding/bond_3ad.h   |    4 +-
>>  drivers/net/bonding/bond_alb.c   |   46 ++++-----------
>>  drivers/net/bonding/bond_alb.h   |    1 -
>>  drivers/net/bonding/bond_main.c  |  121 +++++++------------------------------
>>  drivers/net/bonding/bond_sysfs.c |    6 --
>>  drivers/net/bonding/bonding.h    |    4 +-
>>  7 files changed, 43 insertions(+), 168 deletions(-)
>>
>>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>index 047af0b..194aaf7 100644
>>--- a/drivers/net/bonding/bond_3ad.c
>>+++ b/drivers/net/bonding/bond_3ad.c
>>@@ -2461,35 +2461,16 @@ out:
>>  	return NETDEV_TX_OK;
>>  }
>>
>>-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev)
>>+void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
>>+			  struct slave *slave)
>>  {
>>-	struct bonding *bond = netdev_priv(dev);
>>-	struct slave *slave = NULL;
>>-	int ret = NET_RX_DROP;
>>-
>>-	if (!(dev->flags&  IFF_MASTER))
>>-		goto out;
>>-
>>-	skb = skb_share_check(skb, GFP_ATOMIC);
>>-	if (!skb)
>>-		goto out;
>>+	if (skb->protocol != PKT_TYPE_LACPDU)
>>+		return;
>>
>>  	if (!pskb_may_pull(skb, sizeof(struct lacpdu)))
>>-		goto out;
>>+		return;
>>
>>  	read_lock(&bond->lock);
>>-	slave = bond_get_slave_by_dev(netdev_priv(dev), orig_dev);
>>-	if (!slave)
>>-		goto out_unlock;
>>-
>>  	bond_3ad_rx_indication((struct lacpdu *) skb->data, slave, skb->len);
>>-
>>-	ret = NET_RX_SUCCESS;
>>-
>>-out_unlock:
>>  	read_unlock(&bond->lock);
>>-out:
>>-	dev_kfree_skb(skb);
>>-
>>-	return ret;
>>  }
>>diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
>>index 2c46a154..56a88ff 100644
>>--- a/drivers/net/bonding/bond_3ad.h
>>+++ b/drivers/net/bonding/bond_3ad.h
>>@@ -258,7 +258,6 @@ struct ad_bond_info {
>>  				 * requested
>>  				 */
>>  	struct timer_list ad_timer;
>>-	struct packet_type ad_pkt_type;
>>  };
>>
>>  struct ad_slave_info {
>>@@ -279,7 +278,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave);
>>  void bond_3ad_handle_link_change(struct slave *slave, char link);
>>  int  bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
>>  int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
>>-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev);
>>+void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
>>+			  struct slave *slave);
>>  int bond_3ad_set_carrier(struct bonding *bond);
>>  #endif //__BOND_3AD_H__
>>
>>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>>index 9bc5de3..96d28f9 100644
>>--- a/drivers/net/bonding/bond_alb.c
>>+++ b/drivers/net/bonding/bond_alb.c
>>@@ -308,49 +308,33 @@ static void rlb_update_entry_from_arp(struct bonding *bond, struct arp_pkt *arp)
>>  	_unlock_rx_hashtbl(bond);
>>  }
>>
>>-static int rlb_arp_recv(struct sk_buff *skb, struct net_device *bond_dev, struct packet_type *ptype, struct net_device *orig_dev)
>>+static void rlb_arp_recv(struct sk_buff *skb, struct bonding *bond,
>>+			 struct slave *slave)
>>  {
>>-	struct bonding *bond;
>>-	struct arp_pkt *arp = (struct arp_pkt *)skb->data;
>>-	int res = NET_RX_DROP;
>>+	struct arp_pkt *arp;
>>
>>-	while (bond_dev->priv_flags&  IFF_802_1Q_VLAN)
>>-		bond_dev = vlan_dev_real_dev(bond_dev);
>>-
>>-	if (!(bond_dev->priv_flags&  IFF_BONDING) ||
>>-	    !(bond_dev->flags&  IFF_MASTER))
>>-		goto out;
>>+	if (skb->protocol != cpu_to_be16(ETH_P_ARP))
>>+		return;
>>
>>+	arp = (struct arp_pkt *) skb->data;
>>  	if (!arp) {
>>  		pr_debug("Packet has no ARP data\n");
>>-		goto out;
>>+		return;
>>  	}
>>
>>-	skb = skb_share_check(skb, GFP_ATOMIC);
>>-	if (!skb)
>>-		goto out;
>>-
>>-	if (!pskb_may_pull(skb, arp_hdr_len(bond_dev)))
>>-		goto out;
>>+	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
>>+		return;
>>
>>  	if (skb->len<  sizeof(struct arp_pkt)) {
>>  		pr_debug("Packet is too small to be an ARP\n");
>>-		goto out;
>>+		return;
>>  	}
>>
>>  	if (arp->op_code == htons(ARPOP_REPLY)) {
>>  		/* update rx hash table for this ARP */
>>-		bond = netdev_priv(bond_dev);
>>  		rlb_update_entry_from_arp(bond, arp);
>>  		pr_debug("Server received an ARP Reply from client\n");
>>  	}
>>-
>>-	res = NET_RX_SUCCESS;
>>-
>>-out:
>>-	dev_kfree_skb(skb);
>>-
>>-	return res;
>>  }
>>
>>  /* Caller must hold bond lock for read */
>>@@ -759,7 +743,6 @@ static void rlb_init_table_entry(struct rlb_client_info *entry)
>>  static int rlb_initialize(struct bonding *bond)
>>  {
>>  	struct alb_bond_info *bond_info =&(BOND_ALB_INFO(bond));
>>-	struct packet_type *pk_type =&(BOND_ALB_INFO(bond).rlb_pkt_type);
>>  	struct rlb_client_info	*new_hashtbl;
>>  	int size = RLB_HASH_TABLE_SIZE * sizeof(struct rlb_client_info);
>>  	int i;
>>@@ -784,13 +767,8 @@ static int rlb_initialize(struct bonding *bond)
>>
>>  	_unlock_rx_hashtbl(bond);
>>
>>-	/*initialize packet type*/
>>-	pk_type->type = cpu_to_be16(ETH_P_ARP);
>>-	pk_type->dev = bond->dev;
>>-	pk_type->func = rlb_arp_recv;
>>-
>>  	/* register to receive ARPs */
>>-	dev_add_pack(pk_type);
>>+	bond->recv_probe = rlb_arp_recv;
>>
>>  	return 0;
>>  }
>>@@ -799,8 +777,6 @@ static void rlb_deinitialize(struct bonding *bond)
>>  {
>>  	struct alb_bond_info *bond_info =&(BOND_ALB_INFO(bond));
>>
>>-	dev_remove_pack(&(bond_info->rlb_pkt_type));
>>-
>>  	_lock_rx_hashtbl(bond);
>>
>>  	kfree(bond_info->rx_hashtbl);
>>diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
>>index 118c28a..6605e9e 100644
>>--- a/drivers/net/bonding/bond_alb.h
>>+++ b/drivers/net/bonding/bond_alb.h
>>@@ -130,7 +130,6 @@ struct alb_bond_info {
>>  	int			lp_counter;
>>  	/* -------- rlb parameters -------- */
>>  	int rlb_enabled;
>>-	struct packet_type	rlb_pkt_type;
>>  	struct rlb_client_info	*rx_hashtbl;	/* Receive hash table */
>>  	spinlock_t		rx_hashtbl_lock;
>>  	u32			rx_hashtbl_head;
>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>index dbe182c..8ba7faa 100644
>>--- a/drivers/net/bonding/bond_main.c
>>+++ b/drivers/net/bonding/bond_main.c
>>@@ -1467,27 +1467,17 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
>>  }
>>
>>  /* On bonding slaves other than the currently active slave, suppress
>>- * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and
>>- * ARP on active-backup slaves with arp_validate enabled.
>>+ * duplicates except for alb non-mcast/bcast.
>>   */
>>  static bool bond_should_deliver_exact_match(struct sk_buff *skb,
>>  					    struct slave *slave,
>>  					    struct bonding *bond)
>>  {
>>  	if (bond_is_slave_inactive(slave)) {
>>-		if (slave_do_arp_validate(bond, slave)&&
>>-		    skb->protocol == __cpu_to_be16(ETH_P_ARP))
>>-			return false;
>>-
>>  		if (bond->params.mode == BOND_MODE_ALB&&
>>  		skb->pkt_type != PACKET_BROADCAST&&
>>  		skb->pkt_type != PACKET_MULTICAST)
>>-				return false;
>>-
>>-		if (bond->params.mode == BOND_MODE_8023AD&&
>>-		    skb->protocol == __cpu_to_be16(ETH_P_SLOW))
>>  			return false;
>>-
>>  		return true;
>>  	}
>>  	return false;
>>@@ -1513,6 +1503,15 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>>  	if (bond->params.arp_interval)
>>  		slave->dev->last_rx = jiffies;
>>
>>+	if (bond->recv_probe) {
>>+		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
>>+
>>+		if (likely(nskb)) {
>>+			bond->recv_probe(nskb, bond, slave);
>>+			dev_kfree_skb(nskb);
>>+		}
>>+	}
>>+
>>  	if (bond_should_deliver_exact_match(skb, slave, bond)) {
>>  		skb->deliver_no_wcard = 1;
>>  		return skb;
>>@@ -2813,48 +2812,26 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
>>  	}
>>  }
>>
>>-static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
>>+static void bond_arp_rcv(struct sk_buff *skb, struct bonding *bond,
>>+			 struct slave *slave)
>>  {
>>  	struct arphdr *arp;
>>-	struct slave *slave;
>>-	struct bonding *bond;
>>  	unsigned char *arp_ptr;
>>  	__be32 sip, tip;
>>
>>-	if (dev->priv_flags&  IFF_802_1Q_VLAN) {
>>-		/*
>>-		 * When using VLANS and bonding, dev and oriv_dev may be
>>-		 * incorrect if the physical interface supports VLAN
>>-		 * acceleration.  With this change ARP validation now
>>-		 * works for hosts only reachable on the VLAN interface.
>>-		 */
>>-		dev = vlan_dev_real_dev(dev);
>>-		orig_dev = dev_get_by_index_rcu(dev_net(skb->dev),skb->skb_iif);
>>-	}
>>-
>>-	if (!(dev->priv_flags&  IFF_BONDING) || !(dev->flags&  IFF_MASTER))
>>-		goto out;
>>+	if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
>>+		return;
>
>What happens if the skb that hold the ARP request/reply is vlan
>tagged? The ARP ptype_base handler for bonding used to be called
>after vlan processing.
>
>>-	bond = netdev_priv(dev);
>>  	read_lock(&bond->lock);
>>
>>-	pr_debug("bond_arp_rcv: bond %s skb->dev %s orig_dev %s\n",
>>-		 bond->dev->name, skb->dev ? skb->dev->name : "NULL",
>>-		 orig_dev ? orig_dev->name : "NULL");
>>+	pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
>>+		 bond->dev->name, skb->dev->name);
>>
>>-	slave = bond_get_slave_by_dev(bond, orig_dev);
>>-	if (!slave || !slave_do_arp_validate(bond, slave))
>>-		goto out_unlock;
>>-
>>-	skb = skb_share_check(skb, GFP_ATOMIC);
>>-	if (!skb)
>>-		goto out_unlock;
>>-
>>-	if (!pskb_may_pull(skb, arp_hdr_len(dev)))
>>+	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
>>  		goto out_unlock;
>>
>>  	arp = arp_hdr(skb);
>>-	if (arp->ar_hln != dev->addr_len ||
>>+	if (arp->ar_hln != bond->dev->addr_len ||
>>  	    skb->pkt_type == PACKET_OTHERHOST ||
>>  	    skb->pkt_type == PACKET_LOOPBACK ||
>>  	    arp->ar_hrd != htons(ARPHRD_ETHER) ||
>>@@ -2863,9 +2840,9 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>>  		goto out_unlock;
>>
>>  	arp_ptr = (unsigned char *)(arp + 1);
>>-	arp_ptr += dev->addr_len;
>>+	arp_ptr += bond->dev->addr_len;
>>  	memcpy(&sip, arp_ptr, 4);
>>-	arp_ptr += 4 + dev->addr_len;
>>+	arp_ptr += 4 + bond->dev->addr_len;
>>  	memcpy(&tip, arp_ptr, 4);
>>
>>  	pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
>>@@ -2888,9 +2865,6 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>>
>>  out_unlock:
>>  	read_unlock(&bond->lock);
>>-out:
>>-	dev_kfree_skb(skb);
>>-	return NET_RX_SUCCESS;
>>  }
>>
>>  /*
>>@@ -3782,48 +3756,6 @@ static struct notifier_block bond_inetaddr_notifier = {
>>  	.notifier_call = bond_inetaddr_event,
>>  };
>>
>>-/*-------------------------- Packet type handling ---------------------------*/
>>-
>>-/* register to receive lacpdus on a bond */
>>-static void bond_register_lacpdu(struct bonding *bond)
>>-{
>>-	struct packet_type *pk_type =&(BOND_AD_INFO(bond).ad_pkt_type);
>>-
>>-	/* initialize packet type */
>>-	pk_type->type = PKT_TYPE_LACPDU;
>>-	pk_type->dev = bond->dev;
>>-	pk_type->func = bond_3ad_lacpdu_recv;
>>-
>>-	dev_add_pack(pk_type);
>>-}
>>-
>>-/* unregister to receive lacpdus on a bond */
>>-static void bond_unregister_lacpdu(struct bonding *bond)
>>-{
>>-	dev_remove_pack(&(BOND_AD_INFO(bond).ad_pkt_type));
>>-}
>>-
>>-void bond_register_arp(struct bonding *bond)
>>-{
>>-	struct packet_type *pt =&bond->arp_mon_pt;
>>-
>>-	if (pt->type)
>>-		return;
>>-
>>-	pt->type = htons(ETH_P_ARP);
>>-	pt->dev = bond->dev;
>>-	pt->func = bond_arp_rcv;
>>-	dev_add_pack(pt);
>>-}
>>-
>>-void bond_unregister_arp(struct bonding *bond)
>>-{
>>-	struct packet_type *pt =&bond->arp_mon_pt;
>>-
>>-	dev_remove_pack(pt);
>>-	pt->type = 0;
>>-}
>>-
>>  /*---------------------------- Hashing Policies -----------------------------*/
>>
>>  /*
>>@@ -3917,14 +3849,14 @@ static int bond_open(struct net_device *bond_dev)
>>
>>  		queue_delayed_work(bond->wq,&bond->arp_work, 0);
>>  		if (bond->params.arp_validate)
>>-			bond_register_arp(bond);
>>+			bond->recv_probe = bond_arp_rcv;
>>  	}
>>
>>  	if (bond->params.mode == BOND_MODE_8023AD) {
>>  		INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
>>  		queue_delayed_work(bond->wq,&bond->ad_work, 0);
>>  		/* register to receive LACPDUs */
>>-		bond_register_lacpdu(bond);
>>+		bond->recv_probe = bond_3ad_lacpdu_recv;
>>  		bond_3ad_initiate_agg_selection(bond, 1);
>>  	}
>>
>>@@ -3935,14 +3867,6 @@ static int bond_close(struct net_device *bond_dev)
>>  {
>>  	struct bonding *bond = netdev_priv(bond_dev);
>>
>>-	if (bond->params.mode == BOND_MODE_8023AD) {
>>-		/* Unregister the receive of LACPDUs */
>>-		bond_unregister_lacpdu(bond);
>>-	}
>>-
>>-	if (bond->params.arp_validate)
>>-		bond_unregister_arp(bond);
>>-
>>  	write_lock_bh(&bond->lock);
>>
>>  	bond->send_grat_arp = 0;
>>@@ -3982,6 +3906,7 @@ static int bond_close(struct net_device *bond_dev)
>>  		 */
>>  		bond_alb_deinitialize(bond);
>>  	}
>>+	bond->recv_probe = NULL;
>>
>>  	return 0;
>>  }
>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>index 5161183..6804efe 100644
>>--- a/drivers/net/bonding/bond_sysfs.c
>>+++ b/drivers/net/bonding/bond_sysfs.c
>>@@ -419,11 +419,6 @@ static ssize_t bonding_store_arp_validate(struct device *d,
>>  		bond->dev->name, arp_validate_tbl[new_value].modename,
>>  		new_value);
>>
>>-	if (!bond->params.arp_validate&&  new_value)
>>-		bond_register_arp(bond);
>>-	else if (bond->params.arp_validate&&  !new_value)
>>-		bond_unregister_arp(bond);
>>-
>>  	bond->params.arp_validate = new_value;
>>
>>  	return count;
>>@@ -998,7 +993,6 @@ static ssize_t bonding_store_miimon(struct device *d,
>>  				bond->dev->name);
>>  			bond->params.arp_interval = 0;
>>  			if (bond->params.arp_validate) {
>>-				bond_unregister_arp(bond);
>>  				bond->params.arp_validate =
>>  					BOND_ARP_VALIDATE_NONE;
>>  			}
>>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>index 8f78166..75a1308 100644
>>--- a/drivers/net/bonding/bonding.h
>>+++ b/drivers/net/bonding/bonding.h
>>@@ -228,6 +228,8 @@ struct bonding {
>>  	struct   slave *primary_slave;
>>  	bool     force_primary;
>>  	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
>>+	void     (*recv_probe)(struct sk_buff *, struct bonding *,
>>+			       struct slave *);
>>  	rwlock_t lock;
>>  	rwlock_t curr_slave_lock;
>>  	s8       kill_timers;
>>@@ -406,8 +408,6 @@ void bond_set_mode_ops(struct bonding *bond, int mode);
>>  int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl);
>>  void bond_select_active_slave(struct bonding *bond);
>>  void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
>>-void bond_register_arp(struct bonding *);
>>-void bond_unregister_arp(struct bonding *);
>>  void bond_create_debugfs(void);
>>  void bond_destroy_debugfs(void);
>>  void bond_debug_register(struct bonding *bond);
>

^ permalink raw reply

* Re: [patch net-next-2.6 2/8] bonding: register slave pointer for rx_handler
From: Nicolas de Pesloüan @ 2011-03-05 14:38 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110305142732.GB8573@psychotron.redhat.com>

Le 05/03/2011 15:27, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 03:06:58PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>> Register slave pointer as rx_handler data. That would eventually prevent
>>> need to loop over slave devices to find the right slave.
>>>
>>> Use synchronize_net to ensure that bond_handle_frame does not get slave
>>> structure freed when working with that.
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>> ---
>>>   drivers/net/bonding/bond_main.c |   17 +++++++++++------
>>>   drivers/net/bonding/bonding.h   |    3 +++
>>>   2 files changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> index 0592e6d..1c19368 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -1495,21 +1495,22 @@ static bool bond_should_deliver_exact_match(struct sk_buff *skb,
>>>
>>>   static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>>>   {
>>> -	struct net_device *slave_dev;
>>> +	struct slave *slave;
>>>   	struct net_device *bond_dev;
>>>
>>>   	skb = skb_share_check(skb, GFP_ATOMIC);
>>>   	if (unlikely(!skb))
>>>   		return NULL;
>>> -	slave_dev = skb->dev;
>>> -	bond_dev = ACCESS_ONCE(slave_dev->master);
>>> +
>>> +	slave = bond_slave_get_rcu(skb->dev);
>>> +	bond_dev = ACCESS_ONCE(slave->dev->master);
>>>   	if (unlikely(!bond_dev))
>>>   		return skb;
>>>
>>>   	if (bond_dev->priv_flags&   IFF_MASTER_ARPMON)
>>> -		slave_dev->last_rx = jiffies;
>>> +		slave->dev->last_rx = jiffies;
>>>
>>> -	if (bond_should_deliver_exact_match(skb, slave_dev, bond_dev)) {
>>> +	if (bond_should_deliver_exact_match(skb, slave->dev, bond_dev)) {
>>>   		skb->deliver_no_wcard = 1;
>>>   		return skb;
>>>   	}
>>
>> Up to this point, it looks like cleanup, and unrelated to the title and description of the patch.
>
> Nope - these are changes connected to the ability to get slave struct
> via bond_slave_get_rcu. Very related.

Agreed.

>>
>> Anyway, the cleanup looks good to me. Should just be in a separate patch.
>>
>>> @@ -1703,7 +1704,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>>   		pr_debug("Error %d calling netdev_set_bond_master\n", res);
>>>   		goto err_restore_mac;
>>>   	}
>>> -	res = netdev_rx_handler_register(slave_dev, bond_handle_frame, NULL);
>>> +	res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
>>> +					 new_slave);
>>
>> And using rx_handler data for this purpose sounds good to me.
>>
>> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>>
>>>   	if (res) {
>>>   		pr_debug("Error %d calling netdev_rx_handler_register\n", res);
>>>   		goto err_unset_master;
>>> @@ -1925,6 +1927,7 @@ err_close:
>>>
>>>   err_unreg_rxhandler:
>>>   	netdev_rx_handler_unregister(slave_dev);
>>> +	synchronize_net();
>>>
>>>   err_unset_master:
>>>   	netdev_set_bond_master(slave_dev, NULL);
>>> @@ -2108,6 +2111,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>>>   	}
>>>
>>>   	netdev_rx_handler_unregister(slave_dev);
>>> +	synchronize_net();
>>>   	netdev_set_bond_master(slave_dev, NULL);
>>>
>>>   	slave_disable_netpoll(slave);
>>> @@ -2222,6 +2226,7 @@ static int bond_release_all(struct net_device *bond_dev)
>>>   		}
>>>
>>>   		netdev_rx_handler_unregister(slave_dev);
>>> +		synchronize_net();
>>>   		netdev_set_bond_master(slave_dev, NULL);
>>>
>>>   		slave_disable_netpoll(slave);
>>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>> index ff4e269..1aac5cd 100644
>>> --- a/drivers/net/bonding/bonding.h
>>> +++ b/drivers/net/bonding/bonding.h
>>> @@ -264,6 +264,9 @@ struct bonding {
>>>   #endif /* CONFIG_DEBUG_FS */
>>>   };
>>>
>>> +#define bond_slave_get_rcu(dev) \
>>> +	((struct slave *) rcu_dereference(dev->rx_handler_data))
>>> +
>>>   /**
>>>    * Returns NULL if the net_device does not belong to any of the bond's slaves
>>>    *
>>
>


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox