Added Recirculation functionality in PNA_NIC

[rupesh-chiluka-marvell]

Part of #1245 issue.

Need to add below declaration in p4c/p4include/bmv2/pna.p4:

const PortId_t PNA_PORT_RECIRCULATE = (PortId_t) 0xfffffffa;

[jafingerhut]

All of this looks correct as far as I can tell. Future new P4 program test cases that exercise this new code with p4c compiling the program, then packets passing through pna_nic in the expected ways, are the best assurance of that.

[rupesh-chiluka-marvell]

All of this looks correct as far as I can tell. Future new P4 program test cases that exercise this new code with p4c compiling the program, then packets passing through pna_nic in the expected ways, are the best assurance of that.

Yes, Will include a test case

[antoninbas]

There is no mention of a recirculation port in the PNA spec. @jafingerhut is this correct?
All I know is that this is the SDN port value defined by the P4Runtime spec as the recirculation port.
I guess targets are free to "choose" (or define) the value used for the recirculation port.

[jafingerhut]

Yes, there is a recirculate() operation in PNA, and if that is done, then the packet should recirculate when it completes the current execution of the main control. There is NO recirculation port like there is in PSA or v1model architectures.

[jafingerhut]

Note: As an implementation technique, it is probably a reasonable idea to have a C++ boolean or 1-bit flag somewhere associated with the packet in the PNA implementation that is initialized to 0/false whenever a packet begins execution of the main control, and it is set to 1/true when the recirculate() extern function is called.

[antoninbas]

I couldn't find a recirculate() extern function in pna.p4? Is there a reference to it somehere.
If PNA specifies such an extern function, then this is what this PR should introduce (not a special port value). Implementation of the extern function can use a special port value if appropriate (the boolean flag approach you suggested may be a bit more difficult within the constraints of bmv2).

[rupesh-chiluka-marvell]

Yes. I didn't find any recirculate() extern function in pna.p4. So, I followed the PSA implementation.

If introducing a new recirculate() function is better approach, we can implement that.

[rupesh-chiluka-marvell]

Hi @antoninbas, Is the current approach Ok? Or should I change it recirculate() extern approach?

[jafingerhut]

One approach is to wait until there is an official PNA definition for how recirculation is done, and behaves, and implement other features that are already well-defined.

[rupesh-chiluka-marvell]

Ok

[antoninbas]

what's the expected value of pna_main_parser_input_metadata.input_port for recirculated packets?

[jafingerhut]

I believe it should be unchanged from the original. Unfortunately some things like this are not yet spelled out in as precise a form in the PNA architecture as they are in the PSA architecture, but I believe "input_port is unchanged after recirculate operation" is a reasonable starting place (and maybe finishing place, too).

[antoninbas]

Thanks. This is not what the current PR does, but it can be done easily. See PSA implementation:

behavioral-model/targets/psa_switch/psa_switch.cpp

Lines 618 to 619 in 28b736c

	phv->get_field("psa_ingress_parser_input_metadata.ingress_port")
	.set(PSA_PORT_RECIRCULATE);

[rupesh-chiluka-marvell]

Ok. Will update the line.

Just a doubt: Why are we setting ingress_port field to PSA_PORT_RECIRCULATE ? The P4 programs will find whether the packet is recirculated on not using recirculated_flag field.

[antoninbas]

According to @jafingerhut, the port should be set to the original ingress port, not PSA_PORT_RECIRCULATE
But we should still set the port to something. With your current implementation, the port will always be set to 0. At least that's what will happen I think based on the reset_metadata call (I didn't test).