[RFC] Alignment- / padding-requirements

[Martin Schwenke]

On Thu, 15 Mar 2018 11:29:23 +1100, Martin Schwenke via samba-technical
<samba-technical at lists.samba.org> wrote:

> On Mon, 05 Mar 2018 09:42:15 +0100, Swen Schillig via samba-technical
> <samba-technical at lists.samba.org> wrote:
> 
> > For the last few days I've been looking at 
> > the memory handing within CTDB and I stumbled over code snippets like
> > 
> >         size = (length+(CTDB_DS_ALIGNMENT-1)) & ~(CTDB_DS_ALIGNMENT-1);
> > 
> > I might be wrong, but what is done here is padding and not alignment
> > and I might be wrong again, but padding memory to a multiple of the
> > wanted alignment does not guarantee the alignment.
> > So, I'm a bit puzzled what is really wanted here.
> > 
> > From the comments around the define 
> > 
> > /* we must align packets to ensure ctdb works on all architectures (eg.
> > sparc) */
> > #define CTDB_DS_ALIGNMENT 8
> > 
> > .. I take it that really alignment is required to make sure 64bit
> > access is 
> > guaranteed on architectures which need it (here SPARC !?)
> > 
> > But the structure(s) which are wanted to be aligned 
> >         struct ctdb_req_call_old
> >         struct ctdb_req_header 
> > do not contain any 64bit values.
> > 
> > So here comes my question, do we really need to do those ugly padding
> > tricks and  
> 
> After talking with people who are smarter than me, I finally have a
> theory for this!  :-)
> 
> 1. malloc() returns 8-byte aligned memory (according to POSIX, the C
>    standard, malloc(3))
> 
> 2. CTDB's packet allocation code pads out to a multiple of 8 bytes
> 
> 3. CTDB writes the padded packets, each starting with a header, out to
>    a socket
> 
> 4. As the receiving CTDB reads data from a socket, it might read the
>    data for the current packet and then the beginning of the next
>    packet into a buffer
> 
>    Padding each packet when sending guarantees that the header
>    at beginning of the next packet is always aligned in the buffer on
>    the receiving side.
> 
> So, the padding is required to guarantee alignment of subsequent headers
> because we're reading the header out of the buffer as a struct.  If we
> were reading and unmarshalling from the buffer as a byte stream then we
> wouldn't need to do this.
> 
> This theory can (probably) be tested.  You could add an assert() to the
> reading code to ensure that each header is aligned.  Then remove the
> "alignment" code and see if you trip the assert().
> 
> Someone please yell if I've got this horribly wrong!

Note that this theory doesn't apply in the current ctdb_io.c code
because it does a memmove() to move residual data to the front of the
buffer.  The previous version seems to have only read up until the end
of the packet.  Not sure about previous versions.

However, this might not be a useful theory after all...

peace & happiness,
martin