Should we support headers which aren't byte-aligned?

[thomascalvert-xlnx]

This question arose whilst working on the eBPF backend, however it applies more generally to the rest of the compiler.

Consider a header which has a size in bits which isn't a multiple of 8. I have never encountered any protocol which would define something like this, but it seems that the P4 language permits it - please correct me if I'm wrong.

header foo {
  bit<3> bar;
}

Currently I believe that the eBPF backend won't deal with this case properly: although it tracks the packet offset in bits there is an assumption that headers start on a byte boundary:

p4c/backends/ebpf/ebpfParser.cpp

Lines 462 to 475 in aefe41f

	unsigned alignment = 0;
	for (auto f : ht->fields) {
	auto ftype = state->parser->typeMap->getType(f);
	auto etype = EBPFTypeFactory::instance->create(ftype);
	auto et = dynamic_cast<IHasWidth *>(etype);
	if (et == nullptr) {
	::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
	"Only headers with fixed widths supported %1%", f);
	return;
	}
	compileExtractField(destination, f, alignment, etype);
	alignment += et->widthInBits();
	alignment %= 8;
	}

No errors or warnings will be generated, but the emitted code won't work properly. I could not find any tests which define non-byte-sized headers either. I have not checked other backends but wouldn't be surprised if there exists similar conditions.

The compiler should either handle such headers correctly, or print an error message when they are encountered. I am in favour of the latter approach since there are no real-world scenarios for such headers, and doing so will allow for future performance optimisations.

[jafingerhut]

I have half a recollection of some change being made to the language spec explicitly saying that implementations are allowed to restrict themselves to header definitions whose total size must be a multiple of 8 bits long.

I definitely recall that the BMv2 software switch is such an implementation, and at least somewhere in the p4c-bm2-ss back end code there is a check that rejects programs at compile time that attempt to define headers whose total length is not a multiple of 8 bits long.

[jafingerhut]

The error message from the BMv2 back end looks like this, if you want to grep through the source code for the error message and see how it is implemented, and do something similar for the EBPF back end:

$ p4c --target bmv2 --arch v1model demo1.p4_16.p4
demo1.p4_16.p4(21): [--Werror=target-error] error: header ethernet_t: Found header with fields totaling 119 bits.  BMv2 target only supports headers with fields totaling a multiple of 8 bits.
header ethernet_t {
       ^^^^^^^^^^

[jnfoster]

AFAIK, this line

Architectures may impose additional constraints on header types—e.g., restricting headers to sizes that are an integer number of bytes.

has been in the P4_16 spec since version 1.0.0, from 2017.