[PATCH iproute2 0/4] ip vrf fixups

[PATCH iproute2 0/4] ip vrf fixups

[David Ahern]

Some minor cleanups to the 'ip vrf' command.

Patch 1 moves the CGROUP_BPF hint to the failure of prog_load since it
fails first.

Patch 2 refactors ipvrf_identify. The action part is moved to a function
that can be used standalone and in the process flipped to fopen/fgets for
robustness should the cgroups file grow larger than 4k.

Patch 3 fixes the path switching to "default" VRF.

Patch 4 moves a task to default VRF when switching namespaces.

David Ahern (4):
  ip vrf: Move kernel config hint to prog_load failure
  ip vrf: Refactor ipvrf_identify
  ip vrf: Fix reset to default VRF
  ip netns: Reset vrf to default VRF on namespace switch

 ip/ip_common.h |   1 +
 ip/ipnetns.c   |   5 +++
 ip/ipvrf.c     | 103 +++++++++++++++++++++++++++++++++++----------------------
 3 files changed, 69 insertions(+), 40 deletions(-)

-- 
2.1.4

[PATCH iproute2 1/4] ip vrf: Move kernel config hint to prog_load failure

[David Ahern]

Move the hint about CGROUP_BPF enabled to prog_load failure since
it fails before the attach. Update the existing error message to
print to stderr.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 ip/ipvrf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index 4d59845416cd..44ad7e07024a 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -170,14 +170,15 @@ static int vrf_configure_cgroup(const char *path, int ifindex)
 	 */
 	prog_fd = prog_load(ifindex);
 	if (prog_fd < 0) {
-		printf("Failed to load BPF prog: '%s'\n", strerror(errno));
+		fprintf(stderr, "Failed to load BPF prog: '%s'\n",
+			strerror(errno));
+		fprintf(stderr, "Kernel compiled with CGROUP_BPF enabled?\n");
 		goto out;
 	}
 
 	if (bpf_prog_attach_fd(prog_fd, cg_fd, BPF_CGROUP_INET_SOCK_CREATE)) {
 		fprintf(stderr, "Failed to attach prog to cgroup: '%s'\n",
 			strerror(errno));
-		fprintf(stderr, "Kernel compiled with CGROUP_BPF enabled?\n");
 		goto out;
 	}
 
-- 
2.1.4

[PATCH iproute2 2/4] ip vrf: Refactor ipvrf_identify

[David Ahern]

Split ipvrf_identify into arg processing and a function that does the
actual cgroup file parsing. The latter function is used in a follow
on patch.

In the process, convert the reading of the cgroups file to use fopen
and fgets just in case the file ever grows beyond 4k. Move printing
of any error message and the vrf name to the caller of the new
vrf_identify.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 ip/ipvrf.c | 69 +++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index 44ad7e07024a..a2669f339691 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -40,14 +40,43 @@ static void usage(void)
 	exit(-1);
 }
 
