diff mbox series

[RFC] add extack errors for iptoken

Message ID 20210331204902.78d87b40@hermes.local
State New
Headers show
Series [RFC] add extack errors for iptoken | expand

Commit Message

Stephen Hemminger April 1, 2021, 3:49 a.m. UTC 
Perhaps the following (NOT TESTED) kernel patch will show you how such error messages
could be added.

Note: requires trickling down the extack parameter, but that is a good thing because
other place like devconf could use it as well.

Comments

Hongren Zheng April 28, 2021, 12:55 p.m. UTC | #1 
> Perhaps the following (NOT TESTED) kernel patch will show you how such error messages

> could be added.


Since this patch has been tested, and we have waited a long time for
comments and there is no further response, I wonder if it is the time
to submit this patch to the kernel.
David Ahern April 28, 2021, 3:32 p.m. UTC | #2 
On 4/28/21 6:55 AM, Hongren Zheng wrote:
>> Perhaps the following (NOT TESTED) kernel patch will show you how such error messages

>> could be added.

> 

> Since this patch has been tested, and we have waited a long time for

> comments and there is no further response, I wonder if it is the time

> to submit this patch to the kernel.

> 


send the patch
Hongren Zheng May 29, 2021, 6:31 a.m. UTC | #3 
On Wed, Apr 28, 2021 at 08:55:08PM +0800, Hongren Zheng wrote:
> > Perhaps the following (NOT TESTED) kernel patch will show you how such error messages

> > could be added.

> 

> Since this patch has been tested, and we have waited a long time for

> comments and there is no further response, I wonder if it is the time

> to submit this patch to the kernel.


Is there any updates?

I'm not quite familiar with "RFC" procedure. Should I send this patch to
netdev mailing list with title "[PATCH] add extack errors for iptoken" now
(I suppose not), or wait for Stephen Hemminger sending it, or wait for
more comments?

-- 
GPG Fingerprint: 1127F188280AE3123619332987E17EEF9B18B6C9
Stephen Hemminger May 31, 2021, 2:42 a.m. UTC | #4 
On Sat, 29 May 2021 14:31:56 +0800
Hongren Zheng <i@zenithal.me> wrote:

> On Wed, Apr 28, 2021 at 08:55:08PM +0800, Hongren Zheng wrote:

> > > Perhaps the following (NOT TESTED) kernel patch will show you how such error messages

> > > could be added.  

> > 

> > Since this patch has been tested, and we have waited a long time for

> > comments and there is no further response, I wonder if it is the time

> > to submit this patch to the kernel.  

> 

> Is there any updates?

> 

> I'm not quite familiar with "RFC" procedure. Should I send this patch to

> netdev mailing list with title "[PATCH] add extack errors for iptoken" now

> (I suppose not), or wait for Stephen Hemminger sending it, or wait for

> more comments?

> 


The kernel changes is already upstream with this commit for 5.12 kernel

commit 3583a4e8d77d44697a21437227dd53fc6e7b2cb5
Author: Stephen Hemminger <stephen@networkplumber.org>
Date:   Wed Apr 7 08:59:12 2021 -0700

    ipv6: report errors for iftoken via netlink extack
    
    Setting iftoken can fail for several different reasons but there
    and there was no report to user as to the cause. Add netlink
    extended errors to the processing of the request.
    
    This requires adding additional argument through rtnl_af_ops
    set_link_af callback.
    
    Reported-by: Hongren Zheng <li@zenithal.me>
    Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

    Reviewed-by: David Ahern <dsahern@kernel.org>

    Signed-off-by: David S. Miller <davem@davemloft.net>
Hongren Zheng May 31, 2021, 5:26 a.m. UTC | #5 
On Sun, May 30, 2021 at 07:42:34PM -0700, Stephen Hemminger wrote:
> On Sat, 29 May 2021 14:31:56 +0800

> Hongren Zheng <i@zenithal.me> wrote:

> 

> > On Wed, Apr 28, 2021 at 08:55:08PM +0800, Hongren Zheng wrote:

> > > > Perhaps the following (NOT TESTED) kernel patch will show you how such error messages

> > > > could be added.  

> > > 

> > > Since this patch has been tested, and we have waited a long time for

> > > comments and there is no further response, I wonder if it is the time

> > > to submit this patch to the kernel.  

> > 

> > Is there any updates?

> > 

> > I'm not quite familiar with "RFC" procedure. Should I send this patch to

> > netdev mailing list with title "[PATCH] add extack errors for iptoken" now

> > (I suppose not), or wait for Stephen Hemminger sending it, or wait for

> > more comments?

> > 

> 

> The kernel changes is already upstream with this commit for 5.12 kernel

