p4lang / tutorials

P4 language tutorials
Apache License 2.0
1.36k stars 887 forks source link

gRPC Error: Register reads are not supported yet (UNIMPLEMENTED) #497

Open rmiguelc opened 1 year ago

rmiguelc commented 1 year ago

I was toying with the P4Runtime exercise and attempted to make it read a register, by editing switch.py with the following code and calling it from my controller:

def readRegister(self, p4info_helper, name, index): 
  request = p4runtime_pb2.ReadRequest()
  request.device_id = self.device_id
  entity = request.entities.add()
  register_entry = entity.register_entry
  register_entry.register_id = p4info_helper.get_registers_id(name)
  register_entry.index.index = index

  print("P4Runtime request: ", request)

  responseStream = self.client_stub.Read(request)

  for response in responseStream:
    print(response)

Unfortunately, the response was that:

gRPC Error: Register reads are not supported yet (UNIMPLEMENTED) [./mycontroller.py:239]

Is this something that might change in the future? How would one go about implementing this RPC call?

jafingerhut commented 1 year ago

It is something that might change in the future, yes, and I hope it does.

This is the root cause of the issue: https://github.com/p4lang/PI/issues/376. As you can see, there have been a volunteer or two since 2018 that has thought about taking up the task of making this enhancement, but no one has completed it yet.

As a workaround, it is possible to use PacketIn and PacketOut packets between controller and network device to inject packets into the P4-programmable device that read and/or write the register, and return read values back to the controller in response packets. A demonstration of that workaround can be found here: https://github.com/jafingerhut/p4-guide/tree/master/ptf-tests/registeraccess

rmiguelc commented 1 year ago

@jafingerhut Thanks for the response earlier. I wanted to use the PackinIn and PacketOut approach, but I've got three questions:

  1. I am also using mininet. Is it possible to pass the --cpu-port parameter to the switches? Figures, one might hammer the command directly into the util source code, but is there a better way to do it?

  2. The controller I'm using was adapted from exercises/p4runtime, but it connects to the switches on ports 50051, 50052, and so on, to push rules to the switches' tables. These are transport layer ports, correct? How to connect them to the ethernet port 510 of each (mininet) switch, so that the switch can also send data to the controller?

  3. It seems the way of sending messages to the switch using the p4runtime example differs from how you implemented it in your ptf-tests. Concretely, in p4runtime we are constructing messages by hand, whereas in your examples it seems we are relying on the p4runtime_sh.shell package. Is either way okay to implement switch-controller communication?

jafingerhut commented 1 year ago
  1. The tutorials technique of constructing Protobuf messages using the P4Runtime schema works, or any other method that sends similar sequences of bytes over the TCP connection with the switch's P4Runtime server listening port. p4runtime_sh.shell is just a different Python library for turning method calls and/or data structures into the appropriate Protobuf messages. There tend to be many different small "wrapper libraries" for creating and parsing Protobuf messages, depending upon who wrote the code.

  2. The way simple_switch_grpc works, you cannot make a P4Runtime API connection to the switch's CPU port. You MUST make the P4Runtime API connection over TCP to the appropriate TCP port on which the simple_switch_grpc is listening for incoming P4Runtime API connections. The default TCP port it listens on is 9559 if you do not specify one on the command line. For the multi-switch exercises in the tutorials repo, they explicitly start each simple_switch_grpc process listening on a different TCP port from each other, because if they all tried listening on the same TCP port, they would be running on the same Linux network namespace and would conflict with each other.

When you specify --cpu-port, you are specifying a port number that when the P4 code sends a packet there, the simple_switch_grpc process says "hey, that packet was sent to the CPU port. Run this bit of code to turn that packet into a PacketIn message and send it over the P4Runtime API TCP connection to the controller program". Also, if the controller sends a PacketOut message to simple_switch_grpc oer the P4Runtime API TCP connection, simple_switch_grpc runs some code that does "Oh, there is a --cpu-port configured. Turn this PacketOut message into a packet that I will then send into the switch on the cpu-port, so the P4 program can receive and process it."

  1. Unless the existing tutorials Python code already provides some way to cause it to run simple_switch_grpc processes with the --cpu-port option, the only way I know is to modify that Python code so that it does.
