| Message ID | 20231206141054.41736-1-maze@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: ipv6: support reporting otherwise unknown prefix flags in RTM_NEWPREFIX | expand |
On Wed, Dec 6, 2023 at 3:10 PM Maciej Żenczykowski <maze@google.com> wrote: > > Lorenzo points out that we effectively clear all unknown > flags from PIO when copying them to userspace in the netlink > RTM_NEWPREFIX notification. > > We could fix this one at a time as new flags are defined, > or in one fell swoop - I choose the latter. > > We could either define 6 new reserved flags (reserved1..6) and handle > them individually (and rename them as new flags are defined), or we > could simply copy the entire unmodified byte over - I choose the latter. > > This unfortunately requires some anonymous union/struct magic, > so we add a static assert on the struct size for a little extra safety. > > Cc: Shirley Ma <mashirle@us.ibm.com> > Cc: David Ahern <dsahern@kernel.org> > Cc: Lorenzo Colitti <lorenzo@google.com> > Fixes: 60872d54d963 ("[IPV6]: Add notification for MIB:ipv6Prefix events.") > Signed-off-by: Maciej Żenczykowski <maze@google.com> > --- > include/net/addrconf.h | 12 ++++++++++-- > include/net/if_inet6.h | 4 ---- > net/ipv6/addrconf.c | 6 +----- > 3 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/include/net/addrconf.h b/include/net/addrconf.h > index 82da55101b5a..8e308c2662d7 100644 > --- a/include/net/addrconf.h > +++ b/include/net/addrconf.h > @@ -31,17 +31,22 @@ struct prefix_info { > __u8 length; > __u8 prefix_len; > > + union __attribute__((packed)) { > + __u8 flags; > + struct __attribute__((packed)) { For non uapi, it is recommended to use __packed instead of __attribute__((packed))
On Wed, Dec 6, 2023 at 6:18 AM Eric Dumazet <edumazet@google.com> wrote: > On Wed, Dec 6, 2023 at 3:10 PM Maciej Żenczykowski <maze@google.com> wrote: > > + union __attribute__((packed)) { > > + __u8 flags; > > + struct __attribute__((packed)) { > > For non uapi, it is recommended to use __packed instead of > __attribute__((packed)) Ah, yes, and checkpatch even finds that. Fixed in v3. On patchworks I also see a complaint about the Fixes tag referencing a non-existing commit: Commit: d993c6f5d7e7 ("net: ipv6: support reporting otherwise unknown prefix flags in RTM_NEWPREFIX") Fixes tag: Fixes: 60872d54d963 ("[IPV6]: Add notification for MIB:ipv6Prefix events.") Has these problem(s): - Target SHA1 does not exist I (automatically) pulled it (via git blame) from tglx-history @ https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git graft... $ git log --oneline -n1 remotes/tglx-history/v2.6.2..60872d54d963eefeb302ebeae15204e4be229c2b I'm not sure... would it be better to just not include a fixes tag at all and just CC stable@?
On Wed, Dec 6, 2023 at 6:03 PM Maciej Żenczykowski <maze@google.com> wrote: > > On patchworks I also see a complaint about the Fixes tag referencing a > non-existing commit: > > Commit: d993c6f5d7e7 ("net: ipv6: support reporting otherwise unknown > prefix flags in RTM_NEWPREFIX") > Fixes tag: Fixes: 60872d54d963 ("[IPV6]: Add notification for > MIB:ipv6Prefix events.") > Has these problem(s): > - Target SHA1 does not exist > > I (automatically) pulled it (via git blame) from tglx-history @ > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git > graft... > $ git log --oneline -n1 > remotes/tglx-history/v2.6.2..60872d54d963eefeb302ebeae15204e4be229c2b > > I'm not sure... would it be better to just not include a fixes tag at > all and just CC stable@? Simply use the generic Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
diff --git a/include/net/addrconf.h b/include/net/addrconf.h index 82da55101b5a..8e308c2662d7 100644 --- a/include/net/addrconf.h +++ b/include/net/addrconf.h @@ -31,17 +31,22 @@ struct prefix_info { __u8 length; __u8 prefix_len; + union __attribute__((packed)) { + __u8 flags; + struct __attribute__((packed)) { #if defined(__BIG_ENDIAN_BITFIELD) - __u8 onlink : 1, + __u8 onlink : 1, autoconf : 1, reserved : 6; #elif defined(__LITTLE_ENDIAN_BITFIELD) - __u8 reserved : 6, + __u8 reserved : 6, autoconf : 1, onlink : 1; #else #error "Please fix <asm/byteorder.h>" #endif + }; + }; __be32 valid; __be32 prefered; __be32 reserved2; @@ -49,6 +54,9 @@ struct prefix_info { struct in6_addr prefix; }; +/* rfc4861 4.6.2: IPv6 PIO is 32 bytes in size */ +static_assert(sizeof(struct prefix_info) == 32); + #include <linux/ipv6.h> #include <linux/netdevice.h> #include <net/if_inet6.h> diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h index 3e454c4d7ba6..f07642264c1e 100644 --- a/include/net/if_inet6.h +++ b/include/net/if_inet6.h @@ -22,10 +22,6 @@ #define IF_RS_SENT 0x10 #define IF_READY 0x80000000 -/* prefix flags */ -#define IF_PREFIX_ONLINK 0x01 -#define IF_PREFIX_AUTOCONF 0x02 - enum { INET6_IFADDR_STATE_PREDAD, INET6_IFADDR_STATE_DAD, diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 3aaea56b5166..2692a7b24c40 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -6149,11 +6149,7 @@ static int inet6_fill_prefix(struct sk_buff *skb, struct inet6_dev *idev, pmsg->prefix_len = pinfo->prefix_len; pmsg->prefix_type = pinfo->type; pmsg->prefix_pad3 = 0; - pmsg->prefix_flags = 0; - if (pinfo->onlink) - pmsg->prefix_flags |= IF_PREFIX_ONLINK; - if (pinfo->autoconf) - pmsg->prefix_flags |= IF_PREFIX_AUTOCONF; + pmsg->prefix_flags = pinfo->flags; if (nla_put(skb, PREFIX_ADDRESS, sizeof(pinfo->prefix), &pinfo->prefix)) goto nla_put_failure;
Lorenzo points out that we effectively clear all unknown flags from PIO when copying them to userspace in the netlink RTM_NEWPREFIX notification. We could fix this one at a time as new flags are defined, or in one fell swoop - I choose the latter. We could either define 6 new reserved flags (reserved1..6) and handle them individually (and rename them as new flags are defined), or we could simply copy the entire unmodified byte over - I choose the latter. This unfortunately requires some anonymous union/struct magic, so we add a static assert on the struct size for a little extra safety. Cc: Shirley Ma <mashirle@us.ibm.com> Cc: David Ahern <dsahern@kernel.org> Cc: Lorenzo Colitti <lorenzo@google.com> Fixes: 60872d54d963 ("[IPV6]: Add notification for MIB:ipv6Prefix events.") Signed-off-by: Maciej Żenczykowski <maze@google.com> --- include/net/addrconf.h | 12 ++++++++++-- include/net/if_inet6.h | 4 ---- net/ipv6/addrconf.c | 6 +----- 3 files changed, 11 insertions(+), 11 deletions(-)