Re: [patch net-next-2.6 V3] net: convert bonding to use rx_handler

[Jiri Pirko <jpirko@redhat.com>]

Sat, Feb 19, 2011 at 11:56:23AM CET, nicolas.2p.debian@gmail.com wrote:
>Le 19/02/2011 09:05, Jiri Pirko a écrit :
>>This patch converts bonding to use rx_handler. Results in cleaner
>>__netif_receive_skb() with much less exceptions needed. Also
>>bond-specific work is moved into bond code.
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>
>>v1->v2:
>>         using skb_iif instead of new input_dev to remember original
>>	device
>>v2->v3:
>>	set orig_dev = skb->dev if skb_iif is set
>>
>
>Why do we need to let the rx_handlers call netif_rx() or __netif_receive_skb()?
>
>Bonding used to be handled with very few overhead, simply replacing
>skb->dev with skb->dev->master. Time has passed and we eventually
>added many special processing for bonding into __netif_receive_skb(),
>but the overhead remained very light.
>
>Calling netif_rx() (or __netif_receive_skb()) to allow nesting would probably lead to some overhead.
>
>Can't we, instead, loop inside __netif_receive_skb(), and deliver
>whatever need to be delivered, to whoever need, inside the loop ?
>
>rx_handler = rcu_dereference(skb->dev->rx_handler);
>while (rx_handler) {
>	/* ...  */
>	orig_dev = skb->dev;
>	skb = rx_handler(skb);
>	/* ... */
>	rx_handler = (skb->dev != orig_dev) ? rcu_dereference(skb->dev->rx_handler) : NULL;
>}
>
>This would reduce the overhead, while still allowing nesting: vlan on
>top on bonding, bridge on top on bonding, ...

I see your point. Makes sense to me. But the loop would have to include
at least processing of ptype_all too. I'm going to cook a follow-up
patch.

>
>That way, we can probably keep the list of crossed devices inside a
>local array, and call deliver_skb() with the current "orig_dev" when
>appropriate. No need to overload sk_buff nor to use a global
>variable.
>
>Of course, this might be a very simplistic view.
>
>Any comments?
>
>	Nicolas.