From 44fa45df3aedc49b6a80d03fb3ed6e5542d54d21 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 13 Jun 2022 20:52:29 -0400 Subject: [PATCH] net/if: Drop ifmediareq and just use the regular ifreq for SIOCGIFMEDIA. This was introduced into the main API in 2010 (d72ede75fb252c24c8a5fcc39395f9ae1c202322), but was actually only fully used for the past month (c2a9a890f3ac7795602d11c0edaa20ac2db48202) when SIOCGIFMEDIA was supported for all *BSD drivers and not just WiFi. Most userland consumers of this structure did not use it correctly, as was the case in #17770, and only worked because in the fallback case the network stack just treated it as if it were an ifreq. Nothing actually used the ifm_count/ifm_ulist (though tentative APIs were exposed for it) as noted by previous commits; and the fact that Haiku's IFM_* declarations are so spartan makes most of the returned values unintelligible to userland without using FreeBSD compat headers. If, in the future, we decide to implement ifmedia listing and selection properly, that should likely be done with separate ioctls instead of having multi-function ones like this. This is technically an ABI break, but in practice it should not matter: ifmediareq::ifm_current aligns with ifreq::ifr_media, so the things that used this structure like our in-tree code did will continue to work. Until this past May, the only other field that was usually set was ifm_active, but in the absence of setting ifm_status all non-Haiku consumers should ignore it completely. The only consumer of this ioctl that I know of out of the tree, wpa_supplicant, still works after these changes. --- headers/os/net/NetworkDevice.h | 3 -- headers/posix/net/if.h | 11 ------- src/add-ons/kernel/network/stack/datalink.cpp | 15 ++++------ src/add-ons/kernel/network/stack/link.cpp | 20 ++++--------- src/kits/network/libnetapi/NetworkDevice.cpp | 29 ++----------------- .../network/libnetapi/NetworkInterface.cpp | 7 ++--- .../compat/freebsd_network/compat/net/if.h | 10 +++++++ .../compat/freebsd_network/device_hooks.c | 1 - 8 files changed, 26 insertions(+), 70 deletions(-) diff --git a/headers/os/net/NetworkDevice.h b/headers/os/net/NetworkDevice.h index addac3414d..c310718878 100644 --- a/headers/os/net/NetworkDevice.h +++ b/headers/os/net/NetworkDevice.h @@ -99,9 +99,6 @@ public: uint32 Flags() const; bool HasLink() const; - int32 CountMedia() const; - int32 GetMediaAt(int32 index) const; - int32 Media() const; status_t SetMedia(int32 media); diff --git a/headers/posix/net/if.h b/headers/posix/net/if.h index 80522f2bc8..2ad9bd418e 100644 --- a/headers/posix/net/if.h +++ b/headers/posix/net/if.h @@ -64,17 +64,6 @@ struct ifaliasreq { uint32_t ifra_flags; }; -/* used with SIOCGIFMEDIA */ -struct ifmediareq { - char ifm_name[IF_NAMESIZE]; - int ifm_current; - int ifm_mask; - int ifm_status; - int ifm_active; - int ifm_count; - int* ifm_ulist; -}; - /* interface flags */ #define IFF_UP 0x0001 diff --git a/src/add-ons/kernel/network/stack/datalink.cpp b/src/add-ons/kernel/network/stack/datalink.cpp index c1652a90c5..de474a58b1 100644 --- a/src/add-ons/kernel/network/stack/datalink.cpp +++ b/src/add-ons/kernel/network/stack/datalink.cpp @@ -918,19 +918,16 @@ interface_protocol_control(net_datalink_protocol* _protocol, int32 option, case SIOCGIFMEDIA: { // get media - if (length > 0 && length < sizeof(ifmediareq)) + const size_t copylen = offsetof(ifreq, ifr_media) + sizeof(ifreq::ifr_media); + if (length > 0 && length < copylen) return B_BAD_VALUE; - struct ifmediareq request; - if (user_memcpy(&request, argument, sizeof(request)) != B_OK) + struct ifreq request; + if (user_memcpy(&request, argument, copylen) != B_OK) return B_BAD_ADDRESS; - // TODO: Support retrieving the media list? - memset(&request, 0, sizeof(struct ifmediareq)); - request.ifm_active = request.ifm_current - = interface->device->media; - - return user_memcpy(argument, &request, sizeof(request)); + request.ifr_media = interface->device->media; + return user_memcpy(argument, &request, copylen); } case SIOCGIFMETRIC: diff --git a/src/add-ons/kernel/network/stack/link.cpp b/src/add-ons/kernel/network/stack/link.cpp index 38486621d9..26c9dea3b3 100644 --- a/src/add-ons/kernel/network/stack/link.cpp +++ b/src/add-ons/kernel/network/stack/link.cpp @@ -455,30 +455,22 @@ link_control(net_protocol* _protocol, int level, int option, void* value, case SIOCGIFMEDIA: { // get media - if (*_length > 0 && *_length < sizeof(ifmediareq)) + const size_t copylen = offsetof(ifreq, ifr_media) + sizeof(ifreq::ifr_media); + if (*_length > 0 && *_length < copylen) return B_BAD_VALUE; net_device_interface* interface; - struct ifmediareq request; - if (!user_request_get_device_interface(value, (ifreq&)request, - interface)) + struct ifreq request; + if (!user_request_get_device_interface(value, request, interface)) return B_BAD_ADDRESS; - if (interface == NULL) return B_DEVICE_NOT_FOUND; - if (user_memcpy(&request, value, sizeof(ifmediareq)) != B_OK) { - put_device_interface(interface); - return B_BAD_ADDRESS; - } - - // We do not support SIOCSIFMEDIA here, so ignore the media list. - memset(&request, 0, sizeof(struct ifmediareq)); - request.ifm_active = request.ifm_current = interface->device->media; + request.ifr_media = interface->device->media; put_device_interface(interface); - return user_memcpy(value, &request, sizeof(struct ifmediareq)); + return user_memcpy(value, &request, copylen); } case SIOCSPACKETCAP: diff --git a/src/kits/network/libnetapi/NetworkDevice.cpp b/src/kits/network/libnetapi/NetworkDevice.cpp index a4a5eddc62..c6ddac7c86 100644 --- a/src/kits/network/libnetapi/NetworkDevice.cpp +++ b/src/kits/network/libnetapi/NetworkDevice.cpp @@ -617,39 +617,14 @@ BNetworkDevice::HasLink() const } -int32 -BNetworkDevice::CountMedia() const -{ - ifmediareq request; - request.ifm_count = 0; - request.ifm_ulist = NULL; - - if (do_request(request, Name(), SIOCGIFMEDIA) != B_OK) - return -1; - - return request.ifm_count; -} - - int32 BNetworkDevice::Media() const { - ifmediareq request; - request.ifm_count = 0; - request.ifm_ulist = NULL; - + ifreq request; if (do_request(request, Name(), SIOCGIFMEDIA) != B_OK) return -1; - return request.ifm_current; -} - - -int32 -BNetworkDevice::GetMediaAt(int32 index) const -{ - // TODO: this could do some caching - return 0; + return request.ifr_media; } diff --git a/src/kits/network/libnetapi/NetworkInterface.cpp b/src/kits/network/libnetapi/NetworkInterface.cpp index d557adfaf0..8c61e775ae 100644 --- a/src/kits/network/libnetapi/NetworkInterface.cpp +++ b/src/kits/network/libnetapi/NetworkInterface.cpp @@ -257,14 +257,11 @@ BNetworkInterface::MTU() const int32 BNetworkInterface::Media() const { - ifmediareq request; - request.ifm_count = 0; - request.ifm_ulist = NULL; - + ifreq request; if (do_request(AF_INET, request, Name(), SIOCGIFMEDIA) != B_OK) return -1; - return request.ifm_current; + return request.ifr_media; } diff --git a/src/libs/compat/freebsd_network/compat/net/if.h b/src/libs/compat/freebsd_network/compat/net/if.h index 7f5aba8508..e89a95ea3f 100644 --- a/src/libs/compat/freebsd_network/compat/net/if.h +++ b/src/libs/compat/freebsd_network/compat/net/if.h @@ -117,6 +117,16 @@ struct if_data { struct timeval ifi_lastchange; /* time of last administrative change */ }; +struct ifmediareq { + char ifm_name[IFNAMSIZ]; /* if name, e.g. "en0" */ + int ifm_current; /* current media options */ + int ifm_mask; /* don't care mask */ + int ifm_status; /* media status */ + int ifm_active; /* active options */ + int ifm_count; /* # entries in ifm_ulist array */ + int *ifm_ulist; /* media words */ +}; + struct ifdrv { char ifd_name[IFNAMSIZ]; /* if name, e.g. "en0" */ unsigned long ifd_cmd; diff --git a/src/libs/compat/freebsd_network/device_hooks.c b/src/libs/compat/freebsd_network/device_hooks.c index 7d90f2caec..7e1e65be43 100644 --- a/src/libs/compat/freebsd_network/device_hooks.c +++ b/src/libs/compat/freebsd_network/device_hooks.c @@ -328,7 +328,6 @@ compat_control(void *cookie, uint32 op, void *arg, size_t length) case SIOCSIFFLAGS: case SIOCSIFMEDIA: - case SIOCGIFMEDIA: case SIOCSIFMTU: { IFF_LOCKGIANT(ifp);