Hello!
> This allows random parties to grab a field in the skbuff
> header, and attach a destructor.
Applications beyond netfilter?
Such objects miss main required property: conservation while
skb_clone/skb_copy. I see _no_ applications for such feature,
beyond storing nfdebug, even nfmark must be copied. I am sorry,
but you invented something really strange.
To be honest, it is the reason why I am pretty apathetic to this.
So that you may consider all written below as pure cavils 8)
> This makes the connection tracking
> code (a netfilter module) much nicer (ie. real reference counts),
Is it possible to make it nil, at least when CONFIG_NETFILTER is not defined
then? Do you see the point? If only netfilter uses it now, ifdef it properly.
You've just allocated 32 mainly useless bytes and zero them
each skb_alloc/clone.
Actually, it is possible to remove this static array from sk_buff at all.
F.e. look at TSI chains in gated. It is pretty expensive thing, but it
has zero offset, if nobody needs it and it is critically more flexible.
Another variant is to use aggregated objects a la skb->dst.
Well, and if to use static array, then it takes sense to allocate
objects in it also statically at least. Or do you plan to build
any binary independant DDI? Then defer this great task to 2.5 or later,
all this skbuff things will be apparently rewritten from scratch
for zero copy in any case.
> can also be used for other things where you need to play with skbs.
Paul. When we want to play, we just add new field to skb and that's all. 8)8)
If game turns out to be not interesting, we delete it. Did you forget that
Linux has public sources?
> + /* Could optimize: only iterate over part actually allocated --RR */
> + for (i = 0; i < sizeof(skb->reserve); i += sizeof(long)) {
If you could, then why you did not? 8)
Also, I remember you liked to talk about coding style. 8)
Please, look at surrounding style and note that size_t and another
posixisms are not used there. It is "unsigned long", if you meaned
really this. But something propts me that you wanted to write
just "int" or "unsigned char" in some places 8)
Alexey
|