From f53451a280b7bb27b5f2b8410ab086122ee77526 Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Tue, 18 Oct 2016 18:16:10 -0700 Subject: [PATCH] Improve field list calculation code (#221) More general support for fixed size constants in calculation (arbitrary width), by re-using deparser code. Added a unit test for non-aligned constants in calculations. Code cleanup. --- include/bm/bm_sim/calculations.h | 99 ++++---------------------------- src/bm_sim/calculations.cpp | 89 ++++++++++++++++++++++++++++ src/bm_sim/extract.h | 59 ++++++++++++++++++- src/bm_sim/fields.cpp | 49 +--------------- tests/test_calculations.cpp | 27 +++++++++ tests/test_checksums.cpp | 2 +- 6 files changed, 186 insertions(+), 139 deletions(-) diff --git a/include/bm/bm_sim/calculations.h b/include/bm/bm_sim/calculations.h index efad64882..2204d7346 100644 --- a/include/bm/bm_sim/calculations.h +++ b/include/bm/bm_sim/calculations.h @@ -226,7 +226,16 @@ class RawCalculation }; -struct BufBuilder { +class BufBuilder { + public: + void push_back_field(header_id_t header, int field_offset); + void push_back_constant(const ByteContainer &v, size_t nbits); + void push_back_header(header_id_t header); + void append_payload(); + + void operator()(const Packet &pkt, ByteContainer *buf) const; + + private: struct field_t { header_id_t header; int field_offset; @@ -241,94 +250,10 @@ struct BufBuilder { header_id_t header; }; + struct Deparse; // defined in calculations.cpp + std::vector > entries{}; bool with_payload{false}; - - void push_back_field(header_id_t header, int field_offset) { - field_t f = {header, field_offset}; - entries.emplace_back(f); - } - - void push_back_constant(const ByteContainer &v, size_t nbits) { - // TODO(antonin): general case - assert(nbits % 8 == 0); - constant_t c = {v, nbits}; - entries.emplace_back(c); - } - - void push_back_header(header_id_t header) { - header_t h = {header}; - entries.emplace_back(h); - } - - void append_payload() { - with_payload = true; - } - - struct Deparse : public boost::static_visitor<> { - Deparse(const PHV &phv, ByteContainer *buf) - : phv(phv), buf(buf) { } - - char *extend(int more_bits) { - int nbits_ = nbits + more_bits; - buf->resize((nbits_ + 7) / 8, '\x00'); - char *ptr = buf->data() + (nbits / 8); - nbits = nbits_; - // needed ? - // if (new_bytes > 0) buf->back() = '\x00'; - return ptr; - } - - int get_offset() const { - return nbits % 8; - } - - void operator()(const field_t &f) { - const Header &header = phv.get_header(f.header); - if (!header.is_valid()) return; - const Field &field = header.get_field(f.field_offset); - // taken from headers.cpp::deparse - const int offset = get_offset(); - field.deparse(extend(field.get_nbits()), offset); - } - - void operator()(const constant_t &c) { - assert(get_offset() == 0); - std::copy(c.v.begin(), c.v.end(), extend(c.nbits)); - } - - void operator()(const header_t &h) { - assert(get_offset() == 0); - const Header &header = phv.get_header(h.header); - if (header.is_valid()) { - header.deparse(extend(header.get_nbytes_packet() * 8)); - } - } - - Deparse(const Deparse &other) = delete; - Deparse &operator=(const Deparse &other) = delete; - - Deparse(Deparse &&other) = delete; - Deparse &operator=(Deparse &&other) = delete; - - const PHV &phv; - ByteContainer *buf; - int nbits{0}; - }; - - void operator()(const Packet &pkt, ByteContainer *buf) const { - buf->clear(); - const PHV *phv = pkt.get_phv(); - Deparse visitor(*phv, buf); - std::for_each(entries.begin(), entries.end(), - boost::apply_visitor(visitor)); - if (with_payload) { - size_t curr = buf->size(); - size_t psize = pkt.get_data_size(); - buf->resize(curr + psize); - std::copy(pkt.data(), pkt.data() + psize, buf->begin() + curr); - } - } }; diff --git a/src/bm_sim/calculations.cpp b/src/bm_sim/calculations.cpp index e717a22d5..b80e58ae7 100644 --- a/src/bm_sim/calculations.cpp +++ b/src/bm_sim/calculations.cpp @@ -29,9 +29,98 @@ #include "xxhash.h" #include "crc_tables.h" +#include "extract.h" namespace bm { +void +BufBuilder::push_back_field(header_id_t header, int field_offset) { + field_t f = {header, field_offset}; + entries.emplace_back(f); +} + +void +BufBuilder::push_back_constant(const ByteContainer &v, size_t nbits) { + constant_t c = {v, nbits}; + entries.emplace_back(c); +} + +void +BufBuilder::push_back_header(header_id_t header) { + header_t h = {header}; + entries.emplace_back(h); +} + +void +BufBuilder::append_payload() { + with_payload = true; +} + +struct BufBuilder::Deparse : public boost::static_visitor<> { + Deparse(const PHV &phv, ByteContainer *buf, int init_offset = 0) + : phv(phv), buf(buf), nbits(init_offset) { } + + char *extend(int more_bits) { + int nbits_ = nbits + more_bits; + buf->resize((nbits_ + 7) / 8, '\x00'); + char *ptr = buf->data() + (nbits / 8); + nbits = nbits_; + return ptr; + } + + int get_offset() const { + return nbits % 8; + } + + void operator()(const field_t &f) { + const Header &header = phv.get_header(f.header); + if (!header.is_valid()) return; + const Field &field = header.get_field(f.field_offset); + // get_offset needs to be called before extend! + const auto offset = get_offset(); + field.deparse(extend(field.get_nbits()), offset); + } + + void operator()(const constant_t &c) { + // get_offset needs to be called before extend! + const auto offset = get_offset(); + extract::generic_deparse(&c.v[0], c.nbits, extend(c.nbits), offset); + } + + void operator()(const header_t &h) { + assert(get_offset() == 0); + const Header &header = phv.get_header(h.header); + if (header.is_valid()) { + header.deparse(extend(header.get_nbytes_packet() * 8)); + } + } + + Deparse(const Deparse &other) = delete; + Deparse &operator=(const Deparse &other) = delete; + + Deparse(Deparse &&other) = delete; + Deparse &operator=(Deparse &&other) = delete; + + const PHV &phv; + ByteContainer *buf; + int nbits{0}; +}; + +void +BufBuilder::operator()(const Packet &pkt, ByteContainer *buf) const { + buf->clear(); + const PHV *phv = pkt.get_phv(); + Deparse visitor(*phv, buf); + std::for_each(entries.begin(), entries.end(), + boost::apply_visitor(visitor)); + if (with_payload) { + size_t curr = buf->size(); + size_t psize = pkt.get_data_size(); + buf->resize(curr + psize); + std::copy(pkt.data(), pkt.data() + psize, buf->begin() + curr); + } +} + namespace hash { uint64_t xxh64(const char *buffer, size_t s) { diff --git a/src/bm_sim/extract.h b/src/bm_sim/extract.h index 3bdaae88a..5c9dcd4ac 100644 --- a/src/bm_sim/extract.h +++ b/src/bm_sim/extract.h @@ -21,12 +21,14 @@ #ifndef BM_SIM_EXTRACT_H_ #define BM_SIM_EXTRACT_H_ +#include // for std::fill, std::copy + namespace bm { namespace extract { -static void generic_extract(const char *data, int bit_offset, - int bitwidth, char *dst) { +static inline void generic_extract(const char *data, int bit_offset, + int bitwidth, char *dst) { int nbytes = (bitwidth + 7) / 8; if (bit_offset == 0 && bitwidth % 8 == 0) { @@ -39,7 +41,7 @@ static void generic_extract(const char *data, int bit_offset, // necessary to ensure correct behavior when shifting right (no sign // extension) - unsigned char *udata = (unsigned char *) data; + auto udata = reinterpret_cast(data); int offset = bit_offset - dst_offset; if (offset == 0) { @@ -64,6 +66,57 @@ static void generic_extract(const char *data, int bit_offset, } } +static inline void generic_deparse(const char *data, int bitwidth, + char *dst, int hdr_offset) { + int nbytes = (bitwidth + 7) / 8; + + if (hdr_offset == 0 && bitwidth % 8 == 0) { + memcpy(dst, data, nbytes); + return; + } + + int field_offset = (nbytes << 3) - bitwidth; + int hdr_bytes = (hdr_offset + bitwidth + 7) / 8; + + int i; + + // necessary to ensure correct behavior when shifting right (no sign + // extension) + auto udata = reinterpret_cast(data); + + // zero out bits we are going to write in dst[0] + dst[0] &= (~(0xFF >> hdr_offset)); + + int offset = field_offset - hdr_offset; + if (offset == 0) { + std::copy(data + 1, data + hdr_bytes, dst + 1); + dst[0] |= udata[0]; + } else if (offset > 0) { // shift left + // don't know if this is very efficient, we memset the remaining bytes to 0 + // so we can use |= and preserve what was originally in dst[0] + std::fill(&dst[1], &dst[hdr_bytes], 0); + for (i = 0; i < hdr_bytes - 1; i++) { + dst[i] |= (udata[i] << offset) | (udata[i + 1] >> (8 - offset)); + } + dst[i] |= udata[i] << offset; + } else { // shift right + offset = -offset; + dst[0] |= (udata[0] >> offset); + if (nbytes == 1) { + // dst[1] is always valid, otherwise we would not need to shift the field + // to the right + dst[1] = udata[0] << (8 - offset); + return; + } + for (i = 1; i < hdr_bytes - 1; i++) { + dst[i] = (udata[i - 1] << (8 - offset)) | (udata[i] >> offset); + } + int tail_offset = (hdr_bytes << 3) - (hdr_offset + bitwidth); + dst[i] &= ((1 << tail_offset) - 1); + dst[i] |= (udata[i - 1] << (8 - offset)); + } +} + } // namespace extract } // namespace bm diff --git a/src/bm_sim/fields.cpp b/src/bm_sim/fields.cpp index ed1b57551..10b8e4665 100644 --- a/src/bm_sim/fields.cpp +++ b/src/bm_sim/fields.cpp @@ -20,8 +20,6 @@ #include -#include - #include "extract.h" namespace bm { @@ -43,52 +41,7 @@ int Field::extract_VL(const char *data, int hdr_offset, int computed_nbits) { } int Field::deparse(char *data, int hdr_offset) const { - if (hdr_offset == 0 && nbits % 8 == 0) { - std::copy(bytes.begin(), bytes.end(), data); - return nbits; - } - - int field_offset = (nbytes << 3) - nbits; - int hdr_bytes = (hdr_offset + nbits + 7) / 8; - - int i; - - // necessary to ensure correct behavior when shifting right (no sign - // extension) - unsigned char *ubytes = (unsigned char *) bytes.data(); - - // zero out bits we are going to write in data[0] - data[0] &= (~(0xFF >> hdr_offset)); - - int offset = field_offset - hdr_offset; - if (offset == 0) { - std::copy(bytes.begin() + 1, bytes.begin() + hdr_bytes, data + 1); - data[0] |= ubytes[0]; - } else if (offset > 0) { // shift left - // don't know if this is very efficient, we memset the remaining bytes to 0 - // so we can use |= and preserve what was originally in data[0] - std::fill(&data[1], &data[hdr_bytes], 0); - for (i = 0; i < hdr_bytes - 1; i++) { - data[i] |= (ubytes[i] << offset) | (ubytes[i + 1] >> (8 - offset)); - } - data[i] |= ubytes[i] << offset; - } else { // shift right - offset = -offset; - data[0] |= (ubytes[0] >> offset); - if (nbytes == 1) { - // data[1] is always valid, otherwise we would not need to shift the field - // to the right - data[1] = ubytes[0] << (8 - offset); - return nbits; - } - for (i = 1; i < hdr_bytes - 1; i++) { - data[i] = (ubytes[i - 1] << (8 - offset)) | (ubytes[i] >> offset); - } - int tail_offset = (hdr_bytes << 3) - (hdr_offset + nbits); - data[i] &= ((1 << tail_offset) - 1); - data[i] |= (ubytes[i - 1] << (8 - offset)); - } - + extract::generic_deparse(&bytes[0], nbits, data, hdr_offset); return nbits; } diff --git a/tests/test_calculations.cpp b/tests/test_calculations.cpp index 40099abb2..e4456a392 100644 --- a/tests/test_calculations.cpp +++ b/tests/test_calculations.cpp @@ -188,6 +188,33 @@ TEST_F(CalculationTest, WithConstant) { ASSERT_EQ(expected, actual); } +TEST_F(CalculationTest, WithConstantNonAligned) { + BufBuilder builder; + + // 0x9fd = 1001 1111 1101 + ByteContainer constant("0x9fd"); + + builder.push_back_constant(ByteContainer("0x00"), 4); + builder.push_back_field(testHeader1, 0); // f16 + builder.push_back_constant(constant, 12); + + Calculation calc(builder, "identity"); + + unsigned char pkt_buf[2 * header_size]; + + for (size_t i = 0; i < sizeof(pkt_buf); i++) { + pkt_buf[i] = 0xff; + } + + Packet pkt = get_pkt((const char *) pkt_buf, sizeof(pkt_buf)); + parser.parse(&pkt); + + auto expected = static_cast(0x0ffff9fd); + auto actual = calc.output(pkt); + + ASSERT_EQ(expected, actual); +} + TEST_F(CalculationTest, WithPayload) { BufBuilder builder; diff --git a/tests/test_checksums.cpp b/tests/test_checksums.cpp index 5d9533bfe..f52206046 100644 --- a/tests/test_checksums.cpp +++ b/tests/test_checksums.cpp @@ -112,7 +112,7 @@ class ChecksumTest : public ::testing::Test { BufBuilder tcp_cksum_engine_builder; tcp_cksum_engine_builder.push_back_field(ipv4Header, 10); // ipv4.srcAddr tcp_cksum_engine_builder.push_back_field(ipv4Header, 11); // ipv4.dstAddr - tcp_cksum_engine_builder.push_back_constant(ByteContainer('\x00'), 8); + tcp_cksum_engine_builder.push_back_constant(ByteContainer(1, '\x00'), 8); tcp_cksum_engine_builder.push_back_field(ipv4Header, 8); // ipv4.protocol tcp_cksum_engine_builder.push_back_field(metaHeader, 0); // for tcpLength tcp_cksum_engine_builder.push_back_field(tcpHeader, 0); // tcp.srcPort