netdev - Re: [PATCH net-next] gre: Drop ip_route_output

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: <Z2PM/8LPMtiYeZ8n@mev-dev.igk.intel.com>
Date: Thu, 19 Dec 2024 08:36:31 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Guillaume Nault <gnault@...hat.com>
Cc: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
	David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org,
	Simon Horman <horms@...nel.org>
Subject: Re: [PATCH net-next] gre: Drop ip_route_output_gre().

On Wed, Dec 18, 2024 at 02:17:16PM +0100, Guillaume Nault wrote:
> We already have enough variants of ip_route_output*() functions. We
> don't need a GRE specific one in the generic route.h header file.
> 
> Furthermore, ip_route_output_gre() is only used once, in ipgre_open(),
> where it can be easily replaced by a simple call to
> ip_route_output_key().
> 
> While there, and for clarity, explicitly set .flowi4_scope to
> RT_SCOPE_UNIVERSE instead of relying on the implicit zero
> initialisation.
> 
> Signed-off-by: Guillaume Nault <gnault@...hat.com>
> ---
>  include/net/route.h | 14 --------------
>  net/ipv4/ip_gre.c   | 17 ++++++++++-------
>  2 files changed, 10 insertions(+), 21 deletions(-)
> 
> diff --git a/include/net/route.h b/include/net/route.h
> index 84cb1e04f5cd..6947a155d501 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -185,20 +185,6 @@ static inline struct rtable *ip_route_output_ports(struct net *net, struct flowi
>  	return ip_route_output_flow(net, fl4, sk);
>  }
>  
> -static inline struct rtable *ip_route_output_gre(struct net *net, struct flowi4 *fl4,
> -						 __be32 daddr, __be32 saddr,
> -						 __be32 gre_key, __u8 tos, int oif)
> -{
> -	memset(fl4, 0, sizeof(*fl4));
> -	fl4->flowi4_oif = oif;
> -	fl4->daddr = daddr;
> -	fl4->saddr = saddr;
> -	fl4->flowi4_tos = tos;
> -	fl4->flowi4_proto = IPPROTO_GRE;
> -	fl4->fl4_gre_key = gre_key;
> -	return ip_route_output_key(net, fl4);
> -}
> -
>  enum skb_drop_reason
>  ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
>  		      dscp_t dscp, struct net_device *dev,
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index f1f31ebfc793..a020342f618d 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -924,15 +924,18 @@ static int ipgre_open(struct net_device *dev)
>  	struct ip_tunnel *t = netdev_priv(dev);
>  
>  	if (ipv4_is_multicast(t->parms.iph.daddr)) {
> -		struct flowi4 fl4;
> +		struct flowi4 fl4 = {
> +			.flowi4_oif = t->parms.link,
> +			.flowi4_tos = t->parms.iph.tos & INET_DSCP_MASK,
> +			.flowi4_scope = RT_SCOPE_UNIVERSE,
> +			.flowi4_proto = IPPROTO_GRE,
> +			.saddr = t->parms.iph.saddr,
> +			.daddr = t->parms.iph.daddr,
> +			.fl4_gre_key = t->parms.o_key,
> +		};
>  		struct rtable *rt;
>  
> -		rt = ip_route_output_gre(t->net, &fl4,
> -					 t->parms.iph.daddr,
> -					 t->parms.iph.saddr,
> -					 t->parms.o_key,
> -					 t->parms.iph.tos & INET_DSCP_MASK,
> -					 t->parms.link);
> +		rt = ip_route_output_key(t->net, &fl4);
>  		if (IS_ERR(rt))
>  			return -EADDRNOTAVAIL;
>  		dev = rt->dst.dev;
> -- 

Looks even better without the memset
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>

> 2.39.2