Repository navigation
windows datapath: support the masked set action - #54
Merged
Merged
Conversation
Implement OVS_ACTION_ATTR_SET_MASKED in the kernel datapath so the forwarding extension can rewrite individual header-field bits instead of whole fields. Each field updater gains an optional mask: when present it folds the action value with the current on-wire value as (old & ~mask) | (value & mask) before the existing compare/checksum/write logic runs, keeping the incremental checksum deltas correct. The dispatcher splits the nested key attribute into its value and equally-sized mask, rejects a masked tunnel set, and the install-time validator and action policy accept the action. Advertising the capability lets ofproto emit set_masked instead of degrading every field rewrite (DSCP, partial MAC/port, ECN) to a full set.
There was a problem hiding this comment.
Pull request overview
This PR adds support for OVS_ACTION_ATTR_SET_MASKED in the Windows kernel datapath (ovsext), enabling masked header rewrites so ofproto can emit set_masked instead of expanding partial rewrites into full-field sets.
Changes:
- Accept and advertise the
OVS_ACTION_ATTR_SET_MASKEDaction in the Windows datapath action policy/feature support checks. - Add a masked-set executor path that splits the nested key payload into
{value, mask}halves and applies per-field masked updates across L2/L3/L4/ARP/ND/MPLS updaters. - Extend existing header updaters to accept an optional mask and fold
(old & ~mask) | (value & mask)before checksum/compare/write logic.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| datapath-windows/ovsext/Flow.c | Extends action policy and install-time action length validation to accept SET_MASKED; marks action as supported. |
| datapath-windows/ovsext/Actions.h | Updates updater function signatures to accept an optional mask pointer. |
| datapath-windows/ovsext/Actions.c | Implements masked-set folding helpers, masked-aware header updaters, a masked-set dispatcher, and execution wiring in the main action loop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The masked-set executor reads a value plus an equally-sized mask, each one struct wide, so for a fixed-size key it needs exactly twice the struct length. The install-time validator only enforced a minimum, so an oversize attribute installed successfully and then failed when a packet hit the flow. Require the exact length for fixed-size keys; variable-length keys remain bounded by the executor.
A masked MPLS set carries a variable-length LSE array, so it has no fixed minimum in the key policy. Enforce that the value and mask halves each hold at least one full label entry, so an undersized payload is rejected at install time rather than dropped at runtime when the executor finds half < sizeof(struct ovs_key_mpls).
ofproto routes OVS_KEY_ATTR_PRIORITY, SKB_MARK and ND_EXTENSIONS through the masked-set path, but the Windows datapath does not carry those fields and the plain-set executor already ignores them. Note in the masked-set dispatcher's default case why it must succeed (tolerate) rather than fail closed: failing would drop flows that set those keys, which plain set has always accepted.
The comment still said the nested key length must be "at least 2x the key minimum", but the check enforces an exact length for fixed-size keys (and a two-label minimum for variable-length MPLS). Align the comment with the code.
A flow that masked-sets a tolerated-but-unhandled key (skb_mark, priority, nd_extensions) hits the dispatcher's default case for every packet. Logging that at INFO would spam the log in production; TRACE keeps it available for diagnosis without the noise, matching the "tolerate, do not fail" intent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements
OVS_ACTION_ATTR_SET_MASKEDin the Windows kernel datapath (ovsext), so the forwarding extension can rewrite individual header-field bits rather than whole fields.Each of the nine field updaters (eth/ipv4/ipv6/tcp/udp/sctp/arp/nd/mpls) gains an optional mask. When present, it folds the action value with the current on-wire value as
(old & ~mask) | (value & mask)before the existing compare/checksum/write logic runs, so the incremental checksum deltas (IPv4/TCP/UDP, SCTP CRC32c, ICMPv6 recompute) stay correct. A new dispatcher splits the nested key attribute into its value and equally-sized mask and rejects a masked tunnel set; the install-time validator and action policy accept the action.Advertising the capability lets ofproto emit
set_maskedinstead of degrading every partial field rewrite (DSCP, ECN, partial MAC/port) to a full set.Live-validated on a Hyper-V test datapath:
dpif/show-dp-featuresnow reportsMasked set action: Yes; a bidirectional IPv6 DNAT (set_masked(ipv6) both directions) pings at 0% loss with correct ICMPv6 checksums; and a pktmon capture of a masked IPv4 rewrite (DSCP+ECN+dst) shows the expected0x81ToS, rewritten destination, and a valid IPv4 header checksum.