Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
rmiguelc opened this issue Feb 24, 2023 · 24 comments
Open

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

rmiguelc opened this issue Feb 24, 2023 · 24 comments

Comments

@rmiguelc
Copy link

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
Copy link
Collaborator

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

This is the root cause of the issue: p4lang/PI#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
Copy link
Author

@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
Copy link
Collaborator

  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
Copy link
Author

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
Copy link
Collaborator

"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
Copy link
Author

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! 😃

@jafingerhut
Copy link
Collaborator

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

@rmiguelc
Copy link
Author

rmiguelc commented Mar 30, 2023

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:

  • Inserted a new packet_in header in my P4 program:
@controller_header("packet_in")
header packet_in_h {
    packet_type_t packet_type;
    bit<48> m2;
    bit<16> m3;
    bit<32> m4;
} //No need for further annotations: the switch knows it must send 4 metadata fields
  • Edited utils/p4runtime_switch.py, function start:
def start(self, controllers):
    args.append("--cpu-port 510") #Added by me after all other args.append() calls
  • Then I edited p4runtime_lib/switch.py to provide a method to read a message:
def fetchMessage(self):
    return self.stream_msg_resp.next() #Blocking call
  • Finally, modified my Python controller to call the method and print the messages. The message type received is StreamMessageResponse.
// This message is already defined in p4runtime.proto file.
// I'm just quoting it for reference
message StreamMessageResponse {
  oneof update {
    MasterArbitrationUpdate arbitration = 1;
    PacketIn packet = 2;
    DigestList digest = 3;
    IdleTimeoutNotification idle_timeout_notification = 4;
    .google.protobuf.Any other = 5;
    // Used by the server to asynchronously report errors which occur when
    // processing StreamMessageRequest messages.
    StreamError error = 6;
  }
}
# In your Python controller, define a new function and write:
msg = sw.fetchMessage()
msg_type = msg.WhichOneof('update') #gRPC method to find which of the oneof fields is set

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
Copy link
Collaborator

@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
Copy link
Author

rmiguelc commented Apr 3, 2023

@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
Copy link
Collaborator

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
Copy link

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
Copy link

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
Copy link

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
Copy link

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
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator

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
Copy link

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link

sunruifeng0602 commented Mar 2, 2024

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants