From: Michal Privoznik <mpriv...@redhat.com>

Currently, virNetlinkBridgeVlanFilterSet() takes @cmd as the
second argument where either RTM_SETLINK or RTM_DELLINK is
expected. Both of these constants come from linux/rtnetlink.h and
thus are undefined when building without netlink. This design
also clashes with the whole point of virnetlink: to offload
netlink dependency onto a single file.

Therefore, drop the argument, turn
virNetlinkBridgeVlanFilterSet() into just setter, effectively,
and introduce virNetlinkBridgeVlanFilterDel() for the case when
RTM_DELLINK would be passed as @cmd.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/770
Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
Reviewed-by: Peter Krempa <pkre...@redhat.com>
---
 src/util/virnetdevbridge.c |  8 ++---
 src/util/virnetlink.c      | 73 +++++++++++++++++++++++++++++++++-----
 src/util/virnetlink.h      |  5 ++-
 3 files changed, 73 insertions(+), 13 deletions(-)

diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 806ccc5fa7..20c7a25585 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -321,7 +321,7 @@ virNetDevBridgeSetupVlans(const char *ifname, const 
virNetDevVlan *virtVlan)
         return 0;
 
     /* The interface will have been automatically added to vlan 1, so remove 
it. */
-    if (virNetlinkBridgeVlanFilterSet(ifname, RTM_DELLINK, 0, 1, &error) < 0) {
+    if (virNetlinkBridgeVlanFilterDel(ifname, 1, &error) < 0) {
         if (error != 0) {
             virReportSystemError(-error,
                                  _("error removing vlan filter from interface 
%1$s"),
@@ -341,7 +341,7 @@ virNetDevBridgeSetupVlans(const char *ifname, const 
virNetDevVlan *virtVlan)
                 flags |= BRIDGE_VLAN_INFO_UNTAGGED;
             }
 
-            if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags,
+            if (virNetlinkBridgeVlanFilterSet(ifname, flags,
                                               virtVlan->nativeTag, &error) < 
0) {
                 goto error;
             }
@@ -349,7 +349,7 @@ virNetDevBridgeSetupVlans(const char *ifname, const 
virNetDevVlan *virtVlan)
 
         for (i = 0; i < virtVlan->nTags; i++) {
             if (virtVlan->tag[i] != virtVlan->nativeTag)
-                if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, 0,
+                if (virNetlinkBridgeVlanFilterSet(ifname, 0,
                                                   virtVlan->tag[i], &error) < 
0) {
                     goto error;
                 }
@@ -357,7 +357,7 @@ virNetDevBridgeSetupVlans(const char *ifname, const 
virNetDevVlan *virtVlan)
     } else {
         /* In native mode, add the single VLAN as pvid untagged. */
         flags = BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED;
-        if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags,
+        if (virNetlinkBridgeVlanFilterSet(ifname, flags,
                                           virtVlan->tag[0], &error) < 0) {
             goto error;
         }
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index e3fcabca15..8f6dd86d0f 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -702,7 +702,7 @@ virNetlinkDelLink(const char *ifname, 
virNetlinkTalkFallback fallback)
 }
 
 /**
- * virNetlinkBridgeVlanFilterSet:
+ * virNetlinkBridgeVlanFilterHelper:
  *
  * @ifname: name of the link
  * @cmd:    netlink command, either RTM_SETLINK or RTM_DELLINK
@@ -717,12 +717,12 @@ virNetlinkDelLink(const char *ifname, 
virNetlinkTalkFallback fallback)
  * non-zero, then a netlink failure occurred, but no error message
  * is generated leaving it up to the caller to handle the condition.
  */
-int
-virNetlinkBridgeVlanFilterSet(const char *ifname,
-                              int cmd,
-                              const unsigned short flags,
-                              const short vid,
-                              int *error)
+static int
+virNetlinkBridgeVlanFilterHelper(const char *ifname,
+                                 int cmd,
+                                 const unsigned short flags,
+                                 const short vid,
+                                 int *error)
 {
     struct ifinfomsg ifm = { .ifi_family = PF_BRIDGE };
     struct bridge_vlan_info vinfo = { .flags = flags, .vid = vid };
@@ -767,6 +767,55 @@ virNetlinkBridgeVlanFilterSet(const char *ifname,
     return 0;
 }
 
+
+/**
+ * virNetlinkBridgeVlanFilterSet:
+ *
+ * @ifname: name of the link
+ * @flags:  flags to use when adding the vlan filter
+ * @vid:    vlan id to add
+ * @error:  netlink error code
+ *
+ * Add a vlan filter from an interface associated with a bridge.
+ *
+ * Returns 0 on success, -1 on error. Additionally, if the @error is
+ * non-zero, then a netlink failure occurred, but no error message
+ * is generated leaving it up to the caller to handle the condition.
+ */
+int
+virNetlinkBridgeVlanFilterSet(const char *ifname,
+                              const unsigned short flags,
+                              const short vid,
+                              int *error)
+{
+    return virNetlinkBridgeVlanFilterHelper(ifname, RTM_SETLINK,
+                                            flags, vid, error);
+}
+
+
+/**
+ * virNetlinkBridgeVlanFilterDel:
+ *
+ * @ifname: name of the link
+ * @vid:    vlan id to remove
+ * @error:  netlink error code
+ *
+ * Remove a vlan filter from an interface associated with a bridge.
+ *
+ * Returns 0 on success, -1 on error. Additionally, if the @error is
+ * non-zero, then a netlink failure occurred, but no error message
+ * is generated leaving it up to the caller to handle the condition.
+ */
+int
+virNetlinkBridgeVlanFilterDel(const char *ifname,
+                              const short vid,
+                              int *error)
+{
+    return virNetlinkBridgeVlanFilterHelper(ifname,
+                                            RTM_DELLINK, 0, vid, error);
+}
+
+
 /**
  * virNetlinkGetNeighbor:
  *
@@ -1346,7 +1395,6 @@ virNetlinkNewLink(const char *ifname G_GNUC_UNUSED,
 
 int
 virNetlinkBridgeVlanFilterSet(const char *ifname G_GNUC_UNUSED,
-                              int cmd G_GNUC_UNUSED,
                               const unsigned short unusedflags G_GNUC_UNUSED,
                               const short vid G_GNUC_UNUSED,
                               int *error)
@@ -1356,6 +1404,15 @@ virNetlinkBridgeVlanFilterSet(const char *ifname 
G_GNUC_UNUSED,
     return -1;
 }
 
+int
+virNetlinkBridgeVlanFilterDel(const char *ifname G_GNUC_UNUSED,
+                              const short vid G_GNUC_UNUSED,
+                              int *error G_GNUC_UNUSED)
+{
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
+    return -1;
+}
+
 int
 virNetlinkGetNeighbor(void **nlData G_GNUC_UNUSED,
                       uint32_t src_pid G_GNUC_UNUSED,
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 327fb426a1..74d4f5b613 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -78,11 +78,14 @@ typedef int (*virNetlinkTalkFallback)(const char *ifname);
 int virNetlinkDelLink(const char *ifname, virNetlinkTalkFallback fallback);
 
 int virNetlinkBridgeVlanFilterSet(const char *ifname,
-                                  int cmd,
                                   const unsigned short flags,
                                   const short vid,
                                   int *error);
 
+int virNetlinkBridgeVlanFilterDel(const char *ifname,
+                                  const short vid,
+                                  int *error);
+
 int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen);
 
 int virNetlinkDumpLink(const char *ifname, int ifindex,
-- 
2.49.0

Reply via email to