oxidecomputer / p4

A P4 compiler
Mozilla Public License 2.0
111 stars 5 forks source link

Comments on the P4 book #55

Open jafingerhut opened 1 week ago

jafingerhut commented 1 week ago

Most of these are nits, but thought I would mention them after reading through the book so far:

========== File https://github.com/oxidecomputer/p4/blob/main/book/text/src/01-02-hello_world.md

"Parsers can also transition to the implicit reject state. This means that the packet will be dropped and not go on to any further processing."

Note that v1model, PSA, PNA, and TNA architectures all define that packets that go to the reject state in the parser actually do go to ingress (or main for PNA) processing after parsing is complete, but they also document that typically your P4 code is likely to want to drop the packet in ingress/main controls if that happens. I can imagine in some debug situations one would like to punt a copy of packets experiencing a parser error to the CPU.

Tofino I believe has chip-wide config to drop packets that reach a parser reject state, without going to ingress/egress processing after that, but I believe it is not the default option.

Typo: "I prime example" -> "A prime example"

========= File https://github.com/oxidecomputer/p4/blob/main/book/text/src/02-01-vlan-switch.md

Kind of a subtle point, but I believe from the language spec point of view, the assignment of false to vlan_ok before calling vlan.apply() is a "dead" assignment. No matter what value it has before vlan.apply(), the copy-out behavior of vlan.apply() should overwrite that value. Since the vlan control never assigns false to this out parameter, and only sometimes assigns true, the end value of vlan_ok could be undefined garbage. Perhaps in your implementation all bool values are predictably initialized to false?

In any case, the way to write this more portably in P4 would be to initialize match = false; before the port_vlan.apply(); call in the body of the vlan control.