| Message ID | 20200921235318.14001-1-jengelh@inai.de |
|---|---|
| State | Changes Requested |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | [iproute2] ip: do not exit if RTM_GETNSID failed | expand |
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.
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);
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.
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.
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 --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);
`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(-)