diff mbox series

[net-next] IPv6: add extack info for inet6_addr_add/del

Message ID 20230717093316.2428813-1-liuhangbin@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] IPv6: add extack info for inet6_addr_add/del | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1342 this patch: 1342
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 1365 this patch: 1365
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1365 this patch: 1365
netdev/checkpatch warning WARNING: line length of 103 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Hangbin Liu July 17, 2023, 9:33 a.m. UTC 
Add extack info for inet6_addr_add(), ipv6_add_addr() and
inet6_addr_del(), which would be useful for users to understand the
problem without having to read kernel code.

Suggested-by: Beniamino Galvani <bgalvani@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv6/addrconf.c | 66 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 48 insertions(+), 18 deletions(-)

Comments

David Ahern July 18, 2023, 2:43 p.m. UTC | #1 
On 7/17/23 3:33 AM, Hangbin Liu wrote:
> Add extack info for inet6_addr_add(), ipv6_add_addr() and
> inet6_addr_del(), which would be useful for users to understand the
> problem without having to read kernel code.
> 
> Suggested-by: Beniamino Galvani <bgalvani@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  net/ipv6/addrconf.c | 66 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 48 insertions(+), 18 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index e5213e598a04..199de4b37f24 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1066,15 +1066,19 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>  	     !(cfg->ifa_flags & IFA_F_MCAUTOJOIN)) ||
>  	    (!(idev->dev->flags & IFF_LOOPBACK) &&
>  	     !netif_is_l3_master(idev->dev) &&
> -	     addr_type & IPV6_ADDR_LOOPBACK))
> +	     addr_type & IPV6_ADDR_LOOPBACK)) {
> +		NL_SET_ERR_MSG(extack, "Cannot assign requested address");
>  		return ERR_PTR(-EADDRNOTAVAIL);
> +	}
>  
>  	if (idev->dead) {
> -		err = -ENODEV;			/*XXX*/
> +		NL_SET_ERR_MSG(extack, "No such device");

ENODEV error string gives the same information. Here we can be more
informative with something like "Device marked as dead".


> +		err = -ENODEV;
>  		goto out;
>  	}
>  
>  	if (idev->cnf.disable_ipv6) {
> +		NL_SET_ERR_MSG(extack, "IPv6 is disabled on this device");
>  		err = -EACCES;
>  		goto out;
>  	}
> @@ -1097,12 +1101,14 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>  
>  	ifa = kzalloc(sizeof(*ifa), gfp_flags | __GFP_ACCOUNT);
>  	if (!ifa) {
> +		NL_SET_ERR_MSG(extack, "No buffer space available");

If I recall correctly, extack messages are not returned for memory
allocation failure since it will be the same as strerror(ENOMEM) and
strerror(ENOBUFS).


>  		err = -ENOBUFS;
>  		goto out;
>  	}
>  
>  	f6i = addrconf_f6i_alloc(net, idev, cfg->pfx, false, gfp_flags);
>  	if (IS_ERR(f6i)) {
> +		NL_SET_ERR_MSG(extack, "Dest allocate failed");
>  		err = PTR_ERR(f6i);
>  		f6i = NULL;
>  		goto out;
> @@ -1142,6 +1148,7 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>  
>  	err = ipv6_add_addr_hash(idev->dev, ifa);
>  	if (err < 0) {
> +		NL_SET_ERR_MSG(extack, "IPv6 address add failed");

Add extack to ipv6_add_addr_hash and convert the debug string in that
function to to return the error.

>  		rcu_read_unlock();
>  		goto out;
>  	}
diff mbox series

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index e5213e598a04..199de4b37f24 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1066,15 +1066,19 @@  ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 	     !(cfg->ifa_flags & IFA_F_MCAUTOJOIN)) ||
 	    (!(idev->dev->flags & IFF_LOOPBACK) &&
 	     !netif_is_l3_master(idev->dev) &&
-	     addr_type & IPV6_ADDR_LOOPBACK))
+	     addr_type & IPV6_ADDR_LOOPBACK)) {
+		NL_SET_ERR_MSG(extack, "Cannot assign requested address");
 		return ERR_PTR(-EADDRNOTAVAIL);
+	}
 
 	if (idev->dead) {
-		err = -ENODEV;			/*XXX*/
+		NL_SET_ERR_MSG(extack, "No such device");
+		err = -ENODEV;
 		goto out;
 	}
 
 	if (idev->cnf.disable_ipv6) {
+		NL_SET_ERR_MSG(extack, "IPv6 is disabled on this device");
 		err = -EACCES;
 		goto out;
 	}
@@ -1097,12 +1101,14 @@  ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 
 	ifa = kzalloc(sizeof(*ifa), gfp_flags | __GFP_ACCOUNT);
 	if (!ifa) {
+		NL_SET_ERR_MSG(extack, "No buffer space available");
 		err = -ENOBUFS;
 		goto out;
 	}
 
 	f6i = addrconf_f6i_alloc(net, idev, cfg->pfx, false, gfp_flags);
 	if (IS_ERR(f6i)) {
+		NL_SET_ERR_MSG(extack, "Dest allocate failed");
 		err = PTR_ERR(f6i);
 		f6i = NULL;
 		goto out;
@@ -1142,6 +1148,7 @@  ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 
 	err = ipv6_add_addr_hash(idev->dev, ifa);
 	if (err < 0) {
+		NL_SET_ERR_MSG(extack, "IPv6 address add failed");
 		rcu_read_unlock();
 		goto out;
 	}
@@ -2488,18 +2495,22 @@  static void addrconf_add_mroute(struct net_device *dev)
 	ip6_route_add(&cfg, GFP_KERNEL, NULL);
 }
 