> 

> commit 3583a4e8d77d44697a21437227dd53fc6e7b2cb5

> Author: Stephen Hemminger <stephen@networkplumber.org>

> Date:   Wed Apr 7 08:59:12 2021 -0700

> 

>     ipv6: report errors for iftoken via netlink extack

>     

>     Setting iftoken can fail for several different reasons but there

>     and there was no report to user as to the cause. Add netlink

>     extended errors to the processing of the request.

>     

>     This requires adding additional argument through rtnl_af_ops

>     set_link_af callback.

>     

>     Reported-by: Hongren Zheng <li@zenithal.me>


No wonder I did not receive this email and kept pinging in this thread.

My email address is i@zenithal.me, not li@zenithal.me

Still thank you for getting this upstreamed!

>     Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

>     Reviewed-by: David Ahern <dsahern@kernel.org>

>     Signed-off-by: David S. Miller <davem@davemloft.net>
diff mbox series

Patch

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 4da61c950e93..479f60ef54c0 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -147,8 +147,8 @@  struct rtnl_af_ops {
 	int			(*validate_link_af)(const struct net_device *dev,
 						    const struct nlattr *attr);
 	int			(*set_link_af)(struct net_device *dev,
-					       const struct nlattr *attr);
-
+					       const struct nlattr *attr,
+					       struct netlink_ext_ack *extack);
 	int			(*fill_stats_af)(struct sk_buff *skb,
 						 const struct net_device *dev);
 	size_t			(*get_stats_af_size)(const struct net_device *dev);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1bdcb33fb561..3485b16a7ff3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2863,7 +2863,7 @@  static int do_setlink(const struct sk_buff *skb,
 
 			BUG_ON(!(af_ops = rtnl_af_lookup(nla_type(af))));
 
-			err = af_ops->set_link_af(dev, af);
+			err = af_ops->set_link_af(dev, af, extack);
 			if (err < 0) {
 				rcu_read_unlock();
 				goto errout;
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 75f67994fc85..2e35f68da40a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1978,7 +1978,8 @@  static int inet_validate_link_af(const struct net_device *dev,
 	return 0;
 }
 
-static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla)
+static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla,
+			    struct netlink_ext_ack *extack)
 {
 	struct in_device *in_dev = __in_dev_get_rcu(dev);
 	struct nlattr *a, *tb[IFLA_INET_MAX+1];
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 120073ffb666..b817086fbf42 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5672,7 +5672,8 @@  static int inet6_fill_link_af(struct sk_buff *skb, const struct net_device *dev,
 	return 0;
 }
 
-static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
+static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token,
+			     struct netlink_ext_ack *extack)
 {
 	struct inet6_ifaddr *ifp;
 	struct net_device *dev = idev->dev;
@@ -5681,14 +5682,29 @@  static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 
 	ASSERT_RTNL();
 
-	if (!token)
+	if (!token) {
 		return -EINVAL;
-	if (dev->flags & (IFF_LOOPBACK | IFF_NOARP))
+	}
+
+	if (dev->flags & IFF_LOOPBACK) {
+		NL_SET_ERR_MSG_MOD(extack, "Device is loopback");
 		return -EINVAL;
-	if (!ipv6_accept_ra(idev))
+	}
+
+	if (dev->flags & IFF_NOARP) {
+		NL_SET_ERR_MSG_MOD(extack, "Device does not do discovery");
 		return -EINVAL;
-	if (idev->cnf.rtr_solicits == 0)
+	}
+
+	if (!ipv6_accept_ra(idev)) {
+		NL_SET_ERR_MSG_MOD(extack, "Device does accept route adverts");
+		return -EINVAL;
+	}
+
+	if (idev->cnf.rtr_solicits == 0) {
+		NL_SET_ERR_MSG(extack, "Device has disabled router solicitation");
 		return -EINVAL;
+	}
 
 	write_lock_bh(&idev->lock);
 
@@ -5796,7 +5812,8 @@  static int inet6_validate_link_af(const struct net_device *dev,
 	return 0;
 }
 
-static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla)
+static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla,
+			     struct netlink_ext_ack *extack)
 {
 	struct inet6_dev *idev = __in6_dev_get(dev);
 	struct nlattr *tb[IFLA_INET6_MAX + 1];
@@ -5809,7 +5826,8 @@  static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla)
 		BUG();
 
 	if (tb[IFLA_INET6_TOKEN]) {
-		err = inet6_set_iftoken(idev, nla_data(tb[IFLA_INET6_TOKEN]));
+		err = inet6_set_iftoken(idev, nla_data(tb[IFLA_INET6_TOKEN]),
+					extack);
 		if (err)
 			return err;
 	}