netdev - Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220204180728.1597731-1-Jacques.De.Laval@westermo.com>
Date:   Fri, 4 Feb 2022 19:07:28 +0100
From:   Jacques de Laval <Jacques.De.Laval@...termo.com>
To:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>
CC:     <netdev@...r.kernel.org>,
        Jacques de Laval <Jacques.De.Laval@...termo.com>
Subject: Re: [PATCH net-next 1/1] net: Add new protocol attribute to IP addresses

On 2022-02-04 16:16 UTC, David Ahern wrote:
> > diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> > index a038feb63f23..caa6b7a5b5ac 100644
> > --- a/include/linux/inetdevice.h
> > +++ b/include/linux/inetdevice.h
> > @@ -148,6 +148,7 @@ struct in_ifaddr {
> >  	unsigned char		ifa_prefixlen;
> >  	__u32			ifa_flags;
> >  	char			ifa_label[IFNAMSIZ];
> > +	unsigned char		ifa_proto;
>
> there is a hole after ifa_prefixlen where this can go and not affect
> struct size.
>
> >  
> >  	/* In seconds, relative to tstamp. Expiry is at tstamp + HZ * lft. */
> >  	__u32			ifa_valid_lft;
> > diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> > index 78ea3e332688..e53d8f4f4166 100644
> > --- a/include/net/addrconf.h
> > +++ b/include/net/addrconf.h
> > @@ -69,6 +69,7 @@ struct ifa6_config {
> >  	u32			preferred_lft;
> >  	u32			valid_lft;
> >  	u16			scope;
> > +	u8			ifa_proto;
> >  };
> >  
> >  int addrconf_init(void);
> > diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
> > index 653e7d0f65cb..f7c270b24167 100644
> > --- a/include/net/if_inet6.h
> > +++ b/include/net/if_inet6.h
> > @@ -73,6 +73,8 @@ struct inet6_ifaddr {
> >  
> >  	struct rcu_head		rcu;
> >  	struct in6_addr		peer_addr;
> > +
> > +	__u8			ifa_proto;
>
> similarly for this struct; couple of holes that you can put this.

Thank you! I will fix this in v2.

> >  };
> >  
> >  struct ip6_sf_socklist {
> > diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
> > index dfcf3ce0097f..2aa46b9c9961 100644
> > --- a/include/uapi/linux/if_addr.h
> > +++ b/include/uapi/linux/if_addr.h
> > @@ -35,6 +35,7 @@ enum {
> >  	IFA_FLAGS,
> >  	IFA_RT_PRIORITY,  /* u32, priority/metric for prefix route */
> >  	IFA_TARGET_NETNSID,
> > +	IFA_PROTO,
> >  	__IFA_MAX,
> >  };
> >  
> > @@ -69,4 +70,7 @@ struct ifa_cacheinfo {
> >  #define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg))
> >  #endif
> >  
> > +/* ifa_protocol */
> > +#define IFAPROT_UNSPEC	0
> 
> *If* the value is just a passthrough (userspace to kernel and back), no
> need for this uapi. However, have you considered builtin protocol labels
> - e.g. for autoconf, LLA, etc. Kernel generated vs RAs vs userspace
> adding it.

Agreed. For my own (very isolated) use case I only need the passthrough,
but I can see that it would make sense to standardize some labels.
I was trying to give this some thought but I have to admit I copped out
because of my limited knowledge on what labels would be reasonable to
reserve.

Based on what you mention, do you think the list bellow would make sense?

#define IFAPROT_UNSPEC		0  /* unspecified */
#define IFAPROT_KERNEL_LO	1  /* loopback */
#define IFAPROT_KERNEL_RA	2  /* auto assigned by kernel from router announcement */
#define IFAPROT_KERNEL_LL	3  /* link-local set by kernel */
#define IFAPROT_STATIC		4  /* set by admin */
#define IFAPROT_AUTO		5  /* DHCP, BOOTP etc. */
#define IFAPROT_LL		6  /* link-local set by userspace */

Or do you think it needs more granularity?