diff --git a/docs/JSON_format.md b/docs/JSON_format.md index 954cdc68c..85b785965 100644 --- a/docs/JSON_format.md +++ b/docs/JSON_format.md @@ -10,7 +10,7 @@ on each attribute. ## Current bmv2 JSON format version -The version described in this document is *2.18*. +The version described in this document is *2.19*. The major version number will be increased by the compiler only when backward-compatibility of the JSON format is broken. After a major version @@ -274,7 +274,7 @@ parser. The attributes for these objects are: in the correct order. Each parser operation is represented by a JSON object, whose attributes are: - `op`: the type of operation, either `extract`, `extract_VL`, `set`, - `verify`, `shift` or `primitive`. + `verify`, `shift`, `advance` or `primitive`. - `parameters`: a JSON array of objects encoding the parameters to the parser operation. Depending on the type of operation, the constraints are different. A description of these constraints is included [later in this @@ -335,7 +335,10 @@ In the `parser_ops` array, the format of the `parameters` array depends on the boolean expression while the second should be an expression resolving to a valid integral value for an error constant (see [here](#errors)). - `shift`: we expect a single parameter, the number of bytes to shift (shifted - packet data will be discarded). + packet data will be discarded). `shift` is deprecated in favor of `advance` + (see below). + - `advance`: we expect a single parameter, the number of bytes to shift, which + can be of type `hexstr`, `field` or `expression`. `advance` replaces `shift`. - `primitive`: introduced for P4_16, where extern methods can be called from parser states. It only takes one parameter, which is itself a JSON object with the following attributes: diff --git a/include/bm/bm_sim/data.h b/include/bm/bm_sim/data.h index 06a4260f6..8e9614149 100644 --- a/include/bm/bm_sim/data.h +++ b/include/bm/bm_sim/data.h @@ -93,6 +93,10 @@ class Data { virtual void export_bytes() {} + //! Returns a value less than zero if Data is negative, a value greater than + //! zero if Data is positive, and zero if Data is zero. + int sign() const { return value.sign(); } + // TODO(Antonin): need to figure out what to do with signed values //! Set the value of Data from any integral type template::value, int>::type = 0> T get() const { diff --git a/include/bm/bm_sim/parser.h b/include/bm/bm_sim/parser.h index 4f6c20512..7a55bea9c 100644 --- a/include/bm/bm_sim/parser.h +++ b/include/bm/bm_sim/parser.h @@ -259,6 +259,10 @@ class ParseState : public NamedP4Object { void add_shift(size_t shift_bytes); + void add_advance_from_data(const Data &shift_bytes); + void add_advance_from_expression(const ArithExpression &shift_bytes); + void add_advance_from_field(header_id_t shift_header, int shift_offset); + void set_key_builder(const ParseSwitchKeyBuilder &builder); void add_switch_case(const ByteContainer &key, const ParseState *next_state); diff --git a/src/bm_sim/P4Objects.cpp b/src/bm_sim/P4Objects.cpp index b215137f0..7ce869c50 100644 --- a/src/bm_sim/P4Objects.cpp +++ b/src/bm_sim/P4Objects.cpp @@ -42,7 +42,7 @@ using std::string; namespace { constexpr int required_major_version = 2; -constexpr int max_minor_version = 18; +constexpr int max_minor_version = 19; // not needed for now // constexpr int min_minor_version = 0; @@ -1031,7 +1031,13 @@ P4Objects::init_parsers(const Json::Value &cfg_root, InitState *init_state) { ArithExpression VL_expr; if (op_type == "extract_VL") { const auto &cfg_VL_expr = cfg_parameters[1]; - assert(cfg_VL_expr["type"].asString() == "expression"); + if (cfg_VL_expr["type"].asString() != "expression") { + throw json_exception( + EFormat() << "Parser 'extract_VL' operation has invalid " + << "length type '" << cfg_VL_expr["type"].asString() + << "', expected 'expression'", + cfg_VL_expr); + } build_expression(cfg_VL_expr["value"], &VL_expr); VL_expr.build(); } @@ -1171,9 +1177,43 @@ P4Objects::init_parsers(const Json::Value &cfg_root, InitState *init_state) { parse_state->add_method_call(action_fn.get()); parse_methods.push_back(std::move(action_fn)); } else if (op_type == "shift") { - assert(cfg_parameters.size() == 1); + if (cfg_parameters.size() != 1) { + throw json_exception( + EFormat() << "Parser 'shift' operation has invalid number of " + << "arguments, expected " << 1, + cfg_parser_op); + } auto shift_bytes = static_cast(cfg_parameters[0].asInt()); parse_state->add_shift(shift_bytes); + } else if (op_type == "advance") { + if (cfg_parameters.size() != 1) { + throw json_exception( + EFormat() << "Parser 'advance' operation has invalid number of " + << "arguments, expected " << 1, + cfg_parser_op); + } + const auto &shift_type = cfg_parameters[0]["type"].asString(); + if (shift_type == "field") { + const auto shift = field_info( + cfg_parameters[0]["value"][0].asString(), + cfg_parameters[0]["value"][1].asString()); + parse_state->add_advance_from_field( + std::get<0>(shift), std::get<1>(shift)); + enable_arith(std::get<0>(shift), std::get<1>(shift)); + } else if (shift_type == "hexstr") { + parse_state->add_advance_from_data( + Data(cfg_parameters[0]["value"].asString())); + } else if (shift_type == "expression") { + ArithExpression expr; + build_expression(cfg_parameters[0]["value"], &expr); + expr.build(); + parse_state->add_advance_from_expression(expr); + } else { + throw json_exception( + EFormat() << "Parser 'advance' operation has unsupported " + << "parameter type '" << shift_type << "'", + cfg_parser_op); + } } else { throw json_exception( EFormat() << "Parser op '" << op_type << "'not supported", diff --git a/src/bm_sim/parser.cpp b/src/bm_sim/parser.cpp index a299ccf63..649f29eaf 100644 --- a/src/bm_sim/parser.cpp +++ b/src/bm_sim/parser.cpp @@ -445,6 +445,66 @@ struct ParserOpShift : ParserOp { } }; +template +struct ParserOpAdvance : ParserOp { + T shift_bytes; + + explicit ParserOpAdvance(const T& shift_bytes) + : shift_bytes(shift_bytes) { } + + void operator()(Packet *pkt, const char *data, + size_t *bytes_parsed) const override; +}; + +template<> +void +ParserOpAdvance::operator()(Packet *pkt, const char *data, + size_t *bytes_parsed) const { + (void) data; + // The JSON should never specify a negative constant as the shift amount. + assert(shift_bytes.sign() >= 0); + const auto shift_bytes_uint = shift_bytes.get(); + BMLOG_DEBUG_PKT(*pkt, "Advancing by {} bytes", shift_bytes_uint); + if (pkt->get_ingress_length() - *bytes_parsed < shift_bytes_uint) + throw parser_exception_core(ErrorCodeMap::Core::PacketTooShort); + *bytes_parsed += shift_bytes_uint; +} + +template<> +void +ParserOpAdvance::operator()(Packet *pkt, const char *data, + size_t *bytes_parsed) const { + (void) data; + static thread_local Data shift_bytes_data; + auto phv = pkt->get_phv(); + shift_bytes.eval(*phv, &shift_bytes_data); + // For a JSON file generated by a P4_16 compiler, the expression should never + // evaluate to a negative value, because the shift amount is of type bit<32>. + assert(shift_bytes_data.sign() >= 0); + const auto shift_bytes_uint = shift_bytes_data.get(); + BMLOG_DEBUG_PKT(*pkt, "Advancing by {} bytes", shift_bytes_uint); + if (pkt->get_ingress_length() - *bytes_parsed < shift_bytes_uint) + throw parser_exception_core(ErrorCodeMap::Core::PacketTooShort); + *bytes_parsed += shift_bytes_uint; +} + +template<> +void +ParserOpAdvance::operator()(Packet *pkt, const char *data, + size_t *bytes_parsed) const { + (void) data; + auto phv = pkt->get_phv(); + auto &f_shift = phv->get_field(shift_bytes.header, shift_bytes.offset); + // For a JSON file generated by a P4_16 compiler, the field should never + // evaluate to a negative value, because the shift amount is of type bit<32>. + assert(f_shift.sign() >= 0); + const auto shift_bytes_uint = f_shift.get(); + BMLOG_DEBUG_PKT(*pkt, "Advancing by {} bytes", shift_bytes_uint); + if (pkt->get_ingress_length() - *bytes_parsed < shift_bytes_uint) + throw parser_exception_core(ErrorCodeMap::Core::PacketTooShort); + *bytes_parsed += shift_bytes_uint; +} + template class ParseVSetCommon : public ParseVSetIface { using Lock = std::unique_lock; @@ -850,6 +910,22 @@ ParseState::add_set_from_expression(header_id_t dst_header, int dst_offset, new ParserOpSet(dst_header, dst_offset, expr)); } +void +ParseState::add_advance_from_data(const Data &shift_bytes) { + parser_ops.emplace_back(new ParserOpAdvance(shift_bytes)); +} + +void +ParseState::add_advance_from_expression(const ArithExpression &shift_bytes) { + parser_ops.emplace_back(new ParserOpAdvance(shift_bytes)); +} + +void +ParseState::add_advance_from_field(header_id_t shift_header, int shift_offset) { + parser_ops.emplace_back(new ParserOpAdvance( + field_t::make(shift_header, shift_offset))); +} + void ParseState::add_verify(const BoolExpression &condition, const ArithExpression &error_expr) { diff --git a/tests/test_parser.cpp b/tests/test_parser.cpp index 344ffbe48..c315108af 100644 --- a/tests/test_parser.cpp +++ b/tests/test_parser.cpp @@ -1787,13 +1787,14 @@ class ParserShiftTest : public ParserTestGeneric { protected: ParseState parse_state; HeaderType headerType; - header_id_t testHeader{0}; + header_id_t testHeader{0}, metaHeader{1}; ParserShiftTest() : parse_state("parse_state", 0), headerType("header_type", 0) { headerType.push_back_field("f8", 8); phv_factory.push_back_header("test", testHeader, headerType); + phv_factory.push_back_header("meta", metaHeader, headerType, true); } Packet get_pkt(const std::string &data) { @@ -1823,6 +1824,49 @@ TEST_F(ParserShiftTest, ShiftOneByte) { f.get()); } +TEST_F(ParserShiftTest, AdvanceByData) { + parse_state.add_advance_from_data(Data(1)); // 1-byte shift + parse_state.add_extract(testHeader); + + std::string packet_data("\xab\xcd"); + auto packet = get_pkt(packet_data); + parse_and_check_no_error(&packet); + const auto &f = packet.get_phv()->get_field(testHeader, 0); // f8 + ASSERT_EQ(static_cast(packet_data.at(1)), + f.get()); +} + +TEST_F(ParserShiftTest, AdvanceByExpression) { + ArithExpression expr; + expr.push_back_load_const(Data(1)); + expr.build(); + parse_state.add_advance_from_expression(expr); // 1-byte shift + parse_state.add_extract(testHeader); + + std::string packet_data("\xab\xcd"); + auto packet = get_pkt(packet_data); + parse_and_check_no_error(&packet); + const auto &f = packet.get_phv()->get_field(testHeader, 0); // f8 + ASSERT_EQ(static_cast(packet_data.at(1)), + f.get()); +} + +TEST_F(ParserShiftTest, AdvanceByField) { + parse_state.add_advance_from_field(metaHeader, 0); + parse_state.add_extract(testHeader); + + std::string packet_data("\xab\xcd"); + auto packet = get_pkt(packet_data); + + auto &f_shift = packet.get_phv()->get_field(metaHeader, 0); // f8 + f_shift.set(1); + + parse_and_check_no_error(&packet); + const auto &f = packet.get_phv()->get_field(testHeader, 0); // f8 + ASSERT_EQ(static_cast(packet_data.at(1)), + f.get()); +} + class ParserExtractVLTest : public ParserTestGeneric { protected: