diff mbox series

[iproute2] ip: do not exit if RTM_GETNSID failed

Message ID 20200921235318.14001-1-jengelh@inai.de
State New
Headers show
Series [iproute2] ip: do not exit if RTM_GETNSID failed | expand

Commit Message

Jan Engelhardt Sept. 21, 2020, 11:53 p.m. UTC
`ip addr` when run under qemu-user-riscv64, fails. This likely is
due to qemu-5.1 not doing translation of RTM_GETNSID calls.

2: host0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 5a:44:da:1a:c4:0b brd ff:ff:ff:ff:ff:ff
request send failed: Operation not supported

Treat the situation similar to an absence of procfs.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 ip/ipnetns.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Stephen Hemminger Sept. 22, 2020, 12:22 a.m. UTC | #1
On Tue, 22 Sep 2020 01:53:18 +0200
Jan Engelhardt <jengelh@inai.de> wrote:

> `ip addr` when run under qemu-user-riscv64, fails. This likely is

> due to qemu-5.1 not doing translation of RTM_GETNSID calls.

> 

> 2: host0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000

>     link/ether 5a:44:da:1a:c4:0b brd ff:ff:ff:ff:ff:ff

> request send failed: Operation not supported

> 

> Treat the situation similar to an absence of procfs.

> 

> Signed-off-by: Jan Engelhardt <jengelh@inai.de>


Not a good idea to hide a platform bug in ip command.

When you do this, you risk creating all sorts of issues for people that
run ip commands in container environments where the send is rejected (perhaps by SELinux)
and then things go off into a different failure.
Jan Engelhardt Sept. 22, 2020, 6:28 a.m. UTC | #2
On Tuesday 2020-09-22 02:22, Stephen Hemminger wrote:
>Jan Engelhardt <jengelh@inai.de> wrote:

>

>> `ip addr` when run under qemu-user-riscv64, fails. This likely is

>> due to qemu-5.1 not doing translation of RTM_GETNSID calls.

>> 

>> 2: host0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000

>>     link/ether 5a:44:da:1a:c4:0b brd ff:ff:ff:ff:ff:ff

>> request send failed: Operation not supported

>> 

>> Treat the situation similar to an absence of procfs.

>> 

>> Signed-off-by: Jan Engelhardt <jengelh@inai.de>

>

>Not a good idea to hide a platform bug in ip command.

>When you do this, you risk creating all sorts of issues for people that

>run ip commands in container environments where the send is rejected (perhaps by SELinux)

>and then things go off into a different failure.


In the very same function you do

  fd = open("/proc/self/ns/net", O_RDONLY);

which equally hides a potential platform bug (namely, forgetting to
mount /proc in a chroot, or in case SELinux was improperly set-up).
Why is this measured two different ways?


From 4222d059c910136f5e2b5c6de96ddaf06828c1d5 Mon Sep 17 00:00:00 2001
From: Jan Engelhardt <jengelh@inai.de>

Date: Tue, 22 Sep 2020 01:41:50 +0200
Subject: [PATCH] ip: add error reporting when RTM_GETNSID failed

`ip addr` when run under qemu-user-riscv64, fails. This likely is
due to qemu-5.1 not doing translation of RTM_GETNSID calls. Aborting
ip completely is the wrong response however. This patch reworks the
error handling.

2: host0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 5a:44:da:1a:c4:0b brd ff:ff:ff:ff:ff:ff
request send failed: Operation not supported

Signed-off-by: Jan Engelhardt <jengelh@inai.de>

