Skip to content

Regression in flow id handling causes it to be immutable #252

@rzezeski

Description

@rzezeski

In commit 70785d9 I regressed the intra-layer handling of the InnerFlowId value, making it immutable during layer processing. E.g., when a packet comes inbound the NAT layer rewrites it, but the firewall layer still sees the previous InnerFlowId value, before the NAT rewrite. I uncovered this issue while writing a new integration test for SNAT+ICMP as part of my work on #231. Furthermore, this regression was hidden in manual testing because of a second aspect of commit 70785d9, which is that it pulled the default firewall rules out of the Oxide VPC implementation. Instead, it was decided that the implementation should provide a blank slate and it is up to Omicron to populate the default/initial rules. Because of this the firewall layer currently holds no rules, and when a layer holds no rules it has no action matches. When it has no action matches the packet simply passes through.

This is once again a lesson for all you kids out there to write integration tests when you can. I got lazy, and the gods have punished me for it. As part of #231 I'm also going to fix this regression, as well as add other NAT tests like SNAT+TCP and 1:1 NAT + ICMP/TCP.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions