Light up multicast_drop counter in dpd/swadm (#89)

Closes:
  * #86

Includes:
  * Update packet-test.sh to list expected counters with `swadm`
  * Remove redundant drop_counter in Filter control

Author: zeeshanlakhani

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

@@ -94,6 +94,21 @@ banner "Links"
 
 ./target/debug/swadm -h '[::1]' link ls || echo "failed to list links"
 
+banner "swadm Checks"
+
+pushd swadm
+
+DENDRITE_TEST_HOST='[::1]' \
+    DENDRITE_TEST_VERBOSITY=3 \
+    cargo test \
+    --no-fail-fast \
+    --test \
+    counters \
+    -- \
+    --ignored
+
+popd
+
 banner "Packet Tests"
 
 set +o errexit

dpd-client/tests/integration_tests/mcast.rs

@@ -1409,6 +1409,14 @@ async fn test_ipv4_multicast_invalid_destination_mac() -> TestResult {
         .await
         .unwrap();
 
+    let port_label_ingress = switch.port_label(ingress).unwrap();
+
+    // Check the Multicast_Drop counter baseline for the ingress port
+    let drop_mcast_baseline = switch
+        .get_counter(&port_label_ingress, Some("multicast_drop"))
+        .await
+        .unwrap();
+
     let result = switch.packet_test(vec![test_pkt], expected_pkts);
 
     check_counter_incremented(
@@ -1421,6 +1429,17 @@ async fn test_ipv4_multicast_invalid_destination_mac() -> TestResult {
     .await
     .unwrap();
 
+    // Verify that the Filter_Drop_Multicast counter also incremented
+    check_counter_incremented(
+        switch,
+        &port_label_ingress,
+        drop_mcast_baseline,
+        1,
+        Some("multicast_drop"),
+    )
+    .await
+    .unwrap();
+
     // Cleanup: Remove both external IPv4 group and underlay IPv6 group
     cleanup_test_group(switch, created_group.group_ip).await;
     cleanup_test_group(switch, internal_multicast_ip).await;
@@ -1486,6 +1505,14 @@ async fn test_ipv6_multicast_invalid_destination_mac() -> TestResult {
         .await
         .unwrap();
 
+    let port_label_ingress = switch.port_label(ingress).unwrap();
+
+    // Check the Multicast_Drop counter baseline for the ingress port
+    let drop_mcast_baseline = switch
+        .get_counter(&port_label_ingress, Some("multicast_drop"))
+        .await
+        .unwrap();
+
     let result = switch.packet_test(vec![test_pkt], expected_pkts);
 
     check_counter_incremented(
@@ -1498,6 +1525,17 @@ async fn test_ipv6_multicast_invalid_destination_mac() -> TestResult {
     .await
     .unwrap();
 
+    // Verify that the Multicast_Drop counter also incremented
+    check_counter_incremented(
+        switch,
+        &port_label_ingress,
+        drop_mcast_baseline,
+        1,
+        Some("multicast_drop"),
+    )
+    .await
+    .unwrap();
+
     cleanup_test_group(switch, created_group.group_ip).await;
 
     result

dpd/p4/sidecar.p4

@@ -107,7 +107,6 @@ control Filter(
 	DirectCounter<bit<32>>(CounterType_t.PACKETS_AND_BYTES) ipv4_ctr;
 	DirectCounter<bit<32>>(CounterType_t.PACKETS_AND_BYTES) ipv6_ctr;
 	Counter<bit<32>, PortId_t>(512, CounterType_t.PACKETS) drop_mcast_ctr;
-	Counter<bit<32>, bit<8>>(DROP_REASON_MAX, CounterType_t.PACKETS) drop_reason_ctr;
 	bit<16> mcast_scope;
 
 	action dropv4() {
@@ -184,7 +183,6 @@ control Filter(
 			if (meta.is_mcast && !meta.is_valid) {
 				drop_mcast();
 				drop_mcast_ctr.count(ig_intr_md.ingress_port);
-				drop_reason_ctr.count(meta.drop_reason);
 				return;
 			} else if (meta.is_mcast && meta.is_valid) {
 				// IPv4 Multicast Address Validation (RFC 1112, RFC 7042)
@@ -223,7 +221,6 @@ control Filter(
 			if (meta.is_mcast && !meta.is_valid) {
 				drop_mcast();
 				drop_mcast_ctr.count(ig_intr_md.ingress_port);
-				drop_reason_ctr.count(meta.drop_reason);
 				return;
 			} else if (meta.is_mcast && meta.is_valid) {
 				// Validate the IPv6 multicast MAC address format (RFC 2464,
@@ -240,7 +237,6 @@ control Filter(
 					hdr.ethernet.dst_mac[39:32] != 8w0x33) {
 						drop_mcast_with_reason(DROP_MULTICAST_INVALID_MAC);
 						drop_mcast_ctr.count(ig_intr_md.ingress_port);
-						drop_reason_ctr.count(meta.drop_reason);
 						return;
 				}
 
@@ -255,7 +251,6 @@ control Filter(
 					hdr.ethernet.dst_mac[7:0] != hdr.ipv6.dst_addr[7:0]) {
 						drop_mcast_with_reason(DROP_MULTICAST_INVALID_MAC);
 						drop_mcast_ctr.count(ig_intr_md.ingress_port);
-						drop_reason_ctr.count(meta.drop_reason);
 						return;
 				}
 			}

dpd/src/counters.rs

@@ -62,6 +62,7 @@ enum CounterId {
     MulticastExt,
     MulticastLL,
     MulticastUL,
+    MulticastDrop,
 }
 
 impl From<CounterId> for u8 {
@@ -84,7 +85,7 @@ struct CounterDescription {
     p4_name: &'static str,
 }
 
-const COUNTERS: [CounterDescription; 13] = [
+const COUNTERS: [CounterDescription; 14] = [
     CounterDescription {
         id: CounterId::Service,
         client_name: "Service",
@@ -150,6 +151,11 @@ const COUNTERS: [CounterDescription; 13] = [
         client_name: "Multicast_Underlay",
         p4_name: "pipe.Egress.underlay_mcast_ctr",
     },
+    CounterDescription {
+        id: CounterId::MulticastDrop,
+        client_name: "Multicast_Drop",
+        p4_name: "pipe.Ingress.filter.drop_mcast_ctr",
+    },
 ];
 
 /// Get the list of names by which end users can refer to a counter.
@@ -416,7 +422,8 @@ pub async fn get_values(
             | CounterId::Multicast
             | CounterId::MulticastExt
             | CounterId::MulticastLL
-            | CounterId::MulticastUL => port_label(switch, idx.idx).await,
+            | CounterId::MulticastUL
+            | CounterId::MulticastDrop => port_label(switch, idx.idx).await,
             CounterId::DropReason | CounterId::EgressDropReason => {
                 reason_label(idx.idx as u8)?
             }

swadm/tests/counters.rs

@@ -0,0 +1,70 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/
+//
+// Copyright 2025 Oxide Computer Company
+
+//! Integration test for swadm P4 counter functionality.
+
+use std::process::Command;
+
+// Path to `swadm` executable.
+const SWADM: &str = env!("CARGO_BIN_EXE_swadm");
+
+fn swadm() -> Command {
+    Command::new(SWADM)
+}
+
+#[test]
+#[ignore]
+fn test_p4_counter_list() {
+    let output = swadm()
+        .arg("-h")
+        .arg("[::1]")
+        .arg("counters")
+        .arg("list")
+        .output()
+        .expect("Failed to execute swadm counters list");
+
+    assert!(
+        output.status.success(),
+        "swadm counters list failed with stderr: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+
+    // Verify output is not empty and contains expected counter information
+    assert!(
+        !stdout.is_empty(),
+        "Counter list output should not be empty"
+    );
+
+    // Expected P4 counters from dpd/src/counters.rs COUNTERS array
+    let expected_counters = [
+        "Service",
+        "Ingress",
+        "Packet",
+        "Egress",
+        "Ingress_Drop_Port",
+        "Ingress_Drop_Reason",
+        "Egress_Drop_Port",
+        "Egress_Drop_Reason",
+        "Unicast",
+        "Multicast",
+        "Multicast_External",
+        "Multicast_Link_Local",
+        "Multicast_Underlay",
+        "Multicast_Drop",
+    ];
+
+    // Verify all expected counters are present in the output
+    for counter in &expected_counters {
+        assert!(
+            stdout.contains(counter),
+            "Counter list should contain '{}' counter. Output: {}",
+            counter,
+            stdout
+        );
+    }
+}