---
 ip/ipnetns.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 46cc235b..e7a45653 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -78,6 +78,8 @@ static int ipnetns_have_nsid(void)
 	if (have_rtnl_getnsid < 0) {
 		fd = open("/proc/self/ns/net", O_RDONLY);
 		if (fd < 0) {
+			fprintf(stderr, "/proc/self/ns/net: %s. "
+				"Continuing anyway.\n", strerror(errno));
 			have_rtnl_getnsid = 0;
 			return 0;
 		}
@@ -85,8 +87,11 @@ static int ipnetns_have_nsid(void)
 		addattr32(&req.n, 1024, NETNSA_FD, fd);
 
 		if (rtnl_send(&rth, &req.n, req.n.nlmsg_len) < 0) {
-			perror("request send failed");
-			exit(1);
+			fprintf(stderr, "rtnl_send(RTM_GETNSID): %s. "
+				"Continuing anyway.\n", strerror(errno));
+			have_rtnl_getnsid = 0;
+			close(fd);
+			return 0;
 		}
 		rtnl_listen(&rth, ipnetns_accept_msg, NULL);
 		close(fd);
-- 
2.28.0
David Ahern Sept. 22, 2020, 11:16 p.m. UTC | #3
On 9/22/20 12:28 AM, Jan Engelhardt wrote:
> 

> On Tuesday 2020-09-22 02:22, Stephen Hemminger wrote:

>> Jan Engelhardt <jengelh@inai.de> wrote:

>>

>>> `ip addr` when run under qemu-user-riscv64, fails. This likely is

>>> due to qemu-5.1 not doing translation of RTM_GETNSID calls.

>>>

>>> 2: host0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000

>>>     link/ether 5a:44:da:1a:c4:0b brd ff:ff:ff:ff:ff:ff

>>> request send failed: Operation not supported

>>>

>>> Treat the situation similar to an absence of procfs.

>>>

>>> Signed-off-by: Jan Engelhardt <jengelh@inai.de>

>>

>> Not a good idea to hide a platform bug in ip command.

>> When you do this, you risk creating all sorts of issues for people that

>> run ip commands in container environments where the send is rejected (perhaps by SELinux)

>> and then things go off into a different failure.

> 

> In the very same function you do

> 

>   fd = open("/proc/self/ns/net", O_RDONLY);

> 

> which equally hides a potential platform bug (namely, forgetting to

> mount /proc in a chroot, or in case SELinux was improperly set-up).

> Why is this measured two different ways?

> 

> 


I think checking for EOPNOTSUPP error is more appropriate than ignoring
all errors.
Stephen Hemminger Sept. 22, 2020, 11:57 p.m. UTC | #4
On Tue, 22 Sep 2020 17:16:46 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 9/22/20 12:28 AM, Jan Engelhardt wrote:
> > 
> > On Tuesday 2020-09-22 02:22, Stephen Hemminger wrote:  
> >> Jan Engelhardt <jengelh@inai.de> wrote:
> >>  
> >>> `ip addr` when run under qemu-user-riscv64, fails. This likely is
> >>> due to qemu-5.1 not doing translation of RTM_GETNSID calls.
> >>>
> >>> 2: host0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
> >>>     link/ether 5a:44:da:1a:c4:0b brd ff:ff:ff:ff:ff:ff
> >>> request send failed: Operation not supported
> >>>
> >>> Treat the situation similar to an absence of procfs.
> >>>
> >>> Signed-off-by: Jan Engelhardt <jengelh@inai.de>  
> >>
> >> Not a good idea to hide a platform bug in ip command.
> >> When you do this, you risk creating all sorts of issues for people that
> >> run ip commands in container environments where the send is rejected (perhaps by SELinux)
> >> and then things go off into a different failure.  
> > 
> > In the very same function you do
> > 
> >   fd = open("/proc/self/ns/net", O_RDONLY);
> > 
> > which equally hides a potential platform bug (namely, forgetting to
> > mount /proc in a chroot, or in case SELinux was improperly set-up).
> > Why is this measured two different ways?
> > 
> >   
> 
> I think checking for EOPNOTSUPP error is more appropriate than ignoring
> all errors.
> 

Right, checking for not supported makes sense, but permission denied
is different.
David Ahern Sept. 23, 2020, 12:11 a.m. UTC | #5
On 9/22/20 5:57 PM, Stephen Hemminger wrote:
> On Tue, 22 Sep 2020 17:16:46 -0600

> David Ahern <dsahern@gmail.com> wrote:

> 

>> On 9/22/20 12:28 AM, Jan Engelhardt wrote:

>>>

>>> On Tuesday 2020-09-22 02:22, Stephen Hemminger wrote:  

>>>> Jan Engelhardt <jengelh@inai.de> wrote:

>>>>  

>>>>> `ip addr` when run under qemu-user-riscv64, fails. This likely is

>>>>> due to qemu-5.1 not doing translation of RTM_GETNSID calls.

>>>>>

>>>>> 2: host0@if5: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000

>>>>>     link/ether 5a:44:da:1a:c4:0b brd ff:ff:ff:ff:ff:ff

>>>>> request send failed: Operation not supported

>>>>>

>>>>> Treat the situation similar to an absence of procfs.

>>>>>

>>>>> Signed-off-by: Jan Engelhardt <jengelh@inai.de>  

>>>>

>>>> Not a good idea to hide a platform bug in ip command.

>>>> When you do this, you risk creating all sorts of issues for people that

>>>> run ip commands in container environments where the send is rejected (perhaps by SELinux)

>>>> and then things go off into a different failure.  

>>>

>>> In the very same function you do

>>>

>>>   fd = open("/proc/self/ns/net", O_RDONLY);

>>>

>>> which equally hides a potential platform bug (namely, forgetting to

>>> mount /proc in a chroot, or in case SELinux was improperly set-up).

>>> Why is this measured two different ways?

>>>

>>>   

>>

>> I think checking for EOPNOTSUPP error is more appropriate than ignoring

>> all errors.

>>

> 

> Right, checking for not supported makes sense, but permission denied

> is different.

> 


Sorry, I meant that comment for the original patch about RTM_GETNSID.
diff mbox series

Patch

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 46cc235b..8fab51cd 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -85,8 +85,9 @@  static int ipnetns_have_nsid(void)
 		addattr32(&req.n, 1024, NETNSA_FD, fd);
 
 		if (rtnl_send(&rth, &req.n, req.n.nlmsg_len) < 0) {
-			perror("request send failed");
-			exit(1);
+			have_rtnl_getnsid = 0;
+			close(fd);
+			return 0;
 		}
 		rtnl_listen(&rth, ipnetns_accept_msg, NULL);
 		close(fd);