rmiguelc commented 1 year ago

Many thanks for the fast response!

When you specify --cpu-port, you are specifying a port number that when the P4 code sends a packet there, the simple_switch_grpc process says "hey, that packet was sent to the CPU port. Run this bit of code to turn that packet into a PacketIn message and send it over the P4Runtime API TCP connection to the controller program".

Alright, so that means the simple_switch_grpc target knows how to fit in the pipes automatically, correct? I'm assuming "this bit of code" is in the switch, and does not have to be programmed by the user?

jafingerhut commented 1 year ago

"this bit of code" I mentioned is included as part of the simple_switch_grpc code. You do not need to write any code to make it work, other than use --cpu-port <number> on the command line when you start it, and you need to write P4 code that can either (a) send packets to this CPU port, or (b) receive packets from this CPU port, or (c) both (a) and (b).

It is up to you and your desired use case which packets you want to send to the CPU port and what their contents are, e.g. what custom packet metadata fields do you want to include with such packets? What headers are included vs. not included?

Similarly it is up to you what packets you want your controller software to send as PacketOut messages (if any), and for any of those, to write your P4 code such that it can process them when they arrive on the CPU port in a way that makes sense for your use case.

I do not know what you mean by "knows how to fit in the pipes automatically".

rmiguelc commented 1 year ago

I do not know what you mean by "knows how to fit in the pipes automatically".

Just that it knows packets whose egress_spec is the CPU_PORT must be forwarded to the controller, and that packets received from the controller are supposed to be injected onto the switch with ingress_port=CPU_PORT.

It is up to you and your desired use case which packets you want to send to the CPU port and what their contents are, e.g. what custom packet metadata fields do you want to include with such packets? What headers are included vs. not included?

Understood. I studied the examples in packetinout and registeraccess. In our P4 code, these custom packet metadata fields are defined in special headers annotated with @controller_header("packet_out") or @controller_header("packet_in"). When implementing a custom action to send to the controller, we must set the egress_spec to the CPU_PORT, and optionally invoke setValid() on the packet_in header and fill in its fields. (This information is for others that may read this issue; let me know if it is correct.)

Thank you for all the help! :smiley:

jafingerhut commented 1 year ago

Everything you said in your previous comment looks correct to me.

rmiguelc commented 1 year ago

I was about to post one more question, but I managed to solve it in the meantime. For anyone attempting this workaround using the utils provided by the tutorials environment, here's how I went about it:

if msg_type == 'packet': print('Correct packet_in stream response detected! Processing packet...')

metadata = msg.packet.metadata
payload = msg.packet.payload

...

else: print('Message type was ' + msg_type + '. Only \'packet\' is accepted. Skipping.')



**IMPORTANT:** Do not forget to invoke `.setValid()` on the header. Furthermore, it must be deparsed before any other headers.

The question (meanwhile solved) had to do with metadata fields being apparently the wrong size. For example, if I write `print(metadata)` in the above function, the output will be something like:
> [metadata_id: 1
value: "\001"
, metadata_id: 2
value: "\377\000\000\000\000\000"
, metadata_id: 3
value: "\377\002"
, metadata_id: 4
value: "\000"
]

Giving the impression that the last metadata field is wrong in size (1 byte, when it should be 4 per our definition above). If metadata fields are sent with the most significant bits set, it correctly prints all 4 bytes. I suspect this is just Python3 abbreviating the output. Keep in mind also that the notation used is apparently octal and may print ASCII characters, which was confusing at first, since I was sending raw hexadecimal output through Scapy like: ``scapy sendp(Ether()/IP()/b'\x77\x00', iface=eth0)``, just don't forget the little preceding 'b' otherwise the payload might not be what you expect.

