netdev - Re: [PATCH net-next 1/2] geneve: Allow users to specify source port range

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: <20250225182647.486772df@kernel.org>
Date: Tue, 25 Feb 2025 18:26:47 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Daniel Borkmann <daniel@...earbox.net>
Cc: pabeni@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] geneve: Allow users to specify source port
 range

On Mon, 24 Feb 2025 16:39:26 +0100 Daniel Borkmann wrote:
> @@ -1083,8 +1087,8 @@ static int geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
>  
>  		use_cache = ip_tunnel_dst_cache_usable(skb, info);
>  		tos = geneve_get_dsfield(skb, dev, info, &use_cache);
> -		sport = udp_flow_src_port(geneve->net, skb,
> -					  1, USHRT_MAX, true);
> +		sport = udp_flow_src_port(geneve->net, skb, geneve->cfg.port_min,

nit: we do still prefer breaking at 80 columns if it doesn't make code
less readable.

> +					  geneve->cfg.port_max, true);
>  
>  		rt = udp_tunnel_dst_lookup(skb, dev, geneve->net, 0, &saddr,
>  					   &info->key,

> @@ -1279,6 +1284,17 @@ static int geneve_validate(struct nlattr *tb[], struct nlattr *data[],
>  		}
>  	}
>  
> +	if (data[IFLA_GENEVE_PORT_RANGE]) {
> +		const struct ifla_geneve_port_range *p =
> +			nla_data(data[IFLA_GENEVE_PORT_RANGE]);

nit: would be more readable as fully separate assignment

> +		if (p->high < p->low) {
> +			NL_SET_ERR_MSG_ATTR(extack, data[IFLA_GENEVE_PORT_RANGE],
> +					    "Invalid source port range");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	return 0;
>  }

> @@ -1450,6 +1451,11 @@ enum ifla_geneve_df {
>  	GENEVE_DF_MAX = __GENEVE_DF_END - 1,
>  };
>  
> +struct ifla_geneve_port_range {
> +	__u16 low;
> +	__u16 high;

I agree with the choice in abstract, but since VXLAN uses byte swapped
fields I think we may be setting an annoying trap for user space
implementations. I'd err on the side of consistency. No?

> +};
> +
>  /* Bareudp section  */
>  enum {
>  	IFLA_BAREUDP_UNSPEC,
-- 
pw-bot: cr