Re: iproute2: no error message when link up command fails.

[Patrick McHardy <kaber@trash.net>]

[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]

Andreas Henriksson wrote:
> On ons, 2008-07-16 at 15:53 -0700, Stephen Hemminger wrote:
>> The netlink message in question is marked as type ERROR but the errno
>> encoded in the message is zero.
>>
>> 		if (h->nlmsg_type == NLMSG_ERROR) {
>> 			struct nlmsgerr *err = (struct nlmsgerr*)NLMSG_DATA(h);
>> 			if (l < sizeof(struct nlmsgerr)) {
>> 				fprintf(stderr, "ERROR truncated\n");
>> 			} else {
>> 				errno = -err->error;
>> 				if (errno == 0) {
>> 					if (answer)
>> 						memcpy(answer, h, h->nlmsg_len);
>> 					return 0;
>> 				}
>> 				perror("RTNETLINK answers");
>> 			}
>>
>> So the netlink library just treats as a successful return.
> Why? This seems like a really bad idea to me, and none of the callers in
> iproute benefits from this as far as I can see.
> 
> Just ripping out the errno == 0 special casing looks like and option to
> me, unless anyone can find a reason for it.

NLMSG_ERROR with errno == 0 is a netlink ACK message.

> (It'll give an error message and an error exit code! The message will be
> strange, but lets blame the kernel for that cosmetic issue. Atleast the
> user got some kind of notification.)
> 
> Moving the "return 0;" inside the "if (answer)" would be another
> (atleast for iproutes callers of the library functions)...
> 
>> To me it looks like the problem is in the kernel sending back
>> a NLMSG_ERROR with errno of zero. Some code path isn't setting
>> it up properly.
> 
> None the less, it would be be good if the application wouldn't poop it's
> pants when it can be avoided - broken kernel or not.

The fix in this case is to propagate the return value from
dev_change_flags().



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 784 bytes --]

rtnetlink: propagate error from dev_change_flags in do_setlink()

Andreas Henriksson <andreas@fatal.se> reported that unlike ifconfig,
iproute doesn't report an error when setting an interface up fails.

Propagate the return value from dev_change_flags() to fix this.

Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a9a7721..ffde766 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -867,7 +867,9 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 		if (ifm->ifi_change)
 			flags = (flags & ifm->ifi_change) |
 				(dev->flags & ~ifm->ifi_change);
-		dev_change_flags(dev, flags);
+		err = dev_change_flags(dev, flags);
+		if (err < 0)
+			goto errout;
 	}
 
 	if (tb[IFLA_TXQLEN])