If something here appears wrong, let me know!
jafingerhut commented 1 year ago

@rmiguelc If you get all of this working, and you are interested, it might be of interest to add a new exercise in the exercises directory that demonstrates these things in the context of this repo.

rmiguelc commented 1 year ago

@jafingerhut Sounds like a good idea! Is it okay for people following the tutorials to edit files in the utility directories? I may have missed it, but there doesn't seem to be a way to set the cpu port through the makefile or the topology JSON.

jafingerhut commented 1 year ago

I think that requiring a tutorial follower to edit such files is a reasonable way to start for you writing up such steps. You could even supply those changes as a patch file, e.g. the output of a command like "diff -c orig_version edited_version > changes-to-make.patch", and then your instructions could say "run this command in the appropriate directory: patch -p1 < changes-to-make.patch".

There may be good ways to avoid such steps, e.g. if we found that there was already a way to provide the cpu port through the makefile or topology JSON (there might be in the topology JSON file a way to provide additional command line arguments to simple_switch_grpc processes, but I am not sure). Or we could consider making changes to various Python files that enabled such options for every exercise in the future.

Suuuuuukang commented 1 year ago

I'm just looking the same problem of reading registers by controller, this does help me a lot, though I didn't finish my own test yet. Many thanks to you all!

Suuuuuukang commented 1 year ago

I was about to use part of code in https://github.com/jafingerhut/p4-guide/tree/master/ptf-tests/registeraccess, but I just met an error 'ModuleNotFoundError: No module named 'p4runtime_shell_utils'. Before meeting it, I finished installing p4runtime-shell. How can I solve this problem by adding some source files or installing some package?

Suuuuuukang commented 1 year ago

I was about to use part of code in https://github.com/jafingerhut/p4-guide/tree/master/ptf-tests/registeraccess, but I just met an error 'ModuleNotFoundError: No module named 'p4runtime_shell_utils'. Before meeting it, I finished installing p4runtime-shell. How can I solve this problem by adding some source files or installing some package?

Sorry, I find this file in 'testlib', maybe it's because I don't add files correctly. Btw, thanks a lot for this magic solution of packet_in and packet_out, which is another function I need.

Suuuuuukang commented 1 year ago

