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

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 :

quoted

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<redacted>

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.