[v1] ipv6: mcast: Add ip6_mc_find_idev() helper

[PATCH RESEND net-next] ipv6: mcast: Add ip6_mc_find_idev() helper

Posted by Yue Haibing 1 month, 2 weeks ago

__ipv6_sock_mc_join() has the same code as ip6_mc_find_dev() to find dev,
extract this into ip6_mc_find_dev() and add ip6_mc_find_idev() to reduce
code duplication and improve readability.

Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
---
 net/ipv6/mcast.c | 76 ++++++++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 41 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 36ca27496b3c..75430ad55c3d 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -169,6 +169,29 @@ static int unsolicited_report_interval(struct inet6_dev *idev)
 	return iv > 0 ? iv : 1;
 }
 
+static struct net_device *ip6_mc_find_dev(struct net *net,
+					  const struct in6_addr *group,
+					  int ifindex)
+{
+	struct net_device *dev = NULL;
+	struct rt6_info *rt;
+
+	if (ifindex == 0) {
+		rcu_read_lock();
+		rt = rt6_lookup(net, group, NULL, 0, NULL, 0);
+		if (rt) {
+			dev = dst_dev(&rt->dst);
+			dev_hold(dev);
+			ip6_rt_put(rt);
+		}
+		rcu_read_unlock();
+	} else {
+		dev = dev_get_by_index(net, ifindex);
+	}
+
+	return dev;
+}
+
 /*
  *	socket join on multicast group
  */
@@ -191,28 +214,13 @@ static int __ipv6_sock_mc_join(struct sock *sk, int ifindex,
 	}
 
 	mc_lst = sock_kmalloc(sk, sizeof(struct ipv6_mc_socklist), GFP_KERNEL);
-
 	if (!mc_lst)
 		return -ENOMEM;
 
 	mc_lst->next = NULL;
 	mc_lst->addr = *addr;
 
-	if (ifindex == 0) {
-		struct rt6_info *rt;
-
-		rcu_read_lock();
-		rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
-		if (rt) {
-			dev = dst_dev(&rt->dst);
-			dev_hold(dev);
-			ip6_rt_put(rt);
-		}
-		rcu_read_unlock();
-	} else {
-		dev = dev_get_by_index(net, ifindex);
-	}
-
+	dev = ip6_mc_find_dev(net, addr, ifindex);
 	if (!dev) {
 		sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
 		return -ENODEV;
@@ -302,32 +310,18 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
 }
 EXPORT_SYMBOL(ipv6_sock_mc_drop);
 
-static struct inet6_dev *ip6_mc_find_dev(struct net *net,
-					 const struct in6_addr *group,
-					 int ifindex)
+static struct inet6_dev *ip6_mc_find_idev(struct net *net,
+					  const struct in6_addr *group,
+					  int ifindex)
 {
-	struct net_device *dev = NULL;
-	struct inet6_dev *idev;
-
-	if (ifindex == 0) {
-		struct rt6_info *rt;
+	struct inet6_dev *idev = NULL;
+	struct net_device *dev;
 
-		rcu_read_lock();
-		rt = rt6_lookup(net, group, NULL, 0, NULL, 0);
-		if (rt) {
-			dev = dst_dev(&rt->dst);
-			dev_hold(dev);
-			ip6_rt_put(rt);
-		}
-		rcu_read_unlock();
-	} else {
-		dev = dev_get_by_index(net, ifindex);
+	dev = ip6_mc_find_dev(net, group, ifindex);
+	if (dev) {
+		idev = in6_dev_get(dev);
+		dev_put(dev);
 	}
-	if (!dev)
-		return NULL;
-
-	idev = in6_dev_get(dev);
-	dev_put(dev);
 
 	return idev;
 }
@@ -374,7 +368,7 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
 	if (!ipv6_addr_is_multicast(group))
 		return -EINVAL;
 
-	idev = ip6_mc_find_dev(net, group, pgsr->gsr_interface);
+	idev = ip6_mc_find_idev(net, group, pgsr->gsr_interface);
 	if (!idev)
 		return -ENODEV;
 
@@ -509,7 +503,7 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf,
 	    gsf->gf_fmode != MCAST_EXCLUDE)
 		return -EINVAL;
 
-	idev = ip6_mc_find_dev(net, group, gsf->gf_interface);
+	idev = ip6_mc_find_idev(net, group, gsf->gf_interface);
 	if (!idev)
 		return -ENODEV;
 
-- 
2.34.1