Now I have a new problem. After I successfully made packet_in and packet_out by 'import p4runtime_sh.shell as sh', and I test my function that write and read value. I can see from s1.log that the register was set to right value, but another operation can not find this value. For example, I set index 1 to 12, while reading the value before set, as 0. [22:24:21.343] [bmv2] [T] [thread 83760] [118.0] [cxt 0] multi-tcp.p4(443) Primitive last_route_selected.read(val, hdr.packet_out.operand1) [22:24:21.343] [bmv2] [T] [thread 83760] [118.0] [cxt 0] Read register 'MyIngress.last_route_selected' at index 1 read value 0 [22:24:21.343] [bmv2] [T] [thread 83760] [118.0] [cxt 0] multi-tcp.p4(445) Primitive last_route_selected.write( ... [22:24:21.343] [bmv2] [T] [thread 83760] [118.0] [cxt 0] Wrote register 'MyIngress.last_route_selected' at index 1 with value 12 But when I try another funtion after this call, the set value '12' disappeared, as log follows. [22:24:52.359] [bmv2] [T] [thread 83760] [120.0] [cxt 0] Read register 'MyIngress.last_route_selected' at index 1 read value 0 [22:24:52.359] [bmv2] [I] [thread 83760] Processing packet-out after-read 0 I want to know how to solve this problem, or using 'sh' method can not make changes in Register for use at any time?

jafingerhut commented 1 year ago

I would recommend double-checking those logs to ensure that (a) nothing ELSE made writes to that register between those two times, and (b) all of the accesses you are looking at are from the log for the SAME simple_switch_grpc process.

If you are running a mininet network of multiple simple_switch_grpc processes, for example, each of those has independent contents for all tables and P4 registers.

If one simple_switch_grpc process is killed or exits for any reason, and you start a new one, then the initial contents of all P4 registers is by default 0 when the P4 program is loaded into the software switch.

Suuuuuukang commented 1 year ago

Many thanks to you! I manage it by deleting 'config' attribute when calling 'sh.setup', which is

image

And then it works! I can easily get the register value same as what I set before. Thanks again and please forgive my poor english lol.

jafingerhut commented 1 year ago

If I remember correctly, the config parameter to the setup method causes the controller to call a P4Runtime API message called SetForwardingPipelineConfig, which replaces any current P4 program loaded into the device with a new one. That also causes all P4 registers to be cleared to an initial state of 0 in the new P4 program loaded (or the same P4 program loaded, if you load the same one multiple times).

Suuuuuukang commented 1 year ago

If I remember correctly, the config parameter to the setup method causes the controller to call a P4Runtime API message called SetForwardingPipelineConfig, which replaces any current P4 program loaded into the device with a new one. That also causes all P4 registers to be cleared to an initial state of 0 in the new P4 program loaded (or the same P4 program loaded, if you load the same one multiple times).

Yes, I have another python file which is based on tutorial/p4runtime, and that file directly calls SetForwardingPipelineConfig function, and I use sh to make packet_in and packet_out, which doesn't need any config attribute. Now I do understand this situation. Thx!

rmiguelc commented 1 year ago

Hey there! I'm glad this thread has helped others. I still intend to write an exercise for p4lang/tutorials on sending packets from the switch to the controller. An idea is a simple L2 switch that learns MAC addresses from incoming packets. Keeps P4 code at its simplest, and the focus on the goal of the exercise, while implementing something that is both familiar and realistic.

In my work, I've come across the need to also inject packets onto the switch from the controller. Unfortunately, my attempts so far have been unsuccessful. Here is the Python3 code I programmed into switch.py:

def pktOut(self, payload, *metadata): 
  # The metadata parameter is a series of tuples whose first and second elements
  # are its value and byte length respectively.
  # e.g. sw.pktOut(payload, (20, 2), (47, 8))
  request = p4runtime_pb2.StreamMessageRequest()
  request.packet.payload = payload
  for i in range(len(metadata)):
    m = request.packet.metadata.add()
    m.metadata_id = i+1
    m.value = int.to_bytes(metadata[i][0], metadata[i][1], 'big')
  print(request)
  self.requests_stream.put(request)

StreamMessageRequests are the same messages used to issue Master Arbitration Updates. When running the controller and using this function, the switch prints the following to its log:

[13:18:18.100] [bmv2] [W] [thread 4714] [P4Runtime] p4::tmp::P4DeviceConfig is deprecated [13:18:18.112] [bmv2] [D] [thread 4714] Set default default entry for table 'tbl_act': act - [13:18:18.112] [bmv2] [D] [thread 4714] Set default default entry for table 'tbl_drop': TopIngress.drop - [13:18:18.112] [bmv2] [D] [thread 4714] Set default default entry for table 'tbl_rmoc395': rmoc395 - [13:18:18.112] [bmv2] [D] [thread 4714] Set default default entry for table 'tbl_rmoc399': rmoc399 - [13:18:18.112] [bmv2] [D] [thread 4714] Set default default entry for table 'tbl_drop_0': TopIngress.drop - [13:18:18.112] [bmv2] [D] [thread 4714] Set default default entry for table 'tbl_rmoc408': rmoc408 - [13:18:18.112] [bmv2] [D] [thread 4714] Set default default entry for table 'tbl_drop_1': TopIngress.drop - (...goes on for a few more lines) [13:18:18.116] [bmv2] [D] [thread 4714] simple_switch target has been notified of a config swap

I searched p4lang for this message, "p4::tmp::P4DeviceConfig is deprecated", and it found: https://github.com/p4lang/PI/blob/dc9c75f3017535f1892cdd758a80a46ded6ce4b6/proto/frontend/src/device_mgr.cpp#LL476C30-L476C30

Since both the message and that function refer to device config, I begin to suspect the request was treated as an arbitration update and not as a PacketOut, even after confirming the type is packet with print(request.WhichOneof('update')). Are we dealing with a use case that has not been implemented, or am I doing something wrong? @jafingerhut any insight on this?

jafingerhut commented 1 year ago

I have a PTF test here: https://github.com/jafingerhut/p4-guide/tree/master/ptf-tests/packetinout

that exercises PacketOut (i.e. from controller to switch) and PacketIn (i.e. from switch to controller) P4Runtime API messages. I would recommend going through the code there, and in the p4-guide/testlib/base_test.py file that it uses, to see what you can learn about how it generates PacketOut messages, and parses incoming PacketIn messages.

rmiguelc commented 1 year ago

I managed to do it! For those interested, here's what I implemented in switch.py:

    def pktOut(self, p4info_helper, payload, *metadata):
        """
        Sends a PACKET_OUT from the controller to the switch.
        The metadata parameter corresponds to the PACKET_OUT fields defined by
        you in your P4 program, and should be filled with tuples containing
        value and length (in bytes) of the field respectively.

        For example, imagine PACKET_OUT was defined in the P4 program as:
        @controller_header("packet_out")
        header pktout_h {
          bit<8> opcode;
          bit<48> m1;
        }

        Then this method should be invoked as follows: 
          sw.pktOut(p4info_helper, payload, (1,1), (200, 6))

        i.e. (opcode=1 in 1 byte, m1=200 in 6 bytes)
        """
        packet_out = p4runtime_pb2.PacketOut()
        packet_out.payload = payload

        metadata_list = []
        for metadata_id, (m_val, m_len) in enumerate(metadata, 1):
        # Enumerate is just an easy way to combine a for-loop with a range-loop
        # 'metadata' is the iterable we wish to iterate on, 1 is the starting
        # value for 'metadata_id'.
            item = p4runtime_pb2.PacketMetadata()
            item.metadata_id = metadata_id
            item.value = m_val.to_bytes(m_len, 'big')
            metadata_list.append(item)
        packet_out.metadata.extend(metadata_list)

        request = p4runtime_pb2.StreamMessageRequest()
        request.packet.CopyFrom(packet_out)

        self.requests_stream.put(request)
sunruifeng0602 commented 8 months ago

I studied the examples in packetinout and registeraccess. In registeraccess,a value is read each time through the index of the register. Is there any way to return the entire register? In some cases, it is incorrect to only return the value of one register. But I did not find a solution in the appeal case. In v1model. p4, the given method register. read() is as follows.

    /***
     * read() reads the state of the register array stored at the
     * specified index, and returns it as the value written to the
     * result parameter.
     *
     * @param index The index of the register array element to be
     *              read, normally a value in the range [0, size-1].
     * @param result Only types T that are bit<W> are currently
     *              supported.  When index is in range, the value of
     *              result becomes the value read from the register
     *              array element.  When index >= size, the final
     *              value of result is not specified, and should be
     *              ignored by the caller.
     */
#if V1MODEL_VERSION >= 20200408
    void read(out T result, in I index);
#else
    void read(out T result, in bit<32> index);
jafingerhut commented 8 months ago

I am not aware of any P4 implementation that can read the entire contents of the register in a single API call, atomically.

There are ways to have an "active copy" of a register and a "currently unused" copy of the same number of elements, and swap those at control-plane-time chosen events, and then read the "currently unused" copy one element at a time. Because it is the currently unused copy and not being updated by the data plane, you can read all of its elements one at a time, and known that none of them are changing, and so get a consistent snapshot of all entries, but of course it is the currently unused copy, so is gradually getting more and more stale over time.