[zeeshanlakhani]

This work includes and/or incorporates:

• Bitmap-based packet replication for softnpu: a P4 program can make use of a port_bitmap field in its egress metadata and set the field during ingress (via table action, metadata assignment, etc.):

  struct egress_metadata_t {
      ...
      bit<128> port_bitmap;
  }
  
  action set_port_bitmap(bit<128> bitmap) {
      egress.port_bitmap = bitmap;
  }

  • This bitmap is read after ingress, ahead of broadcast/unicast (drop > non-zero bitmap > broadcast > unicast), and it'll show up in dtrace metadata dumps for debugging, just like the other fields.
  • Checks have been added as well, including a check_port_bitmap requiring a bit<N>.
  • The p4rs::bitmap_to_port_list() function skips the ingress port and bits at or above the given radix. If a non-zero bitmap filters down to no ports, the packet gets dropped. A zeroed bitmap falls back to broadcast/unicast handling. Bit N (value 2^N) is port N, which is the same encoding 128w1 << port produces via shl_le.
  • Each selected port runs egress with its own copy of headers and metadata.
  • drop is now checked before any port selection takes place, enforcing that the egress pipeline never runs on a dropped packet.

• The pipeline codegen no longer hardcodes the ingress_metadata_t/egress_metadata_t types, which are now derived from the control parameters. The generated locals use fixed names (i.e., __ingress_metadata, __egress_metadata) in order to prevent P4 parameter names from shadowing pipeline locals.

• Fixtures have been updated, including the in-repo sidecar-lite example, which now makes use of a mcast_replication_v6 table whose set_port_bitmap action drives the port_bitmap field.

  • The associated mcast test fixture demonstrates dendrite-style MAC splitting, i.e. unicast in the ingress pipeline and replicas in the egress pipeline.

• The addition of shift operators (<<, >>). Binary expressions follow operator precedence, associating operators at equal precedence from left to right.

• The addition of bit-slice assignment (lval[hi:lo] = expr, P4-16 8.6), and slice reads are fixed now too.

  • Note: only bit<W> slices are lowered, whereas varbit slicing results in a language error (8.10), and int<W>/compile-time int slicing are reported as SoftNPU/x4c restrictions.

• Fixes and tightening around unsigned comparisons and diagnostic carets, which are now pulled from the token spans themselves, so that operators and full dotted names get marked properly.

• bit<W> -> bit<V> widening keeps the existing value (for sub-byte widths too).

  • Implicit bit widening is an x4c extension. P4-16 8.11 requires explicit casts between different bit widths, but x4c does not parse explicit casts as of yet.

• New HLIR checks on initializers, assignments, and slice bounds: runtime (unsized) int is rejected, and x4c/SoftNPU limitations get bubbled up instead of looking just like type errors.

  • The use_p4! macro now parses every file before AST checking, returning semantic errors as a syn::Error.

• Unaligned bit<N> action parameters have been fixed. For example, VLAN 300 returned as 44 (and 256 as 0!).

• New check: a table's default_action must appear in its actions list (P4-16 14.2.1.3).

• Bug fix: <= never lexed (s/">="/"<="/ for LessThanEquals).

[rcgoodfellow]

What does the Replicate extern get us that a simple bitmap in egress metadata does not? I think this could be implemented in a simpler way by just using the port_bitmap metadata field introduced in this PR and then implementing multicast replication similar to how broadcast replication is already done.

Having replication state in metadata as a field also makes things a bit more debuggable, as then it's a concrete value in the state of the pipeline that things like dtrace scripts can observe.

[zeeshanlakhani]

What does the Replicate extern get us that a simple bitmap in egress metadata does not? I think this could be implemented in a simpler way by just using the port_bitmap metadata field introduced in this PR and then implementing multicast replication similar to how broadcast replication is already done.

Having replication state in metadata as a field also makes things a bit more debuggable, as then it's a concrete value in the state of the pipeline that things like dtrace scripts can observe.

Yep. I'm actually taking this approach in my cleanup work that will go in soon. The original thinking was to match and model the PRE being a fixed-fucntion block on hardware with an extern, while also avoiding some extra magic field naming.

The other thing pushing me in this direction is that sidecar.p4 already does it this way. The mcast_grp_a/mcast_grp_b fields are metadata and get matched on downstream in mcast_tag_check, and the egress keys-on tbl_decap_ports for eg_intr_md.egress_rid, allowing later stages to read the state back and invalidate a group. The extern doesn't give us this ability.

I'll continue to move to the simpler mechanism.

[zeeshanlakhani]

@rcgoodfellow back up for review.

[rcgoodfellow]

Thanks @zeeshanlakhani. Things are looking much better after moving from an extern to metadata for multicast egress replication. Also some great compiler additions here! Comments follow.

[rcgoodfellow]

slice_is_contiguous calls reversed_slice_range and returns is_some on the result (effectively throwing away the calculated indices) and then generate_slice calls reversed_slice_range again with an unwrap to get the actual start and end indices. It's seems like the flow could be improved here to only call reversed_slice_range once and then match on the resulting Option for generate_slice obviating the need for the extra calls and the unwrap.

But a higher-level question for the entire implementation: is generate_slice_read_arith sufficiently general for both cases? It's a LOT simpler than going through all the index calculations and it may actually be faster code at the end of the day to perform the slice operation through a simple shift on an integer rather than futzing with index extraction over the bitvec.

[zeeshanlakhani]

Yeah, you're right. It should be general enough already and had handled every non-contiguous slice plus all the non-byte-multiple widths > 8.

The next push will use arithmetic for all slice reads and writes. I've dropped reversed_slice_range, slice_is_contiguous, and generate_slice methods.

Note: one thing did come up during this edit: the mask was (1u128 << w) - 1, which overflows for a full [127:0]. It never hit previously because only the range path took that case; it's u128::MAX >> (128 - w) now.

[rcgoodfellow]

What required us to move away from a derived default impl?

[zeeshanlakhani]

I needed the bitvecs to maintain their declared widths, but the derived Default implementation left them empty, and slice reads panic in load_le on empty bitvecs.

We've been getting away with this through is_empty() guards in p4rs and with resize in slice assignment.

[rcgoodfellow]

Why are we dropping the error diagnostic for undefined call names?

[zeeshanlakhani]

Actually, I'll restore it, similar reasoning initially to the one above.

[rcgoodfellow]

It's a bit odd that a function called parse_binary can return expressions that are not binary expressions.

[zeeshanlakhani]

Good point. It's precedence related really, so I've renamed it to parse_with_precedence.

[rcgoodfellow]

is a broadcast drop somehow different than a unicast drop? Seems like it should be the case that if egress.drop is set, it should not really matter what egress.broadcast or egress.port are.

[zeeshanlakhani]

Nope, it's the same drop semantics. One action covers drop with broadcast set and the other with the unicast port set (drop_precedes_broadcast drop_precedes_unicast in hub.rs).

On main, broadcast packet copies ran through egress before checking drop (note: egress can clear it). Now, drop is checked before selecting any ports. Packets with drop set after ingress never actually reach egress.

[rcgoodfellow]

Thanks @zeeshanlakhani. LGTM