* [PATCH net v1] dev_ioctl: fix the type of ifr_flags
@ 2024-09-11 3:46 Atlas Yu
2024-09-11 3:56 ` Stephen Hemminger
0 siblings, 1 reply; 6+ messages in thread
From: Atlas Yu @ 2024-09-11 3:46 UTC (permalink / raw)
To: atlas.yu, kuba, edumazet, davem, pabeni; +Cc: netdev
SIOCxIFFAGS call dev_get_flags/dev_change_flags under the hood, which
take a unsigned int flags instead of short. This mismatch makes it
impossible to get/set IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO that are
beyond 16 bits.
Signed-off-by: Atlas Yu <atlas.yu@canonical.com>
---
include/uapi/linux/if.h | 2 +-
net/core/dev_ioctl.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 797ba2c1562a..b612b6cd7446 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -244,7 +244,7 @@ struct ifreq {
struct sockaddr ifru_broadaddr;
struct sockaddr ifru_netmask;
struct sockaddr ifru_hwaddr;
- short ifru_flags;
+ unsigned int ifru_flags;
int ifru_ivalue;
int ifru_mtu;
struct ifmap ifru_map;
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 8592c052c0f4..4b317561e503 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -145,7 +145,7 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
switch (cmd) {
case SIOCGIFFLAGS: /* Get interface flags */
- ifr->ifr_flags = (short) dev_get_flags(dev);
+ ifr->ifr_flags = dev_get_flags(dev);
return 0;
case SIOCGIFMETRIC: /* Get the metric on the interface
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v1] dev_ioctl: fix the type of ifr_flags
2024-09-11 3:46 [PATCH net v1] dev_ioctl: fix the type of ifr_flags Atlas Yu
@ 2024-09-11 3:56 ` Stephen Hemminger
2024-09-11 4:20 ` Atlas Yu
[not found] ` <CAB55eyUZ=D-vnQZNaNEvLu6gVp33OXvsjJMmdeMiYqVR1FJ2XQ@mail.gmail.com>
0 siblings, 2 replies; 6+ messages in thread
From: Stephen Hemminger @ 2024-09-11 3:56 UTC (permalink / raw)
To: Atlas Yu; +Cc: kuba, edumazet, davem, pabeni, netdev
On Wed, 11 Sep 2024 11:46:08 +0800
Atlas Yu <atlas.yu@canonical.com> wrote:
> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> index 797ba2c1562a..b612b6cd7446 100644
> --- a/include/uapi/linux/if.h
> +++ b/include/uapi/linux/if.h
> @@ -244,7 +244,7 @@ struct ifreq {
> struct sockaddr ifru_broadaddr;
> struct sockaddr ifru_netmask;
> struct sockaddr ifru_hwaddr;
> - short ifru_flags;
> + unsigned int ifru_flags;
> int ifru_ivalue;
> int ifru_mtu;
> struct ifmap ifru_map;
NAK
This breaks userspace ABI. There is no guarantee that
older application correctly zeros the upper flag bits.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v1] dev_ioctl: fix the type of ifr_flags
2024-09-11 3:56 ` Stephen Hemminger
@ 2024-09-11 4:20 ` Atlas Yu
[not found] ` <CAB55eyUZ=D-vnQZNaNEvLu6gVp33OXvsjJMmdeMiYqVR1FJ2XQ@mail.gmail.com>
1 sibling, 0 replies; 6+ messages in thread
From: Atlas Yu @ 2024-09-11 4:20 UTC (permalink / raw)
To: stephen; +Cc: atlas.yu, davem, edumazet, kuba, netdev, pabeni
On Wed, Sep 11, 2024 at 11:56 AM
Stephen Hemminger <stephen@networkplumber.org> wrote:
> On Wed, 11 Sep 2024 11:46:08 +0800
> Atlas Yu <atlas.yu@canonical.com> wrote:
> > diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> > index 797ba2c1562a..b612b6cd7446 100644
> > --- a/include/uapi/linux/if.h
> > +++ b/include/uapi/linux/if.h
> > @@ -244,7 +244,7 @@ struct ifreq {
> > struct sockaddr ifru_broadaddr;
> > struct sockaddr ifru_netmask;
> > struct sockaddr ifru_hwaddr;
> > - short ifru_flags;
> > + unsigned int ifru_flags;
> > int ifru_ivalue;
> > int ifru_mtu;
> > struct ifmap ifru_map;
>
> NAK
> This breaks userspace ABI. There is no guarantee that
> older application correctly zeros the upper flag bits.
Thanks, any suggestions though? How about introducing
another ioctl request for these extended bits.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v1] dev_ioctl: fix the type of ifr_flags
[not found] ` <CAB55eyUZ=D-vnQZNaNEvLu6gVp33OXvsjJMmdeMiYqVR1FJ2XQ@mail.gmail.com>
@ 2024-09-11 5:00 ` Stephen Hemminger
2024-09-11 7:21 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2024-09-11 5:00 UTC (permalink / raw)
To: Atlas Yu; +Cc: kuba, edumazet, davem, pabeni, netdev
On Wed, 11 Sep 2024 12:17:24 +0800
Atlas Yu <atlas.yu@canonical.com> wrote:
> On Wed, Sep 11, 2024 at 11:56 AM
> Stephen Hemminger <stephen@networkplumber.org> wrote:
>
> > On Wed, 11 Sep 2024 11:46:08 +0800
> > Atlas Yu <atlas.yu@canonical.com> wrote:
> > > diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> > > index 797ba2c1562a..b612b6cd7446 100644
> > > --- a/include/uapi/linux/if.h
> > > +++ b/include/uapi/linux/if.h
> > > @@ -244,7 +244,7 @@ struct ifreq {
> > > struct sockaddr ifru_broadaddr;
> > > struct sockaddr ifru_netmask;
> > > struct sockaddr ifru_hwaddr;
> > > - short ifru_flags;
> > > + unsigned int ifru_flags;
> > > int ifru_ivalue;
> > > int ifru_mtu;
> > > struct ifmap ifru_map;
> >
> > NAK
> > This breaks userspace ABI. There is no guarantee that
> > older application correctly zeros the upper flag bits.
>
> Thanks, any suggestions though? How about introducing
> another ioctl request for these extended bits.
Doing anything with ioctl's is hard, there are so many layers to deal with.
Mostly, networking has treated ioctl's as legacy API and switched to using
netlink for any new features.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v1] dev_ioctl: fix the type of ifr_flags
2024-09-11 5:00 ` Stephen Hemminger
@ 2024-09-11 7:21 ` Eric Dumazet
2024-09-11 8:30 ` Atlas Yu
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2024-09-11 7:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Atlas Yu, kuba, davem, pabeni, netdev
On Wed, Sep 11, 2024 at 7:00 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Wed, 11 Sep 2024 12:17:24 +0800
> Atlas Yu <atlas.yu@canonical.com> wrote:
>
> > On Wed, Sep 11, 2024 at 11:56 AM
> > Stephen Hemminger <stephen@networkplumber.org> wrote:
> >
> > > On Wed, 11 Sep 2024 11:46:08 +0800
> > > Atlas Yu <atlas.yu@canonical.com> wrote:
> > > > diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> > > > index 797ba2c1562a..b612b6cd7446 100644
> > > > --- a/include/uapi/linux/if.h
> > > > +++ b/include/uapi/linux/if.h
> > > > @@ -244,7 +244,7 @@ struct ifreq {
> > > > struct sockaddr ifru_broadaddr;
> > > > struct sockaddr ifru_netmask;
> > > > struct sockaddr ifru_hwaddr;
> > > > - short ifru_flags;
> > > > + unsigned int ifru_flags;
> > > > int ifru_ivalue;
> > > > int ifru_mtu;
> > > > struct ifmap ifru_map;
> > >
> > > NAK
> > > This breaks userspace ABI. There is no guarantee that
> > > older application correctly zeros the upper flag bits.
> >
> > Thanks, any suggestions though? How about introducing
> > another ioctl request for these extended bits.
>
> Doing anything with ioctl's is hard, there are so many layers to deal with.
> Mostly, networking has treated ioctl's as legacy API and switched to using
> netlink for any new features.
Note that rtnetlink gets the 32bit flags already, this has been the
case for 20 years or more.
include/uapi/linux/rtnetlink.h:566: unsigned ifi_flags;
/* IFF_* flags */
net/core/rtnetlink.c : rtnl_fill_ifinfo(...)
ifm->ifi_flags = dev_get_flags(dev);
iproute2 also displays more than 16 bits in print_link_flags()
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v1] dev_ioctl: fix the type of ifr_flags
2024-09-11 7:21 ` Eric Dumazet
@ 2024-09-11 8:30 ` Atlas Yu
0 siblings, 0 replies; 6+ messages in thread
From: Atlas Yu @ 2024-09-11 8:30 UTC (permalink / raw)
To: edumazet; +Cc: atlas.yu, davem, kuba, netdev, pabeni, stephen
> On Wed, Sep 11, 2024 at 3:21 PM
> Eric Dumazet <edumazet@google.com> wrote:
>
> Note that rtnetlink gets the 32bit flags already, this has been the
> case for 20 years or more.
>
> include/uapi/linux/rtnetlink.h:566: unsigned ifi_flags;
> /* IFF_* flags */
>
> net/core/rtnetlink.c : rtnl_fill_ifinfo(...)
>
> ifm->ifi_flags = dev_get_flags(dev);
>
> iproute2 also displays more than 16 bits in print_link_flags()
Thank you Eric, I am switching to the netlink API for my use case.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-11 8:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-11 3:46 [PATCH net v1] dev_ioctl: fix the type of ifr_flags Atlas Yu
2024-09-11 3:56 ` Stephen Hemminger
2024-09-11 4:20 ` Atlas Yu
[not found] ` <CAB55eyUZ=D-vnQZNaNEvLu6gVp33OXvsjJMmdeMiYqVR1FJ2XQ@mail.gmail.com>
2024-09-11 5:00 ` Stephen Hemminger
2024-09-11 7:21 ` Eric Dumazet
2024-09-11 8:30 ` Atlas Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).