Skip to content

[swadm,multicast] add multicast subcommand (->list, ->get, ->add, ->del) - #366

Open
zeeshanlakhani wants to merge 16 commits into
zl/mcast-type-validationfrom
zl/mcast-swadm
Open

zeeshanlakhani wants to merge 16 commits into
zl/mcast-type-validationfrom
zl/mcast-swadm

Conversation

@zeeshanlakhani

@zeeshanlakhani zeeshanlakhani commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

This adds a swadm multicast subcommand (aliased as mcast as well) for inspecting, creating, and deleting the multicast groups programmed on the switch:

  • multicast list [-t <tag>] [-k external|underlay] renders all groups as a
    table view, filtered by tag, by replication kind, or both.
  • multicast get <group-ip> shows one group's full configuration.
  • multicast add <ext-ip> -i <underlay> -m <mac> -v <vni> [--member port/link:dir]... [--vlan <id>] [-s <src>]... [-t <tag>] creates an external group and the underlay group that it maps to.
  • multicast del <group-ip> [-t <tag>] [--underlay-only] deletes the pair, or just an unreferenced underlay group with --underlay-only.

The kind filter matches on the response type: external means an external multicast group with a NAT target and no (underlying) direction membership; underlay groups reside in the reserved ff04::/64 subnet and replicate to the member ports (to guest subscribers).

add/del subcommands work together as a pair (of groups) by default: add creates the underlay group, then the external group that maps to it (1:1, rolling the underlay back out if the external create fails), and del checks both (shared) tags before removing either group. The -t flag defaults to the client tag on both subcommands, similar to arp add. del --underlay-only removes unreferenced underlay groups, including any left behind after a failed pair deletion.

Includes:

  • multicast get runs through TabWriter, like list; list headers use Title Casing, like nat/attached.
  • add flags match what nat add has, i.e. -i underlay, -m mac, -v vni, and --member is long-only.
  • clap parses right into the new types from [dpd,dpd-api,dpd-types] make external multicast constraints self-enforcing #396 (type-validation), with bad addresses failing at arg parse-time.
  • tests moved onto the common expectorate setup from Regression tests for swadm #350 (per Cory's review), seeded through swadm itself, which removes the tests' need for tokio and the dropshot client.

@zeeshanlakhani zeeshanlakhani changed the title [swadm,multicast] add multicast inspection subcommand [swadm,multicast] add multicast inspection subcommand (->list, ->get) Sep 5, 2026
@zeeshanlakhani
zeeshanlakhani removed this pull request from stack #363 September 9, 2026 01:59
This work constrains group and SSM source validation based on a variety
of specs, linked throughout the PR. Previously, DPD was accepting groups and
(S, G) sources that no packet could actually exercise, including ones
that can never leave a host/link or, worse, sources a switch can't forward.

Here's the list (the first classifier one was the most obvious):

| address                        | verdict | spec                          |
|--------------------------------|---------|-------------------------------|
| ff3x::/32 in ff30::/12 only    | is SSM  | RFC 4607 §1; RFC 3306 is ASM  |
| ff3x:: group ID > 0x40000000   | accept  | RFC 4607 §4.3                 |
| v6 scopes other than 4/5/8/e   | reject  | RFC 7346; no Ethernet realm   |
| class E (240.0.0.0/4) source   | reject  | RFC 6890 "Source: False"      |
| 0.0.0.0/8 source               | reject  | RFC 6890 "Forwardable: False" |
| 100.64.0.0/10 source           | accept  | RFC 6598, operator-internal   |
| source/group family mismatch   | reject  | (local choice)                |
| 232.0.0.0 (SSM null)           | reject  | RFC 4607 §4.3                 |
| v4-mapped/compat v6 source     | reject  | RFC 4291 §2.5.5, aliases IPv4 |
The external-group docs say “IPv4 and non-admin-local IPv6 multicast traffic,”
but external groups can use admin-local IPv6 too. The exception,
however, is the `ff04::/64` underlay range, which is reserved for the
underlay mcast group mapped from the external mcast group. This makes
the information explicit.

The old wording is in a blessed spec, so this needs a new API version:
`MCAST_EXTERNAL_SCOPE_DOCS`.
This adds a READ-only (no-mutation) `swadm multicast` subcommand
(aliased as `mcast` as well) for inspecting the multicast groups
programmed on the switch.

The two commands are essentially a LIST and GET:

- `multicast list [-t <tag>] [-k external|underlay]` renders all groups as a
  table view, filtered by tag, by replication kind, or both.
