Re: [PATCH net-next] geneve: use netlink_ext_ack for error reporting in rtnl operations

[David Miller]

From: Girish Moodalbail 
Date: Wed,  9 Aug 2017 01:09:28 -0700

> Add extack error messages for failure paths while creating/modifying
> geneve devices. Once extack support is added to iproute2, more
> meaningful and helpful error messages will be displayed making it easy
> for users to discern what went wrong.
> 
> Before:
> ===
> $ ip link add gen1 address 0:1:2:3:4:5:6 type geneve id 200 \
>   remote 192.168.13.2
> RTNETLINK answers: Invalid argument
> 
> After:
> ==
> $ ip link add gen1 address 0:1:2:3:4:5:6 type geneve id 200 \
>   remote 192.168.13.2
> Error: Provided link layer address is not Ethernet
> 
> Also, netdev_dbg() calls used to log errors associated with Netlink
> request have been removed.
> 
> Signed-off-by: Girish Moodalbail 

Applied, thanks.

[PATCH net-next] geneve: use netlink_ext_ack for error reporting in rtnl operations

[Girish Moodalbail]

Add extack error messages for failure paths while creating/modifying
geneve devices. Once extack support is added to iproute2, more
meaningful and helpful error messages will be displayed making it easy
for users to discern what went wrong.

Before:
===
$ ip link add gen1 address 0:1:2:3:4:5:6 type geneve id 200 \
  remote 192.168.13.2
RTNETLINK answers: Invalid argument

After:
==
$ ip link add gen1 address 0:1:2:3:4:5:6 type geneve id 200 \
  remote 192.168.13.2
Error: Provided link layer address is not Ethernet

Also, netdev_dbg() calls used to log errors associated with Netlink
request have been removed.

Signed-off-by: Girish Moodalbail 
---
 drivers/net/geneve.c | 128 ---
 1 file changed, 92 insertions(+), 36 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 745d57ae..977dbcc 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1086,21 +1086,33 @@ static int geneve_validate(struct nlattr *tb[], struct 
nlattr *data[],
   struct netlink_ext_ack *extack)
 {
if (tb[IFLA_ADDRESS]) {
-   if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+   if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN) {
+   NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
+   "Provided link layer address is not 
Ethernet");
return -EINVAL;
+   }
 
-   if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+   if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) {
+   NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
+   "Provided Ethernet address is not 
unicast");
return -EADDRNOTAVAIL;
+   }
}
 
-   if (!data)
+   if (!data) {
+   NL_SET_ERR_MSG(extack,
+  "Not enough attributes provided to perform the 
operation");
return -EINVAL;
+   }
 
if (data[IFLA_GENEVE_ID]) {
__u32 vni =  nla_get_u32(data[IFLA_GENEVE_ID]);
 
-   if (vni >= GENEVE_VID_MASK)
+   if (vni >= GENEVE_VID_MASK) {
+   NL_SET_ERR_MSG_ATTR(extack, data[IFLA_GENEVE_ID],
+   "Geneve ID must be lower than 
16777216");
return -ERANGE;
+   }
}
 
return 0;
@@ -1158,6 +1170,7 @@ static bool geneve_dst_addr_equal(struct ip_tunnel_info 
*a,
 }
 
 static int geneve_configure(struct net *net, struct net_device *dev,
+   struct netlink_ext_ack *extack,
const struct ip_tunnel_info *info,
bool metadata, bool ipv6_rx_csum)
 {
@@ -1166,8 +1179,11 @@ static int geneve_configure(struct net *net, struct 
net_device *dev,
bool tun_collect_md, tun_on_same_port;
int err, encap_len;
 
-   if (metadata && !is_tnl_info_zero(info))
+   if (metadata && !is_tnl_info_zero(info)) {
+   NL_SET_ERR_MSG(extack,
+  "Device is externally controlled, so attributes 
(VNI, Port, and so on) must not be specified");
return -EINVAL;
+   }
 
geneve->net = net;
geneve->dev = dev;
@@ -1188,11 +1204,17 @@ static int geneve_configure(struct net *net, struct 
net_device *dev,
dev->needed_headroom = encap_len + ETH_HLEN;
 
if (metadata) {
-   if (tun_on_same_port)
+   if (tun_on_same_port) {
+   NL_SET_ERR_MSG(extack,
+  "There can be only one externally 
controlled device on a destination port");
return -EPERM;
+   }
} else {
-   if (tun_collect_md)
+   if (tun_collect_md) {
+   NL_SET_ERR_MSG(extack,
+  "There already exists an externally 
controlled device on this destination port");
return -EPERM;
+   }
}
 
dst_cache_reset(>info.dst_cache);
@@ -1214,31 +1236,41 @@ static void init_tnl_info(struct ip_tunnel_info *info, 
__u16 dst_port)
info->key.tp_dst = htons(dst_port);
 }
 
-static int geneve_nl2info(struct net_device *dev, struct nlattr *tb[],
- struct nlattr *data[], struct ip_tunnel_info *info,
- bool *metadata, bool *use_udp6_rx_checksums,
- bool changelink)
+static int geneve_nl2info(struct nlattr *tb[], struct nlattr *data[],
+ struct netlink_ext_ack *extack,
+ struct ip_tunnel_info *info, bool *metadata,
+ bool *use_udp6_rx_checksums, bool changelink)
 {
-   if (data[IFLA_GENEVE_REMOTE] &&