Support expression of type as objectValue

[hesingh]

It's an enhancement for the behavioral-model to support an Array Index as an expression. The expression is a JsonObject and thus this repo's code needs to process the expression as an objectValue.

If one would like to test new JSON generated by p4c bmv2 backend with an expression type described above, see this PR which has changed p4c.

p4lang/p4c#2118

The P4 program used to generate new JSON is available at this repo:

https://github.com/IETF-Hackathon/p4-ipv6-switch-ml/blob/master/ipv6-switch-ml-bmv2.p4

[hesingh]

The JSON file generated from new p4c bmv2 backend via this PR (p4lang/p4c#2118) is attached. Please also look for DEREFERENCE_STACK keyword in the JSON file.

output-json.txt

[jafingerhut]

Attached files are a bit more convenient than multi-page text copy-and-paste for users of the content. You do have to rename them with a .txt suffix, or enclose them in a .zip file, for Github to take them as attachments.

[antoninbas]

A smaller example would have helped as well. I doubt you need such a large program to demonstrate the issue.
Right off the bat, I see 2 issues with the JSON:

• "DEREFERENCE_STACK" is not a thing, the JSON documentation states that the operand is "dereference_header_stack"
• I don't think a field reference like ["hdr", "ml", "idx"] is correct. Field references should be arrays of size 2 (aka 2-tuple).

[antoninbas]

BTW, I doubt that we need any changes to the supported JSON format or the P4Objects.cpp implementation. dereference_header_stack is really no different from other operands that can be used in expressions.

[hesingh]

A smaller example would have helped as well. I doubt you need such a large program to demonstrate the issue.
Right off the bat, I see 2 issues with the JSON:

• "DEREFERENCE_STACK" is not a thing, the JSON documentation states that the operand is "dereference_header_stack"
• I don't think a field reference like ["hdr", "ml", "idx"] is correct. Field references should be arrays of size 2 (aka 2-tuple).

When Chris Dodd suggested the above format in ``["hdr", "ml", "idx"]` , you said, "Chris figured out the format quick" which made me think the format is correct. This is why I developed the JSON output in this format. I can change the format to a tuple. Also, let me prepare a shorter P4 program.

[hesingh]

Attached files are a bit more convenient than multi-page text copy-and-paste for users of the content. You do have to rename them with a .txt suffix, or enclose them in a .zip file, for Github to take them as attachments.

A day or two back, github failed to attach a .txt file for me. This is why I gave up and had to add the contents of the file.

[hesingh]

BTW, I doubt that we need any changes to the supported JSON format or the P4Objects.cpp implementation. dereference_header_stack is really no different from other operands that can be used in expressions.

Isn't the new JSON format such that the expression is a JsonObject? Then how does P4Objects.cpp handle the expression?

[hesingh]

In the PR (p4lang/p4c#2118) branch, I trimmed down the P4 test to test runtime array index and checked in code.

Please see p4c/testdata/p4_16_samples/ipv6-switch-ml-bmv2.p4

The above P4 code includes p4c/testdata/p4_16_samples/ml-headers.p4

[hesingh]

After trimming the P4 program, I generated new JSON and attached the JSON output in this Issue - thanks. I will look into change p4c code next to generate a tuple.

[antoninbas]

Chris had the JSON structure right. It's fairly easy to figure out the exact syntax by looking at other generated JSON files and at the documentation.

Isn't the new JSON format such that the expression is a JsonObject? Then how does P4Objects.cpp handle the expression?

I don't understand the question, but looking at your new JSON file, I can see that even the basic structure is wrong:

"primitives" : [
        {
          "op" : "assign",
          "parameters" : [
            {
              "type" : "field",
              "value" : ["vector[0]", "e"]
            },
            {
              "type" : "field",
              "value" : [
                {
                  "type" : "header",
                  "value" : {
                    "value" : {
                      "op" : "DEREFERENCE_STACK",
                      "left" : {
                        "type" : "stack",
                        "value" : "pool_0"
                      },
                      "right" : {
                        "type" : "field",
                        "value" : ["hdr", "ml", "idx"]
                      }
                    },
                    "type" : "expression"
                  }
                }
              ]
            }
          ],

The second parameter must be an "expression" with a dereference_header_stack operator. I don't know why the outer type is a "field". That is clearly not a valid bmv2 JSON. It should look like this: https://github.com/p4lang/behavioral-model/blob/master/tests/testdata/unions_e2e_options_count.json#L312. Once more it's easy to generate examples IMO, just use a P4 expression that is already supported, such as <header>.<field> = <header>.<field> + X instead of <header.field> = <header>[X].<field>. Unless the compiler does some transformation, you should expect the JSON representations for these 2 assignments to have a similar structure, but with a different operator.

[hesingh]

• I don't think a field reference like ["hdr", "ml", "idx"] is correct. Field references should be arrays of size 2 (aka 2-tuple).

How do I store three entries in a array of size 2? What if the number of entries in the JSON Array is four? Please give an example for how four entries are stored in an array of size 2. Only thing I can think of is nesting, but how does nesting work with four entries?

[hesingh]

Chris had the JSON structure right. It's fairly easy to figure out the exact syntax by looking at other generated JSON files and at the documentation.

I didn't say that Chris was incorrect about the type being an expression. It's the three entries in the Json array of Chris' example that are not correct because today you said, the array can only have two entries.

[hesingh]

I don't understand the question, but looking at your new JSON file, I can see that even the basic structure is wrong:

"primitives" : [
        {
          "op" : "assign",
          "parameters" : [
            {
              "type" : "field",
              "value" : ["vector[0]", "e"]
            },
            {
              "type" : "field",
              "value" : [
                {
                  "type" : "header",
                  "value" : {
                    "value" : {
                      "op" : "DEREFERENCE_STACK",
                      "left" : {
                        "type" : "stack",
                        "value" : "pool_0"
                      },
                      "right" : {
                        "type" : "field",
                        "value" : ["hdr", "ml", "idx"]
                      }
                    },
                    "type" : "expression"
                  }
                }
              ]
            }
          ],

The second parameter must be an "expression" with a dereference_header_stack operator. I don't know why the outer type is a "field". That is clearly not a valid bmv2 JSON. It should look like this: https://github.com/p4lang/behavioral-model/blob/master/tests/testdata/unions_e2e_options_count.json#L312. Once more it's easy to generate examples IMO, just use a P4 expression that is already supported, such as <header>.<field> = <header>.<field> + X instead of <header.field> = <header>[X].<field>. Unless the compiler does some transformation, you should expect the JSON representations for these 2 assignments to have a similar structure, but with a different operator.

I changed code and generated new JSON which looks correct but has one extra type as expression - I will look into it. One type in the JSON is header because we have op as assign and the assignment is dealing with a header. The only issue left pending then is the array size of 2 and number of entries in array are greater than two.

"primitives" : [
        {
          "op" : "assign",
          "parameters" : [
            {
              "type" : "field",
              "value" : ["vector[0]", "e"]
            },
            {
              "type" : "expression",
              "value" : {
                "type" : "expression",
                "value" : [
                  {
                    "type" : "header",
                    "value" : {
                      "type" : "expression",
                      "value" : {
                        "op" : "dereference_header_stack",
                        "left" : {
                          "type" : "stack",
                          "value" : "pool_0"
                        },
                        "right" : {
                          "type" : "field",
                          "value" : ["hdr", "ml", "idx"]
                        }
                      }
                    }
                  }
                ]
              }
            }
          ],
          "source_info" : {
            "filename" : "../testdata/p4_16_samples/ipv6-switch-ml-bmv2.p4",
            "line" : 99,
            "column" : 8,
            "source_fragment" : "hdr.vector[0].e = pool[hdr.ml.idx].val"

[jafingerhut]

@hesingh I am not sure if this "array of size 2" thing you are mentioning is the same thing that @antoninbas is describing.

The header stack arrays can be any finite size, 2 or otherwise.

The array size 2 limit that I think Antonin is mentioning is the list of strings like ["hdr", "ml", "idx"] in the BMv2 JSON file, which if I understand correctly he believes is not supported by simple_switch.

[hesingh]

@hesingh I am not sure if this "array of size 2" thing you are mentioning is the same thing that @antoninbas is describing.

The header stack arrays can be any finite size, 2 or otherwise.

The array size 2 limit that I think Antonin is mentioning is the list of strings like ["hdr", "ml", "idx"] in the BMv2 JSON file, which if I understand correctly he believes is not supported by simple_switch.

@jafingerhut The array size I am mentioning has got nothing to do with the header stack size. The P4 code is pool[hdr.ml.idx] and the hdr.ml.idx is stored in a json array as three strings. If the json array can only contain two entries, the indirection of hrd.ml.idx will not work. This is why, with my JSON output, I publish the source_info which includes the line of P4 code related to the JSON. The source_info shows the indirection.

[jafingerhut]

@hesingh You can choose to write names like foo.bar.baz.gah.guh.blech in P4_16 if you have deeply enough nested instances, but regardless of how many parts there are to the names in the P4_16 source code, they all get flattened down to 2 parts in the BMv2 JSON. That is an implementation detail of BMv2 JSON files, and has nothing to do with anything in the p4_16 language spec.

[hesingh]

@hesingh You can choose to write names like foo.bar.baz.gah.guh.blech in P4_16 if you have deeply enough nested instances, but regardless of how many parts there are to the names in the P4_16 source code, they all get flattened down to 2 parts in the BMv2 JSON. That is an implementation detail of BMv2 JSON files, and has nothing to do with anything in the p4_16 language spec.

Ah, got it - thanks! Now I need to find the implementation details in bmv2 to figure out how to setup my 2-entry JSON array.

[hesingh]

It should look like this: https://github.com/p4lang/behavioral-model/blob/master/tests/testdata/unions_e2e_options_count.json#L312. Once more it's easy to generate examples IMO, just use a P4 expression that is already supported, such as <header>.<field> = <header>.<field> + X instead of <header.field> = <header>[X].<field>. Unless the compiler does some transformation, you should expect the JSON representations for these 2 assignments to have a similar structure, but with a different operator.

The JSON example you gave from behavioral-model does not have the P4 code I could find. Using P4 code and generating JSON, one gets the line of code as source_info to understand what code generated what JSON. I agree, some JSON code can still be understood without P4 source code.

I did what you suggested and see the generated JSON using default p4c (does not have my code changes). The line of code is at the bottom of the log. The P4 code is hdr.ipv6.version, but only ipv6 and version are in the json array: "value" : ["ipv6", "version"]. In Chris's note, hdr is also included. I changed my p4c code to not include the path any more.

"primitives" : [
        {
          "op" : "assign",
          "parameters" : [
            {
              "type" : "field",
              "value" : ["ipv6", "version"]
            },
            {
              "type" : "expression",
              "value" : {
                "type" : "expression",
                "value" : {
                  "op" : "&",
                  "left" : {
                    "type" : "expression",
                    "value" : {
                      "op" : "+",
                      "left" : {
                        "type" : "field",
                        "value" : ["ipv6", "version"]
                      },
                      "right" : {
                        "type" : "hexstr",
                        "value" : "0x02"
                      }
                    }
                  },
                  "right" : {
                    "type" : "hexstr",
                    "value" : "0x0f"
                  }
                }
              }
            }
          ],
          "source_info" : {
            "filename" : "/tmp/p4c/testdata/p4_16_samples/ipv6-switch-ml-bmv2.p4",
            "line" : 101,
            "column" : 10,
            "source_fragment" : "hdr.ipv6.version = hdr.ipv6.version + 2"
          }
        }
      ]

I have another JSON issue to fix for this code pool[hdr.ml.idx].val. The val member is not included in any JSON yet.

[hesingh]

@antoninbas After more code changes, the right side of assignment statement has its val emitted in JSON as well. This is what Chris had also included. See the output json below. However, this JSON still fails behavioral-model testing with the following error:

terminate called after throwing an instance of 'std::runtime_error'
  what():  in Json::Value::operator[](char const*)const: requires objectValue

"primitives" : [
        {
          "op" : "assign",
          "parameters" : [
            {
              "type" : "field",
              "value" : ["vector[0]", "e"]
            },
            {
              "type" : "expression",
              "value" : {
                "type" : "expression",
                "value" : [
                  {
                    "type" : "expression",
                    "value" : {
                      "op" : "dereference_header_stack",
                      "left" : {
                        "type" : "stack",
                        "value" : "pool_0"
                      },
                      "right" : {
                        "type" : "field",
                        "value" : ["ml", "idx"]
                      }
                    }
                  },
                  "val"
                ]
              }
            }
          ],
          "source_info" : {
            "filename" : "../testdata/p4_16_samples/ipv6-switch-ml-bmv2.p4",
            "line" : 99,
            "column" : 8,
            "source_fragment" : "hdr.vector[0].e = pool[hdr.ml.idx].val"
          }
        }
      ]

[hesingh]

@antoninbas If the comment on the following line is correct

behavioral-model/third_party/jsoncpp/include/jsoncpp/json.h

Line 468 in 7192f75

* values of an #objectValue or #arrayValue can be accessed using operator[]()

Then why doesn't the line of code below also include and OR for arrayValue?

https://github.com/p4lang/behavioral-model/blob/master/third_party/jsoncpp/src/jsoncpp.cpp#L3506

[antoninbas]

So I had some more time to look into this issue. What I found is that bmv2 supports runtime indices for header stack access as long as the expression is used as a r-value and not a l-value. In other words:

• <hs>[<index expression>].<field> = X is not supported, but
• <h>.<field> = <hs>[<index expression>].<field> is supported just fine using the access_field and dereference_header_stack operations.

For the r-value case, I verified support by following my own advice... I started with the following P4 program:

#include <v1model.p4>

header h1_t {
    bit<32> v;
    bit<32> idx;
}

header h2_t {
    bit<32> v;
}

struct headers {
    h1_t h1;
    h2_t[3] h2;
}

struct metadata { }

parser parserI(packet_in pkt,
               out headers hdr,
               inout metadata meta,
               inout standard_metadata_t stdmeta) {
    state start {
        pkt.extract(hdr.h1);
        pkt.extract(hdr.h2[0]);
        pkt.extract(hdr.h2[1]);
        pkt.extract(hdr.h2[2]);
        transition accept;
    }
}

control cIngress(inout headers hdr,
                 inout metadata meta,
                 inout standard_metadata_t stdmeta) {
    apply {
        // hdr.h1.v = hdr.h2[hdr.h1.idx].v;
        hdr.h1.v = hdr.h2[1].v;
        stdmeta.egress_spec = stdmeta.ingress_port;
    }
}

control cEgress(inout headers hdr,
                inout metadata meta,
                inout standard_metadata_t stdmeta) {
    apply { }
}

control vc(inout headers hdr,
           inout metadata meta) {
    apply { }
}

control uc(inout headers hdr,
           inout metadata meta) {
    apply { }
}

control DeparserI(packet_out packet,
                  in headers hdr)
{
    apply {
        packet.emit(hdr);
    }
}

V1Switch(parserI(),
         vc(),
         cIngress(),
         cEgress(),
         uc(),
         DeparserI()) main;

I generated the JSON for it the hand-modified it to implement the hdr.h1.v = hdr.h2[hdr.h1.idx].v assignment (which is commented-out in the P4 program above). I was able to observe the expected behavior when generating test packets with scapy.

For reference, this is the original JSON:
runtime_index.1.json.txt
This is the hand-modified JSON:
runtime_index.2.json.txt

As for l-value support, this requires a big change to the bmv2 code. The "expression evaluation engine" currently only returns values, not references. I'll open an issue for this but I don't really have the time to look into it these days.

[antoninbas]

BTW, here is the relevant part of the JSON file:

      "primitives" : [
        {
          "op" : "assign",
          "parameters" : [
            {
              "type" : "field",
              "value" : ["h1", "v"]
            },
            {
              "type" : "expression",
              "value": {
                  "type": "expression",
                  "value": {
                      "op": "access_field",
                      "left": {
                          "type": "expression",
                          "value": {
                              "op": "dereference_header_stack",
                              "left": {
                                  "type": "header_stack",
                                  "value": "h2"
                              },
                              "right": {
                                  "type": "field",
                                  "value": ["h1", "idx"]
                              }
                          }
                      },
                      "right": 0
                  }
              }
            }
          ],
          "source_info" : {
            "filename" : "runtime_index.p4",
            "line" : 37,
            "column" : 8,
            "source_fragment" : "hdr.h1.v = hdr.h2[hdr.h1.idx].v"
          }
        }

[antoninbas]

I opened a new Github issue for the non-supported case I described: #838

I will close this issue now. The title is not really helpful and it's important to make the difference between the case which is already supported and the case which is not.