Checksum field of a packet, should it be at head or tail?

Question

I'm working on a small project that communicates with a FPGA board. It sends data to FPGA and sometimes the FPGA will send data back. So my colleague and I are going to design a simple communication protocol.

We decide to add a checksum field to our protocol. But we have different ideas about where to put it. I suggest putting it at the head of a packet and he prefers putting it at tail.

To put it in code, here is my suggestion:

typedef struct {
    uint32_t magic_number;
    uint16_t frame_type;
    uint16_t length;
    uint16_t checksum;
    uint8_t data[];
}blah_blah;

And here is his:

typedef struct {
    uint32_t magic_number;
    uint16_t frame_type;
    uint16_t length;
    uint8_t data[];  //let's assume it is a valid syntax
    uint16_t checksum;  
}blah_blah;

My point is that I can declare the struct easily and directly cast the last data field to another struct that I want, iff the variable length part is at the end of the packet. And his argument is FPGA reads and writes data by clock, so it's more convenient to update checksum each clock and finally put or check it at the end of the packet.

My knowledge about digital design is very limited, so I don't know which one is a better idea. Where should I put the checksum field?

Edit: Somebody reminds me about strict aliasing when casting a pointer. Even if I use memcpy to avoid casting, it is still easier for me to get checksum in the head. If the checksum is in the tail then I have to cast the buffer to a const char* then add the length offset and then cast it to a const uint16_t* to get it.

Answer 1

For the embedded device, it depends on the hardware. If you're doing it a byte at a time via interrupts, probably better at the end. (you calculate it as you go reading/writing RX/TX registers and then tack it on, or check it, depending on whether you're reading or writing.)

If its DMA driven and two separate blocks, or only one of it is DMA driven, then it probably doesn't matter. You'll end up calculating the checksum/crc after all the data has reached you.

For the FPGA, however, it's almost certainly easier for them to 'tack on' the checksum at the end after the last byte has gone out, rather than pre-calculate the checksum on the packet, then insert it in the middle, then send out the data.

Answer 2

There are a number of ways to do this.

Except for the fact that uint8_t data[]; must be at the end of the struct.

Thus, as I suggested above, don't put the checksum inside the struct. You could even leave the data out of the struct [or not].

As I suggested in my top comment, a buffer could look like:

struct|data|csum

And, checksumming all of that, could sum to zero if the packet is valid.

---

Here's some sample code for one way that I've seen used (e.g. SDLC, etc.). It is crude and only checks some errors. It allow for the data payload to be in a different buffer.

typedef struct {
    uint32_t magic_number;
    uint16_t frame_type;
    uint16_t length;
    uint8_t data[];
} blah_blah;

#define CSUMSEED        0x1234
#define MAGIC           0xDEADBEEF

uint16_t
calcsum(uint16_t csum,const void *ptr,uint16_t length)
{

    uint8_t *data = ptr;
    for (;  length != 0;  --length, ++data)
        csum += *data;

    return csum;
}

void
sendone(int socket,uint16_t type,const void *data,uint16_t length)
{
    blah_blah hdr;

    uint16_t csum = CSUMSEED;

    hdr.magic_number = MAGIC;
    hdr.frame_type = type;
    hdr.length = length;

    csum = calcsum(csum,&hdr,sizeof(hdr));
    csum = calcsum(csum,data,length);

    // convert csum to "inverse" ???
    csum = ~csum;

    // could use writev here instead ...
    write(socket,&hdr,sizeof(hdr));
    write(socket,data,length);
    write(socket,&csum,sizeof(csum));
}

void
recvone(int socket,blah_blah *hdr,void *data)
// data -- must be large enough to hold _maximum_ payload
{

    uint16_t csum = CSUMSEED;
    ssize_t length;

    // get the header struct
    length = read(socket,hdr,sizeof(*hdr));
    if (length != sizeof(*hdr))
        error();

    // magic number must match
    if (hdr.magic != MAGIC)
        error()

    // get checksum for header
    csum = calcsum(csum,hdr,sizeof(*hdr));

    // get the data
    length = read(socket,data,hdr->length + sizeof(uint16_t));
    if (length != (hdr->length + sizeof(uint16_t))
        error();

    // get remaining checksum (including the trailing/appended CRC)
    csum = calcsum(data,data,length);
    if (csum != 0)
        error();
}

You could adapt this to embed the data at the end of the struct if you wish [as shown in the struct definition] with some slight modifications.

---

Here's a slight modification that appends the checksum. The receiver compares the checksum gotten from read with the calculated checksum:

typedef struct {
    uint32_t magic_number;
    uint16_t frame_type;
    uint16_t length;
    uint8_t data[];
} blah_blah;

#define CSUMSEED        0x1234
#define MAGIC           0xDEADBEEF

uint16_t
calcsum(uint16_t csum,const void *ptr,uint16_t length)
{

    uint8_t *data = ptr;
    for (;  length != 0;  --length, ++data)
        csum += *data;

    return csum;
}

void
sendone(int socket,uint16_t type,const void *data,uint16_t length)
{
    blah_blah hdr;

    uint16_t csum = CSUMSEED;

    hdr.magic_number = MAGIC;
    hdr.frame_type = type;
    hdr.length = length;

    csum = calcsum(csum,&hdr,sizeof(hdr));
    csum = calcsum(csum,data,length);

    // could use writev here instead ...
    write(socket,&hdr,sizeof(hdr));
    write(socket,data,length);
    write(socket,&csum,sizeof(csum));
}

void
recvone(int socket,blah_blah *hdr,void *data)
// data -- must be large enough to hold _maximum_ payload
{

    uint16_t csum = CSUMSEED;
    ssize_t length;

    // get the header struct
    length = read(socket,hdr,sizeof(*hdr));
    if (length != sizeof(*hdr))
        error();

    // magic number must match
    if (hdr.magic != MAGIC)
        error()

    // get checksum for header
    csum = calcsum(csum,hdr,sizeof(*hdr));

    // get the data
    length = read(socket,data,hdr->length);
    if (length != hdr->length)
        error();

    // get data checksum
    csum = calcsum(data,data,length);

    // get message checksum
    uint16_t csum2;
    length = read(socket,&csum2,sizeof(csum2));
    if (length != sizeof(csum2))
        error();

    // validate the checksum
    if (csum2 != csum)
        error();
}