Hi Vitali,

Again, I'm not convinced this is 100% feasible.  I understand, I think, what you're getting at.  If you want to use the field, you defined a structure like struct my_arg { void *old, ... my data ... }.  However, this isn't like C++ where the destructors for all layers are called.  There is only 1 destructor for the layer that created the skb (AFAIK) - all the layers simply increment their reference to it (which is decremented by a free).  Thus there's really no way to do what you want with minimal impact on existing code - the ideal behaviour would be to define a common struct (i.e. { void * olddata, void *thisdata }, that is filled with whatever data that layer needs when it increments the refcount and then have kfree_skb unwind the pointer as needed.  This would require a massive chang

 e to the code though, increase the amount of memory usage unnecessarily, and be prone to memory leaks if it's not done carefully.

You say "There is only 1 destructor for the layer that created the skb
(AFAIK)", why not a "There is only 1 destructor argument for the layer
that created the skb (AFAIK)"?
Where is the impact? only a void * destructor_arg that only the
creator of skb is allowed to use. Moreover it can be put under feature
flag.

quoted

This can be used also for sock_w_free() destructor. Current code is
assuming that the socket destructor is always sock_w_free(), if
tomorrow, this callback is changed or removed, we will have to change
packet_mmap code (called in packet_skb_destruct()). Such mechanism
should make replacement of destructor/destructor arg more independent
toward layers.

We're talking about skb destructors, not socket destructors.  sock_wfree is not a destructor - it releases the resources allocated to the skb back to the socket that the resources are from.

I agree, I'm just talking about destructor/destructor_arg pair.

Also, from what I can tell, this is how the Linux kernel works - if someone's change breaks the code, they're responsible for fixing it.  Changing how sock_wfree is fairly significant enough that we don't need to worry about making it easier - it'll probably be so significant that there will be a lot of changes anyways.  It's also highly unlikely to happen I think.

I don't want to break this code. Just add a variable in sk_buff that
can be used as the destructor is ! kind of CB[] that is not
overwritten by sub layer ! or at least backuped.

Thanks,
Johann




-- 
Johann Baudy
johaahn@gmail.com