diff mbox series

[RFC] vlan: use new bridge ioctl's

Message ID 20171121201445.4253-1-sergey.matyukevich.os@quantenna.com
State Accepted
Headers show
Series [RFC] vlan: use new bridge ioctl's | expand

Commit Message

Sergey Matyukevich Nov. 21, 2017, 8:14 p.m. UTC 
Legacy ioctl's through SIOCDEVPRIVATE are deprecated.
Follow the approach taken by bridge-utils and make
use of new bridge ioctl's whenever possible.

For example, using legacy ioctl's breaks dynamic VLAN
mode on 32-bit Linux systems running 64-bit kernels.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 src/ap/vlan_full.c        | 110 +++++++++++++++++++++++++++++-----------------
 src/common/linux_bridge.h |  15 +++++++
 2 files changed, 85 insertions(+), 40 deletions(-)

Comments

Jouni Malinen Jan. 2, 2019, 11:56 a.m. UTC | #1 
On Tue, Nov 21, 2017 at 11:14:45PM +0300, Sergey Matyukevich wrote:
> Legacy ioctl's through SIOCDEVPRIVATE are deprecated.
> Follow the approach taken by bridge-utils and make
> use of new bridge ioctl's whenever possible.
> 
> For example, using legacy ioctl's breaks dynamic VLAN
> mode on 32-bit Linux systems running 64-bit kernels.

Thanks, applied with some cleanup. Mainly, I changes this to use the
existing bridge functions in src/drivers/linux_ioctl.c instead of
implementing practically the same ioctl() calls in vlan_full.c.
Sergey Matyukevich Jan. 8, 2019, 9:16 a.m. UTC | #2 
Hello Jouni,

> On Tue, Nov 21, 2017 at 11:14:45PM +0300, Sergey Matyukevich wrote:
> > Legacy ioctl's through SIOCDEVPRIVATE are deprecated.
> > Follow the approach taken by bridge-utils and make
> > use of new bridge ioctl's whenever possible.
> > 
> > For example, using legacy ioctl's breaks dynamic VLAN
> > mode on 32-bit Linux systems running 64-bit kernels.
> 
> Thanks, applied with some cleanup. Mainly, I changes this to use the
> existing bridge functions in src/drivers/linux_ioctl.c instead of
> implementing practically the same ioctl() calls in vlan_full.c.

Thanks for cleanup. AFAIR, I decided not to use those functions from
linux_ioctl.c to avoid breaking hostapd for non-Linux platforms.
But after another look at vlan_full.c, it is Linux-specific anyway.

Regards,
Sergey
diff mbox series

Patch

diff --git a/src/ap/vlan_full.c b/src/ap/vlan_full.c
index aa42335b9..b797f4b60 100644
--- a/src/ap/vlan_full.c
+++ b/src/ap/vlan_full.c
@@ -135,6 +135,7 @@  static int br_delif(const char *br_name, const char *if_name)
 	struct ifreq ifr;
 	unsigned long args[2];
 	int if_index;
+	int ret;
 
 	wpa_printf(MSG_DEBUG, "VLAN: br_delif(%s, %s)", br_name, if_name);
 	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
@@ -153,19 +154,23 @@  static int br_delif(const char *br_name, const char *if_name)
 		return -1;
 	}
 
-	args[0] = BRCTL_DEL_IF;
-	args[1] = if_index;
-
 	os_strlcpy(ifr.ifr_name, br_name, sizeof(ifr.ifr_name));
-	ifr.ifr_data = (void *) args;
 
-	if (ioctl(fd, SIOCDEVPRIVATE, &ifr) < 0 && errno != EINVAL) {
-		/* No error if interface already removed. */
-		wpa_printf(MSG_ERROR, "VLAN: %s: ioctl[SIOCDEVPRIVATE,"
-			   "BRCTL_DEL_IF] failed for br_name=%s if_name=%s: "
-			   "%s", __func__, br_name, if_name, strerror(errno));
-		close(fd);
-		return -1;
+	ifr.ifr_ifindex = if_index;
+	ret = ioctl(fd, SIOCBRDELIF, &ifr);
+	if (ret < 0) {
+		args[0] = BRCTL_DEL_IF;
+		args[1] = if_index;
+
+		ifr.ifr_data = (void *) args;
+		if (ioctl(fd, SIOCDEVPRIVATE, &ifr) < 0 && errno != EINVAL) {
+			/* No error if interface already removed. */
+			wpa_printf(MSG_ERROR, "VLAN: %s: ioctl[SIOCDEVPRIVATE,"
+					"BRCTL_DEL_IF] failed for br_name=%s if_name=%s: "
+					"%s", __func__, br_name, if_name, strerror(errno));
+			close(fd);
+			return -1;
+		}
 	}
 
 	close(fd);
@@ -186,6 +191,7 @@  static int br_addif(const char *br_name, const char *if_name)
 	struct ifreq ifr;
 	unsigned long args[2];
 	int if_index;
+	int ret;
 
 	wpa_printf(MSG_DEBUG, "VLAN: br_addif(%s, %s)", br_name, if_name);
 	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
@@ -204,24 +210,34 @@  static int br_addif(const char *br_name, const char *if_name)
 		return -1;
 	}
 
-	args[0] = BRCTL_ADD_IF;
-	args[1] = if_index;
-
 	os_strlcpy(ifr.ifr_name, br_name, sizeof(ifr.ifr_name));
-	ifr.ifr_data = (void *) args;
 