Re: [PATCH RESEND net-next] ipv6: mcast: Add ip6_mc_find_idev() helper

Posted by Paolo Abeni 1 month, 1 week ago

On 8/18/25 12:10 PM, Yue Haibing wrote:
> @@ -302,32 +310,18 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
>  }
>  EXPORT_SYMBOL(ipv6_sock_mc_drop);
>  
> -static struct inet6_dev *ip6_mc_find_dev(struct net *net,
> -					 const struct in6_addr *group,
> -					 int ifindex)
> +static struct inet6_dev *ip6_mc_find_idev(struct net *net,
> +					  const struct in6_addr *group,
> +					  int ifindex)
>  {
> -	struct net_device *dev = NULL;
> -	struct inet6_dev *idev;
> -
> -	if (ifindex == 0) {
> -		struct rt6_info *rt;
> +	struct inet6_dev *idev = NULL;
> +	struct net_device *dev;
>  
> -		rcu_read_lock();
> -		rt = rt6_lookup(net, group, NULL, 0, NULL, 0);
> -		if (rt) {
> -			dev = dst_dev(&rt->dst);
> -			dev_hold(dev);
> -			ip6_rt_put(rt);
> -		}
> -		rcu_read_unlock();
> -	} else {
> -		dev = dev_get_by_index(net, ifindex);
> +	dev = ip6_mc_find_dev(net, group, ifindex);
> +	if (dev) {
> +		idev = in6_dev_get(dev);
> +		dev_put(dev);
>  	}
> -	if (!dev)
> -		return NULL;
> -
> -	idev = in6_dev_get(dev);
> -	dev_put(dev);

Not so minor nit: if you omit the last chunk (from 'if (dev) {' onwards,
unneeded), the patch will be much more obvious and smaller. Also you
could clarify a bit the commit message.

You can retain Dawid's ack when posting the next version

/P

Re: [PATCH RESEND net-next] ipv6: mcast: Add ip6_mc_find_idev() helper

Posted by Yue Haibing 1 month, 1 week ago

On 2025/8/21 17:42, Paolo Abeni wrote:
> On 8/18/25 12:10 PM, Yue Haibing wrote:
>> @@ -302,32 +310,18 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
>>  }
>>  EXPORT_SYMBOL(ipv6_sock_mc_drop);
>>  
>> -static struct inet6_dev *ip6_mc_find_dev(struct net *net,
>> -					 const struct in6_addr *group,
>> -					 int ifindex)
>> +static struct inet6_dev *ip6_mc_find_idev(struct net *net,
>> +					  const struct in6_addr *group,
>> +					  int ifindex)
>>  {
>> -	struct net_device *dev = NULL;
>> -	struct inet6_dev *idev;
>> -
>> -	if (ifindex == 0) {
>> -		struct rt6_info *rt;
>> +	struct inet6_dev *idev = NULL;
>> +	struct net_device *dev;
>>  
>> -		rcu_read_lock();
>> -		rt = rt6_lookup(net, group, NULL, 0, NULL, 0);
>> -		if (rt) {
>> -			dev = dst_dev(&rt->dst);
>> -			dev_hold(dev);
>> -			ip6_rt_put(rt);
>> -		}
>> -		rcu_read_unlock();
>> -	} else {
>> -		dev = dev_get_by_index(net, ifindex);
>> +	dev = ip6_mc_find_dev(net, group, ifindex);
>> +	if (dev) {
>> +		idev = in6_dev_get(dev);
>> +		dev_put(dev);
>>  	}
>> -	if (!dev)
>> -		return NULL;
>> -
>> -	idev = in6_dev_get(dev);
>> -	dev_put(dev);
> 
> Not so minor nit: if you omit the last chunk (from 'if (dev) {' onwards,
> unneeded), the patch will be much more obvious and smaller. Also you
> could clarify a bit the commit message.

Thanks, it is indeed more clear, will do this in v2

> You can retain Dawid's ack when posting the next version
> 
> /P
> 
>

Re: [PATCH RESEND net-next] ipv6: mcast: Add ip6_mc_find_idev() helper

Posted by Dawid Osuchowski 1 month, 2 weeks ago

On 2025-08-18 12:10 PM, Yue Haibing wrote:
> __ipv6_sock_mc_join() has the same code as ip6_mc_find_dev() to find dev,
> extract this into ip6_mc_find_dev() and add ip6_mc_find_idev() to reduce
> code duplication and improve readability.

nit: the commit msg is a tad bit confusing

> 
> Signed-off-by: Yue Haibing <yuehaibing@huawei.com>

Regardless of the nit above,

Reviewed-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>

Thanks,
Dawid

> ---
>   net/ipv6/mcast.c | 76 ++++++++++++++++++++++--------------------------
>   1 file changed, 35 insertions(+), 41 deletions(-)
> 
> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index 36ca27496b3c..75430ad55c3d 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -169,6 +169,29 @@ static int unsolicited_report_interval(struct inet6_dev *idev)
>   	return iv > 0 ? iv : 1;
>   }
>   
> +static struct net_device *ip6_mc_find_dev(struct net *net,
> +					  const struct in6_addr *group,
> +					  int ifindex)
> +{
> +	struct net_device *dev = NULL;
> +	struct rt6_info *rt;
> +
> +	if (ifindex == 0) {
> +		rcu_read_lock();
> +		rt = rt6_lookup(net, group, NULL, 0, NULL, 0);
> +		if (rt) {
> +			dev = dst_dev(&rt->dst);
> +			dev_hold(dev);
> +			ip6_rt_put(rt);
> +		}
> +		rcu_read_unlock();
> +	} else {
> +		dev = dev_get_by_index(net, ifindex);
> +	}
> +
> +	return dev;
> +}
> +
>   /*
>    *	socket join on multicast group
>    */
> @@ -191,28 +214,13 @@ static int __ipv6_sock_mc_join(struct sock *sk, int ifindex,
>   	}
>   
>   	mc_lst = sock_kmalloc(sk, sizeof(struct ipv6_mc_socklist), GFP_KERNEL);
> -
>   	if (!mc_lst)
>   		return -ENOMEM;
>   
>   	mc_lst->next = NULL;
>   	mc_lst->addr = *addr;
>   
> -	if (ifindex == 0) {
> -		struct rt6_info *rt;
> -
> -		rcu_read_lock();
> -		rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
> -		if (rt) {
> -			dev = dst_dev(&rt->dst);
> -			dev_hold(dev);
> -			ip6_rt_put(rt);
> -		}
> -		rcu_read_unlock();
> -	} else {
> -		dev = dev_get_by_index(net, ifindex);
> -	}
> -
> +	dev = ip6_mc_find_dev(net, addr, ifindex);
>   	if (!dev) {
>   		sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
>   		return -ENODEV;
> @@ -302,32 +310,18 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
>   }
>   EXPORT_SYMBOL(ipv6_sock_mc_drop);
>   
> -static struct inet6_dev *ip6_mc_find_dev(struct net *net,
> -					 const struct in6_addr *group,
> -					 int ifindex)
> +static struct inet6_dev *ip6_mc_find_idev(struct net *net,
> +					  const struct in6_addr *group,
> +					  int ifindex)
>   {
> -	struct net_device *dev = NULL;
> -	struct inet6_dev *idev;
> -
> -	if (ifindex == 0) {
> -		struct rt6_info *rt;
> +	struct inet6_dev *idev = NULL;
> +	struct net_device *dev;
>   
> -		rcu_read_lock();
> -		rt = rt6_lookup(net, group, NULL, 0, NULL, 0);
> -		if (rt) {
> -			dev = dst_dev(&rt->dst);
> -			dev_hold(dev);
> -			ip6_rt_put(rt);
> -		}
> -		rcu_read_unlock();
> -	} else {
> -		dev = dev_get_by_index(net, ifindex);
> +	dev = ip6_mc_find_dev(net, group, ifindex);
> +	if (dev) {
> +		idev = in6_dev_get(dev);
> +		dev_put(dev);
>   	}
> -	if (!dev)
> -		return NULL;
> -
> -	idev = in6_dev_get(dev);
> -	dev_put(dev);
>   
>   	return idev;
>   }
> @@ -374,7 +368,7 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
>   	if (!ipv6_addr_is_multicast(group))
>   		return -EINVAL;
>   
> -	idev = ip6_mc_find_dev(net, group, pgsr->gsr_interface);
> +	idev = ip6_mc_find_idev(net, group, pgsr->gsr_interface);
>   	if (!idev)
>   		return -ENODEV;
>   
> @@ -509,7 +503,7 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf,
>   	    gsf->gf_fmode != MCAST_EXCLUDE)
>   		return -EINVAL;
>   
> -	idev = ip6_mc_find_dev(net, group, gsf->gf_interface);
> +	idev = ip6_mc_find_idev(net, group, gsf->gf_interface);
>   	if (!idev)
>   		return -ENODEV;
>