You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I wanted to create an issue to record some thoughts for small clean-up kinds of changes to the PSA implementation code, while reading and reviewing it. All of the comments are for code in the file targets/psa_switch/psa_switch.cpp.
Right now the field psa_ingress_input_metadata.ingress_timestamp is assigned a value in several different places. It seems cleaner to do so exactly once, in the method ingress_thread(), just before performing the ingress_parser processing. That is the time that the PSA spec says should be recorded, regardless of whether the packet is new from a port, resubmitted, or recirculated. By recording it earlier in other places, there might be significant packet queueing latency between when it is recorded, and when parsing begins.
Filling in the value of egress_timestamp should be done similarly: in one place in the code, not several, just before performing the egress_parser processing.
Right now when a packet is recirculated, there is code that explicitly assigns values to psa_ingress_input_metadata.ingress_port and to psa_ingress_input_metadata.packet_path. It seems those code be removed, since ingress_thread will assign the same values to those fields, copied from the corresponding fields of psa_ingress_parser_input_metadata.
TBD: For a recirculated packet, when it goes through the ingress_thread() method after being recirculated, does the assignment port_t ingress_port = packet->get_ingress_port() return PSA_PORT_RECIRCULATE? It should, but if it does that, how?
The text was updated successfully, but these errors were encountered:
@peteli3 I wanted to record these comments somewhere, but I do not think they necessarily need to be done in the current PR you are working on for PSA resubmit.
Regarding my TBD question above, I have confirmed by running the test program psa-recirculate-no-meta-bmv2.p4 with a simple test packet, and console logging enabled with the --log-console option, that after a recirculate operation is done on the packet, the log line prints
Processing packet received on port 0
just like it did when the packet first arrived. I would expect that after correction, it should print:
Processing packet received on port 4294967290
where 4294967290 is the decimal value of the hex 0xfffffffa == PSA_PORT_RECIRCULATE.
I wanted to create an issue to record some thoughts for small clean-up kinds of changes to the PSA implementation code, while reading and reviewing it. All of the comments are for code in the file targets/psa_switch/psa_switch.cpp.
Right now the field psa_ingress_input_metadata.ingress_timestamp is assigned a value in several different places. It seems cleaner to do so exactly once, in the method ingress_thread(), just before performing the ingress_parser processing. That is the time that the PSA spec says should be recorded, regardless of whether the packet is new from a port, resubmitted, or recirculated. By recording it earlier in other places, there might be significant packet queueing latency between when it is recorded, and when parsing begins.
Filling in the value of egress_timestamp should be done similarly: in one place in the code, not several, just before performing the egress_parser processing.
Right now when a packet is recirculated, there is code that explicitly assigns values to psa_ingress_input_metadata.ingress_port and to psa_ingress_input_metadata.packet_path. It seems those code be removed, since ingress_thread will assign the same values to those fields, copied from the corresponding fields of psa_ingress_parser_input_metadata.
TBD: For a recirculated packet, when it goes through the ingress_thread() method after being recirculated, does the assignment
port_t ingress_port = packet->get_ingress_port()
return PSA_PORT_RECIRCULATE? It should, but if it does that, how?The text was updated successfully, but these errors were encountered: