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

[Eric Dumazet <eric.dumazet@gmail.com>]

Le vendredi 18 février 2011 à 19:47 +0100, Jiri Pirko a écrit :
> Fri, Feb 18, 2011 at 05:14:30PM CET, eric.dumazet@gmail.com wrote:
> >Le vendredi 18 février 2011 à 16:50 +0100, Patrick McHardy a écrit :
> >> On 18.02.2011 15:58, Jiri Pirko wrote:
> >> > Fri, Feb 18, 2011 at 03:46:45PM CET, kaber@trash.net wrote:
> >> >> Am 18.02.2011 15:27, schrieb Eric Dumazet:
> >> >>> Le vendredi 18 février 2011 à 15:14 +0100, Jiri Pirko a écrit :
> >> >>>
> >> >>>> Do not know how to do it better. As for percpu variable, not only
> >> >>>> origdev would have to be remembered but also probably skb pointer to
> >> >>>> know if it's the first run on the skb or not. Can't really figure out a
> >> >>>> better solution. Can you?
> >> >>>
> >> >>> I'll try and let you know.
> >> >>
> >> >> Why not simply do a lookup on skb->iif?
> >> > 
> >> > Well I was trying to avoid iterating over list of devices for each
> >> > incoming frame.
> >> > 
> >> 
> >> Well, there are a couple of holes on 64 bit, perhaps you can rearrange
> >> things and eliminate either iif or input_dev without increasing size
> >> since they appear to be redundant.
> >
> >Jiri
> >
> >I dont understand why netif_rx() is needed in your patch.
> 
> I used netif_rx() because bridge and macvlan does that too. I did not see
> a reason to not to do the same.
> 
> >
> >Can we stack 10 bond devices or so ???
> >
> >If we avoid this stage and call the real thing (netif_receive_skb()),
> >then we dont need adding a field in each skb, since it can be carried by
> >a global variable (per cpu of course)
> >
> I'm probably missing something. How do netif_receive_skb() and
> netif_rx() differ in this point of view, since both are calling:
> "ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);"
> ?
> 
> Still I see a problem with the percpu global variable. We would have to
> store skb pointer there as well and in each __netif_receive_skb() call it
> would have to be checked if it's different from the current one.
> In that case store new skb and orig_Dev.
> 
> Leaving aside that global variables are evil in general, I still think
> this is not nicer solution then to add skb->input_dev (although I
> understand your arguments).

Really I must miss something about "global variables" thing/fear.

Kernel is full of global variables, they are not evil if properly used.

Take a look at net/core/dev.c :

static DEFINE_PER_CPU(int, xmit_recursion);

For an example of what I have in mind.