Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetworkInterface.Linux: take into account physical link status for OperationalStatus and GetIsNetworkAvailable #44867

Merged
merged 1 commit into from
Nov 26, 2020
Merged

NetworkInterface.Linux: take into account physical link status for OperationalStatus and GetIsNetworkAvailable #44867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.NetworkChange.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ public enum NetworkChangeKind
None = -1,
AddressAdded = 0,
AddressRemoved = 1,
LinkAdded = 2,
LinkRemoved = 3,
AvailabilityChanged = 4
AvailabilityChanged = 2
}

public delegate void NetworkChangeEvent(int socket, NetworkChangeKind kind);
Expand Down
5 changes: 3 additions & 2 deletions src/libraries/Native/Unix/System.Native/pal_interfaceaddresses.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,9 @@ int32_t SystemNative_GetNetworkInterfaces(int32_t * interfaceCount, NetworkInter
nii->SupportsMulticast = 1;
}

// Get administrative state as best guess for now.
nii->OperationalState = (ifaddrsEntry->ifa_flags & IFF_UP) ? OperationalStatus_Up : OperationalStatus_Down;
// OperationalState returns whether the interface can transmit data packets.
// The administrator must have enabled the interface (IFF_UP), and the cable must be plugged in (IFF_RUNNING).
nii->OperationalState = ((ifaddrsEntry->ifa_flags & (IFF_UP|IFF_RUNNING)) == (IFF_UP|IFF_RUNNING)) ? OperationalStatus_Up : OperationalStatus_Down;
Copy link
Member

Choose a reason for hiding this comment

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

Is the IFF_RUNNING reliable? e.g. would it also work for bridges, VPNs and interface types other than ethernet?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. I think so because they have the UP and RUNNING flags set on my system:

enp37s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
lo: flags=73<UP,LOOPBACK,RUNNING>  mtu 65536
tun0: flags=4305<UP,POINTOPOINT,RUNNING,NOARP,MULTICAST>  mtu 1360
virbr0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
vnet0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500

Copy link
Member

Choose a reason for hiding this comment

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

It may be. for me it looks like:

br-9a8c3c79be53: flags=4099<UP,BROADCAST,MULTICAST>  mtu 1500
        inet 172.18.0.1  netmask 255.255.0.0  broadcast 172.18.255.255
        inet6 fe80::42:9cff:fe2a:1051  prefixlen 64  scopeid 0x20<link>
        ether 02:42:9c:2a:10:51  txqueuelen 0  (Ethernet)
        RX packets 92528  bytes 17957517 (17.9 MB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 117337  bytes 121598151 (121.5 MB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

br-9ee3891a5abb: flags=4099<UP,BROADCAST,MULTICAST>  mtu 1500
        inet 172.19.0.1  netmask 255.255.0.0  broadcast 172.19.255.255
        inet6 fe80::42:17ff:fec0:867b  prefixlen 64  scopeid 0x20<link>
        ether 02:42:17:c0:86:7b  txqueuelen 0  (Ethernet)
        RX packets 118  bytes 19808 (19.8 KB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 104  bytes 69596 (69.5 KB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

docker0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 172.17.0.1  netmask 255.255.0.0  broadcast 172.17.255.255
        inet6 fe80::42:daff:fed5:e06e  prefixlen 64  scopeid 0x20<link>
        inet6 fc00::1  prefixlen 64  scopeid 0x0<global>
        inet6 fe80::1  prefixlen 64  scopeid 0x20<link>
        ether 02:42:da:d5:e0:6e  txqueuelen 0  (Ethernet)
        RX packets 2403844  bytes 145172137 (145.1 MB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 3343620  bytes 18861164993 (18.8 GB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0**

but I don't know how to reliably tell if the bride is up or now.

Also for you if tun0 and virbr0 is up & running. can you really send traffic to it?
I think I'm ok claiming they are up if Linux claim them as such regardless of that the actual ability to communicate. That still seems like improvement over just using IFF_UP.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also for you if tun0 and virbr0 is up & running. can you really send traffic to it?

Using RUNNING for virbr0 seems appropriate. When I shut down all VMs, status for virbr0 changes to:

virbr0: flags=4099<UP,BROADCAST,MULTICAST>  mtu 1500

}

if (ifaddrsEntry->ifa_addr == NULL)
Expand Down
10 changes: 2 additions & 8 deletions src/libraries/Native/Unix/System.Native/pal_networkchange.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,9 @@ static NetworkChangeKind ReadNewLinkMessage(struct nlmsghdr* hdr)
assert(hdr != NULL);
struct ifinfomsg* ifimsg;
ifimsg = (struct ifinfomsg*)NLMSG_DATA(hdr);
if (ifimsg->ifi_family == AF_INET)
if (ifimsg->ifi_family == AF_UNSPEC)
Copy link
Member

Choose a reason for hiding this comment

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

Checking for AF_INET was simply wrong before?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and on the managed side LinkAdded was ignored. So there is no possibility of regression here.

Copy link
Member

Choose a reason for hiding this comment

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

My guess that in the past presence of IPv4 address was considered as "available". Perhaps pre-dates IPv6

{
if ((ifimsg->ifi_flags & IFF_UP) != 0)
{
return LinkAdded;
}
return AvailabilityChanged;
}

return None;
Expand Down Expand Up @@ -111,9 +108,6 @@ void SystemNative_ReadEvents(int32_t sock, NetworkChangeEvent onNetworkChange)
case RTM_NEWLINK:
onNetworkChange(sock, ReadNewLinkMessage(hdr));
break;
case RTM_DELLINK:
onNetworkChange(sock, LinkRemoved);
break;
Copy link
Member

Choose a reason for hiding this comment

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

Why is handling for RTM_DELLINK no longer needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Similar LinkRemoved was not handled on managed side, so I removed it here also.

case RTM_NEWROUTE:
case RTM_DELROUTE:
{
Expand Down
4 changes: 1 addition & 3 deletions src/libraries/Native/Unix/System.Native/pal_networkchange.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ typedef enum
None = -1,
AddressAdded = 0,
AddressRemoved = 1,
LinkAdded = 2,
LinkRemoved = 3,
AvailabilityChanged = 4,
AvailabilityChanged = 2,
} NetworkChangeKind;

typedef void (*NetworkChangeEvent)(int32_t sock, NetworkChangeKind notificationKind);
Expand Down