-	if (ioctl(fd, SIOCDEVPRIVATE, &ifr) < 0) {
+	ifr.ifr_ifindex = if_index;
+	ret = ioctl(fd, SIOCBRADDIF, &ifr);
+	if (ret < 0) {
 		if (errno == EBUSY) {
 			/* The interface is already added. */
 			close(fd);
 			return 1;
 		}
 
-		wpa_printf(MSG_ERROR, "VLAN: %s: ioctl[SIOCDEVPRIVATE,"
-			   "BRCTL_ADD_IF] failed for br_name=%s if_name=%s: "
-			   "%s", __func__, br_name, if_name, strerror(errno));
-		close(fd);
-		return -1;
+		args[0] = BRCTL_ADD_IF;
+		args[1] = if_index;
+
+		ifr.ifr_data = (void *) args;
+		if (ioctl(fd, SIOCDEVPRIVATE, &ifr) < 0) {
+			if (errno == EBUSY) {
+				/* The interface is already added. */
+				close(fd);
+				return 1;
+			}
+
+			wpa_printf(MSG_ERROR, "VLAN: %s: ioctl[SIOCDEVPRIVATE,"
+					"BRCTL_ADD_IF] failed for br_name=%s if_name=%s: "
+					"%s", __func__, br_name, if_name, strerror(errno));
+			close(fd);
+			return -1;
+		}
 	}
 
 	close(fd);
@@ -233,6 +249,7 @@  static int br_delbr(const char *br_name)
 {
 	int fd;
 	unsigned long arg[2];
+	int ret;
 
 	wpa_printf(MSG_DEBUG, "VLAN: br_delbr(%s)", br_name);
 	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
@@ -241,15 +258,18 @@  static int br_delbr(const char *br_name)
 		return -1;
 	}
 
-	arg[0] = BRCTL_DEL_BRIDGE;
-	arg[1] = (unsigned long) br_name;
+	ret = ioctl(fd, SIOCBRDELBR, br_name);
+	if (ret < 0) {
+		arg[0] = BRCTL_DEL_BRIDGE;
+		arg[1] = (unsigned long) br_name;
 
-	if (ioctl(fd, SIOCGIFBR, arg) < 0 && errno != ENXIO) {
-		/* No error if bridge already removed. */
-		wpa_printf(MSG_ERROR, "VLAN: %s: BRCTL_DEL_BRIDGE failed for "
-			   "%s: %s", __func__, br_name, strerror(errno));
-		close(fd);
-		return -1;
+		if (ioctl(fd, SIOCGIFBR, arg) < 0 && errno != ENXIO) {
+			/* No error if bridge already removed. */
+			wpa_printf(MSG_ERROR, "VLAN: %s: BRCTL_DEL_BRIDGE failed for "
+					"%s: %s", __func__, br_name, strerror(errno));
+			close(fd);
+			return -1;
+		}
 	}
 
 	close(fd);
@@ -269,6 +289,7 @@  static int br_addbr(const char *br_name)
 	int fd;
 	unsigned long arg[4];
 	struct ifreq ifr;
+	int ret;
 
 	wpa_printf(MSG_DEBUG, "VLAN: br_addbr(%s)", br_name);
 	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
@@ -277,20 +298,29 @@  static int br_addbr(const char *br_name)
 		return -1;
 	}
 
-	arg[0] = BRCTL_ADD_BRIDGE;
-	arg[1] = (unsigned long) br_name;
-
-	if (ioctl(fd, SIOCGIFBR, arg) < 0) {
- 		if (errno == EEXIST) {
+	ret = ioctl(fd, SIOCBRADDBR, br_name);
+	if (ret < 0) {
+		if (errno == EEXIST) {
 			/* The bridge is already added. */
 			close(fd);
 			return 1;
-		} else {
-			wpa_printf(MSG_ERROR, "VLAN: %s: BRCTL_ADD_BRIDGE "
-				   "failed for %s: %s",
-				   __func__, br_name, strerror(errno));
-			close(fd);
-			return -1;
+		}
+
+		arg[0] = BRCTL_ADD_BRIDGE;
+		arg[1] = (unsigned long) br_name;
+
+		if (ioctl(fd, SIOCGIFBR, arg) < 0) {
+			if (errno == EEXIST) {
+				/* The bridge is already added. */
+				close(fd);
+				return 1;
+			} else {
+				wpa_printf(MSG_ERROR, "VLAN: %s: BRCTL_ADD_BRIDGE "
+						"failed for %s: %s",
+						__func__, br_name, strerror(errno));
+				close(fd);
+				return -1;
+			}
 		}
 	}
 
diff --git a/src/common/linux_bridge.h b/src/common/linux_bridge.h
index 7b768464f..84386e60f 100644
--- a/src/common/linux_bridge.h
+++ b/src/common/linux_bridge.h
@@ -9,6 +9,21 @@ 
 #ifndef LINUX_BRIDGE_H
 #define LINUX_BRIDGE_H
 
+/* This ioctl is defined in linux/sockios.h */
+
+#ifndef SIOCBRADDBR
+#define SIOCBRADDBR 0x89a0
+#endif
+#ifndef SIOCBRDELBR
+#define SIOCBRDELBR 0x89a1
+#endif
+#ifndef SIOCBRADDIF
+#define SIOCBRADDIF 0x89a2
+#endif
+#ifndef SIOCBRDELIF
+#define SIOCBRDELIF 0x89a3
+#endif
+
 /* This interface is defined in linux/if_bridge.h */
 
 #define BRCTL_GET_VERSION 0