jamal wrote: > On Fri, 2006-30-06 at 19:18 +0200, Patrick McHardy wrote: > >>Thomas Graf wrote: >> >>>* Thomas Graf <[EMAIL PROTECTED]> 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. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
