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

[Jiri Pirko <jpirko@redhat.com>]

Wed, Mar 02, 2011 at 10:54:03PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 02/03/2011 22:12, Jiri Pirko a écrit :
>>Wed, Mar 02, 2011 at 09:47:50PM CET, nicolas.2p.debian@gmail.com wrote:
>>>Hi Jiri,
>>>
>>>Do you plan to call the bonding ARP handler from inside bond_handle_frame()?
>>
>>I do - it's part of patchset I've cooked (going to test that tomorrow).
>>
>>>A few days ago
>>>(http://marc.info/?l=linux-netdev&m=129883949022340&w=2), I noticed
>>>that it is not possible to call the bonding ARP handler from inside
>>>the bonding rx_handler, because some frame processing may be required
>>>after the bonding rx_handler call, to put the frame in a suitable
>>>state for the bonding ARP handler.
>>
>>Do you see another scenario besides the next one?
>
>None that currently work, but eth0 -> bond0 -> br0 -> br0.100 should work too.
>
>>>This is at least true with the following setup, eth0 ->  bond0 ->
>>>bond0.100, where the ARP frames are VLAN tagged at the time the
>>>bonding rx_handler process them.
>>
>>Isn't this scenario resolved by vlan_on_bond_hook ?
>>
>>eth0
>>   ->rx_handler ->  another round
>>bond0
>>   ->vlan_hwaccel_do_receive ->  __netif_receive_skb
>>bond0.100
>>   ->vlan_on_bond_hook ->  reinject to bond0
>
>Yes, it is, but this hack does not solve the eth0 -> bond0 -> br0 -> br0.100 configuration.
>
>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 :(

>
>My question about whether the skb is shared between the protocol
>handlers (in another thread) also target at this idea.
>
>You will probably told me I'm free to propose patchs for all that,
>and you are right. Just missing the time to do so.
>
>	Nicolas.