Re: [PATCH net-next v2 1/2] seg6: skip the route refcount in seg6local IPv6 lookups

[Gabriel Goller <g.goller@proxmox.com>]

Hi Yuya,

> [snip]
> @@ -372,17 +374,41 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
>  		rt = net->ipv6.ip6_blk_hole_entry;
>  		dst = &rt->dst;
>  		dst_hold(dst);
> +		flags &= ~RT6_LOOKUP_F_DST_NOREF;
>  	}
>  
>  	skb_dst_drop(skb);
> -	skb_dst_set(skb, dst);
> +	if ((flags & RT6_LOOKUP_F_DST_NOREF) && !dst->rt_uncached_list)
> +		skb_dst_set_noref(skb, dst);
> +	else
> +		skb_dst_set(skb, dst);
>  	return dst->error;
>  }
>  
> +static int seg6_lookup_nexthop_noref(struct sk_buff *skb,
> +				     struct in6_addr *nhaddr, u32 tbl_id)
> +{
> +	return seg6_lookup_any_nexthop_noref(skb, nhaddr, tbl_id, false, 0);
> +}
> +
>  int seg6_lookup_nexthop(struct sk_buff *skb,
>  			struct in6_addr *nhaddr, u32 tbl_id)
>  {
> -	return seg6_lookup_any_nexthop(skb, nhaddr, tbl_id, false, 0);
> +	int err;
> +
> +	rcu_read_lock();
> +	err = seg6_lookup_nexthop_noref(skb, nhaddr, tbl_id);
> +	if (!skb_dst_force(skb)) {
> +		struct dst_entry *dst;
> +
> +		dst = &dev_net(skb->dev)->ipv6.ip6_blk_hole_entry->dst;

I think we should use ip6_null_entry instead of ip6_blk_hole_entry here.
Previously this path (failure to acquire the dst ref) was reached through
ip6_pol_route() and ip6_hold_safe(), which uses ip6_null_entry and returns
-ENETUNREACH as an error.
After this patch, the error is triggered here and we use ip6_blk_hole_entry,
which returns -EINVAL.

What do you think?

> +		dst_hold(dst);
> +		skb_dst_set(skb, dst);
> +		err = dst->error;
> +	}
> +	rcu_read_unlock();
> +
> +	return err;
>  }
>  
>  static __u8 seg6_flv_lcblock_octects(const struct seg6_flavors_info *finfo)
> [snip]

Thanks,
Gabriel