Re: [PATCH net-next v3] net: ip6_tunnel: use tunnel parameters for fill_forward_path route lookup

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4061 bytes --]

> Reuse the flowi6 template t->fl.u.ip6 built by ip6_tnl_link_config() in
> ip6_tnl_fill_forward_path(), aligning the fast-path route lookup with
> the slow path in ipxip6_tnl_xmit(). This automatically inherits the
> correct conditional FLOWLABEL masking based on the
> IP6_TNL_F_USE_ORIG_FLOWLABEL flag.
> 
> Return -EOPNOTSUPP when IP6_TNL_F_USE_ORIG_TCLASS,
> IP6_TNL_F_USE_ORIG_FLOWLABEL or IP6_TNL_F_USE_ORIG_FWMARK is set,
> or for collect_md tunnels, since fill_forward_path has no access to
> the original skb and cannot recover the per-packet traffic class,
> flowlabel, mark or tunnel destination needed for the route lookup.
> 
> Reviewed-by: David Ahern <dsahern@kernel.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> Changes in v3:
> - Return EOPNOTSUPP when IP6_TNL_F_USE_ORIG_TCLASS is set or for collect_md tunnels
> - Link to v2: https://lore.kernel.org/r/20260722-ip6ip6-route-lookup-fill_forward_path-v2-1-ff0b7b013699@kernel.org
> 
> Changes in v2:
> - Reuse flowi6 template for the route lookup
> - Return EOPNOTSUPP if the tunnel is configured with
>   IP6_TNL_F_USE_ORIG_FWMARK or IP6_TNL_F_USE_ORIG_FLOWLABEL
> - Use flowi6 fields to setup net_device_path
> - Link to v1: https://lore.kernel.org/r/20260708-ip6ip6-route-lookup-fill_forward_path-v1-1-863b9647102e@kernel.org
> ---
>  net/ipv6/ip6_tunnel.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)

commenting on sashiko's report:
https://sashiko.dev/#/patchset/20260724-ip6ip6-route-lookup-fill_forward_path-v3-1-7b7991538614%40kernel.org

- This is a pre-existing issue, but since you are explicitly handling
  collect_md tunnels here, should the IPv4 equivalent in net/ipv4/ipip.c
  also reject collect_md tunnels?
  - this issue has not been introduced by this patch and it is fixed in
    the following patch:
    https://lore.kernel.org/netdev/20260725-ipip-fill-forward-path-fix-v1-1-bc69fd3127d5@kernel.org/T/#u

- This isn't a bug introduced by this patch, but does this lockless memcpy of
  t->fl.u.ip6 and t->parms risk data corruption from concurrent updates?
  - As pointed out by sashiko, this issue has not been introduced by this patch
    and it is present in the slow-path as well (ipxip6_tnl_xmit()). I guess we
    should fix it with a dedicated patch.

Regards,
Lorenzo

> 
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index bf8e40af60b0..97c3f61d627b 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -1845,24 +1845,30 @@ static int ip6_tnl_fill_forward_path(struct net_device_path_ctx *ctx,
>  				     struct net_device_path *path)
>  {
>  	struct ip6_tnl *t = netdev_priv(ctx->dev);
> -	struct flowi6 fl6 = {
> -		.daddr = t->parms.raddr,
> -	};
>  	struct dst_entry *dst;
> +	struct flowi6 fl6;
>  	int err;
>  
> -	if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) {
> -		/* encaplimit option is currently not supported is
> -		 * sw-acceleration path.
> -		 */
> +	if (t->parms.flags & (IP6_TNL_F_USE_ORIG_TCLASS |
> +			      IP6_TNL_F_USE_ORIG_FLOWLABEL |
> +			      IP6_TNL_F_USE_ORIG_FWMARK))
>  		return -EOPNOTSUPP;
> -	}
> +
> +	if (t->parms.collect_md)
> +		return -EOPNOTSUPP;
> +
> +	if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
> +		return -EOPNOTSUPP;
> +
> +	memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
> +	fl6.flowi6_mark = t->parms.fwmark;
> +	fl6.flowi6_proto = 0;
>  
>  	dst = ip6_route_output(dev_net(ctx->dev), NULL, &fl6);
>  	if (!dst->error) {
>  		path->type = DEV_PATH_TUN;
> -		path->tun.src_v6 = t->parms.laddr;
> -		path->tun.dst_v6 = t->parms.raddr;
> +		path->tun.src_v6 = fl6.saddr;
> +		path->tun.dst_v6 = fl6.daddr;
>  		path->tun.l3_proto = IPPROTO_IPV6;
>  		path->dev = ctx->dev;
>  		ctx->dev = dst->dev;
> 
> ---
> base-commit: 89d8006259b81dd25c962f6cc8d7ab268d6ea426
> change-id: 20260708-ip6ip6-route-lookup-fill_forward_path-9fc45a9118e9
> 
> Best regards,
> -- 
> Lorenzo Bianconi <lorenzo@kernel.org>
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]