11:21 a.m.
Hi,
I'm using libvirt-lxc with mac80211_hwsim to construct virtual wireless network
for NetworkManager testing. Currently, I have to move the interfaces to the
domains manually, as sharing wan 802.11 wireless link with a LXC namespace does
not work:
<hostdev mode='capabilities' type='net'>
<source><interface>wlan0</interface></source>
</hostdev>
The wireless links can only be associated to namespaces by moving the
underlying PHY instead of the network interface.
I'm following up with patches that fix the problem for me; please take a look.
Thank you
Lubo
11:21 a.m.
New subject: [libvirt] [PATCH 1/2] virnetdev: move virNetDevSysfsFile() upwards
...and provide a bogus implementation for non-Linux. No functional change.
We'd like to use it from within virNetDevSetNamespace() to determine if we need
to move a 802.11 PHY to a namespace when moving the interface.
---
src/util/virnetdev.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 9a6d4e7..ee60f09 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -114,6 +114,28 @@ struct _virNetDevMcastList {
virNetDevMcastEntryPtr *entries;
};
+#ifdef __linux__
+# define NET_SYSFS "/sys/class/net/"
+
+static int
+virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname,
+ const char *file)
+{
+
+ if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) <
0)
+ return -1;
+ return 0;
+}
+#else
+static int
+virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname,
+ const char *file)
+{
+ return -1;
+}
+#endif
+
+
#if defined(HAVE_STRUCT_IFREQ)
static int virNetDevSetupControlFull(const char *ifname,
struct ifreq *ifr,
@@ -1612,18 +1634,6 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED,
#ifdef __linux__
-# define NET_SYSFS "/sys/class/net/"
-
-static int
-virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname,
- const char *file)
-{
-
- if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) <
0)
- return -1;
- return 0;
-}
-
static int
virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname,
const char *file)
--
2.1.0
12:04 p.m.
New subject: [libvirt] [PATCH 1/2] virnetdev: move virNetDevSysfsFile() upwards
On 14.04.2015 18:21, Lubomir Rintel wrote:
...and provide a bogus implementation for non-Linux. No functional
change.
We'd like to use it from within virNetDevSetNamespace() to determine if we need
to move a 802.11 PHY to a namespace when moving the interface.
---
src/util/virnetdev.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 9a6d4e7..ee60f09 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -114,6 +114,28 @@ struct _virNetDevMcastList {
virNetDevMcastEntryPtr *entries;
};
+#ifdef __linux__
+# define NET_SYSFS "/sys/class/net/"
+
+static int
+virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname,
+ const char *file)
+{
+
+ if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file)
< 0)
+ return -1;
+ return 0;
+}
+#else
+static int
+virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname,
+ const char *file)
Not that I reviewed the whole patch set (maybe tomorrow), but these
arguments are missing ATTRIBUTE_UNUSED. Just a small nit that can be
fixed before pushing. No need for v2.
ACK
Michal
11:21 a.m.
New subject: [libvirt] [PATCH 2/2] lxc: move wireless PHYs to a network namespace
The 802.11 interfaces can not be moved by themselves, their Phy has to move too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
---
src/util/virnetdev.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index ee60f09..df48763 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -584,6 +584,8 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
{
int rc;
char *pid = NULL;
+ char *phy_path = NULL;
+
const char *argv[] = {
"ip", "link", "set", ifname, "netns",
NULL, NULL
};
@@ -591,6 +593,35 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1)
return -1;
+ /* The 802.11 wireless devices only move together with their PHY. */
+ if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") >= 0) {
+ int len;
+ char *phy = NULL;
+
+ len = virFileReadAllQuiet(phy_path, 1024, &phy);
+ VIR_FREE(phy_path);
+
+ /* Remove a line break. */
+ if (len > 0)
+ phy[len - 1] = '\0';
+
+ if (len >= 0) {
+ const char *iwargv[] = {
+ "iw", "phy", phy, "set", "netns",
NULL, NULL
+ };
+
+ iwargv[5] = pid;
+ rc = virRun(iwargv, NULL);
+
+ VIR_FREE(phy);
+
+ if (rc == 0) {
+ VIR_FREE(pid);
+ return rc;
+ }
+ }
+ }
+
argv[5] = pid;
rc = virRun(argv, NULL);
--
2.1.0
10:23 a.m.
New subject: [libvirt] [PATCH 2/2] lxc: move wireless PHYs to a network namespace
On 14.04.2015 18:21, Lubomir Rintel wrote:
The 802.11 interfaces can not be moved by themselves, their Phy has
to move too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
---
src/util/virnetdev.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index ee60f09..df48763 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -584,6 +584,8 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
{
int rc;
char *pid = NULL;
+ char *phy_path = NULL;
+
const char *argv[] = {
"ip", "link", "set", ifname, "netns",
NULL, NULL
};
@@ -591,6 +593,35 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1)
return -1;
+ /* The 802.11 wireless devices only move together with their PHY. */
+ if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") >= 0) {
+ int len;
+ char *phy = NULL;
+
+ len = virFileReadAllQuiet(phy_path, 1024, &phy);
+ VIR_FREE(phy_path);
+
+ /* Remove a line break. */
+ if (len > 0)
+ phy[len - 1] = '\0';
+
+ if (len >= 0) {
+ const char *iwargv[] = {
+ "iw", "phy", phy, "set",
"netns", NULL, NULL
+ };
+
+ iwargv[5] = pid;
+ rc = virRun(iwargv, NULL);
+
+ VIR_FREE(phy);
+
+ if (rc == 0) {
+ VIR_FREE(pid);
+ return rc;
+ }
+ }
+ }
+
argv[5] = pid;
rc = virRun(argv, NULL);
I understand what you mean, but the code style could be better. I'll
polish and resend the patch in your name.
Michal
12:58 p.m.
New subject: [libvirt] [PATCH 2/2] lxc: move wireless PHYs to a network namespace
On Thu, 2015-04-16 at 17:23 +0200, Michal Privoznik wrote:
On 14.04.2015 18:21, Lubomir Rintel wrote:
> The 802.11 interfaces can not be moved by themselves, their Phy has
> to move too.
>
> If there are other interfaces, they have to move too -- hopefully
> it's not too
> confusing. This is a less-invasive alternative to defining a new
> hostdev type
> for PHYs.
> ---
> src/util/virnetdev.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index ee60f09..df48763 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -584,6 +584,8 @@ int virNetDevSetNamespace(const char *ifname,
> pid_t pidInNs)
> {
> int rc;
> char *pid = NULL;
> + char *phy_path = NULL;
> +
> const char *argv[] = {
> "ip", "link", "set", ifname,
"netns", NULL, NULL
> };
> @@ -591,6 +593,35 @@ int virNetDevSetNamespace(const char *ifname,
> pid_t pidInNs)
> if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1)
> return -1;
>
> + /* The 802.11 wireless devices only move together with their
> PHY. */
> + if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") >=
> 0) {
> + int len;
> + char *phy = NULL;
> +
> + len = virFileReadAllQuiet(phy_path, 1024, &phy);
> + VIR_FREE(phy_path);
> +
> + /* Remove a line break. */
> + if (len > 0)
> + phy[len - 1] = '\0';
> +
> + if (len >= 0) {
> + const char *iwargv[] = {
> + "iw", "phy", phy, "set",
"netns", NULL, NULL
> + };
> +
> + iwargv[5] = pid;
> + rc = virRun(iwargv, NULL);
> +
> + VIR_FREE(phy);
> +
> + if (rc == 0) {
> + VIR_FREE(pid);
> + return rc;
> + }
> + }
> + }
> +
> argv[5] = pid;
> rc = virRun(argv, NULL);
>
>
I understand what you mean, but the code style could be better.
I'm not too sure. You reversed the logic with your changes and now it
no longer works.
It's quite expected for "ip link set" to fail for a 802.11 device --
you must not fail in that case.
I'll
polish and resend the patch in your name.
Michal
1:40 p.m.
New subject: [libvirt] [PATCH] virnetdev: fix moving of 802.11 phys
There was a couple of problems with the style fixes applied to the original
patch:
1.) virFileReadAllQuiet comparison was incorrectly parenthesized when moved
into a condition, causing the len to be set to the result of comparison. This,
together with the removed underflow check would underflow the phy buffer.
2.) The logic was broken. Failure to call "ip" would abort the function, thus
the "iw" branch would never be reached.
This aims to fix the issues and work around possible style complains :)
Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk>
---
src/util/virnetdev.c | 46 +++++++++++++++++++++-------------------------
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 98ce152..e4fcd81 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -478,40 +478,36 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
char *phy_path = NULL;
int len;
- const char *argv[] = {
- "ip", "link", "set", ifname, "netns",
NULL, NULL
- };
-
- const char *iwargv[] = {
- "iw", "phy", NULL, "set", "netns", NULL,
NULL
- };
-
if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1)
return -1;
- argv[5] = pid;
- if (virRun(argv, NULL) < 0)
- goto cleanup;
-
/* The 802.11 wireless devices only move together with their PHY. */
if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") < 0)
goto cleanup;
- if ((len = virFileReadAllQuiet(phy_path, 1024, &phy) < 0)) {
- if (errno == ENOENT) {
- /* Okay, this is not a wireless card. Claim success. */
- ret = 0;
- }
- goto cleanup;
- }
+ if ((len = virFileReadAllQuiet(phy_path, 1024, &phy)) <= 0) {
+ /* Not a wireless device. */
+ const char *argv[] = {
+ "ip", "link", "set", ifname, "netns",
NULL, NULL
+ };
- /* Remove a line break. */
- phy[len - 1] = '\0';
+ argv[5] = pid;
+ if (virRun(argv, NULL) < 0)
+ goto cleanup;
- iwargv[2] = phy;
- iwargv[5] = pid;
- if (virRun(iwargv, NULL) < 0)
- goto cleanup;
+ } else {
+ const char *argv[] = {
+ "iw", "phy", NULL, "set", "netns",
NULL, NULL
+ };
+
+ /* Remove a line break. */
+ phy[len - 1] = '\0';
+
+ argv[2] = phy;
+ argv[5] = pid;
+ if (virRun(argv, NULL) < 0)
+ goto cleanup;
+ }
ret = 0;
cleanup:
--
2.4.2
11:11 a.m.
New subject: [libvirt] [PATCH] virnetdev: fix moving of 802.11 phys
On 01.06.2015 20:40, Lubomir Rintel wrote:
There was a couple of problems with the style fixes applied to the
original
patch:
1.) virFileReadAllQuiet comparison was incorrectly parenthesized when moved
into a condition, causing the len to be set to the result of comparison. This,
together with the removed underflow check would underflow the phy buffer.
2.) The logic was broken. Failure to call "ip" would abort the function, thus
the "iw" branch would never be reached.
This aims to fix the issues and work around possible style complains :)
Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk>
---
src/util/virnetdev.c | 46 +++++++++++++++++++++-------------------------
1 file changed, 21 insertions(+), 25 deletions(-)
ACked and pushed.
Michal
3442
days inactive
3492
days old
7 comments
2 participants
participants (2)
-
Lubomir Rintel -
Michal Privoznik