Skip to content

Commit 3254541

Browse files
P4 cleanup and a return to 18 stages
1 parent e8a6811 commit 3254541

File tree

30 files changed

+859
-627
lines changed

30 files changed

+859
-627
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. With multicast as a default feature, sidecar.p4
4+
# needs 18 stages. Specifying the number of stages isn't strictly necessary,
5+
# but it allows us to track when we exceed the current ceiling. The underlying
6+
# intention is to grow deliberately and thoughtfully, given the limited space
7+
# on the ASIC.
8+
TOFINO_STAGES=18
129

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

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

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,16 @@ export RUST_BACKTRACE=1
33
source .github/buildomat/common.sh
44
source .github/buildomat/linux.sh
55

6-
wd=`pwd`
6+
wd=$(pwd)
77
export WS=$wd
88
MODEL_STARTUP_TIMEOUT=${MODEL_STARTUP_TIMEOUT:=5}
99
STARTUP_TIMEOUT=${STARTUP_TIMEOUT:=120}
1010

1111
BUILD_FEATURES=tofino_asic
1212

1313
CODEGEN_FEATURES=--multicast
14-
SWADM_FEATURES="--features=multicast"
1514

15+
TOFINO_STAGES=18
1616
function cleanup {
1717
set +o errexit
1818
set +o pipefail
@@ -62,29 +62,33 @@ fi
6262

6363
banner "Test"
6464
sudo -E ./tools/veth_setup.sh
65-
id=`id -un`
66-
gr=`id -gn`
65+
id=$(id -un)
66+
gr=$(id -gn)
6767
sudo -E mkdir -p /work
6868
sudo -E chown $id:$gr /work
69-
sudo -E ./tools/run_tofino_model.sh &> /work/simulator.log &
69+
sudo -E ./tools/run_tofino_model.sh &>/work/simulator.log </dev/null &
70+
stty sane 2>/dev/null || true
7071
sleep $MODEL_STARTUP_TIMEOUT
71-
sudo -E ./tools/run_dpd.sh -m 127.0.0.1 &> /work/dpd.log &
72+
sudo -E ./tools/run_dpd.sh -m 127.0.0.1 &>/work/dpd.log </dev/null &
73+
stty sane 2>/dev/null || true
7274
echo "waiting for dpd to come online"
7375
set +o errexit
7476

7577
SLEEP_TIME=5
76-
iters=$(( $STARTUP_TIMEOUT / $SLEEP_TIME ))
77-
while [ 1 ] ; do
78-
./target/debug/swadm --host '[::1]' build-info 2> /dev/null
79-
if [ $? == 0 ]; then
80-
break
81-
fi
82-
iters=$(($iters - 1))
83-
if [ $iters = 0 ]; then
84-
echo "dpd failed to come online in $STARTUP_TIMEOUT seconds"
85-
exit 1
86-
fi
87-
sleep $SLEEP_TIME
78+
iters=$(($STARTUP_TIMEOUT / $SLEEP_TIME))
79+
while [ 1 ]; do
80+
./target/debug/swadm --host '[::1]' build-info 2>/dev/null
81+
rc=$?
82+
stty sane 2>/dev/null || true
83+
if [ $rc == 0 ]; then
84+
break
85+
fi
86+
iters=$(($iters - 1))
87+
if [ $iters = 0 ]; then
88+
echo "dpd failed to come online in $STARTUP_TIMEOUT seconds"
89+
exit 1
90+
fi
91+
sleep $SLEEP_TIME
8892
done
8993
set -o errexit
9094

@@ -100,7 +104,6 @@ DENDRITE_TEST_HOST='[::1]' \
100104
DENDRITE_TEST_VERBOSITY=3 \
101105
cargo test \
102106
--no-fail-fast \
103-
$SWADM_FEATURES \
104107
--test \
105108
counters \
106109
-- \

asic/src/chaos/table.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub const ROUTE_IPV4: &str = "pipe.Ingress.l3_router.routes_ipv4";
2020
pub const ROUTE_IPV6: &str = "pipe.Ingress.l3_router.routes_ipv6";
2121
pub const ARP_IPV4: &str = "pipe.Ingress.l3_router.arp_ipv4";
2222
pub const NEIGHBOR_IPV6: &str = "pipe.Ingress.l3_router.neighbor_ipv6";
23-
pub const MAC_REWRITE: &str = "pipe.Ingress.mac_rewrite.mac_rewrite";
23+
pub const MAC_REWRITE: &str = "pipe.Egress.unicast_mac_rewrite.mac_rewrite";
2424
pub const SWITCH_IPV4_ADDR: &str = "pipe.Ingress.filter.switch_ipv4_addr";
2525
pub const SWITCH_IPV6_ADDR: &str = "pipe.Ingress.filter.switch_ipv6_addr";
2626
pub const NAT_INGRESS_IPV4: &str = "pipe.Ingress.nat_ingress.ingress_ipv4";
@@ -42,7 +42,7 @@ pub(crate) const MCAST_ROUTE_IPV4: &str =
4242
pub(crate) const MCAST_ROUTE_IPV6: &str =
4343
"pipe.Ingress.l3_router.MulticastRouter6.tbl";
4444
pub(crate) const MCAST_MAC_REWRITE: &str =
45-
"pipe.Egress.mac_rewrite.mac_rewrite";
45+
"pipe.Egress.mcast_mac_rewrite.mac_rewrite";
4646
pub(crate) const MCAST_DECAP_PORTS: &str =
4747
"pipe.Egress.mcast_egress.tbl_decap_ports";
4848
pub(crate) const MCAST_PORT_ID_MAPPING: &str =

asic/src/softnpu/table.rs

Lines changed: 101 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ pub struct Table {
2323
}
2424

2525
// soft-npu table names
26+
// Route tables are idx-only in sidecar-lite; route_ttl_is_1 is ignored here.
27+
// TODO: remove compat once sidecar-lite updates route keys/actions:
28+
// - p4/sidecar-lite.p4: add route_ttl_is_1 to route table keys and add
29+
// ttl_exceeded actions; set ingress.route_ttl_is_1 in router.
30+
// - p4/softnpu.p4: add route_ttl_is_1 to ingress metadata.
31+
// - scadm + softnpu tests: encode/decode idx + ttl in route keys.
2632
const ROUTER_V4_RT: &str = "ingress.router.v4_route.rtr";
2733
const ROUTER_V4_IDX: &str = "ingress.router.v4_idx.rtr";
2834
const ROUTER_V6_RT: &str = "ingress.router.v6_route.rtr";
@@ -44,16 +50,19 @@ const _PROXY_ARP: &str = "ingress.pxarp.proxy_arp";
4450
const SWITCH_ADDR4: &str = "pipe.Ingress.filter.switch_ipv4_addr";
4551
const SWITCH_ADDR6: &str = "pipe.Ingress.filter.switch_ipv6_addr";
4652
const ROUTER4_LOOKUP_RT: &str =
47-
"pipe.Ingress.l3_router.Router4.lookup_idx.route";
53+
"pipe.Ingress.l3_router.router4.lookup_idx.route";
4854
const ROUTER4_LOOKUP_IDX: &str =
49-
"pipe.Ingress.l3_router.Router4.lookup_idx.lookup";
55+
"pipe.Ingress.l3_router.router4.lookup_idx.lookup";
5056
const ROUTER6_LOOKUP_RT: &str =
51-
"pipe.Ingress.l3_router.Router6.lookup_idx.route";
57+
"pipe.Ingress.l3_router.router6.lookup_idx.route";
5258
const ROUTER6_LOOKUP_IDX: &str =
53-
"pipe.Ingress.l3_router.Router6.lookup_idx.lookup";
59+
"pipe.Ingress.l3_router.router6.lookup_idx.lookup";
5460
const NDP: &str = "pipe.Ingress.l3_router.Ndp.tbl";
5561
const ARP: &str = "pipe.Ingress.l3_router.Arp.tbl";
56-
const DPD_MAC_REWRITE: &str = "pipe.Ingress.mac_rewrite.mac_rewrite";
62+
const DPD_UNICAST_MAC_REWRITE: &str =
63+
"pipe.Egress.unicast_mac_rewrite.mac_rewrite";
64+
#[cfg(feature = "multicast")]
65+
const DPD_MCAST_MAC_REWRITE: &str = "pipe.Egress.mcast_mac_rewrite.mac_rewrite";
5766
const NAT_INGRESS4: &str = "pipe.Ingress.nat_ingress.ingress_ipv4";
5867
const NAT_INGRESS6: &str = "pipe.Ingress.nat_ingress.ingress_ipv6";
5968
const ATTACHED_SUBNET_INGRESS4: &str =
@@ -85,8 +94,12 @@ impl TableOps<Handle> for Table {
8594
SWITCH_ADDR6 => (Some(LOCAL_V6.into()), Some(SWITCH_ADDR6.into())),
8695
NDP => (Some(RESOLVER_V6.into()), Some(NDP.into())),
8796
ARP => (Some(RESOLVER_V4.into()), Some(ARP.into())),
88-
DPD_MAC_REWRITE => {
89-
(Some(MAC_REWRITE.into()), Some(DPD_MAC_REWRITE.into()))
97+
DPD_UNICAST_MAC_REWRITE => {
98+
(Some(MAC_REWRITE.into()), Some(DPD_UNICAST_MAC_REWRITE.into()))
99+
}
100+
#[cfg(feature = "multicast")]
101+
DPD_MCAST_MAC_REWRITE => {
102+
(Some(MAC_REWRITE.into()), Some(DPD_MCAST_MAC_REWRITE.into()))
90103
}
91104
NAT_INGRESS4 => (Some(NAT_V4.into()), Some(NAT_INGRESS4.into())),
92105
NAT_INGRESS6 => (Some(NAT_V6.into()), Some(NAT_INGRESS6.into())),
@@ -135,9 +148,22 @@ impl TableOps<Handle> for Table {
135148
let action_data = data.action_to_ir().unwrap();
136149

137150
trace!(hdl.log, "entry_add called");
138-
trace!(hdl.log, "table: {}", table);
139-
trace!(hdl.log, "match_data:\n{:#?}", match_data);
140-
trace!(hdl.log, "action_data:\n{:#?}", action_data);
151+
trace!(hdl.log, "table: {table}");
152+
trace!(hdl.log, "match_data:\n{match_data:#?}");
153+
trace!(hdl.log, "action_data:\n{action_data:#?}");
154+
155+
let is_route_table =
156+
matches!(dpd_table.as_str(), ROUTER4_LOOKUP_RT | ROUTER6_LOOKUP_RT);
157+
if is_route_table {
158+
if route_ttl_is_1(&match_data.fields) {
159+
trace!(hdl.log, "skipping ttl==1 route entry for {dpd_table}");
160+
return Ok(());
161+
}
162+
if action_data.action == "ttl_exceeded" {
163+
trace!(hdl.log, "skipping ttl_exceeded action for {dpd_table}");
164+
return Ok(());
165+
}
166+
}
141167

142168
let keyset_data = keyset_data(match_data.fields, &table);
143169

@@ -440,7 +466,23 @@ impl TableOps<Handle> for Table {
440466
}
441467
("rewrite_dst", params)
442468
}
443-
(DPD_MAC_REWRITE, "rewrite") => {
469+
(DPD_UNICAST_MAC_REWRITE, "rewrite") => {
470+
let mut params = Vec::new();
471+
for arg in action_data.args {
472+
match arg.value {
473+
ValueTypes::U64(v) => {
474+
let mac = v.to_le_bytes();
475+
params.extend_from_slice(&mac[0..6]);
476+
}
477+
ValueTypes::Ptr(v) => {
478+
params.extend_from_slice(v.as_slice());
479+
}
480+
}
481+
}
482+
("rewrite", params)
483+
}
484+
#[cfg(feature = "multicast")]
485+
(DPD_MCAST_MAC_REWRITE, "rewrite") => {
444486
let mut params = Vec::new();
445487
for arg in action_data.args {
446488
match arg.value {
@@ -545,10 +587,10 @@ impl TableOps<Handle> for Table {
545587
};
546588
let action = action.to_string();
547589
trace!(hdl.log, "sending request to softnpu");
548-
trace!(hdl.log, "table: {}", table);
549-
trace!(hdl.log, "action: {:#?}", action);
550-
trace!(hdl.log, "keyset_data:\n{:#?}", keyset_data);
551-
trace!(hdl.log, "parameter_data:\n{:#?}", parameter_data);
590+
trace!(hdl.log, "table: {table}");
591+
trace!(hdl.log, "action: {action:#?}");
592+
trace!(hdl.log, "keyset_data:\n{keyset_data:#?}");
593+
trace!(hdl.log, "parameter_data:\n{parameter_data:#?}");
552594

553595
let msg = ManagementRequest::TableAdd(TableAdd {
554596
table,
@@ -576,9 +618,9 @@ impl TableOps<Handle> for Table {
576618
let action_data = data.action_to_ir().unwrap();
577619

578620
trace!(hdl.log, "entry_update called");
579-
trace!(hdl.log, "table: {}", table);
580-
trace!(hdl.log, "match_data:\n{:#?}", match_data);
581-
trace!(hdl.log, "action_data:\n{:#?}", action_data);
621+
trace!(hdl.log, "table: {table}");
622+
trace!(hdl.log, "match_data:\n{match_data:#?}");
623+
trace!(hdl.log, "action_data:\n{action_data:#?}");
582624

583625
//TODO implement in softnpu
584626
Ok(())
@@ -593,17 +635,31 @@ impl TableOps<Handle> for Table {
593635
None => return Ok(()),
594636
Some(id) => id.clone(),
595637
};
638+
let dpd_table = match &self.dpd_id {
639+
None => return Ok(()),
640+
Some(id) => id.clone(),
641+
};
596642
let match_data = key.key_to_ir().unwrap();
597643

598644
trace!(hdl.log, "entry_del called");
599-
trace!(hdl.log, "table: {}", table);
600-
trace!(hdl.log, "match_data:\n{:#?}", match_data);
645+
trace!(hdl.log, "table: {table}");
646+
trace!(hdl.log, "match_data:\n{match_data:#?}");
647+
648+
let is_route_table =
649+
matches!(dpd_table.as_str(), ROUTER4_LOOKUP_RT | ROUTER6_LOOKUP_RT);
650+
if is_route_table && route_ttl_is_1(&match_data.fields) {
651+
trace!(
652+
hdl.log,
653+
"skipping ttl==1 route entry delete for {dpd_table}"
654+
);
655+
return Ok(());
656+
}
601657

602658
let keyset_data = keyset_data(match_data.fields, &table);
603659

604660
trace!(hdl.log, "sending request to softnpu");
605-
trace!(hdl.log, "table: {}", table);
606-
trace!(hdl.log, "keyset_data:\n{:#?}", keyset_data);
661+
trace!(hdl.log, "table: {table}");
662+
trace!(hdl.log, "keyset_data:\n{keyset_data:#?}");
607663

608664
let msg =
609665
ManagementRequest::TableRemove(TableRemove { keyset_data, table });
@@ -641,12 +697,12 @@ fn keyset_data(match_data: Vec<MatchEntryField>, table: &str) -> Vec<u8> {
641697
let mut data: Vec<u8> = Vec::new();
642698
match table {
643699
RESOLVER_V4 => {
644-
// "nexthop_ipv4" => bit<32>
700+
// "nexthop" => bit<32>
645701
serialize_value_type(&x, &mut data);
646702
keyset_data.extend_from_slice(&data[..4]);
647703
}
648704
RESOLVER_V6 => {
649-
// "nexthop_ipv4" => bit<128>
705+
// "nexthop" => bit<128>
650706
let mut buf = Vec::new();
651707
serialize_value_type(&x, &mut buf);
652708
buf.reverse();
@@ -656,10 +712,12 @@ fn keyset_data(match_data: Vec<MatchEntryField>, table: &str) -> Vec<u8> {
656712
serialize_value_type(&x, &mut data);
657713
keyset_data.extend_from_slice(&data[..2]);
658714
}
659-
ROUTER_V4_RT => {
660-
// "idx" => exact => bit<16>
661-
serialize_value_type(&x, &mut data);
662-
keyset_data.extend_from_slice(&data[..2]);
715+
ROUTER_V4_RT | ROUTER_V6_RT => {
716+
// sidecar-lite route keys are idx-only.
717+
if m.name == "idx" {
718+
serialize_value_type(&x, &mut data);
719+
keyset_data.extend_from_slice(&data[..2]);
720+
}
663721
}
664722
NAT_V4 => {
665723
// "dst_addr" => hdr.ipv4.dst: exact => bit<32>
@@ -749,3 +807,18 @@ fn serialize_value_type_be(x: &ValueTypes, data: &mut Vec<u8>) {
749807
}
750808
}
751809
}
810+
811+
fn route_ttl_is_1(fields: &[MatchEntryField]) -> bool {
812+
fields.iter().any(|field| {
813+
if field.name != "route_ttl_is_1" {
814+
return false;
815+
}
816+
match &field.value {
817+
MatchEntryValue::Value(ValueTypes::U64(v)) => *v != 0,
818+
MatchEntryValue::Value(ValueTypes::Ptr(v)) => {
819+
v.first().is_some_and(|b| *b != 0)
820+
}
821+
_ => false,
822+
}
823+
})
824+
}

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)