-static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
+static struct inet6_dev *addrconf_add_dev(struct net_device *dev, struct netlink_ext_ack *extack)
 {
 	struct inet6_dev *idev;
 
 	ASSERT_RTNL();
 
 	idev = ipv6_find_idev(dev);
-	if (IS_ERR(idev))
+	if (IS_ERR(idev)) {
+		NL_SET_ERR_MSG(extack, "No such device");
 		return idev;
+	}
 
-	if (idev->cnf.disable_ipv6)
+	if (idev->cnf.disable_ipv6) {
+		NL_SET_ERR_MSG(extack, "IPv6 is disabled on this device");
 		return ERR_PTR(-EACCES);
+	}
 
 	/* Add default multicast route */
 	if (!(dev->flags & IFF_LOOPBACK) && !netif_is_l3_master(dev))
@@ -2919,21 +2930,29 @@  static int inet6_addr_add(struct net *net, int ifindex,
 
 	ASSERT_RTNL();
 
-	if (cfg->plen > 128)
+	if (cfg->plen > 128) {
+		NL_SET_ERR_MSG(extack, "IPv6 address prefix length larger than 128");
 		return -EINVAL;
+	}
 
 	/* check the lifetime */
-	if (!cfg->valid_lft || cfg->preferred_lft > cfg->valid_lft)
+	if (!cfg->valid_lft || cfg->preferred_lft > cfg->valid_lft) {
+		NL_SET_ERR_MSG(extack, "IPv6 address lifetime invalid");
 		return -EINVAL;
+	}
 
-	if (cfg->ifa_flags & IFA_F_MANAGETEMPADDR && cfg->plen != 64)
+	if (cfg->ifa_flags & IFA_F_MANAGETEMPADDR && cfg->plen != 64) {
+		NL_SET_ERR_MSG(extack, "IPv6 address with mngtmpaddr flag must have prefix length 64");
 		return -EINVAL;
+	}
 
 	dev = __dev_get_by_index(net, ifindex);
-	if (!dev)
+	if (!dev) {
+		NL_SET_ERR_MSG(extack, "Unable to find the interface");
 		return -ENODEV;
+	}
 
-	idev = addrconf_add_dev(dev);
+	idev = addrconf_add_dev(dev, extack);
 	if (IS_ERR(idev))
 		return PTR_ERR(idev);
 
@@ -2941,8 +2960,10 @@  static int inet6_addr_add(struct net *net, int ifindex,
 		int ret = ipv6_mc_config(net->ipv6.mc_autojoin_sk,
 					 true, cfg->pfx, ifindex);
 
-		if (ret < 0)
+		if (ret < 0) {
+			NL_SET_ERR_MSG(extack, "Multicast auto join failed");
 			return ret;
+		}
 	}
 
 	cfg->scope = ipv6_addr_scope(cfg->pfx);
@@ -2999,22 +3020,29 @@  static int inet6_addr_add(struct net *net, int ifindex,
 }
 
 static int inet6_addr_del(struct net *net, int ifindex, u32 ifa_flags,
-			  const struct in6_addr *pfx, unsigned int plen)
+			  const struct in6_addr *pfx, unsigned int plen,
+			  struct netlink_ext_ack *extack)
 {
 	struct inet6_ifaddr *ifp;
 	struct inet6_dev *idev;
 	struct net_device *dev;
 
-	if (plen > 128)
+	if (plen > 128) {
+		NL_SET_ERR_MSG(extack, "IPv6 address prefix length larger than 128");
 		return -EINVAL;
+	}
 
 	dev = __dev_get_by_index(net, ifindex);
-	if (!dev)
+	if (!dev) {
+		NL_SET_ERR_MSG(extack, "Unable to find the interface");
 		return -ENODEV;
+	}
 
 	idev = __in6_dev_get(dev);
-	if (!idev)
+	if (!idev) {
+		NL_SET_ERR_MSG(extack, "No such address on the device");
 		return -ENXIO;
+	}
 
 	read_lock_bh(&idev->lock);
 	list_for_each_entry(ifp, &idev->addr_list, if_list) {
@@ -3037,6 +3065,8 @@  static int inet6_addr_del(struct net *net, int ifindex, u32 ifa_flags,
 		}
 	}
 	read_unlock_bh(&idev->lock);
+
+	NL_SET_ERR_MSG(extack, "IPv6 address not found");
 	return -EADDRNOTAVAIL;
 }
 
@@ -3079,7 +3109,7 @@  int addrconf_del_ifaddr(struct net *net, void __user *arg)
 
 	rtnl_lock();
 	err = inet6_addr_del(net, ireq.ifr6_ifindex, 0, &ireq.ifr6_addr,
-			     ireq.ifr6_prefixlen);
+			     ireq.ifr6_prefixlen, NULL);
 	rtnl_unlock();
 	return err;
 }
@@ -3378,7 +3408,7 @@  static void addrconf_dev_config(struct net_device *dev)
 		return;
 	}
 
-	idev = addrconf_add_dev(dev);
+	idev = addrconf_add_dev(dev, NULL);
 	if (IS_ERR(idev))
 		return;
 
@@ -4692,7 +4722,7 @@  inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	ifa_flags &= IFA_F_MANAGETEMPADDR;
 
 	return inet6_addr_del(net, ifm->ifa_index, ifa_flags, pfx,
-			      ifm->ifa_prefixlen);
+			      ifm->ifa_prefixlen, extack);
 }
 
 static int modify_prefix_route(struct inet6_ifaddr *ifp,