Subject: Re: [patch net-next-2.6] bonding: remove skb_share_check in handle_frame

From: Nicolas de Pesloüan<[email-masked]>
Date: Thu, 03 Mar 2011 09:37:58 +0100

Le 03/03/2011 07:14, Jiri Pirko a écrit :
> Wed, Mar 02, 2011 at 10:54:03PM CET, [email-masked] wrote:
[snip]
>> All those handlers that call netif_rx() or __netif_receive_skb()
>> sound horrible to me. Can you imagine the global overhead of the
>> above receive path?
>>
>> The reason why I suggested you introduce the goto another_round is
>> because most - if not all - stacking configurations could/should be
>> handled simply by returning the right skb from the rx_handler and let
>> __netif_receive_skb() loop. And by having the right orig_dev logic
>> inside __netif_receive_skb(), it could be possible to remove the
>> current vlan_on_bond_hook hack.
>
> Well that wouldn't solve the problem. if we just got anorther_round the
> packed would not be delivered to bond0.100 but only to bond0. That's
> also unwanted. Well, this think shouldn't have been added here in the
> first place :(

Yes, the packet would be delivered "exact match" to bond0, and this is what the bonding ARP handler 
expects, because it registered at this level, then deliver "exact match or NULL" to bond0.100, if 
appropriate, to whatever other protocol handlers.

There is a subtile difference between ptype_all and ptype_base delivery:

- The ptype_all handlers will receive the frame between two call to rx_handlers.
- The ptype_base handlers will receive the frame after all rx_handlers (and hard coded handlers like 
vlan) were called.

So a ptype_all handler might receive vlan tagged frame, if it register below the interface that 
untag the frame, while a ptype_base handler will always receive the untagged frame, because 
ptype_base delivery is done at the end of __netif_receive_skb().

I don't know whether it is desirable, but it used to be required for the bonding ARP monitor to work.

May be we should add some extra properties to protocol handler, so they can tell 
__netif_receive_skb() what they expect :

- a property to tell whether the protocol handler want the exact frame that cross the interface they 
registered on or the final frame, after all rx_handlers.
- a property to tell what they expect a the orig_dev value : the lowest interface known to 
__netif_receive_skb() (f_packet) or the interface one level below the interface they registered on 
(bonding ARP monitor).

Having those properties, I consider we would be in a position to remove most special hacks we 
currently have to handle particular stacking configuration.

	Nicolas.