Refactor container port mapping logic#14469
Refactor container port mapping logic#14469OneBlue wants to merge 20 commits intofeature/wsl-for-appsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors WSLA container port-mapping handling to carry richer mapping metadata (protocol + binding address), centralize VM-port allocation via RAII, and adjust the WSLA test harness accordingly.
Changes:
- Introduces RAII-based VM port allocation/mapping types (
VmPortAllocation,VMPortMapping) and updates VM/session APIs to use them. - Extends port-mapping metadata and IDL structs to include protocol + binding address, with new IP parsing helpers.
- Updates WSLA test launcher and Windows tests to use the new port-mapping shape/behavior (including bridged-mode expectations).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLATests.cpp | Updates port-mapping and inspect tests for bridged networking + binding address changes. |
| src/windows/wslasession/WSLAVirtualMachine.h | Adds VmPortAllocation/VMPortMapping and updates port-mapping/port-allocation APIs. |
| src/windows/wslasession/WSLAVirtualMachine.cpp | Implements new RAII allocation/mapping behavior and relay mapping entry points. |
| src/windows/wslasession/WSLASession.h | Updates testing-only VM port-map COM methods to use unsigned short ports. |
| src/windows/wslasession/WSLASession.cpp | Adapts MapVmPort/UnmapVmPort to new VM port-mapping API. |
| src/windows/wslasession/WSLAContainerMetadata.h | Extends serialized container port metadata (protocol + binding address). |
| src/windows/wslasession/WSLAContainer.h | Switches container port storage to ContainerPortMapping with VMPortMapping. |
| src/windows/wslasession/WSLAContainer.cpp | Refactors port processing, allocation, mapping/unmapping, and recovery logic around new mapping types. |
| src/windows/wslaservice/inc/wslaservice.idl | Extends WSLAPortMapping and updates MapVmPort/UnmapVmPort signatures. |
| src/windows/common/wslutil.h | Adds IP parsing helper declarations. |
| src/windows/common/wslutil.cpp | Implements IP parsing helpers with localized user errors. |
| src/windows/common/WSLAContainerLauncher.h | Extends AddPort API to accept protocol + binding address and stores address lifetime. |
| src/windows/common/WSLAContainerLauncher.cpp | Populates new WSLAPortMapping fields and ensures binding address pointer stability. |
| localization/strings/en-US/Resources.resw | Adds a localized “invalid IP address” message. |
There was a problem hiding this comment.
Pull request overview
This PR refactors WSLA container port-mapping to introduce explicit protocol + binding-address handling and to centralize VM port allocation/mapping via new VMPortMapping / VmPortAllocation types. It updates container creation/recovery and test coverage to reflect the new port-mapping behavior.
Changes:
- Introduces
VMPortMapping+VmPortAllocationabstractions and refactors VM port allocation/mapping APIs accordingly. - Extends
WSLAPortMapping(IDL + launcher + metadata) withProtocolandBindingAddress, and threads these through container create/open/start flows. - Updates WSLA port-mapping tests and adds new negative-path validation cases.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLATests.cpp | Updates port-mapping tests for IPv6 bind behavior and adds error-path assertions. |
| src/windows/wslrelay/localhost.cpp | Adds address family information to port relay TraceLogging events. |
| src/windows/wslasession/WSLAVirtualMachine.h | Adds new port allocation/mapping types and updates Map/Unmap/TryAllocate APIs. |
| src/windows/wslasession/WSLAVirtualMachine.cpp | Implements new allocation/mapping logic and binding address parsing/stringification. |
| src/windows/wslasession/WSLASession.h | Tracks test-only VM port allocations to support MapVmPort/UnmapVmPort changes. |
| src/windows/wslasession/WSLASession.cpp | Refactors MapVmPort/UnmapVmPort to allocate/reserve ports and map using VMPortMapping. |
| src/windows/wslasession/WSLAContainerMetadata.h | Extends serialized container metadata port mapping schema with protocol + binding address. |
| src/windows/wslasession/WSLAContainer.h | Refactors container port mapping representation and adds MapPorts/UnmapPorts helpers. |
| src/windows/wslasession/WSLAContainer.cpp | Refactors port processing, mapping/unmapping lifecycle, and container recovery for port reservations. |
| src/windows/wslaservice/inc/wslaservice.idl | Extends WSLAPortMapping and updates MapVmPort/UnmapVmPort signatures. |
| src/windows/common/wslutil.h | Adds IPv4/IPv6 address parsing helpers. |
| src/windows/common/wslutil.cpp | Implements address parsing helpers with localized invalid-IP errors. |
| src/windows/common/WSLAContainerLauncher.h | Extends AddPort to include protocol and binding address; stores binding address strings. |
| src/windows/common/WSLAContainerLauncher.cpp | Populates new WSLAPortMapping fields and ensures binding address lifetime. |
| localization/strings/en-US/Resources.resw | Adds localized message for invalid IP address. |
There was a problem hiding this comment.
Pull request overview
Refactors container port mapping to support protocol/binding address metadata and centralize VM port allocation/mapping behavior across session/container lifecycles.
Changes:
- Introduces
VMPortMapping/VmPortAllocationRAII types and updates VM port mapping APIs to consume them. - Extends port mapping schema/IDL/metadata to include protocol and binding address; updates container creation/open/start/cleanup flows accordingly.
- Updates tests and adds new negative-path coverage (invalid IP, invalid protocol/family, unsupported UDP/custom bind).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLATests.cpp | Updates networking mode expectations, IPv6 binding, and adds new port-mapping error-path tests. |
| src/windows/wslrelay/localhost.cpp | Adds address-family tracing to relay start/stop and port mapping logs. |
| src/windows/wslasession/WSLAVirtualMachine.h | Adds VmPortAllocation / VMPortMapping types; updates VM port mapping/allocation API signatures. |
| src/windows/wslasession/WSLAVirtualMachine.cpp | Implements new mapping/allocation logic and validation based on protocol/binding address. |
| src/windows/wslasession/WSLASession.h | Adds test-only allocation tracking for VM ports and updates MapVmPort signatures. |
| src/windows/wslasession/WSLASession.cpp | Refactors MapVmPort/UnmapVmPort to reuse allocations with reference counting. |
| src/windows/wslasession/WSLAContainerMetadata.h | Extends serialized port mapping metadata with protocol and binding address. |
| src/windows/wslasession/WSLAContainer.h | Introduces ContainerPortMapping wrapper and stores container network mode. |
| src/windows/wslasession/WSLAContainer.cpp | Refactors port processing/allocation/mapping/unmapping; serializes extended metadata. |
| src/windows/wslaservice/inc/wslaservice.idl | Extends WSLAPortMapping with protocol and binding address; updates MapVmPort signatures. |
| src/windows/common/wslutil.h | Adds ParseIpv4/ParseIpv6 address helpers. |
| src/windows/common/wslutil.cpp | Implements IP parsing with localized user error on invalid IP. |
| src/windows/common/WSLAContainerLauncher.h | Extends AddPort to accept protocol and binding address; stores binding address backing strings. |
| src/windows/common/WSLAContainerLauncher.cpp | Implements extended port mapping population. |
| src/windows/WslcSDK/wslcsdk.h | Removes a stray blank line in port mapping struct. |
| src/windows/WslcSDK/wslcsdk.cpp | Populates protocol and binding address in converted port mappings. |
| localization/strings/en-US/Resources.resw | Adds localized “Invalid IP address” message. |
… into user/oneblue/ports-2
There was a problem hiding this comment.
Pull request overview
Refactors container/VM port mapping to support protocol + binding address, introduce RAII-based VM port allocations, and improve diagnostics/tests around port relay behavior.
Changes:
- Introduces
VmPortAllocation/VMPortMappingabstractions and updates VM map/unmap + allocation APIs. - Extends port mapping schema/IDL (protocol + binding address) and updates container create/open/start mapping behavior.
- Updates Windows tests and relay logging, adding negative-path coverage for invalid mappings.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLATests.cpp | Updates networking mode expectations, enables IPv6 HTTP checks, and adds validation of port-mapping error paths. |
| src/windows/wslrelay/localhost.cpp | Adds address-family context to port relay logs. |
| src/windows/wslasession/WSLAVirtualMachine.h | Adds VmPortAllocation/VMPortMapping types and refactors VM port APIs to use them. |
| src/windows/wslasession/WSLAVirtualMachine.cpp | Implements new mapping/allocation logic and mode-specific validation for NAT/VirtioProxy. |
| src/windows/wslasession/WSLASession.h | Adds test-only tracking for VM port allocations and updates testing COM method signatures. |
| src/windows/wslasession/WSLASession.cpp | Implements reference-counted MapVmPort/UnmapVmPort using new allocation model. |
| src/windows/wslasession/WSLAContainerMetadata.h | Extends serialized port metadata with protocol + binding address. |
| src/windows/wslasession/WSLAContainer.h | Introduces ContainerPortMapping and reshapes container port state to use VMPortMapping. |
| src/windows/wslasession/WSLAContainer.cpp | Reworks port-mapping processing, serialization, create/open behavior, and runtime map/unmap lifecycle. |
| src/windows/wslaservice/inc/wslaservice.idl | Extends WSLAPortMapping with protocol + binding address and tweaks test-only port mapping method signatures. |
| src/windows/common/wslutil.h | Exposes IP parsing helpers. |
| src/windows/common/wslutil.cpp | Adds localized-err IP parsing helpers using inet_pton. |
| src/windows/common/WSLAContainerLauncher.h | Extends AddPort to accept protocol + binding address (with defaults). |
| src/windows/common/WSLAContainerLauncher.cpp | Stores binding address strings and populates new port-mapping fields. |
| src/windows/WslcSDK/wslcsdk.h | Minor formatting touch (no behavior). |
| src/windows/WslcSDK/wslcsdk.cpp | Populates protocol + binding address when translating SDK port mappings. |
| localization/strings/en-US/Resources.resw | Adds localized “Invalid IP address” message. |
| if (BindAddress.Ipv4.sin_family == AF_INET6) | ||
| { | ||
| return IN6_IS_ADDR_LOOPBACK(&BindAddress.Ipv6.sin6_addr); | ||
| } | ||
| else | ||
| { | ||
| return IN4ADDR_ISLOOPBACK(&BindAddress.Ipv4); |
|
|
||
| VMPortMapping::~VMPortMapping() | ||
| { | ||
| Unmap(); |
| uint16_t VmPort{}; | ||
| uint16_t ContainerPort{}; | ||
| int Family{}; | ||
| int Protocol{}; | ||
| std::string BindingAddress; | ||
|
|
||
| // Runtime-only field. Not serialized to JSON. | ||
| bool MappedToHost{}; | ||
|
|
||
| NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(WSLAPortMapping, HostPort, VmPort, ContainerPort, Family); | ||
| NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(WSLAPortMapping, HostPort, VmPort, ContainerPort, Family, Protocol, BindingAddress); |
| void AddPort( | ||
| uint16_t WindowsPort, | ||
| uint16_t ContainerPort, | ||
| int Family, | ||
| int Protocol = IPPROTO_TCP, | ||
| const std::string& BindingAddress = "127.0.0.1"); |
| std::shared_ptr<VmPortAllocation> WSLAVirtualMachine::AllocatePort(int Family, int Protocol) | ||
| { | ||
| std::lock_guard lock{m_lock}; | ||
|
|
||
| std::set<uint16_t> allocatedRange; | ||
|
|
||
| // Add ports to the allocated list until we have enough | ||
| for (auto i = CONTAINER_PORT_RANGE.first; i <= CONTAINER_PORT_RANGE.second && allocatedRange.size() < Count; i++) | ||
| for (auto i = CONTAINER_PORT_RANGE.first; i <= CONTAINER_PORT_RANGE.second; i++) | ||
| { | ||
| if (!m_allocatedPorts.contains(i)) | ||
| { | ||
| WI_VERIFY(allocatedRange.insert(i).second); | ||
| WI_VERIFY(m_allocatedPorts.insert(i).second); | ||
| return std::make_shared<VmPortAllocation>(i, Family, Protocol, *this); | ||
| } | ||
| } | ||
|
|
||
| // Fail if we couldn't find enough free ports. | ||
| THROW_HR_IF_MSG( | ||
| HRESULT_FROM_WIN32(ERROR_NO_SYSTEM_RESOURCES), | ||
| allocatedRange.size() < Count, | ||
| "Failed to allocate %u ports, only %zu available", | ||
| Count, | ||
| allocatedRange.size()); | ||
|
|
||
| // Reserve the ports we found. | ||
| m_allocatedPorts.insert(allocatedRange.begin(), allocatedRange.end()); | ||
|
|
||
| return allocatedRange; | ||
| // Fail if we couldn't find a port. | ||
| THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_NO_SYSTEM_RESOURCES), "Failed to allocate port"); | ||
| } |
| void VMPortMapping::AssignVmPort(const std::shared_ptr<VmPortAllocation>& Port) | ||
| { | ||
| WI_ASSERT(!VmPort); | ||
| VmPort = std::move(Port); | ||
| } |
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed