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

[jamal <hadi@cyberus.ca>]

On Fri, 2006-30-06 at 22:08 +0200, Thomas Graf wrote:
> * jamal <hadi@cyberus.ca> 2006-06-30 15:34

> > So assuming it is taken in mirred (i havent thought of where it is
> > decremented), why would using the ifindex be better?
> 
> The issue exists regardless of mirred/ifb. As soon as the packet is
> queued for the first time we leave netif_receive_skb() and the dev
> reference is dropped. Therefore in order to allow functionality
> like tcf_match_indev() at egress we have to either take a reference
> or ensure that we can catch the unlikely case of the device having
> disappeared. I think everyone would agree to use device pointers
> if only mirred would acquire it.

Thomas, this is certainly more reasonable explanation. 

On Fri, 2006-30-06 at 13:17 -0700, David Miller wrote:

> We have a
> lot of bits of state that sit in the sk_buff but which are used by
> a tiny minority, yet the space consumption is eaten by everyone.
> 
>
> Maybe when the IFB and mirred are in more widespread use we can
> investigate a different approach.
> 

Not unreasonable.
So saving the bits for the majority trumps the need for slightly more
cycles in mirred/ifb until they get more widespread.


> Another part of this issue is that if you use a pointer there is no
> clean place to clean up the input_dev reference within the scope it is
> actually used. 
> 
>
>  The only reliable place is kfree_skb() which is far
> beyond the scope that this ->input_dev thing is used. 
> 

And i cant think of a good place to do it. 

>  We have a lot
> of problems in some parts of the networking because object references
> are held for too long.  One example is all the awful side effects of
> the way the bridge netfilter code holds onto object references for
> long periods of time in the netfilter call chains.
> 
> And we know the refcounting is broken.  And one way we know to fix
> the refcounting without incurring the cost upon everyone is to
> use an interface index and only make the input_dev users eat the
> atomic operation incurred by grabbing the reference.
> 
> And even for the input_dev users, it isn't such a big deal, there
> are other costs already associated with hitting these actions and
> devices that use input_dev, the atomic reference grab there should
> be lost in the noise I think.

Alright guys - lets get the change in. I wish we had this specific
discussion sooner. 

Thomas, can you post your latest incarnation so i can mow comment more
peacefully? ;->

cheers,
jamal