Use PF_LINK on SunOS

[gwr]

I've been working on updates to arcate/eng/common/cross/build-rootfs.sh
and discovered it doing some ill-advised header copying for illumos.
With this small configuration change, that's no longer needed.
As the comment in the change explains:

SunOS defines both AF_LINK and AF_PACKET but AF_LINK is preferred.
Using AF_PACKET on SunOS requires access to system-private headers.

[Copilot]

Pull request overview

This pull request modifies the native networking code to prefer AF_LINK over AF_PACKET on SunOS/illumos platforms. SunOS defines both AF_LINK and AF_PACKET constants, but using AF_PACKET requires access to system-private headers, which creates unnecessary build dependencies. The change ensures that the codebase uses AF_LINK (the preferred API) on SunOS by adding !defined(TARGET_SUNOS) conditions to the existing AF_PACKET preprocessor checks.

Changes:

• Modified conditional compilation in pal_maphardwaretype.c to exclude SunOS from using AF_PACKET code paths
• Modified conditional compilation in pal_interfaceaddresses.c to exclude SunOS from AF_PACKET header includes
• Added explanatory comments documenting why SunOS should use AF_LINK instead of AF_PACKET

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/native/libs/System.Native/pal_maphardwaretype.c	Updated compile-time conditionals at lines 13 and 35 to exclude TARGET_SUNOS from AF_PACKET code paths, forcing the use of AF_LINK for hardware type mapping on SunOS
src/native/libs/System.Native/pal_interfaceaddresses.c	Updated compile-time conditional at line 52 to exclude TARGET_SUNOS from AF_PACKET header includes, ensuring AF_LINK headers are used instead

[am11]

some ill-advised header copying for illumos.

It was discussed with illumos channel when I set it up. It is build-only quirk and shouldn't block the real work which is completion of port. Let’s look for a proper replacement that doesn’t come at the cost of a reduced or compromised implementation.

[gwr]

I'm not sure why I would do that when the #elif defined(AF_LINK) section that follows does what we want on SunOS. Did you see that?

[gwr]

some ill-advised header copying for illumos.

It was discussed with illumos channel when I set it up. It is build-only quirk and shouldn't block the real work which is completion of port. Let’s look for a proper replacement that doesn’t come at the cost of a reduced or compromised implementation.

I don't see any reduced functionality here. This is just making it use the AF_LINK code paths.

[am11]

This is just making it use the AF_LINK code paths.

Have you validated it in a fresh docker? If so, I don't see how it would be behaving properly given the order at call-site:

runtime/src/native/libs/System.Native/pal_interfaceaddresses.c

Lines 228 to 254 in a48fa8e

	#if defined(AF_PACKET)
	else if (family == AF_PACKET)
	{
	if (onLinkLayerFound != NULL)
	{
	struct sockaddr_ll* sall = (struct sockaddr_ll*)current->ifa_addr;
	if (sall->sll_halen > sizeof(sall->sll_addr))
	{
	// sockaddr_ll->sll_addr has a maximum capacity of 8 bytes (unsigned char sll_addr[8])
	// so if we get a address length greater than that, we truncate it to 8 bytes.
	// This is following the kernel docs where they always treat physical addresses with a maximum of 8 bytes.
	// However in WSL we hit an issue where sll_halen was 16 bytes so the memcpy_s below would fail because it was greater.
	sall->sll_halen = sizeof(sall->sll_addr);
	}
	LinkLayerAddressInfo lla;
	memset(&lla, 0, sizeof(LinkLayerAddressInfo));
	lla.InterfaceIndex = interfaceIndex;
	lla.NumAddressBytes = sall->sll_halen;
	lla.HardwareType = MapHardwareType(sall->sll_hatype);
	memcpy_s(&lla.AddressBytes, sizeof_member(LinkLayerAddressInfo, AddressBytes), &sall->sll_addr, sall->sll_halen);
	onLinkLayerFound(context, current->ifa_name, &lla);
	}
	}
	#elif defined(AF_LINK)

[gwr]

This is just making it use the AF_LINK code paths.

Have you validated it in a fresh docker? If so, I don't see how it would be behaving properly given the order at call-site:

runtime/src/native/libs/System.Native/pal_interfaceaddresses.c

Lines 228 to 254 in a48fa8e

	#if defined(AF_PACKET)
	else if (family == AF_PACKET)
	{
	if (onLinkLayerFound != NULL)
	{
	struct sockaddr_ll* sall = (struct sockaddr_ll*)current->ifa_addr;
	if (sall->sll_halen > sizeof(sall->sll_addr))
	{
	// sockaddr_ll->sll_addr has a maximum capacity of 8 bytes (unsigned char sll_addr[8])
	// so if we get a address length greater than that, we truncate it to 8 bytes.
	// This is following the kernel docs where they always treat physical addresses with a maximum of 8 bytes.
	// However in WSL we hit an issue where sll_halen was 16 bytes so the memcpy_s below would fail because it was greater.
	sall->sll_halen = sizeof(sall->sll_addr);
	}
	LinkLayerAddressInfo lla;
	memset(&lla, 0, sizeof(LinkLayerAddressInfo));
	lla.InterfaceIndex = interfaceIndex;
	lla.NumAddressBytes = sall->sll_halen;
	lla.HardwareType = MapHardwareType(sall->sll_hatype);
	memcpy_s(&lla.AddressBytes, sizeof_member(LinkLayerAddressInfo, AddressBytes), &sall->sll_addr, sall->sll_halen);
	onLinkLayerFound(context, current->ifa_name, &lla);
	}
	}
	#elif defined(AF_LINK)

Thanks. I've only gone as far as building this.
What's a good test for this library?

[am11]

I've been working on updates to arcate/eng/common/cross/build-rootfs.sh

illumos support in build-rootfs.sh was originally added for the dotnet prereq Docker image. Some time ago the illumos image build itself was removed due to staleness(although previously built images still exist). So changes in build-rootfs will need to be validated manually:

# fresh docker
$ cd runtime
$ git branch --show-current # make sure it's this branch
# remove the extra headers download block from build-rootfs.sh

# run docker interactively
$ docker run --rm -v$(pwd):/runtime -w /runtime -e ROOTFS_DIR=/crossrootfs/x64 -it ubuntu
$ eng/common/native/install-dependencies.sh
$ apt install -y libmpc-dev
$ eng/common/cross/build-rootfs.sh noble illumos x64
$ ./build.sh clr+libs --cross --os illumos --arch x64 --gcc

What's a good test for this library?

src/libraries/System.Net.NetworkInformation/tests/FunctionalTests

I was actually referring to validation of build test and also fixing the logic, I think it needs #if defined(AF_PACKET) && !defined(TARGET_SUNOS) on pal_interfaceaddresses.c's line 228 as well.

[gwr]

OK. This seems to work as well as the previous did.

System.Net.NetworkInformation.Functional.Tests Results
Overall: 210/239 tests passed (87.9% pass rate), 29 failures, 0 skipped
Failed Test Categories (All throw PlatformNotSupportedException):
(pre-existing failures)