-static int ipvrf_identify(int argc, char **argv)
+static int vrf_identify(pid_t pid, char *name, size_t len)
 {
 	char path[PATH_MAX];
 	char buf[4096];
 	char *vrf, *end;
-	int fd, rc = -1;
+	FILE *fp;
+
+	snprintf(path, sizeof(path), "/proc/%d/cgroup", pid);
+	fp = fopen(path, "r");
+	if (!fp)
+		return -1;
+
+	memset(name, 0, len);
+
+	while (fgets(buf, sizeof(buf), fp)) {
+		vrf = strstr(buf, "::/vrf/");
+		if (vrf) {
+			vrf += 7;  /* skip past "::/vrf/" */
+			end = strchr(vrf, '\n');
+			if (end)
+				*end = '\0';
+
+			strncpy(name, vrf, len - 1);
+			break;
+		}
+	}
+
+	fclose(fp);
+
+	return 0;
+}
+
+static int ipvrf_identify(int argc, char **argv)
+{
+	char vrf[32];
+	int rc;
 	unsigned int pid;
-	ssize_t n;
 
 	if (argc < 1)
 		pid = getpid();
@@ -56,35 +85,15 @@ static int ipvrf_identify(int argc, char **argv)
 	else if (get_unsigned(&pid, argv[0], 10))
 		invarg("Invalid pid\n", argv[0]);
 
-	snprintf(path, sizeof(path), "/proc/%d/cgroup", pid);
-	fd = open(path, O_RDONLY);
-	if (fd < 0) {
-		fprintf(stderr,
-			"Failed to open cgroups file: %s\n", strerror(errno));
-		return -1;
-	}
-
-	n = read(fd, buf, sizeof(buf) - 1);
-	if (n < 0) {
-		fprintf(stderr,
-			"Failed to read cgroups file: %s\n", strerror(errno));
-		goto out;
-	}
-	buf[n] = '\0';
-	vrf = strstr(buf, "::/vrf/");
-	if (vrf) {
-		vrf += 7;  /* skip past "::/vrf/" */
-		end = strchr(vrf, '\n');
-		if (end)
-			*end = '\0';
-
-		printf("%s\n", vrf);
+	rc = vrf_identify(pid, vrf, sizeof(vrf));
+	if (!rc) {
+		if (vrf[0] != '\0')
+			printf("%s\n", vrf);
+	} else {
+		fprintf(stderr, "Failed to lookup vrf association: %s\n",
+			strerror(errno));
 	}
 
-	rc = 0;
-out:
-	close(fd);
-
 	return rc;
 }
 
-- 
2.1.4

[PATCH iproute2 3/4] ip vrf: Fix reset to default VRF

[David Ahern]

Path in vrf_switch for "default" VRF is supposed to be MNT/vrf not
MNT/default. Also, default_vrf flag is redundant with ifindex. Remove
the flag in favor of ifindex != 0.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 ip/ipvrf.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index a2669f339691..de2ec5c120cb 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -202,16 +202,15 @@ static int vrf_configure_cgroup(const char *path, int ifindex)
 static int vrf_switch(const char *name)
 {
 	char path[PATH_MAX], *mnt, pid[16];
-	int ifindex = name_is_vrf(name);
-	bool default_vrf = false;
+	int ifindex = 0;
 	int rc = -1, len, fd = -1;
 
-	if (!ifindex) {
-		if (strcmp(name, "default")) {
+	if (strcmp(name, "default")) {
+		ifindex = name_is_vrf(name);
+		if (!ifindex) {
 			fprintf(stderr, "Invalid VRF name\n");
 			return -1;
 		}
-		default_vrf = true;
 	}
 
 	mnt = find_cgroup2_mount();
@@ -221,8 +220,8 @@ static int vrf_switch(const char *name)
 	/* path to cgroup; make sure buffer has room to cat "/cgroup.procs"
 	 * to the end of the path
 	 */
-	len = snprintf(path, sizeof(path) - sizeof(CGRP_PROC_FILE), "%s%s/%s",
-		       mnt, default_vrf ? "" : "/vrf", name);
+	len = snprintf(path, sizeof(path) - sizeof(CGRP_PROC_FILE), "%s/vrf/%s",
+		       mnt, ifindex ? name : "");
 	if (len > sizeof(path) - sizeof(CGRP_PROC_FILE)) {
 		fprintf(stderr, "Invalid path to cgroup2 mount\n");
 		goto out;
@@ -233,7 +232,7 @@ static int vrf_switch(const char *name)
 		goto out;
 	}
 
-	if (!default_vrf && vrf_configure_cgroup(path, ifindex))
+	if (ifindex && vrf_configure_cgroup(path, ifindex))
 		goto out;
 
 	/*
-- 
2.1.4

[PATCH iproute2 4/4] ip netns: Reset vrf to default VRF on namespace switch

[David Ahern]

A vrf is local to a namespace. Drop any VRF association before trying
to exec a command in the new namespace.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 ip/ip_common.h |  1 +
 ip/ipnetns.c   |  5 +++++
 ip/ipvrf.c     | 14 ++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index 28763e81e4a4..ab6a83431fd6 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -58,6 +58,7 @@ int do_tcp_metrics(int argc, char **argv);
 int do_ipnetconf(int argc, char **argv);
 int do_iptoken(int argc, char **argv);
 int do_ipvrf(int argc, char **argv);
+void vrf_reset(void);
 
 int iplink_get(unsigned int flags, char *name, __u32 filt_mask);
 
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index db9a541769f1..8201b94a1620 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -387,6 +387,11 @@ static int netns_exec(int argc, char **argv)
 	if (netns_switch(argv[0]))
 		return -1;
 
+	/* we just changed namespaces. clear any vrf association
+	 * with prior namespace before exec'ing command
+	 */
+	vrf_reset();
+
 	/* ip must return the status of the child,
 	 * but do_cmd() will add a minus to this,
 	 * so let's add another one here to cancel it.
diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index de2ec5c120cb..dc8364a43a57 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -277,6 +277,20 @@ static int ipvrf_exec(int argc, char **argv)
 	return -cmd_exec(argv[1], argv + 1, !!batch_mode);
 }
 
+/* reset VRF association of current process to default VRF;
+ * used by netns_exec
+ */
+void vrf_reset(void)
+{
+	char vrf[32];
+
+	if (vrf_identify(getpid(), vrf, sizeof(vrf)) ||
+	    (vrf[0] == '\0'))
+		return;
+
+	vrf_switch("default");
+}
+
 int do_ipvrf(int argc, char **argv)
 {
 	if (argc == 0) {
-- 
2.1.4

Re: [PATCH iproute2 0/4] ip vrf fixups

[Stephen Hemminger]

On Thu, 15 Dec 2016 12:06:58 -0800
David Ahern <dsa@cumulusnetworks.com> wrote:

> Some minor cleanups to the 'ip vrf' command.
> 
> Patch 1 moves the CGROUP_BPF hint to the failure of prog_load since it
> fails first.
> 
> Patch 2 refactors ipvrf_identify. The action part is moved to a function
> that can be used standalone and in the process flipped to fopen/fgets for
> robustness should the cgroups file grow larger than 4k.
> 
> Patch 3 fixes the path switching to "default" VRF.
> 
> Patch 4 moves a task to default VRF when switching namespaces.
> 
> David Ahern (4):
>   ip vrf: Move kernel config hint to prog_load failure
>   ip vrf: Refactor ipvrf_identify
>   ip vrf: Fix reset to default VRF
>   ip netns: Reset vrf to default VRF on namespace switch
> 
>  ip/ip_common.h |   1 +
>  ip/ipnetns.c   |   5 +++
>  ip/ipvrf.c     | 103 +++++++++++++++++++++++++++++++++++----------------------
>  3 files changed, 69 insertions(+), 40 deletions(-)
> 

Applied thanks.