By using the helper, knowledge of network namespaces can be kept to within the helper.
Signed-off-by: Andrew Lunn andrew@lunn.ch --- vis/vis.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vis/vis.c b/vis/vis.c index c1f8dad..d1640b6 100644 --- a/vis/vis.c +++ b/vis/vis.c @@ -325,7 +325,7 @@ static int parse_orig_list(struct globals *globals) char path[1024]; struct vis_list_entry *v_entry;
- snprintf(path, sizeof(path), "/sys/kernel/debug/batman_adv/%s/originators", globals->interface); + debugfs_make_path(DEBUG_BATIF_PATH_FMT "/" "originators", globals->interface, path, sizeof(path)); fbuf = read_file(path); if (!fbuf) return -1;
When running within a network namespace, access to files within debugfs have to take into account the network name space. Each namespace has its own directory under /sys/kernel/debug/batman_adv/netns.
Signed-off-by: Andrew Lunn andrew@lunn.ch --- batadv_query.c | 2 +- debugfs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- vis/vis.h | 2 +- 3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/batadv_query.c b/batadv_query.c index c289b80..fe7c007 100644 --- a/batadv_query.c +++ b/batadv_query.c @@ -31,7 +31,7 @@ #include <sys/types.h> #include "debugfs.h"
-#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s" +#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s%s" #define DEBUG_TRANSTABLE_GLOBAL "transtable_global" #define DEBUG_ORIGINATORS "originators"
diff --git a/debugfs.c b/debugfs.c index fc39322..35c1bdf 100644 --- a/debugfs.c +++ b/debugfs.c @@ -20,11 +20,15 @@
#include "debugfs.h" #include <errno.h> +#include <fcntl.h> +#include <limits.h> #include <stdio.h> #include <string.h> #include <sys/mount.h> #include <sys/stat.h> #include <sys/statfs.h> +#include <sys/types.h> +#include <unistd.h>
#ifndef DEBUGFS_MAGIC #define DEBUGFS_MAGIC 0x64626720 @@ -42,16 +46,59 @@ static const char *debugfs_known_mountpoints[] = { NULL, };
+/* Return the current net namespace number. 0 is never a valid + * namespace, so use it to return that there is no name space + * support. + */ + +static unsigned int debugfs_get_netns_inum(void) +{ + char net_path[] = "/proc/self/ns/net"; + struct stat netst; + int netns; + + netns = open(net_path, O_RDONLY); + if (netns < 0) { + if (errno == ENOENT) + /* Probably means no netns support in the kernel */ + return 0; + + fprintf(stderr, + "Error - can't open /proc/self/ns/net for read: %s\n", + strerror(errno)); + return 0; + } + + if (fstat(netns, &netst) < 0) { + fprintf(stderr, "Stat of netns failed: %s\n", + strerror(errno)); + return 0; + } + close (netns); + + return netst.st_ino; +} + /* construct a full path to a debugfs element */ int debugfs_make_path(const char *fmt, const char *mesh_iface, char *buffer, int size) { + unsigned int ns = debugfs_get_netns_inum(); + char ns_dir[PATH_MAX]; + if (strlen(debugfs_mountpoint) == 0) { buffer[0] = '\0'; return -1; }
- return snprintf(buffer, size, fmt, debugfs_mountpoint, mesh_iface); + if (ns) { + snprintf(ns_dir, PATH_MAX, "netns/%u/", ns); + return snprintf(buffer, size, fmt, debugfs_mountpoint, ns_dir, + mesh_iface); + } else { + return snprintf(buffer, size, fmt, debugfs_mountpoint, "", + mesh_iface); + } }
static int debugfs_found; diff --git a/vis/vis.h b/vis/vis.h index 969ca33..9e5dc35 100644 --- a/vis/vis.h +++ b/vis/vis.h @@ -37,7 +37,7 @@ #define UPDATE_INTERVAL 10
#define SYS_IFACE_PATH "/sys/class/net" -#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s" +#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s%s" #define SYS_MESH_IFACE_FMT SYS_IFACE_PATH"/%s/batman_adv/mesh_iface" #define SYS_IFACE_STATUS_FMT SYS_IFACE_PATH"/%s/batman_adv/iface_status"
On Thursday 28 January 2016 04:53:14 Andrew Lunn wrote:
When running within a network namespace, access to files within debugfs have to take into account the network name space. Each namespace has its own directory under /sys/kernel/debug/batman_adv/netns.
Signed-off-by: Andrew Lunn andrew@lunn.ch
batadv_query.c | 2 +- debugfs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- vis/vis.h | 2 +- 3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/batadv_query.c b/batadv_query.c index c289b80..fe7c007 100644 --- a/batadv_query.c +++ b/batadv_query.c @@ -31,7 +31,7 @@ #include <sys/types.h> #include "debugfs.h"
-#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s" +#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s%s" #define DEBUG_TRANSTABLE_GLOBAL "transtable_global" #define DEBUG_ORIGINATORS "originators"
diff --git a/debugfs.c b/debugfs.c index fc39322..35c1bdf 100644 --- a/debugfs.c +++ b/debugfs.c @@ -20,11 +20,15 @@
#include "debugfs.h" #include <errno.h> +#include <fcntl.h> +#include <limits.h> #include <stdio.h> #include <string.h> #include <sys/mount.h> #include <sys/stat.h> #include <sys/statfs.h> +#include <sys/types.h> +#include <unistd.h>
#ifndef DEBUGFS_MAGIC #define DEBUGFS_MAGIC 0x64626720 @@ -42,16 +46,59 @@ static const char *debugfs_known_mountpoints[] = { NULL, };
+/* Return the current net namespace number. 0 is never a valid
- namespace, so use it to return that there is no name space
- support.
- */
+static unsigned int debugfs_get_netns_inum(void) +{
- char net_path[] = "/proc/self/ns/net";
- struct stat netst;
- int netns;
- netns = open(net_path, O_RDONLY);
- if (netns < 0) {
if (errno == ENOENT)/* Probably means no netns support in the kernel */return 0;fprintf(stderr,"Error - can't open /proc/self/ns/net for read: %s\n",strerror(errno));return 0;- }
- if (fstat(netns, &netst) < 0) {
fprintf(stderr, "Stat of netns failed: %s\n",strerror(errno));return 0;
Aren't we leaking the netns file descriptor here? You could just set netst.st_ino = 0 instead ...
Rest looks good to me :)
Cheers, Simon
On Fri, Jan 29, 2016 at 01:18:13PM +0100, Simon Wunderlich wrote:
On Thursday 28 January 2016 04:53:14 Andrew Lunn wrote:
When running within a network namespace, access to files within debugfs have to take into account the network name space. Each namespace has its own directory under /sys/kernel/debug/batman_adv/netns.
Signed-off-by: Andrew Lunn andrew@lunn.ch
batadv_query.c | 2 +- debugfs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- vis/vis.h | 2 +- 3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/batadv_query.c b/batadv_query.c index c289b80..fe7c007 100644 --- a/batadv_query.c +++ b/batadv_query.c @@ -31,7 +31,7 @@ #include <sys/types.h> #include "debugfs.h"
-#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s" +#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s%s" #define DEBUG_TRANSTABLE_GLOBAL "transtable_global" #define DEBUG_ORIGINATORS "originators"
diff --git a/debugfs.c b/debugfs.c index fc39322..35c1bdf 100644 --- a/debugfs.c +++ b/debugfs.c @@ -20,11 +20,15 @@
#include "debugfs.h" #include <errno.h> +#include <fcntl.h> +#include <limits.h> #include <stdio.h> #include <string.h> #include <sys/mount.h> #include <sys/stat.h> #include <sys/statfs.h> +#include <sys/types.h> +#include <unistd.h>
#ifndef DEBUGFS_MAGIC #define DEBUGFS_MAGIC 0x64626720 @@ -42,16 +46,59 @@ static const char *debugfs_known_mountpoints[] = { NULL, };
+/* Return the current net namespace number. 0 is never a valid
- namespace, so use it to return that there is no name space
- support.
- */
+static unsigned int debugfs_get_netns_inum(void) +{
- char net_path[] = "/proc/self/ns/net";
- struct stat netst;
- int netns;
- netns = open(net_path, O_RDONLY);
- if (netns < 0) {
if (errno == ENOENT)/* Probably means no netns support in the kernel */return 0;fprintf(stderr,"Error - can't open /proc/self/ns/net for read: %s\n",strerror(errno));return 0;- }
- if (fstat(netns, &netst) < 0) {
fprintf(stderr, "Stat of netns failed: %s\n",strerror(errno));return 0;Aren't we leaking the netns file descriptor here?
Yes, it is leaking. I will fix that in a v2.
Thanks Andrew
The debugfs helper code has the ability to mount the debugfs file system if it is not already mounted. However, it cannot do this after the capabilities have been dropped. So perform the mount early.
This is especially important when using network name spaces. Each namespace has its own /sys, so the mount of debugfs in the global namespace is not visible in other namespaces.
Signed-off-by: Andrew Lunn andrew@lunn.ch --- main.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/main.c b/main.c index 452d9ae..b1c5ec5 100644 --- a/main.c +++ b/main.c @@ -30,6 +30,7 @@ #include <unistd.h> #endif #include "alfred.h" +#include "debugfs.h" #include "packet.h" #include "list.h"
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0}, };
+ /* We need full capabilities to mount debugfs, so do that now */ + debugfs_mount(NULL); + ret = reduce_capabilities(); if (ret < 0) return NULL;
On Thursday 28 January 2016 04:53:15 Andrew Lunn wrote:
The debugfs helper code has the ability to mount the debugfs file system if it is not already mounted. However, it cannot do this after the capabilities have been dropped. So perform the mount early.
This is especially important when using network name spaces. Each namespace has its own /sys, so the mount of debugfs in the global namespace is not visible in other namespaces.
Signed-off-by: Andrew Lunn andrew@lunn.ch
main.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/main.c b/main.c index 452d9ae..b1c5ec5 100644 --- a/main.c +++ b/main.c @@ -30,6 +30,7 @@ #include <unistd.h> #endif #include "alfred.h" +#include "debugfs.h" #include "packet.h" #include "list.h"
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0}, };
- /* We need full capabilities to mount debugfs, so do that now */
- debugfs_mount(NULL);
- ret = reduce_capabilities(); if (ret < 0) return NULL;
Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in alfred ...
Cheers, Simon
On Fri, Jan 29, 2016 at 01:14:53PM +0100, Simon Wunderlich wrote:
On Thursday 28 January 2016 04:53:15 Andrew Lunn wrote:
The debugfs helper code has the ability to mount the debugfs file system if it is not already mounted. However, it cannot do this after the capabilities have been dropped. So perform the mount early.
This is especially important when using network name spaces. Each namespace has its own /sys, so the mount of debugfs in the global namespace is not visible in other namespaces.
Signed-off-by: Andrew Lunn andrew@lunn.ch
main.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/main.c b/main.c index 452d9ae..b1c5ec5 100644 --- a/main.c +++ b/main.c @@ -30,6 +30,7 @@ #include <unistd.h> #endif #include "alfred.h" +#include "debugfs.h" #include "packet.h" #include "list.h"
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0}, };
- /* We need full capabilities to mount debugfs, so do that now */
- debugfs_mount(NULL);
- ret = reduce_capabilities(); if (ret < 0) return NULL;
Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in alfred ...
Yes, the other calls within this binary are probably failing, due to reduced capabilities. I can remove them in a v3 patch.
Andrew
Hi Andrew,
On Tuesday 02 February 2016 03:25:12 Andrew Lunn wrote:
On Fri, Jan 29, 2016 at 01:14:53PM +0100, Simon Wunderlich wrote:
On Thursday 28 January 2016 04:53:15 Andrew Lunn wrote:
[...]
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0},
};
/* We need full capabilities to mount debugfs, so do that now */
debugfs_mount(NULL);
ret = reduce_capabilities(); if (ret < 0)
return NULL;
Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in alfred ...
Yes, the other calls within this binary are probably failing, due to reduced capabilities. I can remove them in a v3 patch.
Andrew
Are you still planning on sending a fixed patchset? :)
Thanks! Simon
On Thu, Mar 10, 2016 at 04:09:18PM +0100, Simon Wunderlich wrote:
Hi Andrew,
On Tuesday 02 February 2016 03:25:12 Andrew Lunn wrote:
On Fri, Jan 29, 2016 at 01:14:53PM +0100, Simon Wunderlich wrote:
On Thursday 28 January 2016 04:53:15 Andrew Lunn wrote:
[...]
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0},
};
/* We need full capabilities to mount debugfs, so do that now */
debugfs_mount(NULL);
ret = reduce_capabilities(); if (ret < 0)
return NULL;
Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in alfred ...
Yes, the other calls within this binary are probably failing, due to reduced capabilities. I can remove them in a v3 patch.
Andrew
Are you still planning on sending a fixed patchset? :)
Hi Simon
I have a fixed up version. But at the moment, it is not clear what is happening with the kernel patches. Maybe we need to change the debugfs API in order to make it easier to build with older kernels?
Andrew
On Thursday 28 January 2016 04:53:13 Andrew Lunn wrote:
By using the helper, knowledge of network namespaces can be kept to within the helper.
I've applied this one, since its a good change even without the namespace changes.
Applied in revision dca3ebf
Thanks! Simon
b.a.t.m.a.n@lists.open-mesh.org
-
Andrew Lunn -
Simon Wunderlich