New 'advance' parser operation

This new operation is more powerful than 'shift', which can only shift
by a fixed amount of bytes. 'advance' takes as a parameter either an
hexadecimal string ('hexstr'), a field ('field'), or an arithmetic
expression ('expression').

The JSON documentation was updated to reflect this change, and the
version number was bumped up to 2.19.

Fixes #727

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:

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<typename T,
@@ -175,7 +179,7 @@ class Data {
     export_bytes();  // not very efficient for fields, we import then export...
   }
 
-  //! Convert the value of Data to any inegral type
+  //! Convert the value of Data to any integral type
   template<typename T,
            typename std::enable_if<std::is_integral<T>::value, int>::type = 0>
   T get() const {

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);

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<size_t>(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",

src/bm_sim/parser.cpp

@@ -445,6 +445,66 @@ struct ParserOpShift : ParserOp {
   }
 };
 
+template <typename T>
+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<Data>::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<size_t>();
+  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<ArithExpression>::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<size_t>();
+  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<field_t>::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<size_t>();
+  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 <typename P, bool with_padding = true>
 class ParseVSetCommon : public ParseVSetIface {
   using Lock = std::unique_lock<std::mutex>;
@@ -850,6 +910,22 @@ ParseState::add_set_from_expression(header_id_t dst_header, int dst_offset,
       new ParserOpSet<ArithExpression>(dst_header, dst_offset, expr));
 }
 
+void
+ParseState::add_advance_from_data(const Data &shift_bytes) {
+  parser_ops.emplace_back(new ParserOpAdvance<Data>(shift_bytes));
+}
+
+void
+ParseState::add_advance_from_expression(const ArithExpression &shift_bytes) {
+  parser_ops.emplace_back(new ParserOpAdvance<ArithExpression>(shift_bytes));
+}
+
+void
+ParseState::add_advance_from_field(header_id_t shift_header, int shift_offset) {
+  parser_ops.emplace_back(new ParserOpAdvance<field_t>(
+      field_t::make(shift_header, shift_offset)));
+}
+
 void
 ParseState::add_verify(const BoolExpression &condition,
                        const ArithExpression &error_expr) {

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<unsigned char>());
 }
 
+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<unsigned char>(packet_data.at(1)),
+            f.get<unsigned char>());
+}
+
+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<unsigned char>(packet_data.at(1)),
+            f.get<unsigned char>());
+}
+
+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<unsigned char>(packet_data.at(1)),
+            f.get<unsigned char>());
+}
+
 
 class ParserExtractVLTest : public ParserTestGeneric {
  protected: