[PATCH net v2] net: ipv6: support reporting otherwise unknown prefix flags in RTM_NEWPREFIX

[PATCH net v2] net: ipv6: support reporting otherwise unknown prefix flags in RTM_NEWPREFIX

[Maciej Żenczykowski]

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)) {
 #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;
-- 
2.43.0.rc2.451.g8631bc7472-goog

Re: [PATCH net v2] net: ipv6: support reporting otherwise unknown prefix flags in RTM_NEWPREFIX

[Eric Dumazet]

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))

Re: [PATCH net v2] net: ipv6: support reporting otherwise unknown prefix flags in RTM_NEWPREFIX

[Maciej Żenczykowski]

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@?

Re: [PATCH net v2] net: ipv6: support reporting otherwise unknown prefix flags in RTM_NEWPREFIX

[Eric Dumazet]

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")