* [PATCH iproute2] ip: do not exit if RTM_GETNSID failed
@ 2020-09-21 23:53 Jan Engelhardt
2020-09-22 0:22 ` Stephen Hemminger
0 siblings, 1 reply; 6+ messages in thread
From: Jan Engelhardt @ 2020-09-21 23:53 UTC (permalink / raw)
To: stephen; +Cc: netdev
`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(-)
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);
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH iproute2] ip: do not exit if RTM_GETNSID failed 2020-09-21 23:53 [PATCH iproute2] ip: do not exit if RTM_GETNSID failed Jan Engelhardt @ 2020-09-22 0:22 ` Stephen Hemminger 2020-09-22 6:28 ` Jan Engelhardt 0 siblings, 1 reply; 6+ messages in thread From: Stephen Hemminger @ 2020-09-22 0:22 UTC (permalink / raw) To: Jan Engelhardt; +Cc: netdev 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH iproute2] ip: do not exit if RTM_GETNSID failed 2020-09-22 0:22 ` Stephen Hemminger @ 2020-09-22 6:28 ` Jan Engelhardt 2020-09-22 23:16 ` David Ahern 0 siblings, 1 reply; 6+ messages in thread From: Jan Engelhardt @ 2020-09-22 6:28 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH iproute2] ip: do not exit if RTM_GETNSID failed 2020-09-22 6:28 ` Jan Engelhardt @ 2020-09-22 23:16 ` David Ahern 2020-09-22 23:57 ` Stephen Hemminger 0 siblings, 1 reply; 6+ messages in thread From: David Ahern @ 2020-09-22 23:16 UTC (permalink / raw) To: Jan Engelhardt, Stephen Hemminger; +Cc: netdev 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH iproute2] ip: do not exit if RTM_GETNSID failed 2020-09-22 23:16 ` David Ahern @ 2020-09-22 23:57 ` Stephen Hemminger 2020-09-23 0:11 ` David Ahern 0 siblings, 1 reply; 6+ messages in thread From: Stephen Hemminger @ 2020-09-22 23:57 UTC (permalink / raw) To: David Ahern; +Cc: Jan Engelhardt, netdev 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH iproute2] ip: do not exit if RTM_GETNSID failed 2020-09-22 23:57 ` Stephen Hemminger @ 2020-09-23 0:11 ` David Ahern 0 siblings, 0 replies; 6+ messages in thread From: David Ahern @ 2020-09-23 0:11 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Jan Engelhardt, netdev 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-23 0:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-21 23:53 [PATCH iproute2] ip: do not exit if RTM_GETNSID failed Jan Engelhardt 2020-09-22 0:22 ` Stephen Hemminger 2020-09-22 6:28 ` Jan Engelhardt 2020-09-22 23:16 ` David Ahern 2020-09-22 23:57 ` Stephen Hemminger 2020-09-23 0:11 ` David Ahern
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).