Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device

[Patrick McHardy <kaber@trash.net>]

jamal wrote:
> On Fri, 2006-30-06 at 19:18 +0200, Patrick McHardy wrote:
> 
>>Thomas Graf wrote:
>>
>>>* Thomas Graf <tgraf@suug.ch> 2006-06-30 18:32
>>>
>>>
>>>>Anyways, I give up. Last time I've been running after you trying
>>>>to fix the many bugs you leave behind. Ever noticed that whenever
>>>>you add some new code it's someone else following up with tons of
>>>>small bugfix patches having a hard time trying to figure out the
>>>>actual intent. I'll just duplicate the code for my purpose, so
>>>>much easier.
>>>
>>>
>>>There you go, leaves ifb broken as-is, at least prevents it
>>>from crashing randomly when the input_dev disappears.
>>
>>That would be a pity. After this patch has been ACKed, could we start
>>over with the other bugs one at a time? I wasn't really able to gather
>>the problems from this thread, but some of the behaviour you mentioned
>>does seem questionable.
>>
> 
> 
> I will summarize what the outstanding issues are, the rest of the "bugs"
> just ignore otherwise the discussion is a waste of time and may
> get out of control.
> 
> 1) ifb references skb->input_dev

That should be fixed by this patch.

> 2) mirred sets the skb->input_dev which is used in #1

I think Thomas was more complaining about the values it is set to
and the fact that only a single redirection is possible for each
packet, no?

> It is possible that when #1 happens infact input_dev is gone because no
> ref count is incremented. Ok, Thomas is that sufficient to discuss the
> crux of the matter?
> At one point many months ago, the logic was for now the likelihood this
> will happen is low but we need to cover for by at least figuring the
> existence of input_dev when referencing it.
> 
> Thomas makes the claim, this can be achieved only by using an ifindex.
> And i havent been able to see how. I have a small performance problem if
> i just use ifindex. Using ifindex will eventually save 32 bits on the
> 64 bit machines. I posed the question as to which was more beneficial
> as a solution that hasnt been addressed.

Are we still talking about this? Its easy: a pointer without taking
a reference can become stale, with ifindex you do the lookup right
when using it, so you either get an result or you don't. It also
saves atomic operations for anyone _not_ using it, which is that
vast majority of users. So the patch clearly makes sense to me.