Re: [PATCH net-next 02/13] sk_buff: add skb extension infrastructure

Willem de Bruijn [off-list ref] wrote:

quoted

+#ifdef CONFIG_SKB_EXTENSIONS
+       __u8                    active_extensions;
+#endif

This byte could be saved by moving the bits to the first byte of the
new extension.

I tried to do this, but could not resolve following problem:
- extensions a and b are active
- skb is cloned
- extension a is to be disabled

In this patch series, we can just clear the 'a' bit from
from clone->active_extensions.

But if its part of the extension itself, we must first need to
be able to duplicate the extension blob before we can disable
the extension, which means that attempt to disable an extension
would now fail on -ENOMEM.  I think disabling should always work.
(its not a problem at the moment for either secpath or bridge
 as both use distinct pointers in sk_buff).

The likely cold cacheline now needs to be checked, but
only if the pointer is non-NULL.

The upside of the 'active_extension' member is that we can keep the
pointer in uninitialized state for the active_extensions == 0 case,
i.e., when allocating a new skb skb->extensions is not initialized.

If exclusively using accessor
functions to access the struct, even this can be avoided if encoding
the first 3 or 7 active extensions in the pointer itself.

Yes, that would work, but requires to init the pointer on skb allocation
plus a few tricks to align the allocation so we have three bits to use
on arches where kmalloc doesn't guarantee 8-byte-aligned pointers.

Would also need a 'plan b' in place when extension number 4 arrives.