netdev - Re: [PATCH net-next v2 05/11] vxlan: Track reserved bits explicitly as part of the configuration

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: <6434a8f3-c5c3-42cd-b7b0-c9c06a3eeab0@blackwall.org>
Date: Fri, 6 Dec 2024 11:44:58 +0200
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Petr Machata <petrm@...dia.com>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 netdev@...r.kernel.org
Cc: Simon Horman <horms@...nel.org>, Ido Schimmel <idosch@...dia.com>,
 mlxsw@...dia.com, Menglong Dong <menglong8.dong@...il.com>,
 Guillaume Nault <gnault@...hat.com>,
 Alexander Lobakin <aleksander.lobakin@...el.com>,
 Breno Leitao <leitao@...ian.org>
Subject: Re: [PATCH net-next v2 05/11] vxlan: Track reserved bits explicitly
 as part of the configuration

On 12/5/24 17:40, Petr Machata wrote:
> In order to make it possible to configure which bits in VXLAN header should
> be considered reserved, introduce a new field vxlan_config::reserved_bits.
> Have it cover the whole header, except for the VNI-present bit and the bits
> for VNI itself, and have individual enabled features clear more bits off
> reserved_bits.
> 
> (This is expressed as first constructing a used_bits set, and then
> inverting it to get the reserved_bits. The set of used_bits will be useful
> on its own for validation of user-set reserved_bits in a following patch.)
> 
> The patch also moves a comment relevant to the validation from the unparsed
> validation site up to the new site. Logically this patch should add the new
> comment, and a later patch that removes the unparsed bits would remove the
> old comment. But keeping both legs in the same patch is better from the
> history spelunking point of view.
> 
> Signed-off-by: Petr Machata <petrm@...dia.com>
> Reviewed-by: Ido Schimmel <idosch@...dia.com>
> ---
> 
> Notes:
> CC: Menglong Dong <menglong8.dong@...il.com>
> CC: Guillaume Nault <gnault@...hat.com>
> CC: Alexander Lobakin <aleksander.lobakin@...el.com>
> CC: Breno Leitao <leitao@...ian.org>
> 
>  drivers/net/vxlan/vxlan_core.c | 41 +++++++++++++++++++++++++---------
>  include/net/vxlan.h            |  1 +
>  2 files changed, 31 insertions(+), 11 deletions(-)
> 

One very minor nit below, if there's another version. :)
Reviewed-by: Nikolay Aleksandrov <razor@...ckwall.org>

> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 257411d1ccca..f6118de81b8a 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
[snip]
> @@ -4080,6 +4083,10 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  			 struct net_device *dev, struct vxlan_config *conf,
>  			 bool changelink, struct netlink_ext_ack *extack)
>  {
> +	struct vxlanhdr used_bits = {
> +		.vx_flags = VXLAN_HF_VNI,
> +		.vx_vni = VXLAN_VNI_MASK,
> +	};
>  	struct vxlan_dev *vxlan = netdev_priv(dev);
>  	int err = 0;
>  
> @@ -4306,6 +4313,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  				    extack);
>  		if (err)
>  			return err;
> +		used_bits.vx_flags |= VXLAN_HF_RCO;
> +		used_bits.vx_vni |= ~VXLAN_VNI_MASK;
>  	}
>  
>  	if (data[IFLA_VXLAN_GBP]) {
> @@ -4313,6 +4322,7 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  				    VXLAN_F_GBP, changelink, false, extack);
>  		if (err)
>  			return err;
> +		used_bits.vx_flags |= VXLAN_GBP_USED_BITS;
>  	}
>  
>  	if (data[IFLA_VXLAN_GPE]) {
> @@ -4321,8 +4331,17 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  				    extack);
>  		if (err)
>  			return err;
> +

minor nit: extra newline here, there isn't one above for GBP

> +		used_bits.vx_flags |= VXLAN_GPE_USED_BITS;
>  	}