hrev56186 adds 4 changesets to branch 'master'
old head: 7de24641a4b997a57682d4add14469ef64aa8471
new head: 44fa45df3aedc49b6a80d03fb3ed6e5542d54d21
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=44fa45df3aed+%5E7de24641a4b9
----------------------------------------------------------------------------
b5fae382545c: network/ethernet: Do not set IFM_ETHER in update_link_state.
All the drivers that support ETHER_GET_LINK_STATE set this correctly;
and more specifically, it should actually not be set on some devices
(WiFi devices, specifically). Since SIOCGIFMEDIA has always been
passed through directly to the driver if it supports it, WiFi
devices worked anyway because this value never made it to userland.
That will soon change, hence this needed to be fixed.
83ac9b727ce8: network/stack: Do not invoke SIOCGIFMEDIA but just return the
already-fetched media.
This functionally disables most of the functionality of the BSD-style
SIOCGIFMEDIA, but it was never used in userland (because if it had,
it would have triggered SMAP violations in the compatibility layer.)
SIOCGIFMEDIA returns BSD-style media values, which mostly overlap
with Haiku ones but have a few differences still. These are taken care
of in the compat layer by ETHER_GET_LINK_STATE, which is where this
media value comes from, but are not by SIOCGIFMEDIA which just passes
back whatever the drivers do.
Fixes #17770, at least for ethernet drivers.
ecf18ba4c89a: freebsd_network: Cleanup around ifmedia handling.
* Remove unused variable.
* Use original value for IFM_AVALID, we do not need to remap this one.
(Actually we do not need to remap IFM_ACTIVE here either, but it makes
some things slightly easier to work with rather than remapping it
in a different location.)
44fa45df3aed: 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.
[ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]
----------------------------------------------------------------------------
10 files changed, 28 insertions(+), 85 deletions(-)
headers/os/net/NetworkDevice.h | 3 --
headers/posix/net/if.h | 11 --------
.../kernel/network/devices/ethernet/ethernet.cpp | 3 --
src/add-ons/kernel/network/stack/datalink.cpp | 19 ++++---------
src/add-ons/kernel/network/stack/link.cpp | 26 +++++-------------
src/kits/network/libnetapi/NetworkDevice.cpp | 29 ++------------------
src/kits/network/libnetapi/NetworkInterface.cpp | 7 ++---
src/libs/compat/freebsd_network/compat/net/if.h | 10 +++++++
.../compat/freebsd_network/compat/net/if_media.h | 3 +-
src/libs/compat/freebsd_network/device_hooks.c | 2 --
############################################################################
Commit: b5fae382545caad6531ea60477d34d6783188b7d
URL: https://git.haiku-os.org/haiku/commit/?id=b5fae382545c
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Tue Jun 14 00:24:17 2022 UTC
network/ethernet: Do not set IFM_ETHER in update_link_state.
All the drivers that support ETHER_GET_LINK_STATE set this correctly;
and more specifically, it should actually not be set on some devices
(WiFi devices, specifically). Since SIOCGIFMEDIA has always been
passed through directly to the driver if it supports it, WiFi
devices worked anyway because this value never made it to userland.
That will soon change, hence this needed to be fixed.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/network/devices/ethernet/ethernet.cpp
b/src/add-ons/kernel/network/devices/ethernet/ethernet.cpp
index e6327194bd..425567ea7a 100644
--- a/src/add-ons/kernel/network/devices/ethernet/ethernet.cpp
+++ b/src/add-ons/kernel/network/devices/ethernet/ethernet.cpp
@@ -66,9 +66,6 @@ update_link_state(ethernet_device *device, bool notify = true)
return B_NOT_SUPPORTED;
}
- state.media |= IFM_ETHER;
- // make sure the media type is returned correctly
-
if (device->media != state.media
|| device->link_quality != state.quality
|| device->link_speed != state.speed) {
############################################################################
Commit: 83ac9b727ce84c2cf08de4f468aadd19a5c3de76
URL: https://git.haiku-os.org/haiku/commit/?id=83ac9b727ce8
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Tue Jun 14 00:32:57 2022 UTC
Ticket: https://dev.haiku-os.org/ticket/17770
network/stack: Do not invoke SIOCGIFMEDIA but just return the already-fetched
media.
This functionally disables most of the functionality of the BSD-style
SIOCGIFMEDIA, but it was never used in userland (because if it had,
it would have triggered SMAP violations in the compatibility layer.)
SIOCGIFMEDIA returns BSD-style media values, which mostly overlap
with Haiku ones but have a few differences still. These are taken care
of in the compat layer by ETHER_GET_LINK_STATE, which is where this
media value comes from, but are not by SIOCGIFMEDIA which just passes
back whatever the drivers do.
Fixes #17770, at least for ethernet drivers.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/network/stack/datalink.cpp
b/src/add-ons/kernel/network/stack/datalink.cpp
index 9d4d7fb512..c1652a90c5 100644
--- a/src/add-ons/kernel/network/stack/datalink.cpp
+++ b/src/add-ons/kernel/network/stack/datalink.cpp
@@ -922,19 +922,15 @@ interface_protocol_control(net_datalink_protocol*
_protocol, int32 option,
return B_BAD_VALUE;
struct ifmediareq request;
- if (user_memcpy(&request, argument, sizeof(ifmediareq))
!= B_OK)
+ if (user_memcpy(&request, argument, sizeof(request)) !=
B_OK)
return B_BAD_ADDRESS;
- // TODO: see above.
- if
(interface->device->module->control(interface->device,
- SIOCGIFMEDIA, &request,
- sizeof(struct ifmediareq)) != B_OK) {
- memset(&request, 0, sizeof(struct ifmediareq));
- request.ifm_active = request.ifm_current
- = interface->device->media;
- }
+ // 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(struct
ifmediareq));
+ return user_memcpy(argument, &request, sizeof(request));
}
case SIOCGIFMETRIC:
diff --git a/src/add-ons/kernel/network/stack/link.cpp
b/src/add-ons/kernel/network/stack/link.cpp
index 12f20ade7e..38486621d9 100644
--- a/src/add-ons/kernel/network/stack/link.cpp
+++ b/src/add-ons/kernel/network/stack/link.cpp
@@ -472,14 +472,10 @@ link_control(net_protocol* _protocol, int level, int
option, void* value,
return B_BAD_ADDRESS;
}
- // TODO: see above.
- if
(interface->device->module->control(interface->device,
- SIOCGIFMEDIA, &request,
- sizeof(struct ifmediareq)) != B_OK) {
- memset(&request, 0, sizeof(struct ifmediareq));
- request.ifm_active = request.ifm_current
- = interface->device->media;
- }
+ // 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;
+
put_device_interface(interface);
return user_memcpy(value, &request, sizeof(struct
ifmediareq));
############################################################################
Commit: ecf18ba4c89aded4187ca0344572d5881c0445cd
URL: https://git.haiku-os.org/haiku/commit/?id=ecf18ba4c89a
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Tue Jun 14 00:34:05 2022 UTC
freebsd_network: Cleanup around ifmedia handling.
* Remove unused variable.
* Use original value for IFM_AVALID, we do not need to remap this one.
(Actually we do not need to remap IFM_ACTIVE here either, but it makes
some things slightly easier to work with rather than remapping it
in a different location.)
----------------------------------------------------------------------------
diff --git a/src/libs/compat/freebsd_network/compat/net/if_media.h
b/src/libs/compat/freebsd_network/compat/net/if_media.h
index 6d6631f523..d11493282e 100644
--- a/src/libs/compat/freebsd_network/compat/net/if_media.h
+++ b/src/libs/compat/freebsd_network/compat/net/if_media.h
@@ -374,11 +374,10 @@ uint64_t ifmedia_baudrate(int);
/*
* Status bits
*/
-#ifndef __HAIKU__
#define IFM_AVALID 0x00000001 /* Active bit valid */
+#ifndef __HAIKU__
#define IFM_ACTIVE 0x00000002 /* Interface attached to
working net */
#else
-#define IFM_AVALID 0x10000000 /* Active bit valid */
#define IFM_ACTIVE 0x00800000 /* same as Haiku's */
#endif
diff --git a/src/libs/compat/freebsd_network/device_hooks.c
b/src/libs/compat/freebsd_network/device_hooks.c
index d62097b127..7d90f2caec 100644
--- a/src/libs/compat/freebsd_network/device_hooks.c
+++ b/src/libs/compat/freebsd_network/device_hooks.c
@@ -299,7 +299,6 @@ compat_control(void *cookie, uint32 op, void *arg, size_t
length)
{
struct ifmediareq mediareq;
ether_link_state_t state;
- status_t status;
if (length < sizeof(ether_link_state_t))
return EINVAL;
############################################################################
Revision: hrev56186
Commit: 44fa45df3aedc49b6a80d03fb3ed6e5542d54d21
URL: https://git.haiku-os.org/haiku/commit/?id=44fa45df3aed
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Tue Jun 14 00:52:29 2022 UTC
Ticket: https://dev.haiku-os.org/ticket/17770
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.
----------------------------------------------------------------------------
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);
