[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

[multicast] prevent VLAN translation via match key enforcement - #195

Merged
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation
Mar 10, 2026
Merged

[multicast] prevent VLAN translation via match key enforcement#195
zeeshanlakhani merged 9 commits into
mainfrom
vlan-translation

Conversation

@zeeshanlakhani

@zeeshanlakhanizeeshanlakhani commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation. Previously, a packet arriving with VLAN 100 destined to a multicast group configured for VLAN 200 would be NAT encapsulated and forwarded, effectively translating the packet to the wrong customer's network.

NAT ingress table matching (mcast_nat.rs, mod.rs):

  • Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address, VLAN header validity, and VLAN ID
  • Each group installs a single NAT entry with an exact VLAN match key. Decapsulated Geneve packets never reach the NatIngress tables (guarded by !hdr.geneve.isValid()), so a separate untagged entry is unnecessary.
  • Packets with the wrong VLAN miss the entry and are not NAT encapsulated

Multicast router VLAN handling (sidecar.p4):

  • forward_vlan(vlan_id) action re-adds the group's configured VLAN on egress
  • Fix MulticastRouter4 using IPv6 ICMP error codes (ICMP6_DST_UNREACH) instead of IPv4 (ICMP_DEST_UNREACH)

Rollback changes:

  • Remove dead NAT rollback branches for internal groups (no NAT entries)
  • Add rollback support for VLAN changes in NAT and route tables

…tering
Underlay changes:
- Restrict internal multicast from admin/site/org scoped (ff04, ff05, ff08)
to just Omicron's reserved subnet (ff04::/64). These underlay addresses are
made unique in Omicron.
- Rename `AdminScopedIpv6` to `UnderlayMulticastIpv6`.
- Simplify P4 to only match ff04::/64.
- Use omicron-common multicast constants for validation.
Source filtering changes:
- Replace IpSrc::Subnet with IpSrc::Any for any-source matching.
- Change IPv6 source filter from exact to LPM match.
- Allow source filters on ASM groups (previously SSM-only).
API changes:
- API v5 adds IpSrc::Any for ASM source filtering.
- API v6 enforces strict underlay subnet validation.
Fixes#107.
Stacked on #189.
This adds VLAN-aware NAT ingress matching to prevent cross-VLAN translation.
Previously, a packet arriving with VLAN 100 destined to a multicast group
configured for VLAN 200 would be NAT encapsulated and forwarded, effectively
translating the packet to the wrong customer's network.
NAT ingress table matching (mcast_nat.rs, mod.rs):
- Add Ipv4VlanMatchKey and Ipv6VlanMatchKey that match on destination address,
VLAN header validity, and VLAN ID
- For groups with VLAN, install two entries: untagged (for decapsulated Geneve
from underlay) and correctly tagged (for customer packets)
- Packets with the wrong VLAN miss both entries and are not NAT encapsulated
Multicast router VLAN handling (sidecar.p4):
- Strip incoming VLAN tag before routing lookup in MulticastRouter4/6
- forward_vlan action re-adds the group's configured VLAN on egress
- Prevents unintended VLAN translation at the routing stage
Rollback changes:
- Remove dead NAT rollback branches for internal groups (no NAT entries)
- Add rollback support for VLAN changes in NAT and route tables
Counter fix:
- The underlay multicast counter condition was unreachable for packets tagged
MULTICAST_TAG_UNDERLAY_EXTERNAL that were not decapped. The check for
== MULTICAST_TAG_UNDERLAY excluded these packets, causing them to fall
through to the external counter.
Pull Request: #194
@zeeshanlakhanizeeshanlakhani self-assigned this Feb 19, 2026
…ation_ids
This includes some cleanup as well.
Base automatically changed from zl/admin-scope-oxnet to mainFebruary 26, 2026 12:07
Comment threaddpd/src/table/mcast/mcast_nat.rs Outdated
Decapsulated Geneve packets never reach the NatIngress tables due to the
outermost !hdr.geneve.isValid() check, so the dual-entry approach (untagged +
tagged) per VLAN group is unnecessary. Each group now installs a single NAT
entry with an exact VLAN match key.
This also contains a fix for MulticastRouter4 (in P4) using IPv6 ICMP error codes
(ICMP6_DST_UNREACH) instead of IPv4 ones (ICMP_DEST_UNREACH), and removes redundant
entry-count assertions from VLAN lifecycle tests.
@zeeshanlakhani

Copy link
Copy Markdown
ContributorAuthor

@FelixMcFelix updated.

@FelixMcFelixFelixMcFelix 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.

I'm concerned about IPV4_LPM_SIZE, but I think I'm happy that this change is more correct. Thanks for acting on the earlier feedback!

Comment threaddpd-client/tests/integration_tests/mcast.rs Outdated
Comment threaddpd-client/tests/integration_tests/table_tests.rs
…acity drop
- Adds EthHdr::hdr_len() to the packet crate to return the wire size of
the L2 header including any 802.1Q tag, replacing duplicate inline ops.
- Add a TODO noting multicast reduces IPv4 LPM capacity from 8187 to
7164.
@zeeshanlakhani
zeeshanlakhani merged commit 375a0f2 into mainMar 10, 2026
6 checks passed
@zeeshanlakhani
zeeshanlakhani deleted the vlan-translation branch March 10, 2026 01:14
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 10, 2026
zeeshanlakhani added a commit to oxidecomputer/omicron that referenced this pull request Mar 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in integration_tests::mcast::test_multicast_vlan_translation_not_possible

2 participants

@zeeshanlakhani@FelixMcFelix