Re: [PATCH 4/6] virNetDevMacVLanTapSetup: Rework to support multiple FDs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/04/2015 07:31 AM, Michal Privoznik wrote:
So, for the multiqueue on macvtaps we are going to need to open
the device multiple times. Currently, this is not supported.
Rework the function, so that upper layers can be reworked too.

Either merge 3 & 4 together, or change the log message (I'm okay with multiple functions at the same level being changed in the same patch).


Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
  src/util/virnetdevmacvlan.c | 66 ++++++++++++++++++++++++---------------------
  1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index d20990b..f7a7d72 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -302,10 +302,9 @@ virNetDevMacVLanTapOpen(const char *ifname,
/**
   * virNetDevMacVLanTapSetup:
- * @tapfd: file descriptor of the macvtap tap
- * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it
- *
- * Returns 0 on success, -1 in case of fatal error, error code otherwise.
+ * @tapfd: array of file descriptors of the macvtap tap
+ * @tapfdSize: number of file descriptors in @tapfd
+ * @vnet_hdr: whether to enable or disable IFF_VNET_HDR
   *
   * Turn the IFF_VNET_HDR flag, if requested and available, make sure
   * it's off in the other cases.
@@ -313,47 +312,52 @@ virNetDevMacVLanTapOpen(const char *ifname,
   * be turned off for some reason. This is reported with -1. Other fatal
   * error is not being able to read the interface flags. In that case the
   * macvtap device should not be used.
+ *

Comparing this to the VNET_HDR stuff for tap devices, I see that in that case lack of support for VNET_HDR isn't fatal - it simply isn't set. My guess is that VNET_HDR support was probably added prior to macvtap, so that it's always there if we're doing macvtap, but I don't know for sure. (I'm just wondering if maybe there's some way code could be shared between the two to reduce maintenance).

The other difference is that IFF_MULTI_QUEUE is set on each fd for a tap device. Is this also needed for macvtap?

+ * Returns 0 on success, -1 in case of fatal error, error code otherwise.
   */
  static int
-virNetDevMacVLanTapSetup(int tapfd, int vnet_hdr)
+virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr)
  {
      unsigned int features;
      struct ifreq ifreq;
      short new_flags = 0;
      int rc_on_fail = 0;
      const char *errmsg = NULL;
+    size_t i;
- memset(&ifreq, 0, sizeof(ifreq));
+    for (i = 0; i < tapfdSize; i++) {
+        memset(&ifreq, 0, sizeof(ifreq));
- if (ioctl(tapfd, TUNGETIFF, &ifreq) < 0) {
-        virReportSystemError(errno, "%s",
-                             _("cannot get interface flags on macvtap tap"));
-        return -1;
-    }
-
-    new_flags = ifreq.ifr_flags;
-
-    if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) {
-        new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR;
-        rc_on_fail = -1;
-        errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap");
-    } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) {
-        if (ioctl(tapfd, TUNGETFEATURES, &features) < 0) {
+        if (ioctl(tapfd[i], TUNGETIFF, &ifreq) < 0) {
              virReportSystemError(errno, "%s",
-                   _("cannot get feature flags on macvtap tap"));
+                                 _("cannot get interface flags on macvtap tap"));
              return -1;
          }
-        if ((features & IFF_VNET_HDR)) {
-            new_flags = ifreq.ifr_flags | IFF_VNET_HDR;
-            errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap");
+
+        new_flags = ifreq.ifr_flags;
+
+        if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) {
+            new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR;
+            rc_on_fail = -1;
+            errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap");
+        } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) {
+            if (ioctl(tapfd[i], TUNGETFEATURES, &features) < 0) {
+                virReportSystemError(errno, "%s",
+                                     _("cannot get feature flags on macvtap tap"));
+                return -1;
+            }
+            if ((features & IFF_VNET_HDR)) {
+                new_flags = ifreq.ifr_flags | IFF_VNET_HDR;
+                errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap");
+            }
          }
-    }
- if (new_flags != ifreq.ifr_flags) {
-        ifreq.ifr_flags = new_flags;
-        if (ioctl(tapfd, TUNSETIFF, &ifreq) < 0) {
-            virReportSystemError(errno, "%s", errmsg);
-            return rc_on_fail;
+        if (new_flags != ifreq.ifr_flags) {
+            ifreq.ifr_flags = new_flags;
+            if (ioctl(tapfd[i], TUNSETIFF, &ifreq) < 0) {
+                virReportSystemError(errno, "%s", errmsg);
+                return rc_on_fail;
+            }
          }
      }
@@ -864,7 +868,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
          if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0)
              goto disassociate_exit;
- if (virNetDevMacVLanTapSetup(rc, vnet_hdr) < 0) {
+        if (virNetDevMacVLanTapSetup(&rc, 1, vnet_hdr) < 0) {
              VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
              goto disassociate_exit;
          }

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]