Re: [patch net-next-2.6 V3] net: convert bonding to use rx_handler
From: Nicolas de Pesloüan <hidden>
Date: 2011-02-25 23:47:03
Le 23/02/2011 20: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.
Did performance test using pktgen and counting incoming packets by
iptables. No regression noted.
Signed-off-by: Jiri Pirko<redacted>
v1->v2:
using skb_iif instead of new input_dev to remember original
device
v2->v3:
do another loop in case skb->dev is changed. That way orig_dev
core can be left untouched.Hi Jiri, Eventually taking enough time for a review. I think we should split this change : 1/ Change __netif_receive_skb() to call rx_handler for diverted net_device, until rx_handler is NULL. 2/ Convert currently existing rx_handlers (bridge and macvlan) to use this new "loop" feature, removing the need to call netif_rx() inside their respective rx_handler and also removing the associated overhead. 3/ Convert bonding to use rx_handlers. Also, on step 1, we definitely need to clarify what orig_dev should be. I now think that orig_dev should be "the device one level below the current one" or NULL if current device was not diverted from another one. It means that we should keep an array of crossed (diverted) devices and the associated orig_dev. This array would be used to pass the right orig_dev to protocol handlers, depending on the device they register on : eth0 -> bond0 -> br0 A protocol handler registered on bond0 would receive eth0 as orig_dev. A protocol handler registered on br0 would receive bond0 as orig_dev. [snip]
quoted hunk ↗ jump to hunk
@@ -3167,32 +3135,8 @@ static int __netif_receive_skb(struct sk_buff *skb)
[snip]
quoted hunk ↗ jump to hunk
+another_round: + + __this_cpu_inc(softnet_data.processed); + #ifdef CONFIG_NET_CLS_ACT if (skb->tc_verd& TC_NCLS) { skb->tc_verd = CLR_TC_NCLS(skb->tc_verd);@@ -3209,8 +3157,7 @@ static int __netif_receive_skb(struct sk_buff *skb) #endif list_for_each_entry_rcu(ptype,&ptype_all, list) { - if (ptype->dev == null_or_orig || ptype->dev == skb->dev || - ptype->dev == orig_dev) { + if (!ptype->dev || ptype->dev == skb->dev) { if (pt_prev) ret = deliver_skb(skb, pt_prev, orig_dev); pt_prev = ptype;@@ -3224,16 +3171,20 @@ static int __netif_receive_skb(struct sk_buff *skb) ncls: #endif
Why do you loop to ptype_all before calling rx_handler ? I don't understand why ptype_all and ptype_base are not handled at the same place in current __netif_receive_skb() but I think we should take the opportunity to change that, unless someone know of a good reason not to do so.
- /* Handle special case of bridge or macvlan */
rx_handler = rcu_dereference(skb->dev->rx_handler);
if (rx_handler) {Nicolas.