- `multicast get <group-ip>` shows one group's full configuration.

The kind filter matches on the response type: `external` means an
external group with a NAT target and no (underlying) direction
membership; `underlay` groups sit in the reserved ff04::/64 subnet and
replicate to the member ports (to guest subscribers).
@zeeshanlakhani
zeeshanlakhani added this pull request to stack #372 September 9, 2026 14:54
Comment thread swadm/tests/multicast.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swadm now has common infra for tests. Can we move this module over to the new expectorate pattern?

#350

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my understanding, this test uses the dropshot client to create/delete multicast assets so that we can then inspect the output using swadm. Doing so requires the updated tokio dependency.

First, I'd like to use the shared expectorate infra for swadm validation as stated above.

Taking it further, can we add multicast asset creation/deletion to swadm so that this test doesn't need the dropshot client at all?

Doing so creeps the scope of this PR, and restricting to dropshot makes sense if you know of a reason swadm definitely should not support multicast creation/deletion. But adding that to swadm sounds like useful functionality and would simplify this test.

Simplifying swadm tests isn't an end goal in itself, but swadm tests are close enough to human/hand testing that I want to be thoughtful. For example, could you then set up a multicast demo just using cli commands? That would be pretty cool.

@cfzimmerman cfzimmerman Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw the "request changes" is for you to please check out the new expectorate test structure. Which was merged since you opened this PR, so apologies for the churn!
Adding create/delete and using tabwriter in multicast_get are your decision.

Comment thread swadm/src/multicast.rs Outdated
@zeeshanlakhani
zeeshanlakhani removed this pull request from stack #372 September 23, 2026 18:38
@zeeshanlakhani
zeeshanlakhani added this pull request to stack #377 September 24, 2026 17:37
@zeeshanlakhani zeeshanlakhani self-assigned this Oct 6, 2026
@zeeshanlakhani
zeeshanlakhani removed this pull request from stack #377 October 6, 2026 02:42
@zeeshanlakhani
zeeshanlakhani added this pull request to stack #397 October 7, 2026 10:17
@zeeshanlakhani zeeshanlakhani added the multicast Multicast-related feature/impl label Oct 7, 2026
@zeeshanlakhani zeeshanlakhani changed the title [swadm,multicast] add multicast inspection subcommand (->list, ->get) [swadm,multicast] add multicast subcommand (->list, ->get, ->add, ->del) Oct 8, 2026
This update moves the swadm multicast testing harness onto
the common expectorate setup from #350, and gives swadm
multicast group create/delete (add/del) semantics, removing
the tests' need for tokio and the dropshot client.

`add/del` subcommands work on a pair (of groups) by default: `add`
creates the underlay group, then the external group that maps to
it (1:1, rolling the underlay back out if the external create fails),
and `del` checks both (shared) tags before removing either group. `-t`
defaults to the client tag on both subcommands, similar to `arp add`.

`del --underlay-only` removes unreferenced underlay groups, including
any left behind after a failed pair deletion.

Includes:

- `multicast get` runs through `TabWriter`, like `list`, for
  consistency.
- `list` headers use Title Casing, like `nat`/`attached`.
- `add` flags match what `nat add` has, i.e. `-i` underlay, `-m` mac,
  `-v` vni, and `--member` is long-only.
- clap parses right into our new types (parent commit ~ type-validation),
  with bad addresses failing at arg parse-time.
- extended test coverage and test helpers
@zeeshanlakhani
zeeshanlakhani removed this pull request from stack #397 October 13, 2026 13:58
@zeeshanlakhani
zeeshanlakhani added this pull request to stack #401 October 13, 2026 13:59

@cfzimmerman cfzimmerman left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow this totally rocks. Tysm for updates after the last review. Awesome functionality and well tested.

Comment thread swadm/tests/cli/cmd.rs
Comment on lines +75 to +81
// CI exports RUST_BACKTRACE=1, which forces anyhow to append a "Stack
// backtrace:" section to every error swadm prints, and stderr would
// differ from a local run and never match an expectation file.
//
// We still keep panic backtraces but turn off the library-level ones,
// just like omdb's test harness does:
// https://github.com/oxidecomputer/omicron/blob/654472b6368b48743f34c0744e2acfb06278f796/dev-tools/omdb/tests/test_all_output.rs#L961-L969

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@zeeshanlakhani
zeeshanlakhani removed this pull request from stack #401 October 21, 2026 01:55
@zeeshanlakhani
zeeshanlakhani added this pull request to stack #410 October 21, 2026 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multicast Multicast-related feature/impl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants