[ovs-dev] [RFC PATCH 4/4] lib/ofpbuf: Compact

[Ben Pfaff]

On Tue, Apr 01, 2014 at 03:14:16PM +0300, Lori Jakab wrote:
> On 3/26/14, 2:16 AM, Ben Pfaff wrote:
> >On Mon, Mar 24, 2014 at 10:59:06AM -0700, Jarno Rajahalme wrote:
> >>This patch shrinks the struct ofpbuf from 104 to 48 bytes on 64-bit
> >>systems, or from 52 to 36 bytes on 32-bit systems (counting in the
> >>'l7' removal by the previous patch).  This may help contribute to
> >>cache efficiency, and will speed up initializing, copying and
> >>manipulating ofpbufs.  This is potentially important for the DPDK
> >>datapath, but the rest of the code base may also see a little benefit.
> >>
> >>Changes are:
> >>
> >>- Remove 'l7' pointer (previous patch).
> >>- Use offsets instead of layer pointers for l2_5, l3, and l4 using
> >>   'l2' as basis.  Usually 'data' is the same as 'l2', but this is not
> >>   always the case (e.g., when parsing or constructing a packet), so it
> >>   can not be easily used as the offset basis.  Also, packet parsing is
> >>   faster if we do not need to maintain the offsets each time we pull
> >>   data from the ofpbuf.
> >>- Use uint32_t for 'allocated' and 'size', as 2^32 is enough even for
> >>   largest possible messages/packets.
> >>- Use packed enum for 'source'.
> >>- Rearrange to avoid unnecessary padding.
> >>- Remove 'private_p', which was used only in two cases, both of which
> >>   had the invariant ('l2' == 'data'), so we can temporarily use 'l2'
> >>   as a private pointer.
> >>
> >>Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> Unfotunately I missed this patch until rebasing my layer 3 patch
> series [1] today.
> 
> Before this patch, I set the 'l2' pointer to NULL and 'l3' to the
> start of the network header to represent a layer 3 packet in a
> struct ofpbuf.  The way struct ofpbuf looks after this patch makes
> the assumption of always having layer 2 headers a lot stronger.
> 
> I see two options for my patches going forward:
> 1) Add back 'l3' and remove 'l3_ofs' from struct ofpbuf (keeping the
> rest as is); or
> 2) Encode a layer 3 packet by setting 'l2' to the network header
> address and set l3_ofs to 0.

Can we do the following?
    3) Replace 'l2' by 'l2_ofs' also?