Skip to content

Commit 079b582

Browse files
P4 cleanup and a return to 18 stages
1 parent 44a949c commit 079b582

File tree

28 files changed

+799
-582
lines changed

28 files changed

+799
-582
lines changed

.github/buildomat/common.sh

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
#!/bin/bash
22

3-
# The tofino2 has 20 stages, and the current sidecar.p4 needs all 20 of them.
4-
# Specifying the number of stages isn't strictly necessary, but it allows us to
5-
# track when we exceed the current ceiling. The underlying intention is to grow
6-
# deliberately and thoughtfully, given the limited space on the ASIC.
7-
#
8-
# Note: this now seems silly since we have maxed out the number of stages, but
9-
# we want to leave this check and note in place should we ever find a way to
10-
# reduce our footprint below 20 stages.
11-
TOFINO_STAGES=20
3+
# The tofino2 has 20 stages. The base sidecar.p4 needs 15 stages, and with
4+
# multicast enabled it needs 18. Specifying the number of stages isn't
5+
# strictly necessary, but it allows us to track when we exceed the current
6+
# ceiling. The underlying intention is to grow deliberately and thoughtfully,
7+
# given the limited space on the ASIC.
8+
TOFINO_STAGES=15
129

1310
# These describe which version of the SDE to download and where to find it
1411
SDE_COMMIT=2a6b33211c9675996dcb99fe939045506667ae94

