diff --git a/.gitignore b/.gitignore index e05bdad3..66bcef63 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ *.sw* out.rs tags +core diff --git a/Cargo.lock b/Cargo.lock index 6e1e2f59..90b1ae1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -529,6 +529,7 @@ dependencies = [ "serde", "serde_tokenstream", "syn", + "tempfile", ] [[package]] diff --git a/codegen/rust/src/expression.rs b/codegen/rust/src/expression.rs index 5b17a728..9412936a 100644 --- a/codegen/rust/src/expression.rs +++ b/codegen/rust/src/expression.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use p4::ast::{BinOp, DeclarationInfo, Expression, ExpressionKind, Lvalue}; use p4::hlir::Hlir; @@ -14,6 +14,22 @@ impl<'a> ExpressionGenerator<'a> { Self { hlir } } + /// Generate an expression, converting a slice read to an owned `BitVec`. + /// + /// Without the conversion, a slice read would borrow as a `BitSlice`, which + /// the bitmath functions do not take. + pub(crate) fn generate_operand(&self, xpr: &Expression) -> TokenStream { + let token_stream = self.generate_expression(xpr); + match &xpr.kind { + ExpressionKind::Index(_, inner) + if matches!(inner.kind, ExpressionKind::Slice(..)) => + { + quote! { #token_stream.to_bitvec() } + } + _ => token_stream, + } + } + pub(crate) fn generate_expression(&self, xpr: &Expression) -> TokenStream { match &xpr.kind { ExpressionKind::BoolLit(v) => { @@ -30,9 +46,9 @@ impl<'a> ExpressionGenerator<'a> { } ExpressionKind::Lvalue(v) => self.generate_lvalue(v), ExpressionKind::Binary(lhs, op, rhs) => { - let lhs_tks = self.generate_expression(lhs.as_ref()); + let lhs_tks = self.generate_operand(lhs.as_ref()); let op_tks = self.generate_binop(*op); - let rhs_tks = self.generate_expression(rhs.as_ref()); + let rhs_tks = self.generate_operand(rhs.as_ref()); let mut ts = TokenStream::new(); match op { BinOp::Add => { @@ -101,32 +117,87 @@ impl<'a> ExpressionGenerator<'a> { ts.extend(op_tks); ts.extend(rhs_tks_); } - _ => { - ts.extend(lhs_tks); - ts.extend(op_tks); - ts.extend(rhs_tks); + BinOp::BitOr | BinOp::BitAnd | BinOp::Xor | BinOp::Mask => { + ts.extend(quote! { + { + let __lhs = #lhs_tks.clone(); + let __rhs = #rhs_tks.clone(); + __lhs #op_tks __rhs + } + }); + } + BinOp::Shl | BinOp::Shr => { + let rhs_tks = match &rhs.kind { + ExpressionKind::IntegerLit(value) => { + self.generate_bit_literal( + 128, + u128::try_from(*value).expect( + "negative shift count reached codegen", + ), + ) + } + _ => rhs_tks, + }; + + let shift = match op { + BinOp::Shl => quote! { p4rs::bitmath::shl_le }, + BinOp::Shr => quote! { p4rs::bitmath::shr_le }, + _ => unreachable!(), + }; + + ts.extend(quote! { + #shift(#lhs_tks.clone(), #rhs_tks.clone()) + }); + } + BinOp::Lt | BinOp::Gt | BinOp::Leq | BinOp::Geq => { + ts.extend(quote! { + p4rs::bitmath::Unsigned(&#lhs_tks) + #op_tks p4rs::bitmath::Unsigned(&#rhs_tks) + }); } } - ts + quote! { (#ts) } } ExpressionKind::Index(lval, xpr) => { let mut ts = self.generate_lvalue(lval); - ts.extend(self.generate_expression(xpr.as_ref())); + // For slices, look up the parent field's bit width + // so that generate_slice can adjust for header.rs byte + // reversal. + if let ExpressionKind::Slice(begin, end) = &xpr.kind { + let name_info = + self.hlir.lvalue_decls.get(lval).unwrap_or_else(|| { + panic!("unresolved lvalue {lval:#?} in slice") + }); + + let field_width = match &name_info.ty { + p4::ast::Type::Bit(w) => *w, + ty => { + panic!( + "slice on non-bit type {ty:?} reached codegen" + ) + } + }; + + let (hi, lo) = Self::slice_bounds(begin, end); + if Self::slice_is_contiguous(hi, lo, field_width) { + ts.extend(self.generate_slice(begin, end, field_width)); + } else { + // Non-contiguous after byte reversal; + // replace the lvalue suffix with arithmetic. + // Fields fit in the u128 load because the + // checker rejects widths over 128. + return Self::generate_slice_read_arith(&ts, hi, lo); + } + } else { + ts.extend(self.generate_expression(xpr.as_ref())); + } ts } - ExpressionKind::Slice(begin, end) => { - let l = match &begin.kind { - ExpressionKind::IntegerLit(v) => *v as usize, - _ => panic!("slice ranges can only be integer literals"), - }; - let l = l + 1; - let r = match &end.kind { - ExpressionKind::IntegerLit(v) => *v as usize, - _ => panic!("slice ranges can only be integer literals"), - }; - quote! { - [#r..#l] - } + ExpressionKind::Slice(_begin, _end) => { + // The HLIR rejects bare slices outside an Index + // expression, so this is unreachable for well-typed + // programs. + unreachable!("bare Slice reached codegen"); } ExpressionKind::Call(call) => { let lv: Vec = call @@ -158,6 +229,81 @@ impl<'a> ExpressionGenerator<'a> { } } + /// Extract compile-time hi and lo from slice bound expressions. + pub(crate) fn slice_bounds( + begin: &Expression, + end: &Expression, + ) -> (P4Bit, P4Bit) { + let hi: P4Bit = match &begin.kind { + ExpressionKind::IntegerLit(v) => *v as usize, + _ => panic!("slice ranges can only be integer literals"), + }; + let lo: P4Bit = match &end.kind { + ExpressionKind::IntegerLit(v) => *v as usize, + _ => panic!("slice ranges can only be integer literals"), + }; + (hi, lo) + } + + /// Whether `[hi:lo]` on a field of `field_width` bits can be + /// expressed as a contiguous bitvec range after byte reversal. + pub(crate) fn slice_is_contiguous( + hi: P4Bit, + lo: P4Bit, + field_width: FieldWidth, + ) -> bool { + // Non-byte-multiple widths have an additional bit-shift in + // header.rs storage that reversed_slice_range does not model. + if field_width > 8 && !field_width.is_multiple_of(8) { + return false; + } + reversed_slice_range(hi, lo, field_width).is_some() + } + + /// Emit a bitvec range index for a contiguous slice `[hi:lo]`. + /// + /// Callers must check [`Self::slice_is_contiguous`] first and route + /// non-contiguous slices to [`Self::generate_slice_read_arith`]; otherwise, + /// this panics. + pub(crate) fn generate_slice( + &self, + begin: &Expression, + end: &Expression, + field_width: FieldWidth, + ) -> TokenStream { + let (hi, lo) = Self::slice_bounds(begin, end); + + assert!( + Self::slice_is_contiguous(hi, lo, field_width), + "non-contiguous slice reads must be handled \ + by the caller via generate_slice_read_arith", + ); + let (start, end) = reversed_slice_range(hi, lo, field_width).unwrap(); + quote! { [#start..#end] } + } + + /// Emit an arithmetic slice read for non-contiguous slices. + /// + /// This loads the field as an integer, shifts and masks to extract + /// the requested bits, then packs everything into a new bitvec. + pub(crate) fn generate_slice_read_arith( + lhs: &TokenStream, + hi: P4Bit, + lo: P4Bit, + ) -> TokenStream { + let slice_width = hi - lo + 1; + let mask_val = (1u128 << slice_width) - 1; + quote! { + { + let __v: u128 = #lhs.load_le(); + let __extracted = (__v >> #lo) & #mask_val; + let mut __out = bitvec![u8, Msb0; 0; #slice_width]; + __out.store_le(__extracted); + __out + } + } + } + pub(crate) fn generate_bit_literal( &self, width: u16, @@ -191,6 +337,8 @@ impl<'a> ExpressionGenerator<'a> { BinOp::BitAnd => quote! { & }, BinOp::BitOr => quote! { | }, BinOp::Xor => quote! { ^ }, + BinOp::Shl => quote! { << }, + BinOp::Shr => quote! { >> }, } } @@ -223,3 +371,193 @@ impl<'a> ExpressionGenerator<'a> { } } } + +/// P4 bit position within a field, where bit 0 is the least significant bit. +type P4Bit = usize; + +/// Width of a P4 header field in bits. +type FieldWidth = usize; + +/// Half-open bitvec range `(start, end)` into the storage representation. +type BitvecRange = (usize, usize); + +/// Map a P4 slice `[hi:lo]` to a bitvec range in byte-reversed storage. +/// +/// Returns `Some(range)` for single-byte and byte-aligned multi-byte slices, +/// and `None` when byte reversal splits `[hi:lo]` across non-adjacent +/// storage, which happens for non-byte-aligned multi-byte slices. +/// +/// header.rs reverses byte order for fields wider than 8 bits. Bit +/// positions within each byte are preserved (Msb0). The mapping from +/// P4 bit positions to storage indices: +/// +/// ```text +/// wire_idx = W - 1 - b +/// wire_byte = wire_idx / 8 +/// bit_in_byte = wire_idx % 8 +/// storage_byte = ceil(W/8) - 1 - wire_byte +/// bitvec_idx = storage_byte * 8 + bit_in_byte +/// ``` +pub(crate) fn reversed_slice_range( + hi: P4Bit, + lo: P4Bit, + field_width: FieldWidth, +) -> Option { + // Wire byte indices for the slice endpoints. P4 bit W-1 is in wire + // byte 0 (MSB-first), and higher bit numbers map to lower byte indices. + let storage_bytes = field_width.div_ceil(8); + let wire_idx = |bit_pos: P4Bit| field_width - 1 - bit_pos; + let storage_byte = |wire_byte: usize| storage_bytes - 1 - wire_byte; + let bitvec_idx = |bit_pos: P4Bit| { + let idx = wire_idx(bit_pos); + storage_byte(idx / 8) * 8 + idx % 8 + }; + + let wire_byte_hi = wire_idx(hi) / 8; + let wire_byte_lo = wire_idx(lo) / 8; + let byte_aligned = (hi + 1).is_multiple_of(8) && lo.is_multiple_of(8); + + match (wire_byte_hi == wire_byte_lo, byte_aligned) { + // Single-byte slice: map each endpoint individually. + (true, _) => { + let (a, b) = (bitvec_idx(hi), bitvec_idx(lo)); + Some((a.min(b), a.max(b) + 1)) + } + // Multi-byte byte-aligned slice: reversed bytes form a + // contiguous block. + (false, true) => Some(( + storage_byte(wire_byte_lo) * 8, + (storage_byte(wire_byte_hi) + 1) * 8, + )), + // Non-byte-aligned multi-byte slice: byte reversal makes the + // bits non-contiguous, so there is no single bitvec range. + (false, false) => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // Verify the reversed slice range mapping against the byte reversal + // in header.rs. For each case we check that the bitvec range lands + // on the correct bits in the reversed storage layout. + + // Sub-byte slices within a single wire byte. + + #[test] + fn slice_32bit_top_nibble() { + // P4 [31:28] on 32-bit: top nibble of wire byte 0. + // Storage: wire byte 0 -> storage byte 3. + // High nibble of storage byte 3 = bitvec [24..28]. + assert_eq!(reversed_slice_range(31, 28, 32), Some((24, 28))); + } + + #[test] + fn slice_32bit_bottom_nibble() { + // P4 [3:0] on 32-bit: bottom nibble of wire byte 3. + // Storage: wire byte 3 -> storage byte 0. + // Low nibble (Msb0) of storage byte 0 = bitvec [4..8]. + assert_eq!(reversed_slice_range(3, 0, 32), Some((4, 8))); + } + + #[test] + fn slice_16bit_top_nibble() { + // P4 [15:12] on 16-bit: top nibble of wire byte 0. + // Storage: wire byte 0 -> storage byte 1. + // High nibble of storage byte 1 = bitvec [8..12]. + assert_eq!(reversed_slice_range(15, 12, 16), Some((8, 12))); + } + + // Full-byte slices (single byte). + + #[test] + fn slice_128bit_top_byte() { + // P4 [127:120] on 128-bit: wire byte 0 -> storage byte 15. + // bitvec [120..128]. + assert_eq!(reversed_slice_range(127, 120, 128), Some((120, 128))); + } + + #[test] + fn slice_16bit_low_byte() { + // P4 [7:0] on 16-bit: wire byte 1 -> storage byte 0. + // bitvec [0..8]. + assert_eq!(reversed_slice_range(7, 0, 16), Some((0, 8))); + } + + #[test] + fn slice_32bit_middle_byte() { + // P4 [23:16] on 32-bit: wire byte 1 -> storage byte 2. + // bitvec [16..24]. + assert_eq!(reversed_slice_range(23, 16, 32), Some((16, 24))); + } + + // Multi-byte byte-aligned slices. + + #[test] + fn slice_128bit_top_two_bytes() { + // P4 [127:112] on 128-bit: wire bytes 0-1 -> storage bytes 14-15. + // bitvec [112..128]. + assert_eq!(reversed_slice_range(127, 112, 128), Some((112, 128))); + } + + #[test] + fn slice_32bit_top_three_bytes() { + // P4 [31:8] on 32-bit: wire bytes 0-2 -> storage bytes 1-3. + // bitvec [8..32]. + assert_eq!(reversed_slice_range(31, 8, 32), Some((8, 32))); + } + + #[test] + fn slice_32bit_bottom_two_bytes() { + // P4 [15:0] on 32-bit: wire bytes 2-3 -> storage bytes 0-1. + // bitvec [0..16]. + assert_eq!(reversed_slice_range(15, 0, 32), Some((0, 16))); + } + + #[test] + fn slice_48bit_upper_24() { + assert_eq!(reversed_slice_range(47, 24, 48), Some((24, 48))); + } + + #[test] + fn slice_8bit_bottom_nibble() { + assert_eq!(reversed_slice_range(3, 0, 8), Some((4, 8))); + } + + #[test] + fn slice_8bit_top_nibble() { + assert_eq!(reversed_slice_range(7, 4, 8), Some((0, 4))); + } + + #[test] + fn slice_8bit_whole_field() { + assert_eq!(reversed_slice_range(7, 0, 8), Some((0, 8))); + } + + #[test] + fn slice_8bit_single_bit() { + assert_eq!(reversed_slice_range(0, 0, 8), Some((7, 8))); + assert_eq!(reversed_slice_range(7, 7, 8), Some((0, 1))); + } + + #[test] + fn slice_4bit_field() { + assert_eq!(reversed_slice_range(3, 0, 4), Some((0, 4))); + assert_eq!(reversed_slice_range(1, 0, 4), Some((2, 4))); + } + + #[test] + fn slice_non_contiguous_returns_none() { + assert_eq!(reversed_slice_range(11, 4, 32), None); + assert_eq!(reversed_slice_range(22, 0, 32), None); + } + + #[test] + fn slice_contiguity() { + assert!(ExpressionGenerator::slice_is_contiguous(15, 8, 16)); + assert!(!ExpressionGenerator::slice_is_contiguous(7, 0, 12)); + assert!(!ExpressionGenerator::slice_is_contiguous(11, 4, 13)); + assert!(!ExpressionGenerator::slice_is_contiguous(19, 12, 32)); + } +} diff --git a/codegen/rust/src/lib.rs b/codegen/rust/src/lib.rs index 4d9ec523..952aad6a 100644 --- a/codegen/rust/src/lib.rs +++ b/codegen/rust/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use std::collections::HashMap; use std::fs; @@ -216,6 +216,9 @@ fn rust_type(ty: &Type) -> TokenStream { quote! { BitVec:: } } Type::Int(_size) => todo!("generate int type"), + Type::ArbitraryInt => { + unreachable!("arbitrary-precision int reached Rust type generation") + } Type::Varbit(_size) => todo!("generate varbit type"), Type::String => quote! { String }, Type::UserDefined(name) => { @@ -250,6 +253,11 @@ fn type_size(ty: &Type, ast: &AST) -> usize { Type::Error => todo!("generate error size"), Type::Bit(size) => *size, Type::Int(size) => *size, + Type::ArbitraryInt => { + unreachable!( + "arbitrary-precision int reached type-size calculation" + ) + } Type::Varbit(size) => *size, Type::String => todo!("generate string size"), Type::UserDefined(name) => { diff --git a/codegen/rust/src/p4struct.rs b/codegen/rust/src/p4struct.rs index 83807e96..fb5028a9 100644 --- a/codegen/rust/src/p4struct.rs +++ b/codegen/rust/src/p4struct.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use crate::Context; use p4::ast::{Struct, Type, AST}; @@ -25,6 +25,7 @@ impl<'a> StructGenerator<'a> { let mut valid_member_size = Vec::new(); let mut to_bitvec_stmts = Vec::new(); let mut dump_statements = Vec::new(); + let mut default_fields = Vec::new(); let fmt = "{}: {}\n".repeat(s.members.len()); let fmt = fmt.trim(); @@ -54,6 +55,9 @@ impl<'a> StructGenerator<'a> { } }); + default_fields.push(quote! { + #name: #ty::default() + }); dump_statements.push(quote! { #name_s.blue(), self.#name.dump() @@ -67,6 +71,9 @@ impl<'a> StructGenerator<'a> { } Type::Bit(size) => { members.push(quote! { pub #name: BitVec:: }); + default_fields.push(quote! { + #name: bitvec![u8, Msb0; 0; #size] + }); dump_statements.push(quote! { #name_s.blue(), p4rs::dump_bv(&self.#name) @@ -81,6 +88,9 @@ impl<'a> StructGenerator<'a> { } Type::Bool => { members.push(quote! { pub #name: bool }); + default_fields.push(quote! { + #name: false + }); dump_statements.push(quote! { #name_s.blue(), self.#name @@ -99,10 +109,18 @@ impl<'a> StructGenerator<'a> { let name = format_ident!("{}", s.name); let mut structure = quote! { - #[derive(Debug, Default, Clone)] + #[derive(Debug, Clone)] pub struct #name { #(#members),* } + + impl Default for #name { + fn default() -> Self { + Self { + #(#default_fields),* + } + } + } }; if !valid_member_size.is_empty() { structure.extend(quote! { diff --git a/codegen/rust/src/pipeline.rs b/codegen/rust/src/pipeline.rs index 851fb19c..fcea47e8 100644 --- a/codegen/rust/src/pipeline.rs +++ b/codegen/rust/src/pipeline.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use crate::{ qualified_table_function_name, qualified_table_name, rust_type, @@ -7,6 +7,7 @@ use crate::{ use p4::ast::{ Control, Direction, MatchKind, PackageInstance, Parser, Table, Type, AST, }; +use p4::check::PORT_BITMAP_FIELD; use p4::hlir::Hlir; use proc_macro2::TokenStream; use quote::{format_ident, quote}; @@ -173,6 +174,17 @@ impl<'a> PipelineGenerator<'a> { self.ctx.pipelines.insert(inst.name.clone(), pipeline); } + /// Whether egress metadata declares `PORT_BITMAP_FIELD` for port selection. + fn has_port_bitmap(&self, egress_meta: &Type) -> bool { + let Type::UserDefined(name) = egress_meta else { + return false; + }; + + self.ast.get_struct(name).is_some_and(|s| { + s.members.iter().any(|m| m.name == PORT_BITMAP_FIELD) + }) + } + fn pipeline_impl_process_packet( &mut self, parser: &Parser, @@ -180,6 +192,21 @@ impl<'a> PipelineGenerator<'a> { egress: &Control, ) -> (TokenStream, TokenStream) { let parsed_type = rust_type(&parser.parameters[1].ty); + + let [_, ingress_meta, _] = ingress.parameters.as_slice() else { + panic!("SoftNPU ingress control must take 3 parameters"); + }; + let [_, _, egress_meta] = egress.parameters.as_slice() else { + panic!("SoftNPU egress control must take 3 parameters"); + }; + + // This metadata uses fixed names so P4 parameter names cannot shadow + // the pipeline's own locals (parsed, port, pkt). + let ingress_meta_var = format_ident!("__ingress_metadata"); + let egress_meta_var = format_ident!("__egress_metadata"); + let ingress_meta_type = rust_type(&ingress_meta.ty); + let egress_meta_type = rust_type(&egress_meta.ty); + // determine table arguments let ingress_tables = ingress.tables(self.ast); //TODO(dry) @@ -201,6 +228,123 @@ impl<'a> PipelineGenerator<'a> { }); } + let egress_ports = { + let (bitmap_set, replicated) = + if self.has_port_bitmap(&egress_meta.ty) { + let port_bitmap = format_ident!("{PORT_BITMAP_FIELD}"); + ( + quote! { #egress_meta_var.#port_bitmap.any() }, + quote! { + p4rs::replicate( + &#egress_meta_var.#port_bitmap, + port, + self.radix, + ) + }, + ) + } else { + (quote! { false }, quote! { Vec::new() }) + }; + // A non-zero bitmap takes precedence, even if filtering excludes + // every selected port. A zero bitmap falls back to broadcast/unicast + // handling. + quote! { + let ports: Vec = if #egress_meta_var.drop { + Vec::new() + } else if #bitmap_set { + #replicated + } else if #egress_meta_var.broadcast { + (0..self.radix) + .filter(|&p| p != port) + .collect() + } else if #egress_meta_var.port.is_empty() { + Vec::new() + } else { + vec![#egress_meta_var.port.load_le()] + }; + } + }; + + let egress_loop = quote! { + ports.into_iter() + .filter_map(|eport| { + let mut igm = #ingress_meta_var.clone(); + let mut egm = #egress_meta_var.clone(); + let mut parsed_ = parsed.clone(); + + // + // Run the egress block + // + + egm.port = { + let mut x = bitvec![mut u8, Msb0; 0; 16]; + x.store_le(eport); + x + }; + + (self.egress)( + &mut parsed_, + &mut igm, + &mut egm, + #(#egress_tbl_args),* + ); + + if egm.drop { + return None; + } + + // + // Create the packet output. + // + + let bv = parsed_.to_bitvec(); + let buf = bv.as_raw_slice(); + let out = packet_out{ + header_data: buf.to_owned(), + payload_data: &pkt.data[parsed_size..], + }; + Some((out, eport)) + }) + .collect() + }; + + let egress_loop_headers = quote! { + ports.into_iter() + .filter_map(|eport| { + let mut igm = #ingress_meta_var.clone(); + let mut egm = #egress_meta_var.clone(); + let mut parsed_ = parsed.clone(); + + // + // Run the egress block + // + + egm.port = { + let mut x = bitvec![mut u8, Msb0; 0; 16]; + x.store_le(eport); + x + }; + + (self.egress)( + &mut parsed_, + &mut igm, + &mut egm, + #(#egress_tbl_args),* + ); + + if egm.drop { + return None; + } + + // + // Create the packet output. + // + + Some((parsed_, eport)) + }) + .collect() + }; + let process_packet = quote! { fn process_packet<'a>( &mut self, @@ -217,7 +361,7 @@ impl<'a> PipelineGenerator<'a> { // Instantiate ingress/egress metadata // - let mut ingress_metadata = ingress_metadata_t{ + let mut #ingress_meta_var = #ingress_meta_type { port: { let mut x = bitvec![mut u8, Msb0; 0; 16]; x.store_le(port); @@ -225,15 +369,17 @@ impl<'a> PipelineGenerator<'a> { }, ..Default::default() }; - let mut egress_metadata = egress_metadata_t::default(); + let mut #egress_meta_var = #egress_meta_type::default(); + #egress_meta_var.port = BitVec::new(); // // Run the parser block // - let accept = (self.parse)(pkt, &mut parsed, &mut ingress_metadata); + let accept = (self.parse)( + pkt, &mut parsed, &mut #ingress_meta_var, + ); if !accept { - // drop the packet softnpu_provider::parser_dropped!(||()); return Vec::new(); } @@ -252,8 +398,8 @@ impl<'a> PipelineGenerator<'a> { (self.ingress)( &mut parsed, - &mut ingress_metadata, - &mut egress_metadata, + &mut #ingress_meta_var, + &mut #egress_meta_var, #(#ingress_tbl_args),* ); @@ -261,78 +407,27 @@ impl<'a> PipelineGenerator<'a> { // Determine egress ports // - let ports = if egress_metadata.broadcast { - let mut ports = Vec::new(); - for p in 0..self.radix { - if p == port { - continue; - } - ports.push(p); - } - ports - } else { - if egress_metadata.port.is_empty() || egress_metadata.drop { - Vec::new() - } else { - vec![egress_metadata.port.load_le()] - } - }; + #egress_ports - let dump = parsed.dump(); + let dump = format!( + "{}\negress:\n{}", + parsed.dump(), + #egress_meta_var.dump(), + ); if ports.is_empty() { softnpu_provider::ingress_dropped!(||(&dump)); return Vec::new(); } - let dump = format!("\n{}", parsed.dump()); + let dump = format!("\n{dump}"); softnpu_provider::ingress_accepted!(||(&dump)); // // Run output of ingress block through egress block on each // egress port. // - let mut result = Vec::new(); - for eport in ports { - - let mut egm = egress_metadata.clone(); - let mut parsed_ = parsed.clone(); - - // - // Run the egress block - // - - egm.port = { - let mut x = bitvec![mut u8, Msb0; 0; 16]; - x.store_le(eport); - x - }; - - (self.egress)( - &mut parsed_, - &mut ingress_metadata, - &mut egm, - #(#egress_tbl_args),* - ); - - if egm.drop { - continue; - } - - // - // Create the packet output. - // - - let bv = parsed_.to_bitvec(); - let buf = bv.as_raw_slice(); - let out = packet_out{ - header_data: buf.to_owned(), - payload_data: &pkt.data[parsed_size..], - }; - result.push((out, eport)) - - } - result + #egress_loop } }; @@ -353,7 +448,7 @@ impl<'a> PipelineGenerator<'a> { // Instantiate ingress/egress metadata // - let mut ingress_metadata = ingress_metadata_t{ + let mut #ingress_meta_var = #ingress_meta_type { port: { let mut x = bitvec![mut u8, Msb0; 0; 16]; x.store_le(port); @@ -361,15 +456,17 @@ impl<'a> PipelineGenerator<'a> { }, ..Default::default() }; - let mut egress_metadata = egress_metadata_t::default(); + let mut #egress_meta_var = #egress_meta_type::default(); + #egress_meta_var.port = BitVec::new(); // // Run the parser block // - let accept = (self.parse)(pkt, &mut parsed, &mut ingress_metadata); + let accept = (self.parse)( + pkt, &mut parsed, &mut #ingress_meta_var, + ); if !accept { - // drop the packet softnpu_provider::parser_dropped!(||()); return Vec::new(); } @@ -388,8 +485,8 @@ impl<'a> PipelineGenerator<'a> { (self.ingress)( &mut parsed, - &mut ingress_metadata, - &mut egress_metadata, + &mut #ingress_meta_var, + &mut #egress_meta_var, #(#ingress_tbl_args),* ); @@ -397,72 +494,27 @@ impl<'a> PipelineGenerator<'a> { // Determine egress ports // - let ports = if egress_metadata.broadcast { - let mut ports = Vec::new(); - for p in 0..self.radix { - if p == port { - continue; - } - ports.push(p); - } - ports - } else { - if egress_metadata.port.is_empty() || egress_metadata.drop { - Vec::new() - } else { - vec![egress_metadata.port.load_le()] - } - }; + #egress_ports - let dump = parsed.dump(); + let dump = format!( + "{}\negress:\n{}", + parsed.dump(), + #egress_meta_var.dump(), + ); if ports.is_empty() { softnpu_provider::ingress_dropped!(||(&dump)); return Vec::new(); } - let dump = format!("\n{}", parsed.dump()); + let dump = format!("\n{dump}"); softnpu_provider::ingress_accepted!(||(&dump)); // // Run output of ingress block through egress block on each // egress port. // - let mut result = Vec::new(); - for eport in ports { - - let mut egm = egress_metadata.clone(); - let mut parsed_ = parsed.clone(); - - // - // Run the egress block - // - - egm.port = { - let mut x = bitvec![mut u8, Msb0; 0; 16]; - x.store_le(eport); - x - }; - - (self.egress)( - &mut parsed_, - &mut ingress_metadata, - &mut egm, - #(#egress_tbl_args),* - ); - - if egm.drop { - continue; - } - - // - // Create the packet output. - // - - result.push((parsed_, eport)) - - } - result + #egress_loop_headers } }; @@ -802,7 +854,7 @@ impl<'a> PipelineGenerator<'a> { ); }); parameter_refs.push(quote! { #pname.clone() }); - offset += n >> 3; + offset += n.div_ceil(8); } Type::Varbit(_n) => { todo!(); @@ -810,6 +862,9 @@ impl<'a> PipelineGenerator<'a> { Type::Int(_n) => { todo!(); } + Type::ArbitraryInt => { + unreachable!("int rejected by the checker"); + } Type::String => { todo!(); } @@ -879,6 +934,17 @@ impl<'a> PipelineGenerator<'a> { let tname = format_ident!("{}", qtfn); action_match_body.extend(quote! { #aname => { + if parameter_data.len() < #offset { + println!( + "add {} entry: action {} expects {} parameter \ + bytes, found {}, ignoring", + #qtfn, + #aname, + #offset, + parameter_data.len(), + ); + return; + } #(#parameter_tokens)* let action: std::sync::Arc StatementGenerator<'a> { panic!("codegen name not resolved for {:#?}", lval) }); - if is_header_member(lval, self.hlir) { - return quote! { #lhs = #rhs.clone(); }; - } - - let rhs = if rhs_ty != &name_info.ty { + let converted = rhs_ty != &name_info.ty; + let rhs = if converted { let converter = self.converter(rhs_ty, &name_info.ty); + let rhs = if matches!(rhs_ty, Type::Bit(_)) { + quote! { (#rhs).to_owned() } + } else { + rhs + }; quote!( #converter(#rhs) ) } else { rhs }; - let rhs = if let Type::Bit(_) = rhs_ty { + if is_header_member(lval, self.hlir) { + return if converted { + quote! { #lhs = #rhs; } + } else { + quote! { #lhs = #rhs.clone(); } + }; + } + + let rhs = if converted { + rhs + } else if let Type::Bit(_) = rhs_ty { // TODO eww, to better to figure out precisely when to_owned // and clone are needed - quote! { #rhs.to_owned().clone() } + quote! { #rhs.to_owned() } } else if let Type::UserDefined(_) = rhs_ty { quote! { #rhs.clone() } } else { @@ -100,6 +112,88 @@ impl<'a> StatementGenerator<'a> { quote! { #lhs = #rhs; } } } + Statement::SliceAssignment(lval, hi, lo, xpr) => { + let eg = ExpressionGenerator::new(self.hlir); + let lhs = eg.generate_lvalue(lval); + let mut rhs = eg.generate_expression(xpr.as_ref()); + + let name_info = + self.hlir.lvalue_decls.get(lval).unwrap_or_else(|| { + panic!( + "unresolved lvalue {lval:#?} in slice assignment" + ) + }); + let field_width = match &name_info.ty { + Type::Bit(w) => *w, + ty => panic!( + "slice assignment on non-bit type {ty:?} reached codegen" + ), + }; + + let (hi_val, lo_val) = + ExpressionGenerator::slice_bounds(hi, lo); + + let slice_ty = Type::Bit(hi_val - lo_val + 1); + let rhs_ty = self + .hlir + .expression_types + .get(xpr.as_ref()) + .unwrap_or_else(|| panic!("type for {xpr:#?}")); + + let converted = rhs_ty != &slice_ty; + if converted { + let converter = self.converter(rhs_ty, &slice_ty); + if matches!(rhs_ty, Type::Bit(_)) { + rhs = quote! { (#rhs).to_owned() }; + } + rhs = quote! { #converter(#rhs) }; + } + let snapshot = if converted { + rhs.clone() + } else { + quote! { #rhs.to_owned() } + }; + + if ExpressionGenerator::slice_is_contiguous( + hi_val, + lo_val, + field_width, + ) { + let slice = eg.generate_slice(hi, lo, field_width); + + // Temporary prevents overlapping borrows when + // LHS and RHS alias (e.g. `x[7:4] = x[3:0]`). + quote! { + { + let __slice_rhs = #snapshot; + if #lhs.is_empty() { + #lhs.resize(#field_width, false); + } + #lhs #slice .copy_from_bitslice(&__slice_rhs); + } + } + } else { + // Non-contiguous after byte reversal; instead, use + // arithmetic (load, mask, shift, store). Fields fit + // in the u128 loads because the checker rejects + // widths over 128. + let slice_width = hi_val - lo_val + 1; + let mask_val = (1u128 << slice_width) - 1; + quote! { + { + let __rhs_val: u128 = #rhs.load_le(); + if #lhs.is_empty() { + #lhs.resize(#field_width, false); + } + let __lhs_val: u128 = #lhs.load_le(); + let __mask: u128 = #mask_val << #lo_val; + let __new = (__lhs_val & !__mask) + | ((__rhs_val & #mask_val) << #lo_val); + #lhs.store_le(__new); + } + } + } + } Statement::Call(c) => match &self.context { StatementContext::Control(control) => { let mut ts = TokenStream::new(); @@ -135,24 +229,11 @@ impl<'a> StatementGenerator<'a> { let name = format_ident!("{}", v.name); let ty = rust_type(&v.ty); let initializer = match &v.initializer { - Some(xpr) => { - let eg = ExpressionGenerator::new(self.hlir); - let mut ini = eg.generate_expression(xpr.as_ref()); - if let ExpressionKind::Lvalue(_) = xpr.kind { - ini = quote! { #ini.clone() }; - } - let ini_ty = - self.hlir.expression_types.get(xpr).unwrap_or_else( - || panic!("type for expression {:#?}", xpr), - ); - if ini_ty != &v.ty { - let converter = self.converter(ini_ty, &v.ty); - quote! { #converter(#ini) } - } else { - ini - } - } - None => quote! { #ty::default() }, + Some(xpr) => self.initializer(xpr, &v.ty), + None => match &v.ty { + Type::Bit(w) => quote! { bitvec![u8, Msb0; 0; #w] }, + _ => quote! { #ty::default() }, + }, }; names.insert( v.name.clone(), @@ -178,9 +259,7 @@ impl<'a> StatementGenerator<'a> { Statement::Constant(c) => { let name = format_ident!("{}", c.name); let ty = rust_type(&c.ty); - let eg = ExpressionGenerator::new(self.hlir); - let initializer = - eg.generate_expression(c.initializer.as_ref()); + let initializer = self.initializer(&c.initializer, &c.ty); quote! { let #name: #ty = #initializer; } @@ -329,25 +408,15 @@ impl<'a> StatementGenerator<'a> { let eg = ExpressionGenerator::new(self.hlir); let mut args = Vec::new(); - for a in &c.args { - let arg_xpr = eg.generate_expression(a.as_ref()); - args.push(arg_xpr); - } - - let lvref: Vec = c - .lval - .name - .split('.') - .map(|x| format_ident!("{}_action_{}", control.name, x)) - .map(|x| quote! { #x }) - .collect(); - + // Control parameters come first in the action function signature + // (@see generate_control_action in control.rs), followed by + // extern references, and then action-specific parameters. for a in &control.parameters { let arg = format_ident!("{}", a.name); args.push(quote! { #arg }); } - // pass externs instantiated at control scope to actions + // Pass externs instantiated at control scope to actions. for x in &control.variables { if let Type::UserDefined(typename) = &x.ty { if self.ast.get_extern(typename).is_some() { @@ -357,6 +426,25 @@ impl<'a> StatementGenerator<'a> { } } + // Action-specific arguments come last. We clone lvalue args to avoid + // moving out from mutable references. + for a in &c.args { + let arg_xpr = eg.generate_expression(a.as_ref()); + if matches!(a.kind, ExpressionKind::Lvalue(_)) { + args.push(quote! { #arg_xpr.clone() }); + } else { + args.push(arg_xpr); + } + } + + let lvref: Vec = c + .lval + .name + .split('.') + .map(|x| format_ident!("{}_action_{x}", control.name)) + .map(|x| quote! { #x }) + .collect(); + tokens.extend(quote! { #(#lvref).*(#(#args),*); }) @@ -427,11 +515,28 @@ impl<'a> StatementGenerator<'a> { let arg_xpr = eg.generate_expression(a.as_ref()); let et = self.hlir.expression_types.get(a.as_ref()).unwrap(); let local_name = format_ident!("arg{}", i); + let param_ty = &control_instance.parameters[i].ty; + + if et != param_ty { + let converter = self.converter(et, param_ty); + let arg_xpr = if matches!(et, Type::Bit(_)) { + quote! { (#arg_xpr).to_owned() } + } else { + arg_xpr + }; + locals.push(quote! { + let mut #local_name = #converter(#arg_xpr); + }); + args.push(quote! { &#local_name }); + continue; + } + if let ExpressionKind::BitLit(_, _) = a.as_ref().kind { locals.push(quote! { let mut #local_name = #arg_xpr; }) } + match control_instance.parameters[i].direction { Direction::Out | Direction::InOut => { match &a.as_ref().kind { @@ -639,15 +744,56 @@ impl<'a> StatementGenerator<'a> { }); } + /// Generate the value bound by a declaration like `bit x = e;` or + /// `const bit c = e;`. + /// + /// Slice reads get owned through `ExpressionGenerator::generate_operand`, + /// lvalues are cloned, and the `converter` fn applies when the expression's + /// type differs from the declared one. + fn initializer(&self, xpr: &Expression, ty: &Type) -> TokenStream { + let eg = ExpressionGenerator::new(self.hlir); + let mut ini = eg.generate_operand(xpr); + if let ExpressionKind::Lvalue(_) = xpr.kind { + ini = quote! { #ini.clone() }; + } + + let ini_ty = self + .hlir + .expression_types + .get(xpr) + .unwrap_or_else(|| panic!("type for expression {xpr:#?}")); + + if ini_ty != ty { + let converter = self.converter(ini_ty, ty); + quote! { #converter(#ini) } + } else { + ini + } + } + + /// Generate a closure converting a value of type `from` to `to`. + /// + /// Integer literals are truncated to `bit` as specified in P4-16 + /// 8.11.1. Implicit bit widening is an x4c extension implemented through + /// `p4rs::bitvec_resize`. fn converter(&self, from: &Type, to: &Type) -> TokenStream { match (from, to) { - (Type::Int(_), Type::Bit(_)) => { - quote! { p4rs::int_to_bitvec } + (Type::Int(_), Type::Bit(0)) => { + quote! { (|_: i128| BitVec::::new()) } + } + (Type::Int(_), Type::Bit(w)) => { + quote! { + (|__v: i128| { + let mut __x = bitvec![u8, Msb0; 0; #w]; + __x.store_le(__v as u128); + __x + }) + } } - (Type::Bit(x), Type::Bit(16)) if *x <= 16 => { - quote! { p4rs::bitvec_to_bitvec16 } + (Type::Bit(_), Type::Bit(w)) => { + quote! { (|__bv| p4rs::bitvec_resize(__bv, #w)) } } - _ => todo!("type converter for {} to {}", from, to), + _ => unreachable!("hlir rejects converting {from} to {to}"), } } } diff --git a/lang/p4-macro/Cargo.toml b/lang/p4-macro/Cargo.toml index ac482249..06308f84 100644 --- a/lang/p4-macro/Cargo.toml +++ b/lang/p4-macro/Cargo.toml @@ -13,3 +13,6 @@ serde.workspace = true [lib] proc-macro = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/lang/p4-macro/src/lib.rs b/lang/p4-macro/src/lib.rs index dabf375c..6763b1e9 100644 --- a/lang/p4-macro/src/lib.rs +++ b/lang/p4-macro/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company //! The [`use_p4!`] macro allows for P4 programs to be directly integrated into //! Rust programs. @@ -23,6 +23,7 @@ //! For documentation on using [Pipeline](../p4rs/trait.Pipeline.html) trait, see the //! [p4rs](../p4rs/index.html) docs. +use std::collections::HashMap; use std::fs; use std::path::Path; use std::sync::Arc; @@ -84,28 +85,30 @@ fn do_use_p4(item: TokenStream) -> Result { ) }; - generate_rs(filename, settings) + generate_rs(filename, settings).map(Into::into) } fn generate_rs( filename: String, settings: GenerationSettings, -) -> Result { +) -> Result { //TODO gracefull error handling let mut ast = AST::default(); - process_file(Arc::new(filename), &mut ast, &settings)?; + let mut sources = HashMap::new(); + process_file(Arc::new(filename), &mut ast, &mut sources)?; + p4_rust::sanitize(&mut ast); - let (hlir, _) = check::all(&ast); + let (hlir, diags) = check::all(&ast); + check(&sources, &diags)?; - let tokens: TokenStream = p4_rust::emit_tokens( + let tokens = p4_rust::emit_tokens( &ast, &hlir, p4_rust::Settings { pipeline_name: settings.pipeline_name.clone(), }, - ) - .into(); + ); Ok(tokens) } @@ -113,7 +116,7 @@ fn generate_rs( fn process_file( filename: Arc, ast: &mut AST, - _settings: &GenerationSettings, + sources: &mut HashMap, Vec>, ) -> Result<(), syn::Error> { let contents = match fs::read_to_string(&*filename) { Ok(c) => c, @@ -128,25 +131,26 @@ fn process_file( process_file( Arc::new(joined.to_str().unwrap().to_string()), ast, - _settings, + sources, )? } else { - process_file(Arc::new(included.clone()), ast, _settings)?; + process_file(Arc::new(included.clone()), ast, sources)?; } } - let (_, diags) = check::all(ast); let lines: Vec<&str> = ppr.lines.iter().map(|x| x.as_str()).collect(); - check(&lines, &diags); - let lxr = lexer::Lexer::new(lines.clone(), filename); + let lxr = lexer::Lexer::new(lines, filename.clone()); let mut psr = parser::Parser::new(lxr); psr.run(ast).unwrap(); - p4_rust::sanitize(ast); + sources.insert(filename, ppr.lines); Ok(()) } // TODO copy pasta from x4c -fn check(lines: &[&str], diagnostics: &Diagnostics) { +fn check( + sources: &HashMap, Vec>, + diagnostics: &Diagnostics, +) -> Result<(), syn::Error> { let errors = diagnostics.errors(); if !errors.is_empty() { let mut err = Vec::new(); @@ -154,9 +158,76 @@ fn check(lines: &[&str], diagnostics: &Diagnostics) { err.push(SemanticError { at: e.token.clone(), message: e.message.clone(), - source: lines[e.token.line].into(), + source: sources + .get(&e.token.file) + .and_then(|lines| lines.get(e.token.line)) + .cloned() + .unwrap_or_default(), }); } - panic!("{}", error::Error::Semantic(err)); + return Err(syn::Error::new( + proc_macro2::Span::call_site(), + error::Error::Semantic(err).to_string(), + )); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn generate(path: &Path) -> Result { + let settings = GenerationSettings::default(); + generate_rs(path.to_str().unwrap().into(), settings) + } + + #[test] + fn macro_rejects_width_in_final_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("wide.p4"); + fs::write(&path, "header wide_t {\n bit<129> field;\n}\n").unwrap(); + + let error = generate(&path).expect_err("bit<129> accepted").to_string(); + + assert!(error.contains("Width 129 exceeds the 128-bit compiler limit")); + assert!(error.contains("bit<129> field;")); + } + + #[test] + fn macro_diagnostic_uses_included_source() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("main.p4"); + let included = dir.path().join("wide.p4"); + fs::write(&path, "#include \n").unwrap(); + fs::write( + &included, + "\n\n\nheader wide_t {\n bit<129> included_field;\n}\n", + ) + .unwrap(); + + let error = generate(&path).expect_err("bit<129> accepted").to_string(); + + assert!(error.contains("Width 129 exceeds the 128-bit compiler limit")); + assert!(error.contains("bit<129> included_field;")); + assert!(error.contains(included.to_str().unwrap())); + } + + #[test] + fn macro_checks_complete_program_after_includes() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("main.p4"); + fs::write( + &path, + "#include \nheader header_t {\n bit<8> field;\n}\n", + ) + .unwrap(); + fs::write( + dir.path().join("structs.p4"), + "struct headers_t {\n header_t hdr;\n}\n", + ) + .unwrap(); + + generate(&path).expect("complete program rejected"); } } diff --git a/lang/p4rs/src/bitmath.rs b/lang/p4rs/src/bitmath.rs index 0f8c0686..6ef6a3f5 100644 --- a/lang/p4rs/src/bitmath.rs +++ b/lang/p4rs/src/bitmath.rs @@ -1,6 +1,7 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use bitvec::prelude::*; +use std::cmp::Ordering; pub fn add_be(a: BitVec, b: BitVec) -> BitVec { let len = usize::max(a.len(), b.len()); @@ -98,6 +99,38 @@ pub fn mod_be(a: BitVec, b: BitVec) -> BitVec { c } +/// Left shift `a` by `b` positions, little-endian byte order. +/// Result width matches `a`. Shifts by the operand width or more produce zero. +pub fn shl_le(a: BitVec, b: BitVec) -> BitVec { + if a.is_empty() { + return a; + } + let len = a.len(); + let x: u128 = a.load_le(); + let y: u128 = if b.is_empty() { 0 } else { b.load_le() }; + let z = if y >= len as u128 { 0 } else { x << (y as u32) }; + let mut c = BitVec::new(); + c.resize(len, false); + c.store_le(z); + c +} + +/// Right shift `a` by `b` positions, little-endian byte order. +/// Result width matches `a`. Shifts by the operand width or more produce zero. +pub fn shr_le(a: BitVec, b: BitVec) -> BitVec { + if a.is_empty() { + return a; + } + let len = a.len(); + let x: u128 = a.load_le(); + let y: u128 = if b.is_empty() { 0 } else { b.load_le() }; + let z = if y >= len as u128 { 0 } else { x >> (y as u32) }; + let mut c = BitVec::new(); + c.resize(len, false); + c.store_le(z); + c +} + pub fn mod_le(a: BitVec, b: BitVec) -> BitVec { let len = usize::max(a.len(), b.len()); @@ -112,10 +145,67 @@ pub fn mod_le(a: BitVec, b: BitVec) -> BitVec { c } +/// An unsigned ordering view of a little-endian `bit` value. +/// +/// Implements the unsigned comparisons on `bit` from P4-16 section [8.6]. +/// [`BitVec`]'s own ordering compares bits lexicographically, which is wrong +/// for byte-reversed multi-byte fields. +/// +/// Note: an empty bit-string compares as zero. +/// +/// [8.6]: https://p4.org/wp-content/uploads/sites/53/2024/10/P4-16-spec-v1.2.5.html#sec-bit-ops +pub struct Unsigned<'a>(pub &'a BitSlice); + +impl Unsigned<'_> { + fn value(&self) -> u128 { + if self.0.is_empty() { + 0 + } else { + self.0.load_le() + } + } +} + +impl Ord for Unsigned<'_> { + fn cmp(&self, other: &Self) -> Ordering { + self.value().cmp(&other.value()) + } +} + +impl PartialOrd for Unsigned<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl PartialEq for Unsigned<'_> { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl Eq for Unsigned<'_> {} + #[cfg(test)] mod tests { use super::*; + #[test] + fn unsigned_orders_by_value() { + let mut a = bitvec![u8, Msb0; 0; 16]; + a.store_le(0x0100u16); + let mut b = bitvec![u8, Msb0; 0; 16]; + b.store_le(0x00ffu16); + let mut one = bitvec![u8, Msb0; 0; 16]; + one.store_le(1u16); + let zero = bitvec![u8, Msb0; 0; 16]; + let empty = BitVec::::new(); + + assert!(Unsigned(&a) > Unsigned(&b)); + assert!(Unsigned(&empty) < Unsigned(&one)); + assert!(Unsigned(&empty) == Unsigned(&zero)); + } + #[test] fn bitmath_add_be() { let mut a = bitvec![mut u8, Msb0; 0; 16]; @@ -265,4 +355,85 @@ mod tests { let cc: u128 = c.load_be(); assert_eq!(cc, 47u128 % 7u128); } + + #[test] + fn bitmath_shl_le() { + let mut a = bitvec![mut u8, Msb0; 0; 16]; + a.store_le(1u128); + let mut b = bitvec![mut u8, Msb0; 0; 16]; + b.store_le(4u128); + + println!("{:?}", a); + println!("{:?}", b); + let c = shl_le(a, b); + println!("{:?}", c); + + let cc: u128 = c.load_le(); + assert_eq!(cc, 1u128 << 4); + } + + #[test] + fn bitmath_shr_le() { + let mut a = bitvec![mut u8, Msb0; 0; 16]; + a.store_le(0x8000u128); + let mut b = bitvec![mut u8, Msb0; 0; 16]; + b.store_le(4u128); + + println!("{:?}", a); + println!("{:?}", b); + let c = shr_le(a, b); + println!("{:?}", c); + + let cc: u128 = c.load_le(); + assert_eq!(cc, 0x8000u128 >> 4); + } + + #[test] + fn bitmath_shifts_at_width_le() { + let mut a = bitvec![mut u8, Msb0; 0; 16]; + a.store_le(1u128); + let mut b = bitvec![mut u8, Msb0; 0; 16]; + b.store_le(16u128); + + assert_eq!(shl_le(a.clone(), b.clone()).load_le::(), 0); + assert_eq!(shr_le(a, b).load_le::(), 0); + } + + #[test] + fn bitmath_shifts_by_zero_width_count() { + let mut value = bitvec![u8, Msb0; 0; 16]; + value.store_le(0x8123u16); + + assert_eq!(shl_le(value.clone(), BitVec::new()), value); + assert_eq!(shr_le(value.clone(), BitVec::new()), value); + } + + #[test] + fn bitmath_shifts_zero_width_value() { + let mut count = bitvec![u8, Msb0; 0; 8]; + count.store_le(1u8); + + for count in [BitVec::new(), count] { + assert!(shl_le(BitVec::new(), count.clone()).is_empty()); + assert!(shr_le(BitVec::new(), count).is_empty()); + } + } + + #[test] + fn bitmath_shl_shr_roundtrip_le() { + let mut a = bitvec![mut u8, Msb0; 0; 32]; + a.store_le(42u128); + let mut b = bitvec![mut u8, Msb0; 0; 32]; + b.store_le(7u128); + + println!("{:?}", a); + println!("{:?}", b); + let shifted = shl_le(a, b.clone()); + println!("{:?}", shifted); + let back = shr_le(shifted, b); + println!("{:?}", back); + + let result: u128 = back.load_le(); + assert_eq!(result, 42u128); + } } diff --git a/lang/p4rs/src/lib.rs b/lang/p4rs/src/lib.rs index 077ddd72..4ad22e8e 100644 --- a/lang/p4rs/src/lib.rs +++ b/lang/p4rs/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company //! This is the runtime support create for `x4c` generated programs. //! @@ -156,9 +156,7 @@ pub struct TableEntry { } pub trait Pipeline: Send { - /// Process an input packet and produce a set of output packets. Normally - /// there will be a single output packet. However, if the pipeline sets - /// `egress_metadata_t.broadcast` there may be multiple output packets. + /// Process an input packet and produce a set of output packets. fn process_packet<'a>( &mut self, port: u16, @@ -239,16 +237,13 @@ impl<'a> packet_in<'a> { //XXX: remove once classifier defined in terms of bitvecs pub fn bitvec_to_biguint(bv: &BitVec) -> table::BigUintKey { - let mut bytes = bv.as_raw_slice().to_vec(); - - // Align the last bits - if let Some(last) = bytes.last_mut() { - *last >>= (8 - bv.len() % 8) % 8; - } - table::BigUintKey { - value: num::BigUint::from_bytes_le(&bytes), - width: bytes.len(), + value: if bv.is_empty() { + num::BigUint::default() + } else { + bv.load_le::().into() + }, + width: bv.len().div_ceil(8), } } @@ -268,9 +263,26 @@ pub fn int_to_bitvec(x: i128) -> BitVec { bv } -pub fn bitvec_to_bitvec16(mut x: BitVec) -> BitVec { - x.resize(16, false); - x +/// Resize `x` to 16 bits, zero-extending or truncating its value. +pub fn bitvec_to_bitvec16(x: BitVec) -> BitVec { + bitvec_resize(x, 16) +} + +/// Resize `x` to `width` bits. +/// +/// Any added high-order bits are zero. +/// +/// This backs x4c's implicit `bit` to `bit` widening, which P4-16 +/// expresses as an explicit cast ([8.11.1]). +/// +/// [8.11.1]: https://p4.org/wp-content/uploads/sites/53/2024/10/P4-16-spec-v1.2.5.html#sec-explicit-casts +pub fn bitvec_resize(x: BitVec, width: usize) -> BitVec { + let value: u128 = if x.is_empty() { 0 } else { x.load_le() }; + let mut resized = bitvec![u8, Msb0; 0; width]; + if width > 0 { + resized.store_le(value); + } + resized } pub fn dump_bv(x: &BitVec) -> String { @@ -342,7 +354,7 @@ pub fn extract_lpm_key( offset: usize, len: usize, ) -> table::Key { - let (addr, len) = match len { + let (addr, prefix_len) = match len { // IPv4 4 => { let data: [u8; 4] = @@ -360,7 +372,10 @@ pub fn extract_lpm_key( } }; - table::Key::Lpm(table::Prefix { addr, len }) + table::Key::Lpm(table::Prefix { + addr, + len: prefix_len, + }) } pub fn extract_bool_action_parameter( @@ -370,19 +385,57 @@ pub fn extract_bool_action_parameter( parameter_data[offset] == 1 } +/// Extract a `bit` action parameter from the control-plane action data. +/// +/// Action data lays out each parameter in declaration order as +/// `size.div_ceil(8)` little-endian bytes, starting at `offset`. x4c keeps +/// every numeric value little-endian inside a program (header fields are +/// byte-reversed on parse and once again on deparse), so the control plane +/// passes these values into this representation with no conversion necessary; +/// the same is true for `exact` and `range` keys. +/// +/// The returned vector has the same layout as [`bitvec_resize`] output and +/// header fields of the same width, so `load_le` recovers the value whenever +/// the width fits the target integer (up to 128 bits for `u128`). pub fn extract_bit_action_parameter( parameter_data: &[u8], offset: usize, size: usize, ) -> BitVec { - let mut byte_size = size >> 3; - if !size.is_multiple_of(8) { - byte_size += 1; + let byte_size = size.div_ceil(8); + let mut bytes = parameter_data[offset..offset + byte_size].to_vec(); + let rem = size % 8; + // Msb0 keeps a partial last byte in its high bits, so the value's top + // `rem` bits move up before the vector is truncated to `size`. + if rem != 0 { + bytes[byte_size - 1] <<= 8 - rem; } - let mut b: BitVec = - BitVec::from_slice(¶meter_data[offset..offset + byte_size]); - b.resize(size, false); - b + let mut bv = BitVec::::from_vec(bytes); + bv.resize(size, false); + bv +} + +/// Collect output ports from a bitmap, excluding the ingress port. +/// +/// Bits at or above `radix` are ignored, which means that a bitmap cannot +/// address a port outside the pipeline. +/// +/// The bitmap is interpreted as a little-endian integer: bit N +/// (i.e., the bit with numeric value 2^N) corresponds to port N. +/// This matches the encoding used by P4 arithmetic (`128w1 << port`) +/// via [`bitmath::shl_le`]. +pub fn replicate( + bitmap: &BitVec, + ingress_port: u16, + radix: u16, +) -> Vec { + if bitmap.is_empty() { + return Vec::new(); + } + let val: u128 = bitmap.load_le(); + (0..radix.min(128)) + .filter(|&p| val & (1u128 << p) != 0 && p != ingress_port) + .collect() } #[cfg(test)] @@ -390,7 +443,109 @@ mod tests { use super::*; use num::bigint::BigUint; - /// Checks [`bitvec_to_biguint`] is semantically equivalent to `load_le` + #[test] + fn bitvec_resize_preserves_value() { + for (source_width, target_width, value) in [ + (4, 8, 1u128), + (4, 16, 0xf), + (8, 12, 0xab), + (12, 16, 0xabc), + (12, 24, 0xabc), + (16, 128, 0xbeef), + (128, 128, u128::MAX), + ] { + let mut input = bitvec![u8, Msb0; 0; source_width]; + input.store_le(value); + + let out = bitvec_resize(input, target_width); + + assert_eq!(out.len(), target_width); + assert_eq!( + out.load_le::(), + value, + "{source_width} -> {target_width}", + ); + } + } + + #[test] + fn extract_bit_action_parameter_sub_byte_widths() { + for (width, value) in [ + (12, 100u16), + (12, 256), + (12, 300), + (12, 4094), + (4, 0xa), + (9, 0x1ab), + (10, 0x2ab), + (11, 0x5ab), + (13, 0x1abc), + (14, 0x2abc), + (15, 0x5abc), + (16, 0xbeef), + ] { + let mut data = vec![0xffu8]; + data.extend_from_slice(&value.to_le_bytes()); + + let out = extract_bit_action_parameter(&data, 1, width); + + assert_eq!(out.len(), width); + assert_eq!(out.load_le::(), value, "bit<{width}> = {value}"); + + let mut expected = bitvec![u8, Msb0; 0; width]; + expected.store_le(value); + assert_eq!(out, expected, "bit<{width}> layout"); + } + } + + #[test] + fn extract_bit_action_parameter_wide_unaligned() { + let mut data = vec![0xa5u8; 16]; + data.push(0x0f); + + let out = extract_bit_action_parameter(&data, 0, 132); + + assert_eq!(out.len(), 132); + let mut expected = vec![0xa5u8; 16]; + expected.push(0xf0); + assert_eq!(out.as_raw_slice(), expected.as_slice()); + } + + #[test] + fn bitvec_resize_empty_values() { + let out = bitvec_resize(BitVec::new(), 8); + assert_eq!(out.len(), 8); + assert_eq!(out.load_le::(), 0); + assert!(bitvec_resize(BitVec::new(), 0).is_empty()); + } + + #[test] + fn bitvec_resize_aligns_slices() { + for offset in 0..8 { + for (source_width, target_width, value) in [ + (4, 8, 0xbu128), + (8, 16, 0xab), + (12, 16, 0xabc), + (16, 24, 0xbeef), + (128, 128, u128::MAX), + ] { + let storage = bitvec![u8, Msb0; 0; offset + source_width]; + let mut input = storage[offset..].to_bitvec(); + input.store_le(value); + + let out = bitvec_resize(input, target_width); + let mut expected = bitvec![u8, Msb0; 0; target_width]; + expected.store_le(value); + + assert_eq!( + out, expected, + "offset {offset}: {source_width} -> {target_width}", + ); + } + } + } + + /// Checks [`bitvec_to_biguint`] is semantically equivalent to `load_le`, /// even with non-byte-aligned widths. #[test] fn bitvec_to_biguint_non_byte_aligned() { @@ -404,4 +559,54 @@ mod tests { ); } } + + #[test] + fn bitvec_to_biguint_parsed_ihl() { + let data = [0x45u8]; + let ihl = data.view_bits::()[4..8].to_bitvec(); + let key = bitvec_to_biguint(&ihl); + + assert_eq!(key.value, BigUint::from(5u8)); + assert_eq!(key.width, 1); + } + + #[test] + fn bitvec_to_biguint_storage_offsets() { + for offset in 0..8 { + for width in 1..=128 { + let storage = bitvec![u8, Msb0; 1; offset + width]; + let mut bv = storage[offset..].to_bitvec(); + let expected = u128::MAX >> (128 - width); + bv.store_le(expected); + bv.set_uninitialized(true); + let key = bitvec_to_biguint(&bv); + + assert_eq!(key.value, BigUint::from(expected)); + assert_eq!(key.width, width.div_ceil(8)); + } + } + } + + #[test] + fn bitvec_to_biguint_empty() { + let key = bitvec_to_biguint(&BitVec::new()); + + assert_eq!(key.value, BigUint::default()); + assert_eq!(key.width, 0); + } + + #[test] + fn replication_radix_is_bounded_by_bitmap_width() { + let mut bitmap = bitvec![u8, Msb0; 0; 128]; + bitmap.store_le(1u128 << 127); + + assert_eq!(replicate(&bitmap, 0, u16::MAX), vec![127]); + } + + #[test] + fn replication_of_empty_bitmap_yields_no_ports() { + let bitmap: BitVec = BitVec::new(); + + assert_eq!(replicate(&bitmap, 0, 4), Vec::::new()); + } } diff --git a/lang/prog/sidecar-lite/src/lib.rs b/lang/prog/sidecar-lite/src/lib.rs index d3b83f28..8525ff16 100644 --- a/lang/prog/sidecar-lite/src/lib.rs +++ b/lang/prog/sidecar-lite/src/lib.rs @@ -1,5 +1,70 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company #![allow(clippy::too_many_arguments)] p4_macro::use_p4!(p4 = "test/src/p4/sidecar-lite.p4", pipeline_name = "main"); + +#[cfg(test)] +mod tests { + use super::*; + use p4rs::{packet_in, Pipeline}; + + fn v6_packet() -> [u8; 62] { + let mut buf = [0u8; 62]; + buf[..6].copy_from_slice(&[0x02, 0, 0, 0, 0, 2]); + buf[6..12].copy_from_slice(&[0x02, 0, 0, 0, 0, 1]); + buf[12..14].copy_from_slice(&0x86ddu16.to_be_bytes()); + buf[14] = 0x60; + buf[18..20].copy_from_slice(&8u16.to_be_bytes()); + buf[20] = 59; + buf[21] = 64; + buf[22..38].copy_from_slice(&[0xfd; 16]); + buf[38..54].copy_from_slice(&[0xfe; 16]); + buf + } + + #[test] + fn routing_miss_drops() { + let mut pipeline = main_pipeline::new(4); + let buf = v6_packet(); + let mut pkt = packet_in::new(&buf); + let out = pipeline.process_packet(0, &mut pkt); + let ports: Vec = out.iter().map(|(_, p)| *p).collect(); + assert_eq!(ports, Vec::::new(), "routing miss must drop"); + } + + #[test] + fn mcast_group_replicates_to_bitmap_ports() { + let mut pipeline = main_pipeline::new(4); + + let group: std::net::Ipv6Addr = "ff0e::1".parse().unwrap(); + let bitmap: u128 = (1 << 1) | (1 << 2) | (1 << 3); + pipeline.add_ingress_mcast_mcast_replication_v6_entry( + "set_port_bitmap", + &u128::from(group).to_le_bytes(), + &bitmap.to_le_bytes(), + 0, + ); + + let mut buf = v6_packet(); + buf[..6].copy_from_slice(&[0x33, 0x33, 0x00, 0x00, 0x00, 0x01]); + buf[38..54].copy_from_slice(&group.octets()); + + let mut pkt = packet_in::new(&buf); + let packet_ports: Vec = pipeline + .process_packet(1, &mut pkt) + .iter() + .map(|(_, port)| *port) + .collect(); + + let mut pkt = packet_in::new(&buf); + let header_ports: Vec = pipeline + .process_packet_headers(1, &mut pkt) + .iter() + .map(|(_, port)| *port) + .collect(); + + assert_eq!(packet_ports, [2, 3], "packet"); + assert_eq!(header_ports, [2, 3], "headers"); + } +} diff --git a/p4/examples/bad/checker/control-data-variables.p4 b/p4/examples/bad/checker/control-data-variables.p4 new file mode 100644 index 00000000..d22e5703 --- /dev/null +++ b/p4/examples/bad/checker/control-data-variables.p4 @@ -0,0 +1,38 @@ +// Valid P4 control-scope data variables rejected by the x4c target. + +header h_t { + bit<8> f; +} + +struct data_t { + bit<8> f; +} + +control ingress(out bit<16> port) { + bit<16> selected = 16w1; + bit<8> scratch; + int<8> signed_value; + bool enabled = true; + h_t h; + data_t data; + + action assign() { + port = selected; + } + + table route { + key = { + selected: exact; + } + actions = { + assign; + } + default_action = assign; + } + + apply { + port = selected; + assign(); + route.apply(); + } +} diff --git a/p4/examples/codegen/double_instantiation.p4 b/p4/examples/codegen/double_instantiation.p4 deleted file mode 100644 index 2bfc0430..00000000 --- a/p4/examples/codegen/double_instantiation.p4 +++ /dev/null @@ -1,58 +0,0 @@ -#include -#include - -SoftNPU( - parse(), - ingress() -) main; - -struct headers_t { } - -parser parse( - packet_in pkt, - out headers_t headers, - inout ingress_metadata_t ingress, -){ - state start { transition accept; } -} - -control resolver( - in bit<32> x, - out bool resolved, -) { - action resolve() { - resolved = true; - } - table arp { - key = { x: exact; } - actions = { resolve; } - default_action = NoAction; - } - apply { arp.apply(); } -} - -control foo() { - resolver() resolver; - apply { - resolver.apply(23w47); - } -} - -control bar() { - resolver() resolver; - apply { resolver.apply(23w1701); } -} - -control ingress( - inout headers_t hdr, - inout ingress_metadata_t ingress, - inout egress_metadata_t egress, -) { - foo() foo; - bar() bar; - - apply { - bar.apply(); - foo.apply(); - } -} diff --git a/p4/examples/control-instances.p4 b/p4/examples/control-instances.p4 new file mode 100644 index 00000000..409a519f --- /dev/null +++ b/p4/examples/control-instances.p4 @@ -0,0 +1,22 @@ +extern Checksum { + bit<16> run(in T data); +} + +control helper() { + apply { } +} + +control ingress() { + Checksum() checksum; + helper() child; + + action use_local() { + bit<8> action_local = 8w1; + } + + apply { + bit<8> apply_local = 8w1; + use_local(); + child.apply(); + } +} diff --git a/p4/examples/expressions.p4 b/p4/examples/expressions.p4 new file mode 100644 index 00000000..4a58d05b --- /dev/null +++ b/p4/examples/expressions.p4 @@ -0,0 +1,6 @@ +control ingress() { + apply { + bit<8> x = 8w1; + bool result = x << 1 == 8w2; + } +} diff --git a/p4/src/ast.rs b/p4/src/ast.rs index 5f52520f..461c56f6 100644 --- a/p4/src/ast.rs +++ b/p4/src/ast.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use std::cmp::{Eq, PartialEq}; use std::collections::HashMap; @@ -302,6 +302,11 @@ pub enum Type { Bit(usize), Varbit(usize), Int(usize), + /// Arbitrary-precision `int`, which is valid only for compile-time values + /// (P4-16 7.1.6.5). + /// + /// `Int(w)` becomes the fixed-width `int`. + ArbitraryInt, String, UserDefined(String), ExternFunction, //TODO actual signature @@ -359,6 +364,7 @@ impl fmt::Display for Type { Type::Bit(size) => write!(f, "bit<{}>", size), Type::Varbit(size) => write!(f, "varbit<{}>", size), Type::Int(size) => write!(f, "int<{}>", size), + Type::ArbitraryInt => write!(f, "int"), Type::String => write!(f, "string"), Type::UserDefined(name) => write!(f, "{}", name), Type::ExternFunction => write!(f, "extern function"), @@ -657,6 +663,8 @@ pub enum BinOp { BitAnd, BitOr, Xor, + Shl, + Shr, } impl BinOp { @@ -673,6 +681,8 @@ impl BinOp { BinOp::BitAnd => "bitwise and", BinOp::BitOr => "bitwise or", BinOp::Xor => "xor", + BinOp::Shl => "shift left", + BinOp::Shr => "shift right", } } @@ -1674,6 +1684,8 @@ impl MatchKind { pub enum Statement { Empty, Assignment(Lvalue, Box), + /// `lval[hi:lo] = expr` (P4-16 spec 8.6). + SliceAssignment(Lvalue, Box, Box, Box), //TODO get rid of this in favor of ExpressionKind::Call ??? Call(Call), If(IfBlock), @@ -1693,6 +1705,12 @@ impl Statement { lval.accept(v); xpr.accept(v); } + Statement::SliceAssignment(lval, hi, lo, xpr) => { + lval.accept(v); + hi.accept(v); + lo.accept(v); + xpr.accept(v); + } Statement::Call(call) => call.accept(v), Statement::If(if_block) => if_block.accept(v), Statement::Variable(var) => var.accept(v), @@ -1714,6 +1732,12 @@ impl Statement { lval.accept_mut(v); xpr.accept_mut(v); } + Statement::SliceAssignment(lval, hi, lo, xpr) => { + lval.accept_mut(v); + hi.accept_mut(v); + lo.accept_mut(v); + xpr.accept_mut(v); + } Statement::Call(call) => call.accept_mut(v), Statement::If(if_block) => if_block.accept_mut(v), Statement::Variable(var) => var.accept_mut(v), @@ -1735,6 +1759,12 @@ impl Statement { lval.mut_accept(v); xpr.mut_accept(v); } + Statement::SliceAssignment(lval, hi, lo, xpr) => { + lval.mut_accept(v); + hi.mut_accept(v); + lo.mut_accept(v); + xpr.mut_accept(v); + } Statement::Call(call) => call.mut_accept(v), Statement::If(if_block) => if_block.mut_accept(v), Statement::Variable(var) => var.mut_accept(v), @@ -1756,6 +1786,12 @@ impl Statement { lval.mut_accept_mut(v); xpr.mut_accept_mut(v); } + Statement::SliceAssignment(lval, hi, lo, xpr) => { + lval.mut_accept_mut(v); + hi.mut_accept_mut(v); + lo.mut_accept_mut(v); + xpr.mut_accept_mut(v); + } Statement::Call(call) => call.mut_accept_mut(v), Statement::If(if_block) => if_block.mut_accept_mut(v), Statement::Variable(var) => var.mut_accept_mut(v), diff --git a/p4/src/check.rs b/p4/src/check.rs index 443ff06f..c97f2f4e 100644 --- a/p4/src/check.rs +++ b/p4/src/check.rs @@ -1,13 +1,13 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use std::collections::HashMap; use crate::ast::{ - Call, Control, DeclarationInfo, Expression, ExpressionKind, Header, Lvalue, - NameInfo, Parser, State, Statement, StatementBlock, Struct, Table, - Transition, Type, VisitorMut, AST, + Call, Control, ControlParameter, DeclarationInfo, Direction, Expression, + ExpressionKind, Header, Lvalue, NameInfo, Parser, State, Statement, + StatementBlock, Struct, Table, Transition, Type, VisitorMut, AST, }; -use crate::hlir::{Hlir, HlirGenerator}; +use crate::hlir::{implicitly_converts, Hlir, HlirGenerator}; use crate::lexer::Token; use colored::Colorize; @@ -77,9 +77,59 @@ pub fn all(ast: &AST) -> (Hlir, Diagnostics) { for h in &ast.headers { diags.extend(&HeaderChecker::check(h, ast)); } + + check_port_bitmap(ast, &mut diags); + (hg.hlir, diags) } +/// The egress port bitmap, e.g. for multicast replication. +/// +/// Bit N (the bit set by `1 << N`) selects port N. The pipeline reads this +/// field after ingress. +/// +/// A non-zero value takes precedence over `broadcast` and `port`. If filtering +/// excludes every selected port, the packet is dropped. +pub const PORT_BITMAP_FIELD: &str = "port_bitmap"; + +/// If declared, require `PORT_BITMAP_FIELD` in SoftNPU egress metadata to be a +/// `bit`. +fn check_port_bitmap(ast: &AST, diags: &mut Diagnostics) { + let Some(egress_struct) = ast + .package_instance + .as_ref() + // SoftNPU(parser, ingress, egress) -> the egress control. + .and_then(|inst| inst.parameters.get(2)) + .and_then(|name| ast.get_control(name)) + // Control parameters are (headers, ingress_meta, egress_meta). + .and_then(|egress| egress.parameters.get(2)) + .and_then(|meta| match &meta.ty { + Type::UserDefined(name) => ast.get_struct(name), + _ => None, + }) + else { + return; + }; + + diags.0.extend( + egress_struct + .members + .iter() + .filter(|m| { + m.name == PORT_BITMAP_FIELD && !matches!(m.ty, Type::Bit(_)) + }) + .map(|m| Diagnostic { + level: Level::Error, + message: format!( + "{PORT_BITMAP_FIELD} in egress metadata {} must be a \ + bit, found {}", + egress_struct.name, m.ty, + ), + token: m.token.clone(), + }), + ); +} + pub struct ControlChecker {} impl ControlChecker { @@ -96,6 +146,8 @@ impl ControlChecker { pub fn check_params(c: &Control, ast: &AST, diags: &mut Diagnostics) { for p in &c.parameters { + check_type_width(&p.ty, &p.ty_token, diags); + check_parameter_type(p, diags); if let Type::UserDefined(typename) = &p.ty { if ast.get_user_defined_type(typename).is_none() { diags.push(Diagnostic { @@ -135,23 +187,50 @@ impl ControlChecker { message: "Table must have a default action".into(), token: t.token.clone(), }); + } else if !t + .actions + .iter() + .any(|action| action.name == t.default_action) + { + // P4-16 14.2.1.3: the default action must be one of the actions + // that appear in the actions list. + diags.push(Diagnostic { + level: Level::Error, + message: format!( + "default action {} is not in the actions of table {}", + t.default_action, t.name, + ), + token: t.token.clone(), + }); } } pub fn check_variables(c: &Control, ast: &AST, diags: &mut Diagnostics) { for v in &c.variables { - if let Type::UserDefined(typename) = &v.ty { - if ast.get_user_defined_type(typename).is_some() { - continue; + match &v.ty { + Type::Bit(w) | Type::Varbit(w) | Type::Int(w) if *w > 128 => { + check_type_width(&v.ty, &v.token, diags); } - if ast.get_control(typename).is_some() { - continue; + Type::UserDefined(typename) + if ast.get_extern(typename).is_some() + || ast.get_control(typename).is_some() => {} + Type::UserDefined(typename) + if ast.get_user_defined_type(typename).is_none() => + { + diags.push(Diagnostic { + level: Level::Error, + message: format!("Typename {typename} not found"), + token: v.token.clone(), + }); } - diags.push(Diagnostic { + _ => diags.push(Diagnostic { level: Level::Error, - message: format!("Typename {} not found", typename), + message: format!( + "control-scope data variable {} is not supported", + v.name, + ), token: v.token.clone(), - }) + }), } } } @@ -166,6 +245,10 @@ impl ControlChecker { Self::check_table_action_reference(c, t, ast, diags); } for a in &c.actions { + for p in &a.parameters { + check_type_width(&p.ty, &p.ty_token, diags); + check_runtime_type(&p.ty, &p.ty_token, diags); + } check_statement_block(&a.statement_block, hlir, diags, ast, true); } } @@ -177,7 +260,7 @@ impl ControlChecker { diags: &mut Diagnostics, ) { for a in &t.actions { - if c.get_action(&a.name).is_none() { + if a.name != "NoAction" && c.get_action(&a.name).is_none() { diags.push(Diagnostic { level: Level::Error, message: format!( @@ -217,64 +300,12 @@ fn check_statement_block( ) { for s in &block.statements { match s { - Statement::Assignment(lval, xpr) => { - let name_info = match hlir.lvalue_decls.get(lval) { - Some(info) => info, - None => { - diags.push(Diagnostic { - level: Level::Error, - message: format!( - "Could not resolve lvalue {}", - &lval.name, - ), - token: lval.token.clone(), - }); - return; - } - }; - - let expression_type = - match hlir.expression_types.get(xpr.as_ref()) { - Some(ty) => ty, - None => { - diags.push(Diagnostic { - level: Level::Error, - message: "Could not determine expression type" - .to_owned(), - token: xpr.token.clone(), - }); - return; - } - }; - - if &name_info.ty != expression_type { - diags.push(Diagnostic { - level: Level::Error, - message: format!( - "Cannot assign {} to {}", - expression_type, &name_info.ty, - ), - token: xpr.token.clone(), - }); - } - } Statement::Empty => {} Statement::Call(c) if in_action => { - let lval = c.lval.pop_right(); - let name_info = match hlir.lvalue_decls.get(&lval) { - Some(info) => info, - None => { - diags.push(Diagnostic { - level: Level::Error, - message: format!( - "Could not resolve lvalue {}", - &c.lval.name, - ), - token: c.lval.token.clone(), - }); - return; - } - }; + let name_info = hlir + .lvalue_decls + .get(&c.lval.pop_right()) + .expect("call lvalue resolved by hlir"); match &name_info.ty { Type::Table => { diags.push(Diagnostic { @@ -299,9 +330,22 @@ fn check_statement_block( _ => {} } } - _ => { - // TODO + Statement::Variable(v) => { + check_type_width(&v.ty, &v.token, diags); + check_runtime_type(&v.ty, &v.token, diags); } + Statement::If(ifb) => { + check_statement_block(&ifb.block, hlir, diags, ast, in_action); + for ei in &ifb.else_ifs { + check_statement_block( + &ei.block, hlir, diags, ast, in_action, + ); + } + if let Some(eb) = &ifb.else_block { + check_statement_block(eb, hlir, diags, ast, in_action); + } + } + _ => {} } } } @@ -317,16 +361,8 @@ impl VisitorMut for ApplyCallChecker<'_> { fn call(&mut self, call: &Call) { let name = call.lval.root(); let names = self.c.names(); - let name_info = match names.get(name) { - Some(info) => info, - None => { - self.diags.push(Diagnostic { - level: Level::Error, - message: format!("{} is undefined", name), - token: call.lval.token.clone(), - }); - return; - } + let Some(name_info) = names.get(name) else { + return; }; match &name_info.ty { @@ -383,7 +419,11 @@ impl ApplyCallChecker<'_> { None => panic!("bug: no type for expression {:?}", arg), }; let param = &ctl.parameters[i]; - if arg_t != ¶m.ty { + let compatible = match param.direction { + Direction::Out | Direction::InOut => arg_t == ¶m.ty, + _ => implicitly_converts(arg, arg_t, ¶m.ty), + }; + if !compatible { self.diags.push(Diagnostic { level: Level::Error, message: format!( @@ -408,6 +448,10 @@ impl ParserChecker { pub fn check(p: &Parser, ast: &AST) -> Diagnostics { let mut diags = Diagnostics::new(); + for param in &p.parameters { + check_parameter_type(param, &mut diags); + } + if !p.decl_only { Self::start_state(p, &mut diags); for s in &p.states { @@ -484,6 +528,8 @@ impl StructChecker { pub fn check(s: &Struct, ast: &AST) -> Diagnostics { let mut diags = Diagnostics::new(); for m in &s.members { + check_type_width(&m.ty, &m.token, &mut diags); + check_runtime_type(&m.ty, &m.token, &mut diags); if let Type::UserDefined(typename) = &m.ty { if ast.get_user_defined_type(typename).is_none() { diags.push(Diagnostic { @@ -507,6 +553,8 @@ impl HeaderChecker { pub fn check(h: &Header, ast: &AST) -> Diagnostics { let mut diags = Diagnostics::new(); for m in &h.members { + check_type_width(&m.ty, &m.token, &mut diags); + check_runtime_type(&m.ty, &m.token, &mut diags); if let Type::UserDefined(typename) = &m.ty { if ast.get_user_defined_type(typename).is_none() { diags.push(Diagnostic { @@ -524,6 +572,60 @@ impl HeaderChecker { } } +/// Rust represents bit values as u128 for literals, shifts, and arithmetic +/// slice operations. +/// +/// Control and action parameters, locals, and struct and header members wider +/// than 128 bits are rejected. +/// +/// TODO: parser parameters and constants are not yet checked. +fn check_type_width(ty: &Type, token: &Token, diags: &mut Diagnostics) { + if let Type::Bit(w) | Type::Varbit(w) | Type::Int(w) = ty { + if *w > 128 { + diags.push(Diagnostic { + level: Level::Error, + message: format!( + "Width {w} exceeds the 128-bit compiler limit", + ), + token: token.clone(), + }); + } + } +} + +/// Reject directional `int` parameters (P4-16 7.1.6.5) and directionless ones, +/// which SoftNPU cannot bind. +fn check_parameter_type(p: &ControlParameter, diags: &mut Diagnostics) { + match (p.direction, &p.ty) { + (Direction::Unspecified, Type::ArbitraryInt) => { + diags.push(Diagnostic { + level: Level::Error, + message: "directionless int parameters are not supported by \ + SoftNPU" + .into(), + token: p.ty_token.clone(), + }) + } + (_, Type::ArbitraryInt) => diags.push(Diagnostic { + level: Level::Error, + message: "int parameters must be directionless".into(), + token: p.ty_token.clone(), + }), + _ => {} + } +} + +/// Reject `int` in positions that hold runtime values (P4-16 7.1.6.5). +fn check_runtime_type(ty: &Type, token: &Token, diags: &mut Diagnostics) { + if let Type::ArbitraryInt = ty { + diags.push(Diagnostic { + level: Level::Error, + message: "int is only valid for compile-time known values".into(), + token: token.clone(), + }); + } +} + fn check_name( name: &str, names: &HashMap, @@ -559,6 +661,8 @@ fn check_statement_lvalues( match stmt { Statement::Empty => {} Statement::Variable(v) => { + check_type_width(&v.ty, &v.token, &mut diags); + check_runtime_type(&v.ty, &v.token, &mut diags); if let Some(expr) = &v.initializer { diags.extend(&check_expression_lvalues( expr.as_ref(), @@ -585,6 +689,10 @@ fn check_statement_lvalues( diags.extend(&check_lvalue(lval, ast, names, None)); diags.extend(&check_expression_lvalues(expr, ast, names)); } + Statement::SliceAssignment(lval, _hi, _lo, expr) => { + diags.extend(&check_lvalue(lval, ast, names, None)); + diags.extend(&check_expression_lvalues(expr, ast, names)); + } Statement::Call(call) => { diags.extend(&check_lvalue(&call.lval, ast, names, None)); for arg in &call.args { @@ -798,6 +906,19 @@ fn check_lvalue( }); } } + Type::ArbitraryInt => { + if parts.len() > 1 { + diags.push(Diagnostic { + level: Level::Error, + message: format!( + "type {} does not have a member {}", + "int".bright_blue(), + parts[1].bright_blue(), + ), + token: lval.token.clone(), + }); + } + } Type::String => { if parts.len() > 1 { diags.push(Diagnostic { @@ -952,124 +1073,329 @@ fn check_lvalue( diags } -pub struct ExpressionTypeChecker { - //ast: &'a mut AST, - //ast: RefCell::, +#[cfg(test)] +mod tests { + use crate::ast::AST; + use crate::lexer::Lexer; + use crate::parser::Parser; + use std::sync::Arc; + + fn check_p4(source: &str) -> super::Diagnostics { + let lines: Vec<&str> = source.lines().collect(); + let filename = Arc::new("test.p4".to_string()); + let lexer = Lexer::new(lines, filename); + let mut parser = Parser::new(lexer); + let mut ast = AST::default(); + parser.run(&mut ast).expect("parse failed"); + let (_hlir, diags) = crate::check::all(&ast); + diags + } + + #[test] + fn width_128_accepted() { + let source = r#" +header h_t { + bit<128> f; +} +struct headers_t { + h_t h; } +control ingress(inout headers_t hdr) { + apply { + bit<128> x = hdr.h.f; + } +} +"#; + let diags = check_p4(source); + let errors = diags.errors(); + assert!( + errors.is_empty(), + "unexpected errors: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); + } -impl ExpressionTypeChecker { - pub fn run(&self) -> (HashMap, Diagnostics) { - // These iterations may seem a bit odd. The reason I'm using numeric - // indices here is that I cannot borrow a mutable node of the AST and - // the AST itself at the same time, and then pass them separately into a - // handler function. So instead I just pass along the mutable AST - // together with the index of the thing I'm going to mutate within the - // AST. Then in the handler function, a mutable reference to the node of - // interest is acquired based on the index. - /* - let mut diags = Diagnostics::new(); - for i in 0..self.ast.constants.len() { - diags.extend(&self.check_constant(i)); - } - for i in 0..self.ast.controls.len() { - diags.extend(&self.check_control(i)); - } - for i in 0..self.ast.parsers.len() { - diags.extend(&self.check_parser(i)); + #[test] + fn control_data_variables_rejected() { + let source = + include_str!("../examples/bad/checker/control-data-variables.p4"); + + let diags = check_p4(source); + let errors = diags.errors(); + let expected = [ + "selected", + "scratch", + "signed_value", + "enabled", + "h", + "data", + ]; + assert_eq!( + errors.len(), + expected.len(), + "{:?}", + errors.iter().map(|diag| &diag.message).collect::>(), + ); + + for (error, name) in errors.iter().zip(expected) { + assert_eq!( + error.message, + format!("control-scope data variable {name} is not supported"), + ); } - diags - */ + } - todo!(); + #[test] + fn control_instances_and_block_locals_accepted() { + let source = include_str!("../examples/control-instances.p4"); + let diags = check_p4(source); + let errors = diags.errors(); + assert!( + errors.is_empty(), + "{:?}", + errors.iter().map(|diag| &diag.message).collect::>(), + ); } - pub fn check_constant(&self, _index: usize) -> Diagnostics { - todo!("global constant expression type check"); + #[test] + fn width_over_128_rejected() { + let source = r#" +header h_t { + bit<129> f; +} +struct metadata_t { + bit<130> f; +} +struct headers_t { + h_t h; +} +control ingress(inout headers_t hdr, in bit<131> parameter) { + bit<132> control_variable; + action a(bit<133> action_parameter) { + bit<134> action_variable; + } + apply { + bit<135> apply_variable; + } +} +"#; + let diags = check_p4(source); + let errors = diags.errors(); + assert_eq!( + errors.len(), + 7, + "expected an error for each declaration site: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); + let messages: Vec<_> = + errors.iter().map(|error| &error.message).collect(); + for width in 129..=135 { + assert!( + messages.iter().any(|message| message + .contains(&format!("Width {width} exceeds"))), + "missing diagnostic for width {width}: {messages:?}", + ); + } } - pub fn check_control(&self, _index: usize) -> Diagnostics { - /* - let c = &mut self.ast.controls[index]; - let mut diags = Diagnostics::new(); - let names = c.names(); - for a in &mut c.actions { - diags.extend( - &self.check_statement_block(&mut a.statement_block, &names) - ) + const PORT_BITMAP_PIPELINE: &str = r#" +struct headers_t { + bit<8> f; +} +struct ingress_metadata_t { + bit<16> port; +} +struct egress_metadata_t { + PORT_BITMAP_TYPE port_bitmap; +} +parser parse(inout headers_t hdr) { + state start { + transition accept; + } +} +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + apply { } +} +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + apply { } +} +SoftNPU(parse(), ingress(), egress()) main; +"#; + + #[test] + fn control_checks_follow_hlir() { + let source = r#" +header h_t { + bit<8> f; +} +struct headers_t { + h_t h; +} +control child(in bool valid, in bit<8> value) { + apply { } +} +control ingress(inout headers_t hdr) { + child() c; + table t { + key = { hdr.h.f: exact; } + actions = { NoAction; } + default_action = NoAction; + } + action nested() { + if (true) { + t.apply(); } - diags - */ - todo!(); + } + apply { + c.apply(hdr.h.isValid(), 5); + } +} +"#; + let diags = check_p4(source); + let errors: Vec<_> = + diags.errors().iter().map(|d| d.message.clone()).collect(); + assert_eq!(errors, ["Cannot apply table within action"]); } - pub fn check_statement_block( - &self, - _sb: &mut StatementBlock, - _names: &HashMap, - ) -> Diagnostics { - todo!(); + #[test] + fn control_out_parameter_rejects_literal() { + let source = r#" +control child(out bit<8> value) { + apply { } +} +control ingress() { + child() c; + apply { + c.apply(5); + } +} +"#; + let diags = check_p4(source); + let errors: Vec<_> = + diags.errors().iter().map(|d| d.message.clone()).collect(); + assert_eq!(errors.len(), 1, "{errors:?}"); + assert!( + errors[0].starts_with("wrong argument type for"), + "{errors:?}", + ); + } - /* - let mut diags = Diagnostics::new(); - for stmt in &mut sb.statements { - match stmt { - Statement::Empty => {} - Statement::Assignment(_, xpr) => { - diags.extend(&self.check_expression(xpr, names)); - todo!() - } - Statement::Call(c) => { todo!() } - Statement::If(ifb) => { todo!() } - Statement::Variable(v) => { todo!() } - Statement::Constant(c) => { todo!() } - } - } - diags - */ + #[test] + fn default_action_must_be_listed() { + let source = r#" +header h_t { + bit<8> f; +} +struct headers_t { + h_t h; +} +control ingress(inout headers_t hdr) { + action hit() { } + table arp { + key = { hdr.h.f: exact; } + actions = { hit; NoAction; } + default_action = NoAction; + } + table routes { + key = { hdr.h.f: exact; } + actions = { hit; } + default_action = NoAction; + } + apply { + arp.apply(); + routes.apply(); + } +} +"#; + let diags = check_p4(source); + let errors: Vec<_> = + diags.errors().iter().map(|d| d.message.clone()).collect(); + assert_eq!( + errors, + ["default action NoAction is not in the actions of table routes"], + ); } - pub fn check_expression( - &self, - _xpr: &mut Expression, - _names: &HashMap, - ) -> Diagnostics { - /* - let mut diags = Diagnostics::new(); - match &mut xpr.kind { - ExpressionKind::BoolLit(_) => { - xpr.ty = Some(Type::Bool) - } - //TODO P4 spec section 8.9.1/8.9.2 - ExpressionKind::IntegerLit(_) => { - xpr.ty = Some(Type::Int(128)) - } - ExpressionKind::BitLit(width, _) => { - xpr.ty = Some(Type::Bit(*width as usize)) - } - ExpressionKind::SignedLit(width, _) => { - xpr.ty = Some(Type::Int(*width as usize)) - } - ExpressionKind::Lvalue(lval) => { - //let ty = lvalue_type(lval, ast, names) - todo!(); - } - ExpressionKind::Binary(lhs, _, rhs) => { - todo!(); - } - ExpressionKind::Index(lval, xpr) => { - todo!(); - } - ExpressionKind::Slice(begin, end) => { - todo!(); - } + #[test] + fn arbitrary_int_rejected_at_runtime() { + let source = r#" +header h_t { + int a; + int<8> b; +} +struct s_t { + int c; +} +parser parse(packet_in pkt, in int d, int j) { + state start { + transition accept; + } +} +control ingress(in int e, int i) { + action act(int f) { + int g; + } + apply { + int h; + } +} +"#; + let runtime = "int is only valid for compile-time known values"; + let directional = "int parameters must be directionless"; + let directionless = + "directionless int parameters are not supported by SoftNPU"; + let diags = check_p4(source); + let errors: Vec<_> = + diags.errors().iter().map(|d| d.message.clone()).collect(); + assert_eq!( + errors, + [ + directional, + directionless, + directional, + directionless, + runtime, + runtime, + runtime, + runtime, + runtime, + ], + ); + } - } - diags - */ - todo!(); + #[test] + fn port_bitmap_accepts_bit_type() { + let source = + PORT_BITMAP_PIPELINE.replace("PORT_BITMAP_TYPE", "bit<128>"); + let diags = check_p4(&source); + let errors = diags.errors(); + assert!( + errors.is_empty(), + "unexpected errors: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); } - pub fn check_parser(&self, _index: usize) -> Diagnostics { - todo!("parser expression type check"); + #[test] + fn port_bitmap_rejects_non_bit_type() { + let source = PORT_BITMAP_PIPELINE.replace("PORT_BITMAP_TYPE", "bool"); + let diags = check_p4(&source); + let errors = diags.errors(); + assert!( + errors.iter().any(|d| d.message.contains( + "port_bitmap in egress metadata egress_metadata_t must be a \ + bit" + )), + "missing port_bitmap type diagnostic: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); } } diff --git a/p4/src/error.rs b/p4/src/error.rs index 60d8e0cb..d5f27f95 100644 --- a/p4/src/error.rs +++ b/p4/src/error.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use crate::lexer::{Kind, Lexer, Token}; use colored::Colorize; @@ -165,12 +165,24 @@ fn carat_line(line: &str, at: &Token) -> String { carat_line.push(' '); } } - for x in line[at.col..].chars() { - if x.is_whitespace() || (Lexer::is_separator(x) && x != '.') { - break; + + let rst = &line[at.col..]; + let on_op = rst.chars().next().is_some_and(|x| { + !x.is_whitespace() && Lexer::is_separator(x) && x != '.' + }); + + for x in rst.chars() { + let end = if on_op { + x.is_whitespace() || x.is_alphanumeric() || x == '_' } else { - carat_line.push('^'); + x.is_whitespace() || (Lexer::is_separator(x) && x != '.') + }; + + if end { + break; } + + carat_line.push('^'); } carat_line } @@ -205,3 +217,31 @@ fn fmt_common( let carat_line = carat_line(source, at); write!(f, " {}", carat_line.bright_red()) } + +#[cfg(test)] +mod tests { + use super::carat_line; + use crate::lexer::{Kind, Token}; + use std::sync::Arc; + + fn token(kind: Kind, col: usize) -> Token { + Token { + kind, + line: 0, + col, + file: Arc::new(String::new()), + } + } + + #[test] + fn carat_marks_operators() { + assert_eq!(carat_line("x + 16w1;", &token(Kind::Plus, 2)), " ^"); + assert_eq!(carat_line("x << y;", &token(Kind::Shl, 2)), " ^^"); + } + + #[test] + fn carat_marks_lvalues() { + let field = Kind::Identifier("hdr.f".into()); + assert_eq!(carat_line("\thdr.f + y;", &token(field, 1)), "\t^^^^^"); + } +} diff --git a/p4/src/hlir.rs b/p4/src/hlir.rs index 979cefd9..faf2ddeb 100644 --- a/p4/src/hlir.rs +++ b/p4/src/hlir.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use crate::ast::{ BinOp, Constant, Control, DeclarationInfo, Expression, ExpressionKind, @@ -67,7 +67,9 @@ impl<'a> HlirGenerator<'a> { self.lvalue(lval, &mut local_names); } for lval in &t.actions { - self.lvalue(lval, &mut local_names); + if lval.name != "NoAction" { + self.lvalue(lval, &mut local_names); + } } } self.statement_block(&c.apply, &mut names); @@ -82,8 +84,87 @@ impl<'a> HlirGenerator<'a> { match stmt { Statement::Empty => {} Statement::Assignment(lval, xpr) => { + let target = self.lvalue(lval, names); + let actual = self.expression(xpr, names); + if let (Some(target), Some(actual)) = (target, actual) { + if !implicitly_converts(xpr, &actual, &target) { + let message = if unsupported_int_conversion( + xpr, &actual, &target, + ) { + UNSUPPORTED_INT_EXPRESSION.into() + } else { + format!("cannot assign {actual} to {target}") + }; + self.diags.push(Diagnostic { + level: Level::Error, + message, + token: xpr.token.clone(), + }); + } + } + } + Statement::SliceAssignment(lval, hi, lo, xpr) => { self.lvalue(lval, names); - self.expression(xpr, names); + self.expression(hi, names); + self.expression(lo, names); + let expression_type = self.expression(xpr, names); + + // Validate slice bounds. + if let Some(name_info) = self.hlir.lvalue_decls.get(lval) { + let width = match &name_info.ty { + Type::Bit(w) => *w, + ty => { + let message = match ty { + Type::Int(_) => format!( + "slice assignment to {ty} is not \ + supported by SoftNPU" + ), + _ => format!( + "slice assignment requires a bit \ + type, got {ty}" + ), + }; + self.diags.push(Diagnostic { + level: Level::Error, + message, + token: lval.token.clone(), + }); + continue; + } + }; + if let (Some((hi_val, lo_val)), Some(expression_type)) = + (self.slice_bounds(hi, lo, width), expression_type) + { + // P4-16 8.6: a slice is a bit l-value. + let expected_ty = + Type::Bit((hi_val - lo_val + 1) as usize); + + if !implicitly_converts( + xpr, + &expression_type, + &expected_ty, + ) { + let message = if unsupported_int_conversion( + xpr, + &expression_type, + &expected_ty, + ) { + UNSUPPORTED_INT_EXPRESSION.into() + } else { + format!( + "slice [{hi_val}:{lo_val}] requires \ + {expected_ty}, got {expression_type}" + ) + }; + + self.diags.push(Diagnostic { + level: Level::Error, + message, + token: xpr.token.clone(), + }); + } + } + } } Statement::Call(c) => { // pop the function name off the lval before resolving @@ -93,10 +174,10 @@ impl<'a> HlirGenerator<'a> { } } Statement::If(ifb) => { - self.expression(ifb.predicate.as_ref(), names); + self.predicate(ifb.predicate.as_ref(), names); self.statement_block(&ifb.block, names); for ei in &ifb.else_ifs { - self.expression(ei.predicate.as_ref(), names); + self.predicate(ei.predicate.as_ref(), names); self.statement_block(&ei.block, names); } if let Some(eb) = &ifb.else_block { @@ -111,8 +192,9 @@ impl<'a> HlirGenerator<'a> { decl: DeclarationInfo::Local, }, ); + if let Some(initializer) = &v.initializer { - self.expression(initializer, names); + self.initializer(&v.ty, initializer, names); } } Statement::Constant(c) => { @@ -123,7 +205,7 @@ impl<'a> HlirGenerator<'a> { decl: DeclarationInfo::Local, }, ); - self.expression(c.initializer.as_ref(), names); + self.initializer(&c.ty, c.initializer.as_ref(), names); } Statement::Transition(_t) => { //TODO @@ -137,6 +219,70 @@ impl<'a> HlirGenerator<'a> { } } + /// Check a declaration's initializer against its declared type. + /// + /// This uses `implicitly_converts`, reporting compile-time `int` expressions + /// other than literals as an x4c limitation rather than a type error. + fn initializer( + &mut self, + declared: &Type, + xpr: &Expression, + names: &mut HashMap, + ) { + let Some(actual) = self.expression(xpr, names) else { + return; + }; + + if !implicitly_converts(xpr, &actual, declared) { + let message = if unsupported_int_conversion(xpr, &actual, declared) + { + UNSUPPORTED_INT_EXPRESSION.into() + } else { + format!("cannot initialize {declared} with {actual}") + }; + self.diags.push(Diagnostic { + level: Level::Error, + message, + token: xpr.token.clone(), + }); + } + } + + fn literal(&mut self, xpr: &Expression, ty: Type) -> Option { + if let Type::Bit(w) | Type::Int(w) = ty { + if w > 128 { + self.diags.push(Diagnostic { + level: Level::Error, + message: format!( + "Width {w} exceeds the 128-bit compiler limit" + ), + token: xpr.token.clone(), + }); + return None; + } + } + self.hlir.expression_types.insert(xpr.clone(), ty.clone()); + Some(ty) + } + + fn predicate( + &mut self, + xpr: &Expression, + names: &mut HashMap, + ) { + let Some(ty) = self.expression(xpr, names) else { + return; + }; + + if ty != Type::Bool { + self.diags.push(Diagnostic { + level: Level::Error, + message: format!("condition must be bool, got {ty}"), + token: xpr.token.clone(), + }); + } + } + fn expression( &mut self, xpr: &Expression, @@ -149,20 +295,17 @@ impl<'a> HlirGenerator<'a> { Some(ty) } ExpressionKind::IntegerLit(_) => { - //TODO P4 spec section 8.9.1/8.9.2 + //TODO P4 spec sections 6.4.3.3 (integer literals) and 8.11.2 + // (implicit int casts) let ty = Type::Int(128); self.hlir.expression_types.insert(xpr.clone(), ty.clone()); Some(ty) } ExpressionKind::BitLit(width, _) => { - let ty = Type::Bit(*width as usize); - self.hlir.expression_types.insert(xpr.clone(), ty.clone()); - Some(ty) + self.literal(xpr, Type::Bit(*width as usize)) } ExpressionKind::SignedLit(width, _) => { - let ty = Type::Int(*width as usize); - self.hlir.expression_types.insert(xpr.clone(), ty.clone()); - Some(ty) + self.literal(xpr, Type::Int(*width as usize)) } ExpressionKind::Lvalue(lval) => { let ty = self.lvalue(lval, names)?; @@ -207,10 +350,12 @@ impl<'a> HlirGenerator<'a> { } }; //TODO less special case-y? - Some(match call.lval.leaf() { + let ty = match call.lval.leaf() { "isValid" => Type::Bool, _ => Type::Void, - }) + }; + self.hlir.expression_types.insert(xpr.clone(), ty.clone()); + Some(ty) } ExpressionKind::List(elements) => { let mut type_elements = Vec::new(); @@ -218,7 +363,9 @@ impl<'a> HlirGenerator<'a> { let ty = self.expression(e.as_ref(), names)?; type_elements.push(Box::new(ty)); } - Some(Type::List(type_elements)) + let ty = Type::List(type_elements); + self.hlir.expression_types.insert(xpr.clone(), ty.clone()); + Some(ty) } } } @@ -279,43 +426,11 @@ impl<'a> HlirGenerator<'a> { }); None } + // P4-16 8.6: a slice is an unsigned bit. Type::Bit(width) => match &xpr.kind { - ExpressionKind::Slice(end, begin) => { - let (begin_val, end_val) = self.slice(begin, end, width)?; - let w = end_val - begin_val + 1; - Some(Type::Bit(w as usize)) - } - _ => { - self.diags.push(Diagnostic { - level: Level::Error, - message: "only slices supported as index arguments" - .into(), - token: lval.token.clone(), - }); - None - } - }, - Type::Varbit(width) => match &xpr.kind { - ExpressionKind::Slice(begin, end) => { - let (begin_val, end_val) = self.slice(begin, end, width)?; - let w = end_val - begin_val + 1; - Some(Type::Varbit(w as usize)) - } - _ => { - self.diags.push(Diagnostic { - level: Level::Error, - message: "only slices supported as index arguments" - .into(), - token: lval.token.clone(), - }); - None - } - }, - Type::Int(width) => match &xpr.kind { - ExpressionKind::Slice(begin, end) => { - let (begin_val, end_val) = self.slice(begin, end, width)?; - let w = end_val - begin_val + 1; - Some(Type::Int(w as usize)) + ExpressionKind::Slice(hi, lo) => { + let (hi, lo) = self.slice_bounds(hi, lo, width)?; + Some(Type::Bit((hi - lo + 1) as usize)) } _ => { self.diags.push(Diagnostic { @@ -327,6 +442,38 @@ impl<'a> HlirGenerator<'a> { None } }, + // TODO: P4-16 8.7 and 8.8 allow slicing int and int. + // SoftNPU, on the other hand, has no int code generation, and + // x4c does not evaluate compile-time int expressions. + Type::Int(w) => { + self.diags.push(Diagnostic { + level: Level::Error, + message: format!( + "slicing int<{w}> is not supported by SoftNPU" + ), + token: lval.token.clone(), + }); + None + } + Type::ArbitraryInt => { + self.diags.push(Diagnostic { + level: Level::Error, + message: "slicing a compile-time int is not supported by \ + x4c" + .into(), + token: lval.token.clone(), + }); + None + } + // P4-16 8.10: varbit supports only assignment and ==/!=. + Type::Varbit(_) => { + self.diags.push(Diagnostic { + level: Level::Error, + message: "cannot index a varbit".into(), + token: lval.token.clone(), + }); + None + } Type::String => { self.diags.push(Diagnostic { level: Level::Error, @@ -370,70 +517,57 @@ impl<'a> HlirGenerator<'a> { } } - fn slice( + /// Validate a slice `[hi:lo]` on a field of `width` bits. + fn slice_bounds( &mut self, - begin: &Expression, - end: &Expression, + hi: &Expression, + lo: &Expression, width: usize, ) -> Option<(i128, i128)> { - // According to P4-16 section 8.5, slice values must be - // known at compile time. For now just enfoce integer - // literals only, we can get fancier later with other - // things that can be figured out at compile time. - let begin_val = match &begin.kind { - ExpressionKind::IntegerLit(v) => *v, - _ => { - self.diags.push(Diagnostic { - level: Level::Error, - message: - "only interger literals are supported as slice bounds" - .into(), - token: begin.token.clone(), - }); - return None; - } - }; - let end_val = match &end.kind { - ExpressionKind::IntegerLit(v) => *v, - _ => { - self.diags.push(Diagnostic { - level: Level::Error, - message: - "only interger literals are supported as slice bounds" - .into(), - token: begin.token.clone(), - }); - return None; - } - }; - let w = width as i128; - if begin_val < 0 || begin_val >= w { + // P4-16 section 8.6: slice bounds must be compile-time + // known values. Only integer literals are accepted; constant + // expressions are not yet supported. + let hi_val = self.slice_bound(hi, width, "upper")?; + let lo_val = self.slice_bound(lo, width, "lower")?; + if hi_val < lo_val { self.diags.push(Diagnostic { level: Level::Error, - message: "slice begin value out of bounds".into(), - token: begin.token.clone(), + message: "slice upper bound must be \ + greater than or equal to the lower bound" + .into(), + token: hi.token.clone(), }); return None; } - if end_val < 0 || end_val >= w { + Some((hi_val, lo_val)) + } + + fn slice_bound( + &mut self, + bound: &Expression, + width: usize, + which: &str, + ) -> Option { + let ExpressionKind::IntegerLit(v) = bound.kind else { self.diags.push(Diagnostic { level: Level::Error, - message: "slice end value out of bounds".into(), - token: begin.token.clone(), + message: "only integer literals are supported by x4c as \ + slice bounds" + .into(), + token: bound.token.clone(), }); return None; - } - if begin_val >= end_val { + }; + + if !(0..width as i128).contains(&v) { self.diags.push(Diagnostic { level: Level::Error, - message: "slice upper bound must be \ - greater than the lower bound" - .into(), - token: begin.token.clone(), + message: format!("slice {which} bound out of bounds"), + token: bound.token.clone(), }); return None; } - Some((begin_val, end_val)) + Some(v) } fn lvalue( @@ -462,6 +596,14 @@ impl<'a> HlirGenerator<'a> { } } + /// Type the binary expression or report an error and return `None`. + /// + /// Shifts need a `bit` left operand and a `bit` or non-negative + /// literal count. + /// + /// Every other operator needs matching operand types, and + /// ordered comparisons also need `bit` operands. Comparisons produce + /// `bool`, while other operators produce the left operand's type. fn binary_expression( &mut self, xpr: &Expression, @@ -472,25 +614,107 @@ impl<'a> HlirGenerator<'a> { ) -> Option { let lhs_ty = self.expression(lhs, names)?; let rhs_ty = self.expression(rhs, names)?; + let valid_count = matches!(&rhs_ty, Type::Bit(_)) + || matches!( + &rhs.kind, + ExpressionKind::IntegerLit(value) if *value >= 0 + ); - // TODO just checking that types are the same for now. - if lhs_ty != rhs_ty { - self.diags.push(Diagnostic { - level: Level::Error, - message: format!( - "cannot {} a {} and a {}", - op.english_verb(), - lhs_ty, - rhs_ty, - ), - token: xpr.token.clone(), - }); + // TODO arithmetic and bitwise operands are only checked to have the + // same type; P4-16 8.6 limits them to bit and int types. + match op { + BinOp::Shl | BinOp::Shr if !valid_count => { + self.diags.push(Diagnostic { + level: Level::Error, + message: "shift count must be a bit type or a \ + non-negative integer literal" + .into(), + token: rhs.token.clone(), + }); + return None; + } + BinOp::Shl | BinOp::Shr if !matches!(&lhs_ty, Type::Bit(_)) => { + let message = match &lhs_ty { + ty if compile_time_int(lhs, ty) => { + "shifting a compile-time int is not supported by x4c" + .into() + } + Type::Int(_) => { + format!("shifting {lhs_ty} is not supported by SoftNPU") + } + _ => format!( + "shift left operand must be a bit type, got {lhs_ty}" + ), + }; + self.diags.push(Diagnostic { + level: Level::Error, + message, + token: lhs.token.clone(), + }); + return None; + } + BinOp::Shl | BinOp::Shr => {} + BinOp::Lt | BinOp::Gt | BinOp::Leq | BinOp::Geq + if !matches!(&lhs_ty, Type::Bit(_)) => + { + let non_integer = [(lhs, &lhs_ty), (rhs, &rhs_ty)] + .into_iter() + .find(|(_, ty)| !integer_type(ty)); + let (message, token) = match non_integer { + Some((operand, ty)) => ( + format!( + "ordered comparison requires bit operands, got \ + {ty}" + ), + operand.token.clone(), + ), + None if compile_time_int(lhs, &lhs_ty) => ( + "ordered comparison of a compile-time int is not \ + supported by x4c" + .into(), + lhs.token.clone(), + ), + None => ( + format!( + "ordered comparison of {lhs_ty} is not supported \ + by SoftNPU" + ), + lhs.token.clone(), + ), + }; + self.diags.push(Diagnostic { + level: Level::Error, + message, + token, + }); + return None; + } + _ if lhs_ty != rhs_ty => { + self.diags.push(Diagnostic { + level: Level::Error, + message: format!( + "cannot {} a {lhs_ty} and a {rhs_ty}", + op.english_verb(), + ), + token: xpr.token.clone(), + }); + return None; + } + _ => {} } - self.hlir - .expression_types - .insert(xpr.clone(), lhs_ty.clone()); - Some(lhs_ty) + // P4-16 8.6: comparisons produce a Boolean value. + let ty = match op { + BinOp::Eq + | BinOp::NotEq + | BinOp::Lt + | BinOp::Gt + | BinOp::Leq + | BinOp::Geq => Type::Bool, + _ => lhs_ty, + }; + self.hlir.expression_types.insert(xpr.clone(), ty.clone()); + Some(ty) } fn parser(&mut self, p: &Parser) { @@ -501,3 +725,536 @@ impl<'a> HlirGenerator<'a> { } } } + +/// Whether a value of type `from` may be used where `to` is expected or +/// asserted. +/// +/// P4-16 [8.11.2] allows an arbitrary-precision `int` to convert implicitly +/// to `bit`, but only integer literals are accepted here, while +/// compile-time `int` expressions such as `1 + 2` are not. +/// +/// x4c also accepts implicit widening from `bit` to `bit` when +/// `W < V`, zero-extending the value. +/// P4-16 [8.11.1] and [8.11.2] require an explicit cast between different bit +/// widths; x4c does not currently parse explicit casts. +/// +/// [8.11.1]: https://p4.org/wp-content/uploads/sites/53/2024/10/P4-16-spec-v1.2.5.html#sec-explicit-casts +/// [8.11.2]: https://p4.org/wp-content/uploads/sites/53/2024/10/P4-16-spec-v1.2.5.html#sec-implicit-casts +pub(crate) fn implicitly_converts( + xpr: &Expression, + from: &Type, + to: &Type, +) -> bool { + from == to + || matches!( + (&xpr.kind, to), + (ExpressionKind::IntegerLit(_), Type::Bit(_)) + ) + || matches!((from, to), (Type::Bit(w), Type::Bit(v)) if w <= v) +} + +const UNSUPPORTED_INT_EXPRESSION: &str = + "int expressions other than literals are not supported by x4c"; + +fn literal_int_expression(xpr: &Expression) -> bool { + match &xpr.kind { + ExpressionKind::IntegerLit(_) => true, + ExpressionKind::Binary(lhs, _, rhs) => { + literal_int_expression(lhs) && literal_int_expression(rhs) + } + _ => false, + } +} + +/// Whether `xpr` is a compile-time `int`: typed `int`, or an integer-typed +/// expression built only from integer literals. +fn compile_time_int(xpr: &Expression, ty: &Type) -> bool { + matches!(ty, Type::ArbitraryInt) + || (matches!(ty, Type::Int(_)) && literal_int_expression(xpr)) +} + +fn integer_type(ty: &Type) -> bool { + matches!(ty, Type::Bit(_) | Type::Int(_) | Type::ArbitraryInt) +} + +/// Whether a failed conversion to `bit` comes from a compile-time `int` +/// expression that's not literal, which x4c cannot evaluate. +fn unsupported_int_conversion( + xpr: &Expression, + from: &Type, + to: &Type, +) -> bool { + !matches!(xpr.kind, ExpressionKind::IntegerLit(_)) + && compile_time_int(xpr, from) + && matches!(to, Type::Bit(_)) +} + +#[cfg(test)] +mod tests { + use crate::ast::{Expression, ExpressionKind, Statement, Type, AST}; + use crate::lexer::Lexer; + use crate::parser::Parser; + use std::sync::Arc; + + fn parse_p4(source: &str) -> AST { + let lines: Vec<&str> = source.lines().collect(); + let filename = Arc::new("test.p4".to_string()); + let lexer = Lexer::new(lines, filename); + let mut parser = Parser::new(lexer); + let mut ast = AST::default(); + parser.run(&mut ast).expect("parse failed"); + ast + } + + fn check_p4(source: &str) -> crate::check::Diagnostics { + let ast = parse_p4(source); + let (_hlir, diags) = crate::check::all(&ast); + diags + } + + fn assert_slice_error(stmt: &str, bound: &str, message: &str) { + let source = format!( + r#" +header h_t {{ + bit<32> f; +}} +struct headers_t {{ + h_t h; +}} +control ingress(inout headers_t hdr) {{ + apply {{ + bit<32> n = 32w3; + {stmt} + }} +}} +"# + ); + let line = source + .lines() + .position(|l| l.contains(stmt)) + .expect("statement line"); + let col = source.lines().nth(line).unwrap().find(bound).unwrap(); + + let diags = check_p4(&source); + let errors = diags.errors(); + + assert_eq!( + errors.len(), + 1, + "expected one error for `{stmt}`, got {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); + assert_eq!(errors[0].message, message, "message for `{stmt}`"); + assert_eq!( + (errors[0].token.line, errors[0].token.col), + (line, col), + "token position for `{stmt}`", + ); + } + + const NON_LITERAL: &str = + "only integer literals are supported by x4c as slice bounds"; + const HI_OOB: &str = "slice upper bound out of bounds"; + const LO_OOB: &str = "slice lower bound out of bounds"; + const HI_LT_LO: &str = + "slice upper bound must be greater than or equal to the lower bound"; + + #[test] + fn slice_read_bound_errors() { + assert_slice_error("bit<8> x = hdr.h.f[n:0];", "n:0]", NON_LITERAL); + assert_slice_error("bit<8> x = hdr.h.f[32:0];", "32:0]", HI_OOB); + assert_slice_error("bit<8> x = hdr.h.f[31:32];", "32]", LO_OOB); + assert_slice_error("bit<8> x = hdr.h.f[3:7];", "3:7]", HI_LT_LO); + } + + #[test] + fn slice_assign_bound_errors() { + assert_slice_error("hdr.h.f[0:n] = 1w0;", "n]", NON_LITERAL); + assert_slice_error("hdr.h.f[32:0] = 1w0;", "32:0]", HI_OOB); + assert_slice_error("hdr.h.f[31:32] = 1w0;", "32]", LO_OOB); + assert_slice_error("hdr.h.f[3:7] = 1w0;", "3:7]", HI_LT_LO); + } + + const SLICE_ASSIGN_CONTEXTS: &[&str] = &[ + "apply { STATEMENT }", + "apply { if (false) { } else if (true) { STATEMENT } }", + "action assign() { STATEMENT } apply { }", + ]; + + fn slice_assign_source(context: &str, slice: &str, rhs: &str) -> String { + let body = + context.replace("STATEMENT", &format!("hdr.h.f{slice} = {rhs};")); + format!( + r#" +header h_t {{ + bit<32> f; +}} +struct headers_t {{ + h_t h; +}} +control ingress(inout headers_t hdr) {{ + {body} +}} +"# + ) + } + + #[test] + fn slice_assign_rhs_type_errors() { + for context in SLICE_ASSIGN_CONTEXTS { + for (rhs, rhs_type) in + [("16w1", "bit<16>"), ("8s1", "int<8>"), ("true", "bool")] + { + let source = slice_assign_source(context, "[7:0]", rhs); + let rhs_start = format!("= {rhs};"); + let (line, text) = source + .lines() + .enumerate() + .find(|(_, text)| text.contains(&rhs_start)) + .unwrap(); + let col = text.find(&rhs_start).unwrap() + 2; + + let diags = check_p4(&source); + let errors = diags.errors(); + assert_eq!( + errors.len(), + 1, + "{context}, {rhs}: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); + assert_eq!( + errors[0].message, + format!("slice [7:0] requires bit<8>, got {rhs_type}"), + "{context}, {rhs}", + ); + assert_eq!( + (errors[0].token.line, errors[0].token.col), + (line, col), + "{context}, {rhs}", + ); + } + } + } + + #[test] + fn slice_assign_rhs_matching_types() { + for context in SLICE_ASSIGN_CONTEXTS { + for rhs in ["8w1", "4w1", "1", "hdr.h.f[15:8]"] { + let source = slice_assign_source(context, "[7:0]", rhs); + let diags = check_p4(&source); + let errors = diags.errors(); + assert!( + errors.is_empty(), + "{context}, {rhs}: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); + } + } + } + + fn errors_for(source: &str) -> Vec { + check_p4(source) + .errors() + .iter() + .map(|d| d.message.clone()) + .collect() + } + + #[test] + fn slice_assign_rejects_comparison_rhs() { + let source = slice_assign_source( + SLICE_ASSIGN_CONTEXTS[0], + "[7:0]", + "hdr.h.f[7:0] == hdr.h.f[15:8]", + ); + assert_eq!( + errors_for(&source), + ["slice [7:0] requires bit<8>, got bool"] + ); + } + + #[test] + fn varbit_index_rejected() { + let source = r#" +header h_t { + varbit<32> v; +} +struct headers_t { + h_t h; +} +control ingress(inout headers_t hdr) { + apply { + bit<8> x = hdr.h.v[7:0]; + } +} +"#; + assert_eq!(errors_for(source), ["cannot index a varbit"]); + } + + #[test] + fn action_assignment_follows_apply_rules() { + let source = r#" +header h_t { + bit<8> f; +} +struct headers_t { + h_t h; +} +control ingress(inout headers_t hdr) { + action set() { + hdr.h.f = 5; + } + action wide() { + hdr.h.f = 16w1; + } + apply { } +} +"#; + assert_eq!(errors_for(source), ["cannot assign bit<16> to bit<8>"]); + } + + fn statement_source(statement: &str) -> String { + format!( + r#" +control ingress() {{ + apply {{ + bit<8> x = 8w1; + {statement} + }} +}} +"# + ) + } + + #[test] + fn initializer_type_errors() { + for (statement, message) in [ + ("bit<4> y = x;", "cannot initialize bit<4> with bit<8>"), + ("bool y = x;", "cannot initialize bool with bit<8>"), + ("if (x) { }", "condition must be bool, got bit<8>"), + ( + "if (false) { } else if (x) { }", + "condition must be bool, got bit<8>", + ), + ("bit<8> y = x + 16w1;", "cannot add a bit<8> and a bit<16>"), + ( + "bit<1> y = x[3];", + "only slices supported as index arguments", + ), + ( + "bit<8> y = 8w1 << 129w1;", + "Width 129 exceeds the 128-bit compiler limit", + ), + ("x = 16w1;", "cannot assign bit<16> to bit<8>"), + ("x = true;", "cannot assign bool to bit<8>"), + ("x = 8s1;", "cannot assign int<8> to bit<8>"), + ( + "bool y = true < false;", + "ordered comparison requires bit operands, got bool", + ), + ( + "bool y = 8s1 < 8s2;", + "ordered comparison of int<8> is not supported by SoftNPU", + ), + ( + "bool y = 1 < 2;", + "ordered comparison of a compile-time int is not supported \ + by x4c", + ), + ( + "bit<8> y = 1 << 2;", + "shifting a compile-time int is not supported by x4c", + ), + ( + "bool y = 1 < true;", + "ordered comparison requires bit operands, got bool", + ), + ( + "bool y = 8s1 < true;", + "ordered comparison requires bit operands, got bool", + ), + ( + "bit<8> y = 1 << true;", + "shift count must be a bit type or a non-negative integer \ + literal", + ), + ("bit<8> y = (1 == 2);", "cannot initialize bit<8> with bool"), + ( + "bool y = (1 == 2) < false;", + "ordered comparison requires bit operands, got bool", + ), + ("bit<8> y = 1 + 2;", super::UNSUPPORTED_INT_EXPRESSION), + ("x = 1 + 2;", super::UNSUPPORTED_INT_EXPRESSION), + ("x[7:0] = 1 + 2;", super::UNSUPPORTED_INT_EXPRESSION), + ( + "int<8> s = 8s1; bit<4> y = s[3:0];", + "slicing int<8> is not supported by SoftNPU", + ), + ( + "int<8> s = 8s1; s[3:0] = 4w1;", + "slice assignment to int<8> is not supported by SoftNPU", + ), + ( + "int c; bit<4> y = c[3:0];", + "slicing a compile-time int is not supported by x4c", + ), + ] { + assert_eq!( + errors_for(&statement_source(statement)), + [message], + "{statement}", + ); + } + } + + #[test] + fn initializer_types_accepted() { + for statement in [ + "bit<8> y = x;", + "bit<16> y = x;", + "bit<32> y = x;", + "const bit<32> c = x;", + "bit<8> y = 5;", + "bool y = x == 8w1;", + "if (x < 8w2) { }", + "const bit<8> c = 5;", + "x = 5;", + "x = 8w2;", + ] { + assert_eq!( + errors_for(&statement_source(statement)), + Vec::::new(), + "{statement}", + ); + } + } + + fn shift_source(expression: &str) -> String { + format!( + r#" +control ingress() {{ + apply {{ + bit<32> x = 32w0x81234567; + bit<8> narrow = 8w1; + bit<128> wide = 128w1; + int<8> signed_value; + bool flag; + bit<32> result = {expression}; + }} +}} +"# + ) + } + + fn shift_expression(ast: &AST) -> &Expression { + let Statement::Variable(result) = + ast.controls[0].apply.statements.last().unwrap() + else { + panic!("expected result declaration"); + }; + result.initializer.as_deref().unwrap() + } + + #[test] + fn shift_count_types_preserve_result_type() { + for op in ["<<", ">>"] { + for count in + ["0", "1", "31", "32", "33", "128", "256", "narrow", "wide"] + { + let source = shift_source(&format!("x {op} {count}")); + let ast = parse_p4(&source); + let (hlir, diags) = crate::check::all(&ast); + let errors = diags.errors(); + assert!( + errors.is_empty(), + "{op} {count}: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); + assert_eq!( + hlir.expression_types.get(shift_expression(&ast)), + Some(&Type::Bit(32)), + "{op} {count}", + ); + } + } + } + + #[test] + fn shift_operand_type_errors() { + const COUNT_ERROR: &str = + "shift count must be a bit type or a non-negative integer literal"; + + for op in ["<<", ">>"] { + for (lhs, rhs, message) in [ + ( + "flag", + "1", + "shift left operand must be a bit type, got bool", + ), + ( + "signed_value", + "1", + "shifting int<8> is not supported by SoftNPU", + ), + ("x", "flag", COUNT_ERROR), + ("x", "signed_value", COUNT_ERROR), + ] { + let source = shift_source(&format!("{lhs} {op} {rhs}")); + let ast = parse_p4(&source); + let ExpressionKind::Binary(left, _, right) = + &shift_expression(&ast).kind + else { + panic!("expected shift expression"); + }; + let token = match lhs { + "x" => &right.token, + _ => &left.token, + }; + let (_, diags) = crate::check::all(&ast); + let errors = diags.errors(); + assert_eq!( + errors.len(), + 1, + "{lhs} {op} {rhs}: {:?}", + errors.iter().map(|d| &d.message).collect::>(), + ); + assert_eq!(errors[0].message, message); + assert_eq!( + (errors[0].token.line, errors[0].token.col), + (token.line, token.col), + ); + } + } + } + + #[test] + fn negative_shift_count_rejected() { + for op in ["<<", ">>"] { + let source = shift_source(&format!("x {op} 1")); + let mut ast = parse_p4(&source); + let Statement::Variable(result) = + ast.controls[0].apply.statements.last_mut().unwrap() + else { + panic!("expected result declaration"); + }; + let ExpressionKind::Binary(_, _, rhs) = + &mut result.initializer.as_mut().unwrap().kind + else { + panic!("expected shift expression"); + }; + rhs.kind = ExpressionKind::IntegerLit(-1); + let token = rhs.token.clone(); + let (_, diags) = crate::check::all(&ast); + let errors = diags.errors(); + assert_eq!(errors.len(), 1); + assert_eq!( + errors[0].message, + "shift count must be a bit type or a non-negative integer literal", + ); + assert_eq!( + (errors[0].token.line, errors[0].token.col), + (token.line, token.col), + ); + } + } +} diff --git a/p4/src/lexer.rs b/p4/src/lexer.rs index 02679d41..2f7c085b 100644 --- a/p4/src/lexer.rs +++ b/p4/src/lexer.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use crate::error::TokenError; use std::fmt; @@ -89,6 +89,7 @@ pub enum Kind { Bang, Tilde, Shl, + Shr, Pipe, Carat, GreaterThanEquals, @@ -217,6 +218,7 @@ impl fmt::Display for Kind { Kind::Bang => write!(f, "operator !"), Kind::Tilde => write!(f, "operator ~"), Kind::Shl => write!(f, "operator <<"), + Kind::Shr => write!(f, "operator >>"), Kind::Pipe => write!(f, "operator |"), Kind::Carat => write!(f, "operator ^"), Kind::GreaterThanEquals => write!(f, "operator >="), @@ -417,6 +419,10 @@ impl<'a> Lexer<'a> { return Ok(t); } + if let Some(t) = self.match_token(">>", Kind::Shr) { + return Ok(t); + } + if let Some(t) = self.match_token(">", Kind::AngleClose) { return Ok(t); } @@ -972,6 +978,7 @@ impl<'a> Lexer<'a> { }, Some('>') => match chars.next() { Some('=') => return &self.cursor[..2], + Some('>') => return &self.cursor[..2], _ => return &self.cursor[..1], }, Some('<') => match chars.next() { diff --git a/p4/src/parser.rs b/p4/src/parser.rs index 61cfd5a2..e407b47c 100644 --- a/p4/src/parser.rs +++ b/p4/src/parser.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use crate::ast::{ self, Action, ActionParameter, ActionRef, BinOp, Call, ConstTableEntry, @@ -144,7 +144,15 @@ impl<'a> Parser<'a> { } lexer::Kind::Int => { - Type::Int(self.parse_optional_width_parameter()?) + let next = self.next_token()?; + let has_width = next.kind == lexer::Kind::AngleOpen; + self.backlog.push(next); + + if has_width { + Type::Int(self.parse_optional_width_parameter()?) + } else { + Type::ArbitraryInt + } } lexer::Kind::Identifier(name) => { @@ -432,29 +440,32 @@ impl<'a> Parser<'a> { Ok(result) } - fn try_parse_binop(&mut self) -> Result, Error> { + fn try_parse_binop(&mut self) -> Result, Error> { let token = self.next_token()?; - match token.kind { - lexer::Kind::GreaterThanEquals => Ok(Some(BinOp::Geq)), - lexer::Kind::AngleClose => Ok(Some(BinOp::Gt)), - lexer::Kind::LessThanEquals => Ok(Some(BinOp::Leq)), - lexer::Kind::AngleOpen => Ok(Some(BinOp::Lt)), - lexer::Kind::NotEquals => Ok(Some(BinOp::NotEq)), - lexer::Kind::DoubleEquals => Ok(Some(BinOp::Eq)), - lexer::Kind::Plus => Ok(Some(BinOp::Add)), - lexer::Kind::Minus => Ok(Some(BinOp::Subtract)), - lexer::Kind::Mod => Ok(Some(BinOp::Mod)), - lexer::Kind::Mask => Ok(Some(BinOp::Mask)), - lexer::Kind::And => Ok(Some(BinOp::BitAnd)), - lexer::Kind::Pipe => Ok(Some(BinOp::BitOr)), - lexer::Kind::Carat => Ok(Some(BinOp::Xor)), + let op = match token.kind { + lexer::Kind::GreaterThanEquals => BinOp::Geq, + lexer::Kind::AngleClose => BinOp::Gt, + lexer::Kind::LessThanEquals => BinOp::Leq, + lexer::Kind::AngleOpen => BinOp::Lt, + lexer::Kind::NotEquals => BinOp::NotEq, + lexer::Kind::DoubleEquals => BinOp::Eq, + lexer::Kind::Plus => BinOp::Add, + lexer::Kind::Minus => BinOp::Subtract, + lexer::Kind::Mod => BinOp::Mod, + lexer::Kind::Mask => BinOp::Mask, + lexer::Kind::And => BinOp::BitAnd, + lexer::Kind::Pipe => BinOp::BitOr, + lexer::Kind::Carat => BinOp::Xor, + lexer::Kind::Shl => BinOp::Shl, + lexer::Kind::Shr => BinOp::Shr, // TODO other binops _ => { self.backlog.push(token); - Ok(None) + return Ok(None); } - } + }; + Ok(Some((op, token))) } pub fn parse_statement_block(&mut self) -> Result { @@ -1034,7 +1045,13 @@ impl<'a, 'b> ControlParser<'a, 'b> { let c = self.parser.parse_constant()?; control.constants.push(c); } - lexer::Kind::Identifier(_) => { + lexer::Kind::Bool + | lexer::Kind::Error + | lexer::Kind::Bit + | lexer::Kind::Varbit + | lexer::Kind::Int + | lexer::Kind::String + | lexer::Kind::Identifier(_) => { self.parser.backlog.push(token); let v = self.parser.parse_variable()?; control.variables.push(v); @@ -1454,6 +1471,7 @@ impl<'a, 'b> StatementParser<'a, 'b> { let token = self.parser.next_token()?; let statement = match token.kind { lexer::Kind::Equals => self.parse_assignment(lval)?, + lexer::Kind::SquareOpen => self.parse_slice_assignment(lval)?, lexer::Kind::ParenOpen => { self.parser.backlog.push(token); self.parse_call(lval)? @@ -1485,6 +1503,25 @@ impl<'a, 'b> StatementParser<'a, 'b> { Ok(Statement::Assignment(lval, expression)) } + /// Parse `lval[hi:lo] = expr`. + /// + /// The opening `[` has already been consumed by this point. + pub fn parse_slice_assignment( + &mut self, + lval: Lvalue, + ) -> Result { + let mut ep = ExpressionParser::new(self.parser); + let hi = ep.run()?; + self.parser.expect_token(lexer::Kind::Colon)?; + let mut ep = ExpressionParser::new(self.parser); + let lo = ep.run()?; + self.parser.expect_token(lexer::Kind::SquareClose)?; + self.parser.expect_token(lexer::Kind::Equals)?; + let mut ep = ExpressionParser::new(self.parser); + let rhs = ep.run()?; + Ok(Statement::SliceAssignment(lval, hi, lo, rhs)) + } + pub fn parse_call(&mut self, lval: Lvalue) -> Result { let args = self.parser.parse_expr_parameters()?; Ok(Statement::Call(Call { lval, args })) @@ -1562,8 +1599,40 @@ impl<'a, 'b> ExpressionParser<'a, 'b> { } pub fn run(&mut self) -> Result, Error> { + self.parse_binary(0) + } + + fn parse_binary(&mut self, min_prec: u8) -> Result, Error> { + let mut lhs = self.parse_primary()?; + while let Some((op, token)) = self.parser.try_parse_binop()? { + let prec = Self::prec(op); + if prec < min_prec { + self.parser.backlog.push(token); + break; + } + let rhs = self.parse_binary(prec + 1)?; + lhs = Expression::new(token, ExpressionKind::Binary(lhs, op, rhs)); + } + Ok(lhs) + } + + fn prec(op: BinOp) -> u8 { + match op { + BinOp::Mask => 1, + BinOp::Eq | BinOp::NotEq => 2, + BinOp::Lt | BinOp::Leq | BinOp::Gt | BinOp::Geq => 3, + BinOp::BitOr => 4, + BinOp::Xor => 5, + BinOp::BitAnd => 6, + BinOp::Shl | BinOp::Shr => 7, + BinOp::Add | BinOp::Subtract => 8, + BinOp::Mod => 9, + } + } + + fn parse_primary(&mut self) -> Result, Error> { let token = self.parser.next_token()?; - let lhs = match token.kind { + let expression = match token.kind { lexer::Kind::TrueLiteral => { Expression::new(token.clone(), ExpressionKind::BoolLit(true)) } @@ -1582,6 +1651,11 @@ impl<'a, 'b> ExpressionParser<'a, 'b> { token.clone(), ExpressionKind::SignedLit(width, value), ), + lexer::Kind::ParenOpen => { + let expression = self.run()?; + self.parser.expect_token(lexer::Kind::ParenClose)?; + expression + } lexer::Kind::Identifier(_) => { self.parser.backlog.push(token.clone()); let lval = self.parser.parse_lvalue("identifier")?; @@ -1610,7 +1684,7 @@ impl<'a, 'b> ExpressionParser<'a, 'b> { ), ) } else { - self.parser.backlog.push(token.clone()); + self.parser.backlog.push(slice_token); self.parser.expect_token(lexer::Kind::SquareClose)?; Expression::new(token, ExpressionKind::Index(lval, xpr)) } @@ -1660,16 +1734,7 @@ impl<'a, 'b> ExpressionParser<'a, 'b> { } }; - // check for binary operator - match self.parser.try_parse_binop()? { - Some(op) => { - // recurse to rhs - let mut ep = ExpressionParser::new(self.parser); - let rhs = ep.run()?; - Ok(Expression::new(token, ExpressionKind::Binary(lhs, op, rhs))) - } - None => Ok(lhs), - } + Ok(expression) } } @@ -1873,3 +1938,54 @@ impl<'a, 'b> SelectParser<'a, 'b> { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::Parser; + use crate::ast::{BinOp, ExpressionKind, Statement, Type, AST}; + use crate::lexer::{Kind, Lexer}; + use std::sync::Arc; + + #[test] + fn nested_binary_types_are_distinct() { + let source = include_str!("../examples/expressions.p4"); + let lexer = Lexer::new( + source.lines().collect(), + Arc::new("expressions.p4".into()), + ); + let mut parser = Parser::new(lexer); + let mut ast = AST::default(); + parser.run(&mut ast).unwrap(); + let Statement::Variable(result) = + ast.controls[0].apply.statements.last().unwrap() + else { + panic!("expected result declaration"); + }; + let result = result.initializer.as_deref().unwrap(); + let ExpressionKind::Binary(shift, BinOp::Eq, rhs) = &result.kind else { + panic!("expected comparison"); + }; + let ExpressionKind::Binary(lhs, BinOp::Shl, count) = &shift.kind else { + panic!("expected shift"); + }; + assert_eq!(result.token.kind, Kind::DoubleEquals); + assert_eq!(shift.token.kind, Kind::Shl); + + let (hlir, diags) = crate::check::all(&ast); + let errors = diags.errors(); + assert!( + errors.is_empty(), + "{:?}", + errors.iter().map(|diag| &diag.message).collect::>(), + ); + for (expression, ty) in [ + (result, Type::Bool), + (shift.as_ref(), Type::Bit(8)), + (lhs.as_ref(), Type::Bit(8)), + (count.as_ref(), Type::Int(128)), + (rhs.as_ref(), Type::Bit(8)), + ] { + assert_eq!(hlir.expression_types.get(expression), Some(&ty)); + } + } +} diff --git a/p4/src/util.rs b/p4/src/util.rs index 769e8105..bf11b3d2 100644 --- a/p4/src/util.rs +++ b/p4/src/util.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use crate::ast::{Lvalue, NameInfo, Type, AST}; use std::collections::HashMap; @@ -18,6 +18,7 @@ pub fn resolve_lvalue( Type::Bit(_) => root.clone(), Type::Varbit(_) => root.clone(), Type::Int(_) => root.clone(), + Type::ArbitraryInt => root.clone(), Type::String => root.clone(), Type::ExternFunction => root.clone(), Type::HeaderMethod => root.clone(), diff --git a/test/src/action_param_offset.rs b/test/src/action_param_offset.rs new file mode 100644 index 00000000..cca33247 --- /dev/null +++ b/test/src/action_param_offset.rs @@ -0,0 +1,63 @@ +use crate::softnpu::{RxFrame, SoftNpu, TxFrame}; +use crate::{expect_frames, muffins}; + +p4_macro::use_p4!( + p4 = "test/src/p4/action_param_offset.p4", + pipeline_name = "action_param_offset", +); + +fn forward_param(vid: u16, port: u16) -> Vec { + let mut buf = vid.to_le_bytes().to_vec(); + buf.extend_from_slice(&port.to_le_bytes()); + buf +} + +// A `bit<12>` action parameter followed by a `bit<16>` one. +// +// x4c lays out action data as ceil(n/8) little-endian bytes per parameter +// (in declaration order). This means that a VID above 255 comes through +// intact and the port is read from the two bytes after the VID. +#[test] +fn action_param_offset() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + + pipeline.add_ingress_retag_entry( + "forward", + &0u16.to_le_bytes(), + &forward_param(300, 2), + 0, + ); + pipeline.add_ingress_retag_entry( + "forward", + &1u16.to_le_bytes(), + &forward_param(4094, 3), + 0, + ); + // Three bytes for a four-byte action is ignored rather than panicking. + pipeline.add_ingress_retag_entry( + "forward", + &3u16.to_le_bytes(), + &[0u8; 3], + 0, + ); + assert_eq!(pipeline.get_ingress_retag_entries().len(), 2); + + let mut npu = SoftNpu::new(4, pipeline, false); + let phy0 = npu.phy(0); + let phy1 = npu.phy(1); + let phy2 = npu.phy(2); + let phy3 = npu.phy(3); + + npu.run(); + + let et = 0; + let msg = muffins!(); + + phy0.send(&[TxFrame::newv(phy2.mac, et, msg.0, 10)])?; + expect_frames!(phy2, &[RxFrame::newv(phy0.mac, et, msg.0, 300)]); + + phy1.send(&[TxFrame::newv(phy3.mac, et, msg.1, 20)])?; + expect_frames!(phy3, &[RxFrame::newv(phy1.mac, et, msg.1, 4094)]); + + Ok(()) +} diff --git a/test/src/dynamic_router_noaddr.rs b/test/src/dynamic_router_noaddr.rs new file mode 100644 index 00000000..23b52b16 --- /dev/null +++ b/test/src/dynamic_router_noaddr.rs @@ -0,0 +1,82 @@ +use bitvec::field::BitField; +use p4rs::{packet_in, Pipeline}; +use std::net::Ipv6Addr; + +p4_macro::use_p4!( + p4 = "test/src/p4/dynamic_router_noaddr.p4", + pipeline_name = "dynamic_router_noaddr", +); + +fn v6_packet(dst: Ipv6Addr) -> [u8; 54] { + let mut buf = [0u8; 54]; + buf[..6].copy_from_slice(&[0x02, 0, 0, 0, 0, 2]); + buf[6..12].copy_from_slice(&[0x02, 0, 0, 0, 0, 1]); + buf[12..14].copy_from_slice(&0x86ddu16.to_be_bytes()); + buf[14] = 0x60; + buf[21] = 64; + buf[22..38].copy_from_slice(&[0xfd; 16]); + buf[38..54].copy_from_slice(&dst.octets()); + buf +} + +fn assert_egress_ether_types( + pipeline: &mut main_pipeline, + buf: &[u8], + expected: &[(u16, u16)], +) { + let mut pkt = packet_in::new(buf); + let packets: Vec<(u16, u16)> = pipeline + .process_packet(2, &mut pkt) + .into_iter() + .map(|(packet, port)| { + let ether_type = u16::from_be_bytes([ + packet.header_data[12], + packet.header_data[13], + ]); + (port, ether_type) + }) + .collect(); + + let mut pkt = packet_in::new(buf); + let headers: Vec<(u16, u16)> = pipeline + .process_packet_headers(2, &mut pkt) + .into_iter() + .map(|(headers, port)| { + (port, headers.ethernet.ether_type.load_le::()) + }) + .collect(); + + assert_eq!(packets, expected, "packet"); + assert_eq!(headers, expected, "headers"); +} + +#[test] +fn local_dst_goes_to_scrimlet() { + let mut pipeline = main_pipeline::new(4); + let dst: Ipv6Addr = "fd00:1000::1".parse().unwrap(); + pipeline.add_ingress_local_local_entry( + "set_local", + &u128::from(dst).to_le_bytes(), + &[], + 0, + ); + + assert_egress_ether_types(&mut pipeline, &v6_packet(dst), &[(0, 0x0901)]); +} + +#[test] +fn remote_dst_is_routed() { + let mut pipeline = main_pipeline::new(4); + let prefix: Ipv6Addr = "fd00:2000::".parse().unwrap(); + let mut key = prefix.octets().to_vec(); + key.push(24); + pipeline.add_ingress_router_router_entry( + "forward", + &key, + &3u16.to_le_bytes(), + 0, + ); + + let dst: Ipv6Addr = "fd00:2000::1".parse().unwrap(); + assert_egress_ether_types(&mut pipeline, &v6_packet(dst), &[(3, 0x86dd)]); +} diff --git a/test/src/hub.rs b/test/src/hub.rs index 9333fb78..1c63c25b 100644 --- a/test/src/hub.rs +++ b/test/src/hub.rs @@ -1,5 +1,6 @@ use crate::softnpu::{RxFrame, SoftNpu, TxFrame}; use crate::{expect_frames, muffins}; +use p4rs::{packet_in, Pipeline}; p4_macro::use_p4!(p4 = "test/src/p4/hub.p4", pipeline_name = "hub2"); @@ -80,3 +81,32 @@ fn hub2() -> Result<(), anyhow::Error> { Ok(()) } + +fn assert_dropped(ingress: u16) { + let mut pipeline = main_pipeline::new(3); + let data = [0u8; 64]; + + let mut pkt = packet_in::new(&data); + assert!( + pipeline.process_packet(ingress, &mut pkt).is_empty(), + "packet" + ); + + let mut pkt = packet_in::new(&data); + assert!( + pipeline + .process_packet_headers(ingress, &mut pkt) + .is_empty(), + "headers" + ); +} + +#[test] +fn drop_precedes_broadcast() { + assert_dropped(2); +} + +#[test] +fn drop_precedes_unicast() { + assert_dropped(3); +} diff --git a/test/src/lib.rs b/test/src/lib.rs index a6d4f38b..6b1234b4 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -1,5 +1,9 @@ +// Copyright 2026 Oxide Computer Company + #![allow(clippy::too_many_arguments)] +#[cfg(test)] +mod action_param_offset; #[cfg(test)] mod arithmetic; #[cfg(test)] @@ -15,6 +19,8 @@ mod dload; #[cfg(test)] mod dynamic_router; #[cfg(test)] +mod dynamic_router_noaddr; +#[cfg(test)] mod headers; #[cfg(test)] mod hub; @@ -23,8 +29,20 @@ mod ipv6; #[cfg(test)] mod mac_rewrite; #[cfg(test)] +mod mcast; +#[cfg(test)] +mod metadata_names; +#[cfg(test)] mod range; #[cfg(test)] +mod shift; +#[cfg(test)] +mod slice_assign; +#[cfg(test)] +mod slice_assign_empty; +#[cfg(test)] +mod slice_read; +#[cfg(test)] mod table_in_egress_and_ingress; #[cfg(test)] mod vlan; diff --git a/test/src/mcast.rs b/test/src/mcast.rs new file mode 100644 index 00000000..b72044b6 --- /dev/null +++ b/test/src/mcast.rs @@ -0,0 +1,364 @@ +use crate::softnpu::{RxFrame, SoftNpu, TxFrame}; +use crate::{expect_frames, muffins}; +use bitvec::field::BitField; +use p4rs::{packet_in, Pipeline}; + +p4_macro::use_p4!(p4 = "test/src/p4/mcast.p4", pipeline_name = "mcast"); + +/// Build a port bitmap for use as action parameter_data. +/// `byte_len` is the byte width of the P4 `bit` field (N / 8). +/// +/// Little-endian encoding: bit N (value 2^N) corresponds to port N, +/// matching how p4rs arithmetic ([`p4rs::bitmath::shl_le`], +/// [`bitvec::field::BitField::load_le`]) interprets bitvec storage. +pub(crate) fn port_bitmap(byte_len: usize, ports: &[u16]) -> Vec { + let mut bitmap = vec![0u8; byte_len]; + for &p in ports { + let byte_idx = (p / 8) as usize; + let bit_idx = p % 8; + assert!(byte_idx < byte_len, "port {p} exceeds bitmap width"); + bitmap[byte_idx] |= 1 << bit_idx; + } + bitmap +} + +fn assert_egress_ports( + pipeline: &mut main_pipeline, + ingress: u16, + expected: &[u16], +) { + let data = [0u8; 64]; + + let mut pkt = packet_in::new(&data); + let packet_ports: Vec = pipeline + .process_packet(ingress, &mut pkt) + .iter() + .map(|(_, port)| *port) + .collect(); + + let mut pkt = packet_in::new(&data); + let header_ports: Vec = pipeline + .process_packet_headers(ingress, &mut pkt) + .iter() + .map(|(_, port)| *port) + .collect(); + + assert_eq!(packet_ports, expected, "packet (ingress {ingress})"); + assert_eq!(header_ports, expected, "headers (ingress {ingress})"); +} + +#[test] +fn bitmap_no_self_replication() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + + // Port 0 is in the bitmap but is also the ingress port. + let bitmap = port_bitmap(16, &[0, 1, 2]); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap", + &0u16.to_le_bytes(), + &bitmap, + 0, + ); + + let mut npu = SoftNpu::new(4, pipeline, false); + let phy0 = npu.phy(0); + let phy1 = npu.phy(1); + let phy2 = npu.phy(2); + let phy3 = npu.phy(3); + + npu.run(); + + let msg = muffins!(); + + // Port 0 should be excluded since it is the ingress port. + phy0.send(&[TxFrame::new(phy1.mac, 0, msg.0)])?; + expect_frames!(phy1, &[RxFrame::new(phy0.mac, 0, msg.0)]); + expect_frames!(phy2, &[RxFrame::new(phy0.mac, 0, msg.0)]); + assert_eq!(phy0.recv_buffer_len(), 0); + assert_eq!(phy3.recv_buffer_len(), 0); + + Ok(()) +} + +#[test] +fn metadata_bit_fields_default_to_sized_zeros() { + let egress = egress_metadata_t::default(); + + assert_eq!(egress.bitmap_a.len(), 128); + assert_eq!(egress.port_bitmap.len(), 128); + assert_eq!(egress.nexthop_v6.len(), 128); + assert_eq!(egress.nexthop_v4.len(), 32); + assert_eq!(egress.port.len(), 16); + assert!(!egress.bitmap_a.any()); + assert!(!egress.port_bitmap.any()); +} + +#[test] +fn no_table_match_yields_no_egress() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + + // Empty bitmap -> no ports set. + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap", + &1u16.to_le_bytes(), + &port_bitmap(16, &[]), + 0, + ); + + for ingress in [0, 1] { + assert_egress_ports(&mut pipeline, ingress, &[]); + } + + Ok(()) +} + +#[test] +fn empty_bitmap_falls_back_to_broadcast() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + let bitmap = port_bitmap(16, &[]); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap_broadcast", + &0u16.to_le_bytes(), + &bitmap, + 0, + ); + + assert_egress_ports(&mut pipeline, 0, &[1, 2, 3]); + + Ok(()) +} + +#[test] +fn empty_bitmap_falls_back_to_unicast() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + pipeline.add_ingress_bitmap_table_entry( + "forward", + &0u16.to_le_bytes(), + &1u16.to_le_bytes(), + 0, + ); + + assert_egress_ports(&mut pipeline, 0, &[1]); + + Ok(()) +} + +fn assert_filtered_bitmap_has_no_egress(action: &str, bitmap_port: u16) { + let mut pipeline = main_pipeline::new(4); + pipeline.add_ingress_bitmap_table_entry( + action, + &0u16.to_le_bytes(), + &port_bitmap(16, &[bitmap_port]), + 0, + ); + + assert_egress_ports(&mut pipeline, 0, &[]); +} + +#[test] +fn filtered_bitmap_ingress_only_precedes_broadcast() { + assert_filtered_bitmap_has_no_egress("set_bitmap_broadcast", 0); +} + +#[test] +fn filtered_bitmap_out_of_radix_only_precedes_unicast() { + assert_filtered_bitmap_has_no_egress("set_bitmap_unicast", 127); +} + +#[test] +fn drop_precedes_nonempty_bitmap() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + let bitmap = port_bitmap(16, &[1, 2]); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap_drop", + &0u16.to_le_bytes(), + &bitmap, + 0, + ); + + assert_egress_ports(&mut pipeline, 0, &[]); + + Ok(()) +} + +#[test] +fn bitmap_precedence_over_broadcast() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + + // Bitmap with only port 1. The bitmap check runs before broadcast, + // so even though broadcast might be set elsewhere, bitmap wins + // when port_bitmap has bits set. + let bitmap = port_bitmap(16, &[1]); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap_broadcast", + &0u16.to_le_bytes(), + &bitmap, + 0, + ); + + let mut npu = SoftNpu::new(4, pipeline, false); + let phy0 = npu.phy(0); + let phy1 = npu.phy(1); + let phy2 = npu.phy(2); + let phy3 = npu.phy(3); + + npu.run(); + + let msg = muffins!(); + + phy0.send(&[TxFrame::new(phy1.mac, 0, msg.0)])?; + expect_frames!(phy1, &[RxFrame::new(phy0.mac, 0, msg.0)]); + assert_eq!(phy2.recv_buffer_len(), 0); + assert_eq!(phy3.recv_buffer_len(), 0); + + Ok(()) +} + +#[test] +fn bitmap_all_ports() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + + // All ports set, equivalent to broadcast. + let bitmap = port_bitmap(16, &[0, 1, 2, 3]); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap", + &0u16.to_le_bytes(), + &bitmap, + 0, + ); + + let mut npu = SoftNpu::new(4, pipeline, false); + let phy0 = npu.phy(0); + let phy1 = npu.phy(1); + let phy2 = npu.phy(2); + let phy3 = npu.phy(3); + + npu.run(); + + let msg = muffins!(); + + // Port 0 is ingress and should be excluded. + phy0.send(&[TxFrame::new(phy1.mac, 0, msg.0)])?; + expect_frames!(phy1, &[RxFrame::new(phy0.mac, 0, msg.0)]); + expect_frames!(phy2, &[RxFrame::new(phy0.mac, 0, msg.0)]); + expect_frames!(phy3, &[RxFrame::new(phy0.mac, 0, msg.0)]); + assert_eq!(phy0.recv_buffer_len(), 0); + + Ok(()) +} + +#[test] +fn per_replica_ingress_metadata_is_isolated() -> Result<(), anyhow::Error> { + let mut pipeline = main_pipeline::new(4); + let bitmap = port_bitmap(16, &[1, 2, 3]); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap", + &0u16.to_le_bytes(), + &bitmap, + 0, + ); + + assert_egress_ports(&mut pipeline, 0, &[1, 2, 3]); + + Ok(()) +} + +#[test] +fn bitmap_ports_beyond_radix_ignored() -> Result<(), anyhow::Error> { + let mut radix_pipeline = main_pipeline::new(4); + + // Port 127 is the top bitmap bit and outside the radix-4 + // pipeline -> ignore it. + let bitmap = port_bitmap(16, &[1, 127]); + radix_pipeline.add_ingress_bitmap_table_entry( + "set_bitmap", + &0u16.to_le_bytes(), + &bitmap, + 0, + ); + + assert_egress_ports(&mut radix_pipeline, 0, &[1]); + + Ok(()) +} + +fn unicast_mac(port: u16) -> [u8; 6] { + let id = 0x20 + port as u8; + [0x02, 0x00, id, id, 0x00, 0x02] +} + +fn replica_mac(port: u16) -> [u8; 6] { + let id = 0x10 + port as u8; + [0x02, 0x00, id, id, 0x00, 0x02] +} + +fn add_mac_rewrite_entries(pipeline: &mut main_pipeline) { + for port in 0u16..4 { + pipeline.add_ingress_mac_mac_rewrite_entry( + "rewrite", + &port.to_le_bytes(), + &unicast_mac(port), + 0, + ); + pipeline.add_egress_mac_mac_rewrite_entry( + "rewrite", + &port.to_le_bytes(), + &replica_mac(port), + 0, + ); + } +} + +fn assert_src_macs(pipeline: &mut main_pipeline, expected: &[(u16, [u8; 6])]) { + let data = [0u8; 64]; + + let mut pkt = packet_in::new(&data); + let packets: Vec<(u16, [u8; 6])> = pipeline + .process_packet(0, &mut pkt) + .into_iter() + .map(|(packet, port)| { + (port, packet.header_data[6..12].try_into().unwrap()) + }) + .collect(); + + let mut pkt = packet_in::new(&data); + let headers: Vec<(u16, [u8; 6])> = pipeline + .process_packet_headers(0, &mut pkt) + .into_iter() + .map(|(headers, port)| { + let src = headers.ethernet.src_addr.load_le::().to_be_bytes(); + (port, src[2..].try_into().unwrap()) + }) + .collect(); + + assert_eq!(packets, expected, "packet"); + assert_eq!(headers, expected, "headers"); +} + +#[test] +fn unicast_rewrites_mac_in_ingress() { + let mut pipeline = main_pipeline::new(4); + add_mac_rewrite_entries(&mut pipeline); + pipeline.add_ingress_bitmap_table_entry( + "forward", + &0u16.to_le_bytes(), + &2u16.to_le_bytes(), + 0, + ); + + assert_src_macs(&mut pipeline, &[(2, unicast_mac(2))]); +} + +#[test] +fn replicas_rewrite_mac_in_egress() { + let mut pipeline = main_pipeline::new(4); + add_mac_rewrite_entries(&mut pipeline); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap", + &0u16.to_le_bytes(), + &port_bitmap(16, &[0, 2, 3]), + 0, + ); + + assert_src_macs(&mut pipeline, &[(2, replica_mac(2)), (3, replica_mac(3))]); +} diff --git a/test/src/metadata_names.rs b/test/src/metadata_names.rs new file mode 100644 index 00000000..f6ed495e --- /dev/null +++ b/test/src/metadata_names.rs @@ -0,0 +1,77 @@ +use bitvec::field::BitField; +use p4rs::{packet_in, Pipeline}; + +macro_rules! assert_forwarding { + ($pipeline:expr) => {{ + let mut pipeline = $pipeline; + let data = [0, 0, 0, 0, 0xaa, 0xbb]; + + for (ingress, ports) in + [(0u16, &[1u16, 2][..]), (1, &[3][..]), (2, &[0, 1, 3][..])] + { + let expected: Vec<_> = + ports.iter().map(|&port| (port, ingress, port)).collect(); + + let mut pkt = packet_in::new(&data); + let packets: Vec<_> = pipeline + .process_packet(ingress, &mut pkt) + .into_iter() + .map(|(packet, port)| { + assert_eq!(packet.header_data.len(), 4); + assert_eq!(packet.payload_data, &data[4..]); + let ingress_port = u16::from_be_bytes( + packet.header_data[..2].try_into().unwrap(), + ); + let egress_port = u16::from_be_bytes( + packet.header_data[2..].try_into().unwrap(), + ); + (port, ingress_port, egress_port) + }) + .collect(); + + let mut pkt = packet_in::new(&data); + let headers: Vec<_> = pipeline + .process_packet_headers(ingress, &mut pkt) + .into_iter() + .map(|(headers, port)| { + ( + port, + headers.result.ingress_port.load_le::(), + headers.result.egress_port.load_le::(), + ) + }) + .collect(); + + assert_eq!(packets, expected); + assert_eq!(headers, expected); + } + }}; +} + +mod shared_parameter_names { + use super::*; + + p4_macro::use_p4!( + p4 = "test/src/p4/metadata_names_shared.p4", + pipeline_name = "metadata_names_shared", + ); + + #[test] + fn forwarding() { + assert_forwarding!(main_pipeline::new(4)); + } +} + +mod wrapper_local_names { + use super::*; + + p4_macro::use_p4!( + p4 = "test/src/p4/metadata_names_locals.p4", + pipeline_name = "metadata_names_locals", + ); + + #[test] + fn forwarding() { + assert_forwarding!(main_pipeline::new(4)); + } +} diff --git a/test/src/p4/action_param_offset.p4 b/test/src/p4/action_param_offset.p4 new file mode 100644 index 00000000..f90e5292 --- /dev/null +++ b/test/src/p4/action_param_offset.p4 @@ -0,0 +1,61 @@ +#include +#include +#include + +SoftNPU( + parse(), + ingress(), + egress() +) main; + +struct headers_t { + ethernet_h ethernet; + vlan_h vlan; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t ingress, +){ + state start { + pkt.extract(hdr.ethernet); + if (hdr.ethernet.ether_type == 16w0x8100) { + transition vlan; + } + transition reject; + } + + state vlan { + pkt.extract(hdr.vlan); + transition accept; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + action forward(bit<12> vid, bit<16> port) { + hdr.vlan.vid = vid; + egress.port = port; + } + + table retag { + key = { ingress.port: exact; } + actions = { forward; NoAction; } + default_action = NoAction; + } + + apply { + retag.apply(); + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { +} diff --git a/test/src/p4/controller_multiple_instantiation.p4 b/test/src/p4/controller_multiple_instantiation.p4 index c8510304..1c67269f 100644 --- a/test/src/p4/controller_multiple_instantiation.p4 +++ b/test/src/p4/controller_multiple_instantiation.p4 @@ -26,7 +26,7 @@ control resolver( } table arp { key = { x: exact; } - actions = { resolve; } + actions = { resolve; NoAction; } default_action = NoAction; } apply { arp.apply(); } diff --git a/test/src/p4/dynamic_router_noaddr.p4 b/test/src/p4/dynamic_router_noaddr.p4 index e05c5e3a..495d10e9 100644 --- a/test/src/p4/dynamic_router_noaddr.p4 +++ b/test/src/p4/dynamic_router_noaddr.p4 @@ -54,7 +54,7 @@ control local( is_local = false; } - action local() { + action set_local() { is_local = true; } @@ -63,7 +63,7 @@ control local( hdr.ipv6.dst: exact; } actions = { - local; + set_local; nonlocal; } default_action = nonlocal; @@ -172,7 +172,7 @@ control ingress( else { // if the packet came from the scrimlet invalidate the header // sidecar header so. - if (ingress.port == 8w1) { + if (ingress.port == 16w1) { hdr.sidecar.setInvalid(); } router.apply(hdr, ingress, egress); diff --git a/test/src/p4/dynamic_router_noaddr_nbr.p4 b/test/src/p4/dynamic_router_noaddr_nbr.p4 index 42d43df2..dc3597e5 100644 --- a/test/src/p4/dynamic_router_noaddr_nbr.p4 +++ b/test/src/p4/dynamic_router_noaddr_nbr.p4 @@ -141,7 +141,7 @@ control router( apply { router.apply(); - if (egress.port != 16w0) { + if (egress.nexthop_v6 != 128w0) { resolver.apply(hdr, egress); } } diff --git a/test/src/p4/hub.p4 b/test/src/p4/hub.p4 index b0d6f4e3..36bd04aa 100644 --- a/test/src/p4/hub.p4 +++ b/test/src/p4/hub.p4 @@ -45,6 +45,16 @@ control ingress( egress.broadcast = true; } + action broadcast_drop() { + egress.broadcast = true; + egress.drop = true; + } + + action unicast_drop() { + egress.port = 16w1; + egress.drop = true; + } + table tbl { key = { ingress.port: exact; @@ -52,11 +62,15 @@ control ingress( actions = { drop; forward; + broadcast_drop; + unicast_drop; } default_action = drop; const entries = { 16w0 : forward(16w1); 16w1 : forward(16w0); + 16w2 : broadcast_drop(); + 16w3 : unicast_drop(); } } diff --git a/test/src/p4/mcast.p4 b/test/src/p4/mcast.p4 new file mode 100644 index 00000000..d9f98909 --- /dev/null +++ b/test/src/p4/mcast.p4 @@ -0,0 +1,129 @@ +#include +#include + +SoftNPU( + parse(), + ingress(), + egress() +) main; + +struct headers_t { + ethernet_t ethernet; +} + +header ethernet_t { + bit<48> dst_addr; + bit<48> src_addr; + bit<16> ether_type; +} + +parser parse( + packet_in pkt, + out headers_t headers, + inout ingress_metadata_t ingress, +){ + state start { + pkt.extract(headers.ethernet); + transition finish; + } + + state finish { + transition accept; + } +} + +control mac_rewrite( + inout headers_t hdr, + inout egress_metadata_t egress, +) { + action rewrite(bit<48> mac) { + hdr.ethernet.src_addr = mac; + } + + table mac_rewrite { + key = { egress.port: exact; } + actions = { rewrite; NoAction; } + default_action = NoAction; + } + + apply { + mac_rewrite.apply(); + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + mac_rewrite() mac; + + action drop() { } + + action forward(bit<16> port) { + egress.port = port; + } + + action set_bitmap(bit<128> bitmap) { + egress.bitmap_a = bitmap; + } + + action set_bitmap_broadcast(bit<128> bitmap) { + egress.bitmap_a = bitmap; + egress.broadcast = true; + } + + action set_bitmap_unicast(bit<128> bitmap) { + egress.bitmap_a = bitmap; + egress.port = 16w1; + } + + action set_bitmap_drop(bit<128> bitmap) { + egress.bitmap_a = bitmap; + egress.drop = true; + } + + table bitmap_table { + key = { + ingress.port: exact; + } + actions = { + drop; + forward; + set_bitmap; + set_bitmap_broadcast; + set_bitmap_unicast; + set_bitmap_drop; + } + default_action = drop; + } + + apply { + bitmap_table.apply(); + egress.port_bitmap = egress.bitmap_a; + if (egress.port_bitmap == 128w0) { + mac.apply(hdr, egress); + } + } + +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + mac_rewrite() mac; + + apply { + if (egress.port_bitmap != 128w0) { + mac.apply(hdr, egress); + } + + // Detect ingress metadata leaking between output copies. + if (ingress.nat == true) { + egress.drop = true; + } + ingress.nat = true; + } +} diff --git a/test/src/p4/metadata_names_locals.p4 b/test/src/p4/metadata_names_locals.p4 new file mode 100644 index 00000000..a19f1a10 --- /dev/null +++ b/test/src/p4/metadata_names_locals.p4 @@ -0,0 +1,53 @@ +#include +#include + +SoftNPU(parse(), ingress(), egress()) main; + +header result_h { + bit<16> ingress_port; + bit<16> egress_port; +} + +struct headers_t { + result_h result; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t input, +) { + state start { + pkt.extract(hdr.result); + hdr.result.ingress_port = input.port; + transition accept; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t parsed, + inout egress_metadata_t output, +) { + apply { + output.port = 16w3; + if (parsed.port == 16w0) { + output.port_bitmap = 128w6; + } else if (parsed.port == 16w2) { + output.broadcast = true; + } + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t input, + inout egress_metadata_t port, +) { + apply { + if (hdr.result.ingress_port != input.port) { + port.drop = true; + } + hdr.result.egress_port = port.port; + } +} diff --git a/test/src/p4/metadata_names_shared.p4 b/test/src/p4/metadata_names_shared.p4 new file mode 100644 index 00000000..8c9a7cca --- /dev/null +++ b/test/src/p4/metadata_names_shared.p4 @@ -0,0 +1,53 @@ +#include +#include + +SoftNPU(parse(), ingress(), egress()) main; + +header result_h { + bit<16> ingress_port; + bit<16> egress_port; +} + +struct headers_t { + result_h result; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t input, +) { + state start { + pkt.extract(hdr.result); + hdr.result.ingress_port = input.port; + transition accept; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t meta, + inout egress_metadata_t output, +) { + apply { + output.port = 16w3; + if (meta.port == 16w0) { + output.port_bitmap = 128w6; + } else if (meta.port == 16w2) { + output.broadcast = true; + } + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t input, + inout egress_metadata_t meta, +) { + apply { + if (hdr.result.ingress_port != input.port) { + meta.drop = true; + } + hdr.result.egress_port = meta.port; + } +} diff --git a/test/src/p4/range.p4 b/test/src/p4/range.p4 index 8b007d4e..f282d569 100644 --- a/test/src/p4/range.p4 +++ b/test/src/p4/range.p4 @@ -53,9 +53,11 @@ control ingress( table power_ranger { key = { hdr.ipv4.dst: range; + hdr.ethernet.ether_type: exact; } actions = { forward; + NoAction; } default_action = NoAction; } diff --git a/test/src/p4/shift.p4 b/test/src/p4/shift.p4 new file mode 100644 index 00000000..7cb324ab --- /dev/null +++ b/test/src/p4/shift.p4 @@ -0,0 +1,65 @@ +#include +#include + +SoftNPU( + parse(), + ingress(), + egress() +) main; + +header result_h { + bit<128> port_mask; + bit<128> unshifted; +} + +struct headers_t { + result_h result; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t ingress, +) { + state start { + pkt.extract(hdr.result); + transition accept; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + action set_bitmap(bit<128> bitmap) { + egress.bitmap_a = bitmap; + } + + table bitmap_table { + key = { + ingress.port: exact; + } + actions = { + set_bitmap; + NoAction; + } + default_action = NoAction; + } + + apply { + bitmap_table.apply(); + egress.port_bitmap = egress.bitmap_a; + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + apply { + hdr.result.port_mask = 128w1 << egress.port; + hdr.result.unshifted = hdr.result.port_mask >> egress.port; + } +} diff --git a/test/src/p4/shift_counts.p4 b/test/src/p4/shift_counts.p4 new file mode 100644 index 00000000..978152b1 --- /dev/null +++ b/test/src/p4/shift_counts.p4 @@ -0,0 +1,78 @@ +#include +#include + +SoftNPU(parse(), ingress(), egress()) main; + +header result_h { + bit<128> count; + bit<32> left_literal; + bit<32> right_literal; + bit<32> left_narrow; + bit<32> right_narrow; + bit<32> left_wide; + bit<32> right_wide; + bit<8> left_large_literal; + bit<8> right_large_literal; + bit<8> left_linear; + bit<8> right_linear; + bit<8> left_right_linear; + bit<8> right_left_linear; + bit<8> add_then_shift; + bit<8> shift_by_sum; + bit<8> grouped_shift; + bit<8> grouped_comparison; +} + +struct headers_t { + result_h result; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t ingress, +) { + state start { + pkt.extract(hdr.result); + transition accept; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + apply { + bit<8> narrow = hdr.result.count[7:0]; + hdr.result.left_literal = 32w0x81234567 << 1; + hdr.result.right_literal = 32w0x81234567 >> 1; + hdr.result.left_narrow = 32w0x81234567 << narrow; + hdr.result.right_narrow = 32w0x81234567 >> narrow; + hdr.result.left_wide = 32w0x81234567 << hdr.result.count; + hdr.result.right_wide = 32w0x81234567 >> hdr.result.count; + hdr.result.left_large_literal = 8w1 << 256; + hdr.result.right_large_literal = + 8w128 >> 1267650600228229401496703205376; + hdr.result.left_linear = 8w1 << 8w1 << 8w2; + hdr.result.right_linear = 8w128 >> 8w1 >> 8w2; + hdr.result.left_right_linear = 8w3 << 8w1 >> 8w2; + hdr.result.right_left_linear = 8w7 >> 8w1 << 8w1; + hdr.result.add_then_shift = 8w1 + 8w2 << 8w1; + hdr.result.shift_by_sum = 8w1 << 8w1 + 8w1; + hdr.result.grouped_shift = 8w1 << (8w1 << 8w2); + if ((8w1 == 8w1) == true) { + hdr.result.grouped_comparison = 8w1; + } else { + hdr.result.grouped_comparison = 8w0; + } + egress.port = 16w1; + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { +} diff --git a/test/src/p4/sidecar-lite.p4 b/test/src/p4/sidecar-lite.p4 index c7052636..b29b1018 100644 --- a/test/src/p4/sidecar-lite.p4 +++ b/test/src/p4/sidecar-lite.p4 @@ -1,5 +1,5 @@ #include -#include +#include #include SoftNPU( @@ -306,7 +306,7 @@ control nat_ingress( hdr.ipv4.dst: exact; ingress.nat_id: range; } - actions = { forward_to_sled; } + actions = { forward_to_sled; NoAction; } default_action = NoAction; } @@ -315,7 +315,7 @@ control nat_ingress( hdr.ipv6.dst: exact; ingress.nat_id: range; } - actions = { forward_to_sled; } + actions = { forward_to_sled; NoAction; } default_action = NoAction; } @@ -324,7 +324,7 @@ control nat_ingress( hdr.ipv6.dst: exact; ingress.nat_id: range; } - actions = { forward_to_sled; } + actions = { forward_to_sled; NoAction; } default_action = NoAction; } @@ -333,7 +333,7 @@ control nat_ingress( hdr.ipv4.dst: exact; ingress.nat_id: range; } - actions = { forward_to_sled; } + actions = { forward_to_sled; NoAction; } default_action = NoAction; } @@ -507,7 +507,7 @@ control mac_rewrite( table mac_rewrite { key = { egress.port: exact; } - actions = { rewrite; } + actions = { rewrite; NoAction; } default_action = NoAction; } @@ -530,10 +530,8 @@ control proxy_arp( hdr.arp.sender_mac = mac; hdr.arp.opcode = 16w2; - //TODO compiler broken for this, should be able to do this in one line. bit<32> tmp = 0; tmp = hdr.arp.sender_ip; - /// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ hdr.arp.sender_ip = hdr.arp.target_ip; hdr.arp.target_ip = tmp; @@ -541,7 +539,7 @@ control proxy_arp( table proxy_arp { key = { hdr.arp.target_ip: range; } - actions = { proxy_arp_reply; } + actions = { proxy_arp_reply; NoAction; } default_action = NoAction; } @@ -550,6 +548,30 @@ control proxy_arp( } } +control mcast_ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + action set_port_bitmap(bit<128> bitmap) { + egress.port_bitmap = bitmap; + } + + table mcast_replication_v6 { + key = { + hdr.ipv6.dst: exact; + } + actions = { set_port_bitmap; NoAction; } + default_action = NoAction; + } + + apply { + if (hdr.ipv6.isValid()) { + mcast_replication_v6.apply(); + } + } +} + control ingress( inout headers_t hdr, inout ingress_metadata_t ingress, @@ -561,6 +583,7 @@ control ingress( resolver() resolver; mac_rewrite() mac; proxy_arp() pxarp; + mcast_ingress() mcast; apply { @@ -633,7 +656,10 @@ control ingress( hdr.inner_udp.setInvalid(); } router.apply(hdr, ingress, egress); - if (egress.port != 16w0) { + if (egress.nexthop_v4 != 32w0) { + resolver.apply(hdr, egress); + } + if (egress.nexthop_v6 != 128w0) { resolver.apply(hdr, egress); } } @@ -653,9 +679,6 @@ control ingress( hdr.sidecar.sc_payload = 128w0x1701d; // scrimlet port - // TODO simply stating 0 here causes bad conversion, initializes - // egress.port as a 128 bit value due to - // StatementGenerator::converter using int_to_bitvec egress.port = 16w0; } return; @@ -669,9 +692,17 @@ control ingress( // check for ingress nat nat.apply(hdr, ingress, egress); - router.apply(hdr, ingress, egress); - if (egress.port != 16w0) { - resolver.apply(hdr, egress); + // check for multicast replication before unicast routing + mcast.apply(hdr, ingress, egress); + + if (egress.port_bitmap == 128w0) { + router.apply(hdr, ingress, egress); + if (egress.nexthop_v4 != 32w0) { + resolver.apply(hdr, egress); + } + if (egress.nexthop_v6 != 128w0) { + resolver.apply(hdr, egress); + } } } diff --git a/test/src/p4/slice_assign.p4 b/test/src/p4/slice_assign.p4 new file mode 100644 index 00000000..eb16f573 --- /dev/null +++ b/test/src/p4/slice_assign.p4 @@ -0,0 +1,66 @@ +// Copyright 2026 Oxide Computer Company + +#include +#include +#include + +SoftNPU( + parse(), + ingress(), + egress() +) main; + +struct headers_t { + ethernet_h ethernet; + ipv4_h ipv4; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t ingress, +){ + state start { + pkt.extract(hdr.ethernet); + transition ipv4; + } + + state ipv4 { + pkt.extract(hdr.ipv4); + transition accept; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + apply { + // Derive multicast dst MAC from ipv4.dst (RFC 1112 section 6.4). + hdr.ethernet.dst[47:24] = 24w0x01005e; + hdr.ethernet.dst[23:16] = hdr.ipv4.dst[23:16]; + hdr.ethernet.dst[15:0] = hdr.ipv4.dst[15:0]; + hdr.ethernet.dst[23:23] = 1w0; + + // Copy ipv4.dst top nibble into its own bottom nibble, + // exercising same-field aliased slice assignment. + // + // Later IP writes do not update the multicast MAC or IPv4 checksum. + hdr.ipv4.dst[3:0] = hdr.ipv4.dst[31:28]; + + // Set a single bit to exercise [n:n] = 1w1. + hdr.ethernet.src[0:0] = 1w1; + + hdr.ipv4.src[19:12] = 8w0x5a; + + egress.port = 16w1; + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { +} diff --git a/test/src/p4/slice_assign_empty.p4 b/test/src/p4/slice_assign_empty.p4 new file mode 100644 index 00000000..ad2d1a5f --- /dev/null +++ b/test/src/p4/slice_assign_empty.p4 @@ -0,0 +1,125 @@ +#include +#include + +// Ports 8 and 9 exercise x4c's implicit bit widening extension. +// P4-16 8.11.1 requires explicit casts between these widths. + +SoftNPU( + parse(), + ingress(), + egress() +) main; + +header result_h { + bit<16> value; +} + +struct headers_t { + result_h result; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t ingress, +) { + state start { + transition accept; + } +} + +control set_value(inout headers_t hdr, in bit<16> value) { + apply { + hdr.result.value = value; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + set_value() setter; + + apply { + hdr.result.setValid(); + hdr.result.value = 16w0; + + if (ingress.port == 16w0) { + egress.port[15:0] = 16w1; + } else if (ingress.port == 16w1) { + // P4-16 8.25 leaves uninitialized locals unspecified. + // SoftNPU initializes bit locals to zero. + bit<16> uninit; + if (uninit == 16w0) { + hdr.result.value = 16w1; + } + egress.port = 16w1; + } else if (ingress.port == 16w2) { + egress.port[8:1] = 8w1; + } else if (ingress.port == 16w3) { + bit<16> high = 16w0x0100; + bit<16> low = 16w0x00ff; + if (high > low) { + hdr.result.value = 16w1; + } + egress.port = 16w1; + } else if (ingress.port == 16w4) { + bit<16> two = 2; + hdr.result.value = 1; + hdr.result.value = hdr.result.value + two; + egress.port = 16w1; + } else if (ingress.port == 16w5) { + setter.apply(hdr, 7); + egress.port = 16w1; + } else if (ingress.port == 16w6) { + hdr.result.value[15:8] = 1; + hdr.result.value[11:4] = 2; + egress.port = 16w1; + } else if (ingress.port == 16w7) { + hdr.result.value = 16w0x0302; + bit<8> high = hdr.result.value[15:8] << 1; + if (high == 8w6) { + hdr.result.value = 16w1 << hdr.result.value[3:0]; + } + egress.port = 16w1; + } else if (ingress.port == 16w8) { + bit<8> narrow = 8w5; + bit<16> wide = narrow; + hdr.result.value[15:0] = narrow; + if (wide == 16w5) { + hdr.result.value = hdr.result.value + 16w0x0100; + } + egress.port = 16w1; + } else if (ingress.port == 16w9) { + bit<8> narrow = 8w0xab; + bit<16> wide = 16w0; + wide = narrow; + hdr.result.value = narrow; + hdr.result.value[15:8] = hdr.result.value[3:0]; + if (hdr.result.value == 16w0x0bab) { + hdr.result.value = hdr.result.value[7:0]; + setter.apply(hdr, hdr.result.value[7:0]); + if (hdr.result.value == wide) { + if (narrow == 8w0xab) { + hdr.result.value = 16w0x01ab; + } + } + } + egress.port = 16w1; + } else if (ingress.port == 16w10) { + bit<0> empty = 0; + empty = 1; + bit<16> wide = empty; + hdr.result.value = wide + 16w4; + egress.port = 16w1; + } + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { +} diff --git a/test/src/p4/slice_read.p4 b/test/src/p4/slice_read.p4 new file mode 100644 index 00000000..3a12d993 --- /dev/null +++ b/test/src/p4/slice_read.p4 @@ -0,0 +1,79 @@ +// Copyright 2026 Oxide Computer Company + +#include +#include +#include + +SoftNPU( + parse(), + ingress(), + egress() +) main; + +struct headers_t { + ethernet_h ethernet; + ipv4_h ipv4; +} + +parser parse( + packet_in pkt, + out headers_t hdr, + inout ingress_metadata_t ingress, +){ + state start { + pkt.extract(hdr.ethernet); + transition ipv4; + } + + state ipv4 { + pkt.extract(hdr.ipv4); + transition accept; + } +} + +control ingress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { + apply { + // Read a sub-byte slice from a non-top byte of a 32-bit field. + // This exercises byte-reversal correctness. + // + // dst IP = 239.171.2.3 = 0xEFAB0203. + // ipv4.dst[23:20] = top nibble of second wire byte = 0xA. + // + // Correctly reversed: storage is [0x03, 0x02, 0xAB, 0xEF]. + // reversed_slice_range(23, 20, 32) maps to bitvec [16..20], + // which is the top nibble of storage byte 2 (0xAB) = 0xA. + // + // The incorrect range [20..24] reads the bottom nibble of storage + // byte 2 (0xAB), which is 0xB. + // + // Header writes record slice results without maintaining IPv4 checksums. + if (hdr.ipv4.dst[23:20] == 4w0xa) { + hdr.ipv4.identification = 16w42; + } + + if (hdr.ipv4.ttl[3:0] == 4w0x5) { + hdr.ipv4.protocol = 8w0x5b; + } + + if (hdr.ipv4.dst[19:12] == 8w0xb0) { + hdr.ipv4.diffserv = 8w0xb0; + } + + if (hdr.ipv4.frag_offset[11:4] == 8w0xab) { + hdr.ipv4.hdr_checksum = 16w0xab; + } + + egress.port = 16w1; + } +} + +control egress( + inout headers_t hdr, + inout ingress_metadata_t ingress, + inout egress_metadata_t egress, +) { +} diff --git a/test/src/p4/softnpu_mcast.p4 b/test/src/p4/softnpu_mcast.p4 new file mode 100644 index 00000000..e9c39bde --- /dev/null +++ b/test/src/p4/softnpu_mcast.p4 @@ -0,0 +1,20 @@ +struct ingress_metadata_t { + bit<16> port; + bool nat; + bit<16> nat_id; + bool drop; +} + +struct egress_metadata_t { + bit<16> port; + bit<128> nexthop_v6; + bit<32> nexthop_v4; + bool drop; + bool broadcast; + bit<128> port_bitmap; + bit<128> bitmap_a; +} + +extern Checksum { + bit<16> run(in T data); +} diff --git a/test/src/p4/vlan_trunk.p4 b/test/src/p4/vlan_trunk.p4 index 8f6123e9..2fe6c4ba 100644 --- a/test/src/p4/vlan_trunk.p4 +++ b/test/src/p4/vlan_trunk.p4 @@ -48,6 +48,7 @@ control ingress( } actions = { forward; + NoAction; } default_action = NoAction; } diff --git a/test/src/p4/vrf_router.p4 b/test/src/p4/vrf_router.p4 index 165fa59e..d2da2029 100644 --- a/test/src/p4/vrf_router.p4 +++ b/test/src/p4/vrf_router.p4 @@ -49,6 +49,7 @@ control ingress( } actions = { forward; + NoAction; } default_action = NoAction; } diff --git a/test/src/range.rs b/test/src/range.rs index 64691661..50664b38 100644 --- a/test/src/range.rs +++ b/test/src/range.rs @@ -21,6 +21,7 @@ fn range() -> Result<(), anyhow::Error> { let end = v4_range_key("4.0.0.0".parse().unwrap()); let mut buf = begin.to_vec(); buf.extend_from_slice(&end); + buf.extend_from_slice(&0x0800u16.to_le_bytes()); pipeline.add_ingress_power_ranger_entry( "forward", @@ -33,6 +34,7 @@ fn range() -> Result<(), anyhow::Error> { let end = v4_range_key("8.0.0.0".parse().unwrap()); let mut buf = begin.to_vec(); buf.extend_from_slice(&end); + buf.extend_from_slice(&0x0800u16.to_le_bytes()); pipeline.add_ingress_power_ranger_entry( "forward", @@ -45,6 +47,7 @@ fn range() -> Result<(), anyhow::Error> { let end = v4_range_key("12.0.0.0".parse().unwrap()); let mut buf = begin.to_vec(); buf.extend_from_slice(&end); + buf.extend_from_slice(&0x0800u16.to_le_bytes()); pipeline.add_ingress_power_ranger_entry( "forward", @@ -57,6 +60,7 @@ fn range() -> Result<(), anyhow::Error> { let end = v4_range_key("16.0.0.0".parse().unwrap()); let mut buf = begin.to_vec(); buf.extend_from_slice(&end); + buf.extend_from_slice(&0x0800u16.to_le_bytes()); pipeline.add_ingress_power_ranger_entry( "forward", diff --git a/test/src/shift.rs b/test/src/shift.rs new file mode 100644 index 00000000..3b0af79e --- /dev/null +++ b/test/src/shift.rs @@ -0,0 +1,229 @@ +use crate::mcast::port_bitmap; +use bitvec::field::BitField; +use p4rs::{packet_in, Pipeline}; + +p4_macro::use_p4!(p4 = "test/src/p4/shift.p4", pipeline_name = "shift"); + +const HEADER_LEN: usize = 32; + +fn assert_egress_shifts(ports: &[u16]) { + let mut pipeline = main_pipeline::new(4); + pipeline.add_ingress_bitmap_table_entry( + "set_bitmap", + &0u16.to_le_bytes(), + &port_bitmap(16, ports), + 0, + ); + + let expected: Vec<(u16, u128, u128)> = + ports.iter().map(|&port| (port, 1u128 << port, 1)).collect(); + let data = [0u8; HEADER_LEN]; + + let mut pkt = packet_in::new(&data); + let packets: Vec<_> = pipeline + .process_packet(0, &mut pkt) + .into_iter() + .map(|(packet, port)| { + assert_eq!(packet.header_data.len(), HEADER_LEN); + let read = |offset: usize| { + u128::from_be_bytes( + packet.header_data[offset..offset + 16].try_into().unwrap(), + ) + }; + (port, read(0), read(16)) + }) + .collect(); + + let mut pkt = packet_in::new(&data); + let headers: Vec<_> = pipeline + .process_packet_headers(0, &mut pkt) + .into_iter() + .map(|(headers, port)| { + ( + port, + headers.result.port_mask.load_le::(), + headers.result.unshifted.load_le::(), + ) + }) + .collect(); + + assert_eq!(packets, expected, "packet"); + assert_eq!(headers, expected, "headers"); +} + +#[test] +fn shift_by_egress_port() { + assert_egress_shifts(&[1, 2]); +} + +#[test] +fn shift_by_higher_egress_port() { + assert_egress_shifts(&[3]); +} + +mod counts { + use bitvec::field::BitField; + use p4rs::{packet_in, Pipeline}; + + p4_macro::use_p4!( + p4 = "test/src/p4/shift_counts.p4", + pipeline_name = "shift_counts", + ); + + const VALUE: u32 = 0x8123_4567; + const HEADER_LEN: usize = 50; + + struct ShiftResults { + literal: [u32; 2], + narrow: [u32; 2], + wide: [u32; 2], + large_literal: [u8; 2], + linear: [u8; 4], + arithmetic: [u8; 2], + grouped: [u8; 2], + } + + fn outputs( + pipeline: &mut main_pipeline, + count: u128, + ) -> [(&'static str, ShiftResults); 2] { + let mut data = [0u8; HEADER_LEN + 2]; + data[..16].copy_from_slice(&count.to_be_bytes()); + data[HEADER_LEN..].copy_from_slice(&[0xaa, 0xbb]); + + let mut pkt = packet_in::new(&data); + let packets = pipeline.process_packet(0, &mut pkt); + assert_eq!(packets.len(), 1, "count {count}"); + + let (packet, port) = &packets[0]; + assert_eq!(*port, 1); + assert_eq!(packet.header_data.len(), HEADER_LEN); + assert_eq!(&packet.header_data[..16], &data[..16]); + assert_eq!(packet.payload_data, &data[HEADER_LEN..]); + let read = |offset| { + u32::from_be_bytes( + packet.header_data[offset..offset + 4].try_into().unwrap(), + ) + }; + let packet_result = ShiftResults { + literal: [read(16), read(20)], + narrow: [read(24), read(28)], + wide: [read(32), read(36)], + large_literal: [packet.header_data[40], packet.header_data[41]], + linear: packet.header_data[42..46].try_into().unwrap(), + arithmetic: packet.header_data[46..48].try_into().unwrap(), + grouped: packet.header_data[48..50].try_into().unwrap(), + }; + + let mut pkt = packet_in::new(&data); + let mut headers = pipeline.process_packet_headers(0, &mut pkt); + assert_eq!(headers.len(), 1, "count {count}"); + let (headers, port) = headers.pop().unwrap(); + assert_eq!(port, 1); + assert_eq!(headers.result.count.load_le::(), count); + let header_result = ShiftResults { + literal: [ + headers.result.left_literal.load_le::(), + headers.result.right_literal.load_le::(), + ], + narrow: [ + headers.result.left_narrow.load_le::(), + headers.result.right_narrow.load_le::(), + ], + wide: [ + headers.result.left_wide.load_le::(), + headers.result.right_wide.load_le::(), + ], + large_literal: [ + headers.result.left_large_literal.load_le::(), + headers.result.right_large_literal.load_le::(), + ], + linear: [ + headers.result.left_linear.load_le::(), + headers.result.right_linear.load_le::(), + headers.result.left_right_linear.load_le::(), + headers.result.right_left_linear.load_le::(), + ], + arithmetic: [ + headers.result.add_then_shift.load_le::(), + headers.result.shift_by_sum.load_le::(), + ], + grouped: [ + headers.result.grouped_shift.load_le::(), + headers.result.grouped_comparison.load_le::(), + ], + }; + + [("packet", packet_result), ("headers", header_result)] + } + + #[test] + fn literal_counts() { + let mut pipeline = main_pipeline::new(2); + for (api, result) in outputs(&mut pipeline, 0) { + assert_eq!(result.literal, [0x0246_8ace, 0x4091_a2b3], "{api}"); + } + } + + #[test] + fn narrow_variable_counts() { + let mut pipeline = main_pipeline::new(2); + for count in [0u128, 1, 31, 32, 33, 128, 255, 256] { + let amount = (count & 0xff) as u32; + let expected = [ + VALUE.checked_shl(amount).unwrap_or(0), + VALUE.checked_shr(amount).unwrap_or(0), + ]; + for (api, result) in outputs(&mut pipeline, count) { + assert_eq!(result.narrow, expected, "{api}, count {count}"); + } + } + } + + #[test] + fn wide_variable_counts() { + let mut pipeline = main_pipeline::new(2); + for count in [0u128, 1, 31, 32, 33, 128, 256, 1u128 << 100, u128::MAX] { + let amount = u32::try_from(count).unwrap_or(u32::MAX); + let expected = [ + VALUE.checked_shl(amount).unwrap_or(0), + VALUE.checked_shr(amount).unwrap_or(0), + ]; + for (api, result) in outputs(&mut pipeline, count) { + assert_eq!(result.wide, expected, "{api}, count {count}"); + } + } + } + + #[test] + fn large_literal_counts() { + let mut pipeline = main_pipeline::new(2); + for (api, result) in outputs(&mut pipeline, 0) { + assert_eq!(result.large_literal, [0, 0], "{api}"); + } + } + + #[test] + fn linear_shifts() { + let mut pipeline = main_pipeline::new(2); + for (api, result) in outputs(&mut pipeline, 0) { + assert_eq!(result.linear, [8, 16, 1, 6], "{api}"); + } + } + + #[test] + fn arithmetic_prec() { + let mut pipeline = main_pipeline::new(2); + for (api, result) in outputs(&mut pipeline, 0) { + assert_eq!(result.arithmetic, [6, 4], "{api}"); + } + } + + #[test] + fn explicit_grouping() { + let mut pipeline = main_pipeline::new(2); + for (api, result) in outputs(&mut pipeline, 0) { + assert_eq!(result.grouped, [16, 1], "{api}"); + } + } +} diff --git a/test/src/slice_assign.rs b/test/src/slice_assign.rs new file mode 100644 index 00000000..0ed29927 --- /dev/null +++ b/test/src/slice_assign.rs @@ -0,0 +1,67 @@ +// Copyright 2026 Oxide Computer Company + +use crate::softnpu::{Interface4, SoftNpu}; + +p4_macro::use_p4!( + p4 = "test/src/p4/slice_assign.p4", + pipeline_name = "slice_assign", +); + +/// Verify bit-slice assignment derives a multicast MAC from ipv4.dst +/// per RFC 1112 section 6.4, using byte-aligned slices on the LHS. +#[test] +fn slice_assign_mcast_mac() -> Result<(), anyhow::Error> { + let pipeline = main_pipeline::new(2); + + let mut npu = SoftNpu::new(2, pipeline, false); + let phy0 = npu.phy(0); + let phy1 = npu.phy(1); + + let if0 = Interface4::new(phy0.clone(), "10.0.0.1".parse().unwrap()); + + npu.run(); + + // Use 239.129.2.3 so bit 23 of the IP (MSB of second byte = 0x81) + // is set, exercising the [23:23] = 0 clear. + if0.send(phy1.mac, "239.129.2.3".parse().unwrap(), b"test")?; + + let frames = phy1.recv(); + let frame = &frames[0]; + + // RFC 1112: 01:00:5e + lower 23 bits of dst IP. + // dst IP = 239.129.2.3, ipv4.dst[23:16] = 0x81. + // After clearing bit 23: 0x81 & 0x7f = 0x01. + // Expected MAC: 01:00:5e:01:02:03 + assert_eq!( + frame.dst, + [0x01, 0x00, 0x5e, 0x01, 0x02, 0x03], + "multicast MAC with bit 23 cleared" + ); + + // Same-field aliased assignment: ipv4.dst[3:0] = ipv4.dst[31:28]. + // dst IP = 0xEF810203, top nibble = 0xE. + // + // After assignment, the bottom nibble becomes 0xE and last byte = 0x0E. + let dst_ip = &frame.payload[16..20]; // ipv4.dst in the IPv4 header + assert_eq!( + dst_ip[3], 0x0E, + "same-field alias: bottom nibble should be top nibble (0xE)" + ); + + // Single-bit set: ethernet.src[0:0] = 1w1. + // Bit 0 is the LSB of the last byte of src MAC. + // The original src MAC's last byte gets bit 0 set. + assert_eq!( + frame.src[5] & 0x01, + 0x01, + "single-bit set: LSB of src MAC last byte" + ); + + assert_eq!( + &frame.payload[12..16], + &[0x0a, 0x05, 0xa0, 0x01], + "non-contiguous ipv4.src[19:12] = 0x5a on 10.0.0.1" + ); + + Ok(()) +} diff --git a/test/src/slice_assign_empty.rs b/test/src/slice_assign_empty.rs new file mode 100644 index 00000000..f79f1e49 --- /dev/null +++ b/test/src/slice_assign_empty.rs @@ -0,0 +1,91 @@ +use bitvec::field::BitField; +use p4rs::{packet_in, Pipeline}; + +p4_macro::use_p4!( + p4 = "test/src/p4/slice_assign_empty.p4", + pipeline_name = "slice_assign_empty", +); + +fn assert_outputs(ingress: u16, expected: &[(u16, u16)]) { + let mut pipeline = main_pipeline::new(4); + let data = []; + + let mut pkt = packet_in::new(&data); + let packets: Vec<_> = pipeline + .process_packet(ingress, &mut pkt) + .into_iter() + .map(|(packet, port)| { + assert_eq!(packet.header_data.len(), 2); + let value = + u16::from_be_bytes(packet.header_data[..2].try_into().unwrap()); + (port, value) + }) + .collect(); + + let mut pkt = packet_in::new(&data); + let headers: Vec<_> = pipeline + .process_packet_headers(ingress, &mut pkt) + .into_iter() + .map(|(headers, port)| (port, headers.result.value.load_le::())) + .collect(); + + assert_eq!(packets.as_slice(), expected); + assert_eq!(headers.as_slice(), expected); +} + +#[test] +fn full_width_egress_port() { + assert_outputs(0, &[(1, 0)]); +} + +// P4-16 8.25 leaves uninitialized locals unspecified, while SoftNPU zeroes +// them out. +#[test] +fn uninitialized_local_is_zero() { + assert_outputs(1, &[(1, 1)]); +} + +#[test] +fn noncontiguous_egress_port() { + assert_outputs(2, &[(2, 0)]); +} + +#[test] +fn ordered_comparison_is_numeric() { + assert_outputs(3, &[(1, 1)]); +} + +#[test] +fn int_literal_assignments_are_sized() { + assert_outputs(4, &[(1, 3)]); +} + +#[test] +fn int_literal_control_argument_is_sized() { + assert_outputs(5, &[(1, 7)]); +} + +#[test] +fn int_literal_slice_assignments_are_sized() { + assert_outputs(6, &[(1, 0x0020)]); +} + +#[test] +fn sliced_shift_operands_are_owned() { + assert_outputs(7, &[(1, 4)]); +} + +#[test] +fn bit_widening_zero_extends() { + assert_outputs(8, &[(1, 0x0105)]); +} + +#[test] +fn bit_widening_operands_are_owned() { + assert_outputs(9, &[(1, 0x01ab)]); +} + +#[test] +fn zero_width_int_literals_are_empty() { + assert_outputs(10, &[(1, 4)]); +} diff --git a/test/src/slice_read.rs b/test/src/slice_read.rs new file mode 100644 index 00000000..aea0f0f6 --- /dev/null +++ b/test/src/slice_read.rs @@ -0,0 +1,140 @@ +// Copyright 2026 Oxide Computer Company + +use bitvec::field::BitField; +use p4rs::{packet_in, Pipeline}; +use pnet::packet::ipv4::{Ipv4Packet, MutableIpv4Packet}; + +use crate::softnpu::{Interface4, SoftNpu}; + +p4_macro::use_p4!( + p4 = "test/src/p4/slice_read.p4", + pipeline_name = "slice_read", +); + +/// Read a sub-byte slice from a multi-byte field and verify the +/// byte-reversal mapping is correct. +#[test] +fn slice_read_top_nibble() -> Result<(), anyhow::Error> { + let pipeline = main_pipeline::new(2); + + let mut npu = SoftNpu::new(2, pipeline, false); + let phy0 = npu.phy(0); + let phy1 = npu.phy(1); + + let if0 = Interface4::new(phy0.clone(), "10.0.0.1".parse().unwrap()); + + npu.run(); + + // dst IP = 239.171.2.3 = 0xEFAB0203. + // ipv4.dst[23:20] = top nibble of 0xAB = 0xA. + if0.send(phy1.mac, "239.171.2.3".parse().unwrap(), b"test")?; + + let frames = phy1.recv(); + let frame = &frames[0]; + let ip = Ipv4Packet::new(&frame.payload).unwrap(); + + // The P4 compares ipv4.dst[23:20] == 0xA and sets identification=42 + // if true. With correct byte reversal the top nibble of 0xAB is 0xA, + // and that branch should be taken. An incorrect [20..24] range + // reads the bottom nibble (0xB) instead, so the comparison + // fails, and identification stays at 0. + assert_eq!( + ip.get_identification(), + 42, + "ipv4.dst[23:20] should be 0xA (top nibble of 0xAB)" + ); + + Ok(()) +} + +fn ipv4_frame(frag_offset: u16) -> [u8; 34] { + let mut buf = [0u8; 34]; + buf[..6].copy_from_slice(&[0x02, 0, 0, 0, 0, 1]); + buf[6..12].copy_from_slice(&[0x02, 0, 0, 0, 0, 0]); + buf[12..14].copy_from_slice(&0x0800u16.to_be_bytes()); + + let mut ip = MutableIpv4Packet::new(&mut buf[14..]).unwrap(); + ip.set_version(4); + ip.set_header_length(5); + ip.set_total_length(20); + ip.set_fragment_offset(frag_offset); + ip.set_ttl(0x35); + ip.set_source("10.0.0.1".parse().unwrap()); + ip.set_destination("239.171.2.3".parse().unwrap()); + + buf +} + +#[derive(Debug, PartialEq)] +struct SliceReads { + port: u16, + identification: u16, + diffserv: u8, + ttl: u8, + protocol: u8, + checksum: u16, +} + +fn slice_reads(frag_offset: u16) -> SliceReads { + let mut pipeline = main_pipeline::new(2); + let buf = ipv4_frame(frag_offset); + + let mut pkt = packet_in::new(&buf); + let packets: Vec<_> = pipeline + .process_packet(0, &mut pkt) + .into_iter() + .map(|(packet, port)| { + let ip = Ipv4Packet::new(&packet.header_data[14..]).unwrap(); + SliceReads { + port, + identification: ip.get_identification(), + diffserv: packet.header_data[15], + ttl: ip.get_ttl(), + protocol: ip.get_next_level_protocol().0, + checksum: ip.get_checksum(), + } + }) + .collect(); + + let mut pkt = packet_in::new(&buf); + let mut headers: Vec<_> = pipeline + .process_packet_headers(0, &mut pkt) + .into_iter() + .map(|(headers, port)| SliceReads { + port, + identification: headers.ipv4.identification.load_le(), + diffserv: headers.ipv4.diffserv.load_le(), + ttl: headers.ipv4.ttl.load_le(), + protocol: headers.ipv4.protocol.load_le(), + checksum: headers.ipv4.hdr_checksum.load_le(), + }) + .collect(); + + assert_eq!(packets, headers, "packet and header outputs differ"); + assert_eq!(headers.len(), 1, "packet should egress on port 1"); + headers.pop().unwrap() +} + +#[test] +fn slice_read_sub_byte_field() { + let reads = slice_reads(0); + assert_eq!(reads.port, 1); + assert_eq!( + reads.protocol, 0x5b, + "ipv4.ttl[3:0] should read 0x5, the low nibble of 0x35" + ); + assert_eq!(reads.ttl, 0x35); +} + +#[test] +fn slice_read_non_contiguous() { + let reads = slice_reads(0x0ab5); + assert_eq!( + reads.diffserv, 0xb0, + "ipv4.dst[19:12] of 0xEFAB0203 should read 0xb0" + ); + assert_eq!( + reads.checksum, 0xab, + "ipv4.frag_offset[11:4] of 0x0ab5 should read 0xab" + ); +} diff --git a/test/src/softnpu.rs b/test/src/softnpu.rs index 3022f964..7967d736 100644 --- a/test/src/softnpu.rs +++ b/test/src/softnpu.rs @@ -6,8 +6,12 @@ use std::net::{Ipv4Addr, Ipv6Addr}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use std::thread::spawn; +use std::time::{Duration, Instant}; use xfr::{ring, FrameBuffer, RingConsumer, RingProducer}; +/// Upper bound on waiting for expected frames before failing a test. +pub const EXPECT_FRAMES_TIMEOUT: Duration = Duration::from_secs(10); + pub fn do_expect_frames( name: &str, phy: &Arc>, @@ -16,15 +20,25 @@ pub fn do_expect_frames( ) { let n = expected.len(); let mut frames = Vec::new(); - loop { - let fs = phy.recv(); - frames.extend_from_slice(&fs); - // TODO this is not a great interface, if frames.len() > n, we should do - // something besides hang forever. - if frames.len() == n { - break; + // `OuterPhy::recv` blocks until at least one frame arrives; we enforce + // the deadline by polling `recv_buffer_len` instead. A frame sent + // to the wrong port then fails the test vs just hanging. + let deadline = Instant::now() + EXPECT_FRAMES_TIMEOUT; + while frames.len() < n { + if phy.recv_buffer_len() > 0 { + frames.extend_from_slice(&phy.recv()); + } else if Instant::now() >= deadline { + panic!( + "[{name}] expected {n} frames, received {} within {:?}", + frames.len(), + EXPECT_FRAMES_TIMEOUT, + ); + } else { + std::thread::sleep(Duration::from_millis(1)); } } + // A single `recv` can return more frames than expected. + assert_eq!(frames.len(), n, "[{name}] frame count"); for i in 0..n { let payload = match frames[i].ethertype { 0x0901 => { @@ -50,6 +64,7 @@ pub fn do_expect_frames( assert_eq!(frames[i].dst, d, "dst"); } assert_eq!(frames[i].ethertype, expected[i].ethertype, "ethertype"); + assert_eq!(frames[i].vid, expected[i].vid, "vid"); assert_eq!(payload, expected[i].payload, "payload"); } } diff --git a/x4c/src/lib.rs b/x4c/src/lib.rs index 62457027..9be10109 100644 --- a/x4c/src/lib.rs +++ b/x4c/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use anyhow::{anyhow, Result}; use clap::Parser;