[PATCH v2 0/4] Fix build without libnl
v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/VYUE7... diff to v1: 1) Patch 1/5 from the original is dropped, no symbol addition to the private syms file, 2) The "virnetlink.h" is included more often in patch 3/4, 3) Some fixes around virNetlinkBridgeVlanFilterSet() stub Michal Prívozník (4): virnetlink: Provide stub for virNetlinkBridgeVlanFilterSet() virnetdevbridge.c: Fix comments in virNetDevBridgeSetupVlans() virnetdevbridge: Include virnetlink.h more often virnetlink: Split virNetlinkBridgeVlanFilterSet() src/util/virnetdevbridge.c | 18 ++++----- src/util/virnetlink.c | 83 ++++++++++++++++++++++++++++++++++---- src/util/virnetlink.h | 5 ++- 3 files changed, 88 insertions(+), 18 deletions(-) -- 2.49.0
From: Michal Privoznik <mprivozn@redhat.com> In virnetlink.c there are two sections: the first one when building WITH_LIBNL support, the other that provides stubs for functions declared in the corresponding header file when building without netlink support. But the stub implementation for virNetlinkBridgeVlanFilterSet() was missing. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virnetlink.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 206646d9d7..e3fcabca15 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -1344,6 +1344,18 @@ 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) +{ + *error = 0; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); + return -1; +} + int virNetlinkGetNeighbor(void **nlData G_GNUC_UNUSED, uint32_t src_pid G_GNUC_UNUSED, -- 2.49.0
From: Michal Privoznik <mprivozn@redhat.com> We still use C89 style of comments. Fix C99 style of comments used in virNetDevBridgeSetupVlans(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virnetdevbridge.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index c79d0c79b7..3b54e2cb1e 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -322,7 +322,7 @@ virNetDevBridgeSetupVlans(const char *ifname, const virNetDevVlan *virtVlan) if (!virtVlan || !virtVlan->nTags) return 0; - // The interface will have been automatically added to vlan 1, so remove it + /* The interface will have been automatically added to vlan 1, so remove it. */ if (virNetlinkBridgeVlanFilterSet(ifname, RTM_DELLINK, 0, 1, &error) < 0) { if (error != 0) { virReportSystemError(-error, @@ -332,7 +332,7 @@ virNetDevBridgeSetupVlans(const char *ifname, const virNetDevVlan *virtVlan) return -1; } - // If trunk mode, add the native VLAN then add the others, if any + /* If trunk mode, add the native VLAN then add the others, if any. */ if (virtVlan->trunk) { size_t i; @@ -357,7 +357,7 @@ virNetDevBridgeSetupVlans(const char *ifname, const virNetDevVlan *virtVlan) } } } else { - // In native mode, add the single VLAN as pvid untagged + /* 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, virtVlan->tag[0], &error) < 0) { -- 2.49.0
From: Michal Privoznik <mprivozn@redhat.com> The whole point of virnetlink.h is that it hides away the build time dependency on netlink. It wraps netlink functions in our functions which then have a stub implementation in case netlink support was disabled. Though, netlink is still Linux specific, so keep it in the '#ifdef __linux__` block to cause a compilation error should anybody try to use any of the wrapped functions on non-Linux. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevbridge.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 3b54e2cb1e..806ccc5fa7 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -30,9 +30,7 @@ #endif #ifdef __linux__ -# if defined(WITH_LIBNL) -# include "virnetlink.h" -# endif +# include "virnetlink.h" # include <linux/sockios.h> # include <linux/param.h> /* HZ */ # include <linux/in6.h> -- 2.49.0
On Tue, May 13, 2025 at 16:43:52 +0200, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The whole point of virnetlink.h is that it hides away the build time dependency on netlink. It wraps netlink functions in our functions which then have a stub implementation in case netlink support was disabled.
Though, netlink is still Linux specific, so keep it in the '#ifdef __linux__` block to cause a compilation error should anybody try to use any of the wrapped functions on non-Linux.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevbridge.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
From: Michal Privoznik <mprivozn@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 <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@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
participants (2)
-
Michal Privoznik -
Peter Krempa