.github/buildomat/packet-test-common.sh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@ if [ x$MULTICAST == x ]; then
1717
CODEGEN_FEATURES=--multicast
1818
SWADM_FEATURES=--features=multicast
1919
fi
20-
20+
21+
if [ x$MULTICAST == x ]; then
22+
TOFINO_STAGES=15
23+
else
24+
TOFINO_STAGES=18
25+
fi
26+
2127
function cleanup {
2228
set +o errexit
2329
set +o pipefail

asic/src/softnpu/table.rs

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,26 @@ impl TableOps<Handle> for Table {
112112
trace!(hdl.log, "match_data:\n{:#?}", match_data);
113113
trace!(hdl.log, "action_data:\n{:#?}", action_data);
114114

115+
// Route tables are idx-only in sidecar-lite and route_ttl_is_1
116+
// is ignored here.
117+
//
118+
// TODO: remove compat once https://github.com/oxidecomputer/sidecar-lite/pull/152
119+
// is merged and sidecar-lite updates route keys/actions accordingly.
120+
let is_route_table = matches!(
121+
self.type_,
122+
TableType::RouteFwdIpv4 | TableType::RouteFwdIpv6
123+
);
124+
if is_route_table {
125+
if route_ttl_is_1(&match_data.fields) {
126+
trace!(hdl.log, "skipping ttl==1 route entry for {name}");
127+
return Ok(());
128+
}
129+
if action_data.action == "ttl_exceeded" {
130+
trace!(hdl.log, "skipping ttl_exceeded action for {name}");
131+
return Ok(());
132+
}
133+
}
134+
115135
let keyset_data = keyset_data(match_data.fields, self.type_);
116136

117137
let (action, parameter_data) = match (
@@ -428,6 +448,22 @@ impl TableOps<Handle> for Table {
428448
}
429449
("rewrite", params)
430450
}
451+
#[cfg(feature = "multicast")]
452+
(TableType::PortMacAddressMcast, "rewrite") => {
453+
let mut params = Vec::new();
454+
for arg in action_data.args {
455+
match arg.value {
456+
ValueTypes::U64(v) => {
457+
let mac = v.to_le_bytes();
458+
params.extend_from_slice(&mac[0..6]);
459+
}
460+
ValueTypes::Ptr(v) => {
461+
params.extend_from_slice(v.as_slice());
462+
}
463+
}
464+
}
465+
("rewrite", params)
466+
}
431467
(TableType::NatIngressIpv4, "forward_ipv4_to")
432468
| (TableType::NatIngressIpv6, "forward_ipv6_to")
433469
| (TableType::AttachedSubnetIpv4, "forward_to_v4")
@@ -573,6 +609,15 @@ impl TableOps<Handle> for Table {
573609
trace!(hdl.log, "table: {name}");
574610
trace!(hdl.log, "match_data:\n{:#?}", match_data);
575611

612+
let is_route_table = matches!(
613+
self.type_,
614+
TableType::RouteFwdIpv4 | TableType::RouteFwdIpv6
615+
);
616+
if is_route_table && route_ttl_is_1(&match_data.fields) {
617+
trace!(hdl.log, "skipping ttl==1 route entry delete for {name}");
618+
return Ok(());
619+
}
620+
576621
let keyset_data = keyset_data(match_data.fields, self.type_);
577622

578623
trace!(hdl.log, "sending request to softnpu");
@@ -632,10 +677,12 @@ fn keyset_data(match_data: Vec<MatchEntryField>, table: TableType) -> Vec<u8> {
632677
serialize_value_type(&x, &mut data);
633678
keyset_data.extend_from_slice(&data[..2]);
634679
}
635-
TableType::RouteIdxIpv4 => {
636-
// "idx" => exact => bit<16>
637-
serialize_value_type(&x, &mut data);
638-
keyset_data.extend_from_slice(&data[..2]);
680+
TableType::RouteFwdIpv4 | TableType::RouteFwdIpv6 => {
681+
// sidecar-lite route keys are idx-only.
682+
if m.name == "idx" {
683+
serialize_value_type(&x, &mut data);
684+
keyset_data.extend_from_slice(&data[..2]);
685+
}
639686
}
640687
TableType::NatIngressIpv4 => {
641688
// "dst_addr" => hdr.ipv4.dst: exact => bit<32>
@@ -725,3 +772,18 @@ fn serialize_value_type_be(x: &ValueTypes, data: &mut Vec<u8>) {
725772
}
726773
}
727774
}
775+
776+
fn route_ttl_is_1(fields: &[MatchEntryField]) -> bool {
777+
fields.iter().any(|field| {
778+
if field.name != "route_ttl_is_1" {
779+
return false;
780+
}
781+
match &field.value {
782+
MatchEntryValue::Value(ValueTypes::U64(v)) => *v != 0,
783+
MatchEntryValue::Value(ValueTypes::Ptr(v)) => {
784+
v.first().is_some_and(|b| *b != 0)
785+
}
786+
_ => false,
787+
}
788+
})
789+
}

asic/src/tofino_common/mod.rs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,16 @@ pub mod ports;
2323
fn table_name(type_: TableType) -> &'static str {
2424
match type_ {
2525
TableType::RouteIdxIpv4 => {
26-
"pipe.Ingress.l3_router.Router4.lookup_idx.lookup"
26+
"pipe.Ingress.l3_router.router4.lookup_idx.lookup"
2727
}
2828
TableType::RouteFwdIpv4 => {
29-
"pipe.Ingress.l3_router.Router4.lookup_idx.route"
29+
"pipe.Ingress.l3_router.router4.lookup_idx.route"
3030
}
3131
TableType::RouteIdxIpv6 => {
32-
"pipe.Ingress.l3_router.Router6.lookup_idx.lookup"
32+
"pipe.Ingress.l3_router.router6.lookup_idx.lookup"
3333
}
3434
TableType::RouteFwdIpv6 => {
35-
"pipe.Ingress.l3_router.Router6.lookup_idx.route"
35+
"pipe.Ingress.l3_router.router6.lookup_idx.route"
3636
}
3737
#[cfg(feature = "multicast")]
3838
TableType::RouteIpv4Mcast => {
@@ -44,13 +44,15 @@ fn table_name(type_: TableType) -> &'static str {
4444
}
4545
TableType::ArpIpv4 => "pipe.Ingress.l3_router.Arp.tbl",
4646
TableType::NeighborIpv6 => "pipe.Ingress.l3_router.Ndp.tbl",
47-
TableType::PortMacAddress => "pipe.Ingress.mac_rewrite.mac_rewrite",
47+
TableType::PortMacAddress => {
48+
"pipe.Egress.unicast_mac_rewrite.mac_rewrite"
49+
}
4850
TableType::PortAddrIpv4 => "pipe.Ingress.filter.switch_ipv4_addr",
4951
TableType::PortAddrIpv6 => "pipe.Ingress.filter.switch_ipv6_addr",
5052
TableType::NatIngressIpv4 => "pipe.Ingress.nat_ingress.ingress_ipv4",
5153
TableType::NatIngressIpv6 => "pipe.Ingress.nat_ingress.ingress_ipv6",
5254
TableType::UplinkIngress => "pipe.Ingress.filter.uplink_ports",
53-
TableType::UplinkEgress => "pipe.Ingress.egress_filter.egress_filter",
55+
TableType::UplinkEgress => "pipe.Egress.egress_filter.egress_filter",
5456
TableType::AttachedSubnetIpv4 => {
5557
"pipe.Ingress.attached_subnet_ingress.attached_subnets_v4"
5658
}
@@ -78,7 +80,9 @@ fn table_name(type_: TableType) -> &'static str {
7880
"pipe.Ingress.nat_ingress.ingress_ipv6_mcast"
7981
}
8082
#[cfg(feature = "multicast")]
81-
TableType::PortMacAddressMcast => "pipe.Egress.mac_rewrite.mac_rewrite",
83+
TableType::PortMacAddressMcast => {
84+
"pipe.Egress.mcast_mac_rewrite.mac_rewrite"
85+
}
8286
#[cfg(feature = "multicast")]
8387
TableType::McastEgressDecapPorts => {
8488
"pipe.Egress.mcast_egress.tbl_decap_ports"
@@ -96,9 +100,13 @@ fn counter_table_name(id: CounterId) -> &'static str {
96100
CounterId::Service => "pipe.Ingress.services.service_ctr",
97101
CounterId::Ingress => "pipe.Ingress.ingress_ctr",
98102
CounterId::Packet => "pipe.Ingress.packet_ctr",
99-
CounterId::Egress => "pipe.Ingress.egress_ctr",
100103
CounterId::DropPort => "pipe.Ingress.drop_port_ctr",
101104
CounterId::DropReason => "pipe.Ingress.drop_reason_ctr",
105+
CounterId::Forwarded => "pipe.Egress.forwarded_ctr",
106+
CounterId::Unicast => "pipe.Egress.unicast_ctr",
107+
CounterId::MulticastLL => "pipe.Egress.link_local_mcast_ctr",
108+
CounterId::EgressDropPort => "pipe.Egress.drop_port_ctr",
109+
CounterId::EgressDropReason => "pipe.Egress.drop_reason_ctr",
102110
#[cfg(feature = "multicast")]
103111
CounterId::Multicast(id) => mulitcast_counter_table_name(id),
104112
}
@@ -107,16 +115,9 @@ fn counter_table_name(id: CounterId) -> &'static str {
107115
#[cfg(feature = "multicast")]
108116
fn mulitcast_counter_table_name(id: MulticastCounterId) -> &'static str {
109117
match id {
110-
MulticastCounterId::EgressDropPort => "pipe.Egress.drop_port_ctr",
111-
MulticastCounterId::EgressDropReason => "pipe.Egress.drop_reason_ctr",
112-
MulticastCounterId::Unicast => "pipe.Egress.unicast_ctr",
113118
MulticastCounterId::Multicast => "pipe.Egress.mcast_ctr",
114119
MulticastCounterId::MulticastExt => "pipe.Egress.external_mcast_ctr",
115-
MulticastCounterId::MulticastLL => "pipe.Egress.link_local_mcast_ctr",
116120
MulticastCounterId::MulticastUL => "pipe.Egress.underlay_mcast_ctr",
117-
MulticastCounterId::MulticastDrop => {
118-
"pipe.Ingress.filter.drop_mcast_ctr"
119-
}
120121
}
121122
}
122123

common/src/counters.rs

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,16 @@ pub struct FecRSCounters {
205205
pub enum CounterId {
206206
Service,
207207
Ingress,
208-
Egress,
209208
Packet,
210209
DropPort,
211210
DropReason,
211+
Forwarded,
212+
Unicast,
213+
/// Link-local IPv6 multicast (ff02::/16). Not feature-gated because
214+
/// link-local forwarding uses standard routing, not replication groups.
215+
MulticastLL,
216+
EgressDropPort,
217+
EgressDropReason,
212218
#[cfg(feature = "multicast")]
213219
Multicast(MulticastCounterId),
214220
}
@@ -227,14 +233,9 @@ pub enum CounterId {
227233
)]
228234
#[cfg(feature = "multicast")]
229235
pub enum MulticastCounterId {
230-
EgressDropPort,
231-
EgressDropReason,
232-
Unicast,
233236
Multicast,
234237
MulticastExt,
235-
MulticastLL,
236238
MulticastUL,
237-
MulticastDrop,
238239
}
239240

240241
impl fmt::Display for CounterId {
@@ -245,10 +246,14 @@ impl fmt::Display for CounterId {
245246
match self {
246247
CounterId::Service => "Service".to_string(),
247248
CounterId::Ingress => "Ingress".to_string(),
248-
CounterId::Egress => "Egress".to_string(),
249249
CounterId::Packet => "Packet".to_string(),
250250
CounterId::DropPort => "Ingress_Drop_Port".to_string(),
251251
CounterId::DropReason => "Ingress_Drop_Reason".to_string(),
252+
CounterId::Forwarded => "Forwarded".to_string(),
253+
CounterId::Unicast => "Unicast".to_string(),
254+
CounterId::MulticastLL => "Multicast_Link_Local".to_string(),
255+
CounterId::EgressDropPort => "Egress_Drop_Port".to_string(),
256+
CounterId::EgressDropReason => "Egress_Drop_Reason".to_string(),
252257
#[cfg(feature = "multicast")]
253258
CounterId::Multicast(id) => id.to_string(),
254259
}
@@ -263,58 +268,43 @@ impl std::str::FromStr for CounterId {
263268
match s.to_lowercase().replace(['_'], "").as_str() {
264269
"service" => Ok(CounterId::Service),
265270
"ingress" => Ok(CounterId::Ingress),
266-
"egress" => Ok(CounterId::Egress),
267271
"packet" => Ok(CounterId::Packet),
268272
"ingressdropport" => Ok(CounterId::DropPort),
269273
"ingressdropreason" => Ok(CounterId::DropReason),
274+
"forwarded" => Ok(CounterId::Forwarded),
275+
"unicast" => Ok(CounterId::Unicast),
276+
"multicastll" | "multicastlinklocal" => Ok(CounterId::MulticastLL),
277+
"egressdropport" => Ok(CounterId::EgressDropPort),
278+
"egressdropreason" => Ok(CounterId::EgressDropReason),
270279
#[cfg(feature = "multicast")]
271280
x => match x {
272-
"egressdropport" => {
273-
Ok(CounterId::Multicast(MulticastCounterId::EgressDropPort))
274-
}
275-
"egressdropreason" => Ok(CounterId::Multicast(
276-
MulticastCounterId::EgressDropReason,
277-
)),
278-
"unicast" => {
279-
Ok(CounterId::Multicast(MulticastCounterId::Unicast))
280-
}
281281
"multicast" => {
282282
Ok(CounterId::Multicast(MulticastCounterId::Multicast))
283283
}
284284
"multicastext" | "multicastexternal" => {
285285
Ok(CounterId::Multicast(MulticastCounterId::MulticastExt))
286286
}
287-
"multicastll" | "multicastlinklocal" => {
288-
Ok(CounterId::Multicast(MulticastCounterId::MulticastLL))
289-
}
290287
"multicastul" | "multicastunderlay" => {
291288
Ok(CounterId::Multicast(MulticastCounterId::MulticastUL))
292289
}
293-
"multicastdrop" => {
294-
Ok(CounterId::Multicast(MulticastCounterId::MulticastDrop))
295-
}
296290
x => Err(format!("No such counter: {x}")),
297291
},
298292
#[cfg(not(feature = "multicast"))]
299293
x => Err(format!("No such counter: {x}")),
300294
}
301295
}
302296
}
297+
303298
#[cfg(feature = "multicast")]
304299
impl fmt::Display for MulticastCounterId {
305300
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
306301
write!(
307302
f,
308303
"{}",
309304
match self {
310-
MulticastCounterId::EgressDropPort => "Egress_Drop_Port",
311-
MulticastCounterId::EgressDropReason => "Egress_Drop_Reason",
312-
MulticastCounterId::Unicast => "Unicast",
313305
MulticastCounterId::Multicast => "Multicast",
314306
MulticastCounterId::MulticastExt => "Multicast_External",
315-
MulticastCounterId::MulticastLL => "Multicast_Link_Local",
316307
MulticastCounterId::MulticastUL => "Multicast_Underlay",
317-
MulticastCounterId::MulticastDrop => "Multicast_Drop",
318308
}
319309
)
320310
}

common/src/illumos.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/
44
//
5-
// Copyright 2025 Oxide Computer Company
5+
// Copyright 2026 Oxide Computer Company
66

77
//! Illumos-specific common modules and operations.
88

dpd-client/tests/integration_tests/mcast.rs

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,14 +1710,6 @@ async fn test_ipv6_multicast_invalid_destination_mac() -> TestResult {
17101710
let ctr_baseline =
17111711
switch.get_counter("multicast_invalid_mac", None).await.unwrap();
17121712

1713-
let port_label_ingress = switch.port_label(ingress).unwrap();
1714-
1715-
// Check the Multicast_Drop counter baseline for the ingress port
1716-
let drop_mcast_baseline = switch
1717-
.get_counter(&port_label_ingress, Some("multicast_drop"))
1718-
.await
1719-
.unwrap();
1720-
17211713
switch.packet_test(vec![test_pkt], expected_pkts).unwrap();
17221714

17231715
check_counter_incremented(
@@ -1730,17 +1722,6 @@ async fn test_ipv6_multicast_invalid_destination_mac() -> TestResult {
17301722
.await
17311723
.unwrap();
17321724

1733-
// Verify that the Multicast_Drop counter also incremented
1734-
check_counter_incremented(
1735-
switch,
1736-
&port_label_ingress,
1737-
drop_mcast_baseline,
1738-
1,
1739-
Some("multicast_drop"),
1740-
)
1741-
.await
1742-
.unwrap();
1743-
17441725
cleanup_test_group(switch, get_group_ip(&created_group), TEST_TAG).await
17451726
}
17461727

dpd-client/tests/integration_tests/table_tests.rs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,7 @@ use crate::integration_tests::common::prelude::*;
4242
// investigating. If it only changes by an entry or two, it's fine to just
4343
// adjust the constant below to match the observed result.
4444
//
45-
// TODO: Multicast drops IPv4 LPM capacity to 7164 (from 8187) due to
46-
// ingress TCAM pressure. Investigate moving MulticastRouter4/6 into the
47-
// egress pipeline to reclaim capacity.
48-
#[cfg(feature = "multicast")]
49-
const IPV4_LPM_SIZE: usize = 7164; // ipv4 forwarding table
50-
#[cfg(not(feature = "multicast"))]
51-
const IPV4_LPM_SIZE: usize = 8187; // ipv4 forwarding table
52-
53-
#[cfg(feature = "multicast")]
54-
const IPV6_LPM_SIZE: usize = 1023; // ipv6 forwarding table
55-
#[cfg(not(feature = "multicast"))]
45+
const IPV4_LPM_SIZE: usize = 8191; // ipv4 forwarding table
5646
const IPV6_LPM_SIZE: usize = 1023; // ipv6 forwarding table
5747

5848
const SWITCH_IPV4_ADDRS_SIZE: usize = 511; // ipv4 addrs assigned to our ports

0 commit comments

Comments
 (0)