1

I am trying to create a client C code for CAPWAP protocol. I tried implementing CAPWAP header using a bit field structure. But after sending this structure over socket using sendto(), when I sniff the packet using wireshark, I find that some extra bits are added in between. I have no clue from where this came from. Requesting for help. Thanks in advance.

wireshark

I tried commenting last few members of the struct to make it 4 byte aligned. Still the isue persists.

This is the original header

struct cw_header
{
unsigned preamble : 8;
unsigned hlen : 5;
unsigned rid : 5;
unsigned wbid : 5;
unsigned t : 1;
unsigned f : 1;
unsigned l : 1;
unsigned w : 1;
unsigned m : 1;
unsigned k : 1;
unsigned flags : 3;
unsigned fragment_id : 16;
unsigned fragment_offset : 13;
unsigned reserved : 3;
uint32_t mac_length : 8;
uint32_t mac_addr[6];
uint32_t padding : 8;
};

I tried commenting these

//uint32_t mac_length : 8;
//uint32_t mac_addr[6];
//uint32_t padding : 8;

This is where the struct is populated

struct cw_header create_cw_header()
{
struct cw_header cw_header;
cw_header.preamble = 0;
cw_header.hlen = 1;
cw_header.rid = 1;
cw_header.wbid = 1; 
cw_header.t = 0;
cw_header.f = 0;
cw_header.l = 1;
cw_header.w = 0;
cw_header.m = 1;
cw_header.k = 0;
cw_header.flags = 0;    
cw_header.fragment_id = 0;  ;
cw_header.fragment_offset = 0;  
cw_header.reserved = 0; 
cw_header.mac_length = 6;
get_mac_address(cw_header.mac_addr);
cw_header.padding = 0;
return cw_header;
};

Here is the output of wireshark for first 32 bits

0000 0000 0010 0001 0000 0100 0000 1010

Expected result : Bits should be in the order mentioned in the struct Error : Extra bits are added in between struct members

Athul Sukumaran
  • 360
  • 4
  • 16

2 Answers2

5

"Bits should be in the order mentioned in the struct"

What order? The C language specifies no order of bits. You cannot portably know if preamble is MSB or LSB.

"Extra bits are added in between struct members"

Yep, the compiler is free to place padding bits or padding bytes between members of a bitfield. Bitfields are divided in abstract, "invisible" units called "storage unit" by the standard, which have a given size - often the same size as the CPU alignment but not necessarily so. Between different such storage units there may or may not be padding. The compiler is also free to let bits that won't fit in one storage unit get placed in the next one.

There's no telling what a particular compiler will do without reading up on the compiler's implementation of bitfields. There is almost no support for them by the C standard.

Add endianess on top of that and you have a proper mess - in your case both CPU endianess and network endianess will matter.

The best solution to all of these problems is to drop bitfields entirely. Instead you should use bit-wise operators, that are far more well-defined, deterministic and portable:

uint32_t cw_preable = something << preamble_offset;
...
uint32_t cw_header = 0;
cw_header |= cw_preamble;
...
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • I am new to this stuff. Can you please explain what must be the value of preamble_offset and something, here? – Athul Sukumaran Aug 06 '19 at 06:53
  • 1
    @AthulSukumaran It's just the bit position. For example if preamble should be allocated on bits 24 to 31, then `preamble_offset` should be 24. – Lundin Aug 06 '19 at 06:56
  • can you please give a sample value for "something" and "rid_offset" (rid is 5 bits long with value 1)? – Athul Sukumaran Aug 06 '19 at 07:00
  • @AthulSukumaran No. I don't know the protocol and I don't know what data you are sending. – Lundin Aug 06 '19 at 07:02
  • Is there any generic data type that I can use instead of "uint32_t". I wanted to cross compile it for a much smaller embedded device. – Athul Sukumaran Aug 06 '19 at 07:30
  • @AthulSukumaran use `uint8_t`... :D – Antti Haapala -- Слава Україні Aug 06 '19 at 07:36
  • @AnttiHaapala I tried that. But static_assert fails. – Athul Sukumaran Aug 06 '19 at 08:47
  • @Lundin But how do I send variable of type uint32_t across UDP sockets? – Athul Sukumaran Aug 06 '19 at 11:15
  • @AthulSukumaran No matter what it is you are sending, you are likely going to serialize it byte by byte using `uint8_t*`. I believe UDP uses big endian(?) so if your CPU is little endian, you have to swap the data around before anything else. – Lundin Aug 06 '19 at 11:23
  • @Lundin I tried serializing the data using char * and sending it over socket using sento(). But I get a different data when I sniffed over wireshark. `uint32_t disc_req[1];` `create_discovery_req(disc_req);` `char *tmp = *disc_req;` `sendto(sockfd, &tmp, sizeof(tmp),` `MSG_CONFIRM, (const struct sockaddr *) &servaddr,` `sizeof(servaddr));` – Athul Sukumaran Aug 06 '19 at 11:43
3

The layout of bitfields is entirely implementation-dependent. However it is common that an unit cannot cross the storage-unit boundary, so in this case it would depend on what size unsigned is. It is also implementation-dependent how uint32_t is defined and how it compares to everything else, and if the uint32_t mac_addr[6]; can overlay rest of the storage unit in uint32_t : 8.

All in all, it isn't going to be pretty. The best course of action would be to use named unsigned char/uint8_t fields and arrays in the struct, with bit-twiddling packing of the smaller members into these.

Antti Haapala -- Слава Україні
  • 129,958
  • 22
  • 279
  • 321
  • What if I had to cross compile this code for another machine ? – Athul Sukumaran Aug 06 '19 at 07:03
  • @AthulSukumaran that **exactly** is the reason why my approach works and yours doesn't. The bitfields are in **no way** portable, you will have to read the manuals of **each individual compiler for each target to ensure how they behave there**. At least with structs containing *only* `uint8_t`s then it is not common to have padding in the structure and it can be ascertained by a `static_assert`. – Antti Haapala -- Слава Україні Aug 06 '19 at 07:03