Compare commits
35 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 5a42f1fcdc | |||
| b14bf469da | |||
| f2e51513a0 | |||
| 4f3b8e6a17 | |||
| 985e71cf0c | |||
| c1a7e5f39b | |||
| 77b02695ab | |||
| 50057212b3 | |||
| c61c2c83ef | |||
| e3e721c529 | |||
| 7bf08eb7cb | |||
| 0862e2b778 | |||
| 9ae002fc72 | |||
| 7749fc4a26 | |||
| 5b0f05f970 | |||
| 36079ddefa | |||
| 9abc69f078 | |||
| f4d251ad6e | |||
| b0d2e09842 | |||
| b0dd011728 | |||
| 2513c9ff81 | |||
| 9ef5501a94 | |||
| 659f2f0b36 | |||
| f3cac3532b | |||
| 6eb56c2e71 | |||
| 93a549eb1a | |||
| f2a998ce76 | |||
| 8556d44e84 | |||
| f8539fae13 | |||
| 80a3188a20 | |||
| 56d89b1d61 | |||
| 2a7828bae8 | |||
| c879325741 | |||
| edf6aa6cd7 | |||
| 163b87484b |
@@ -1,40 +0,0 @@
|
||||
From 76cdc7adf55723ff8da146bd3c15c64d0afd5d93 Mon Sep 17 00:00:00 2001
|
||||
From: Martin Kletzander <mkletzan@redhat.com>
|
||||
Date: Tue, 27 Feb 2024 16:20:12 +0100
|
||||
Subject: [PATCH] Fix off-by-one error in udevListInterfacesByStatus
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Ever since this function was introduced in 2012 it could've tried
|
||||
filling in an extra interface name. That was made worse in 2019 when
|
||||
the caller functions started accepting NULL arrays of size 0.
|
||||
|
||||
This is assigned CVE-2024-1441.
|
||||
|
||||
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Reported-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
|
||||
Fixes: 5a33366f5c0b18c93d161bd144f9f079de4ac8ca
|
||||
Fixes: d6064e2759a24e0802f363e3a810dc5a7d7ebb15
|
||||
Reviewed-by: Ján Tomko <jtomko@redhat.com>
|
||||
(cherry picked from commit c664015fe3a7bf59db26686e9ed69af011c6ebb8)
|
||||
---
|
||||
src/interface/interface_backend_udev.c | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
|
||||
index fb6799ed94..4091483060 100644
|
||||
--- a/src/interface/interface_backend_udev.c
|
||||
+++ b/src/interface/interface_backend_udev.c
|
||||
@@ -222,7 +222,7 @@ udevListInterfacesByStatus(virConnectPtr conn,
|
||||
g_autoptr(virInterfaceDef) def = NULL;
|
||||
|
||||
/* Ensure we won't exceed the size of our array */
|
||||
- if (count > names_len)
|
||||
+ if (count >= names_len)
|
||||
break;
|
||||
|
||||
path = udev_list_entry_get_name(dev_entry);
|
||||
--
|
||||
2.43.0
|
||||
|
||||
@@ -1,41 +0,0 @@
|
||||
From 3499354e12a1c1832bf4030693a64e03ceb79d05 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
|
||||
Date: Wed, 5 Jun 2024 11:16:21 +0100
|
||||
Subject: [PATCH] interface: fix udev reference leak with invalid flags
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
The udevInterfaceGetXMLDesc method takes a reference on the udev
|
||||
driver as its first action. If the virCheckFlags() condition
|
||||
fails, however, this reference is never released.
|
||||
|
||||
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
|
||||
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
---
|
||||
src/interface/interface_backend_udev.c | 4 +++-
|
||||
1 file changed, 3 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
|
||||
index fdf11a8318..e1a50389c9 100644
|
||||
--- a/src/interface/interface_backend_udev.c
|
||||
+++ b/src/interface/interface_backend_udev.c
|
||||
@@ -1027,12 +1027,14 @@ static char *
|
||||
udevInterfaceGetXMLDesc(virInterfacePtr ifinfo,
|
||||
unsigned int flags)
|
||||
{
|
||||
- struct udev *udev = udev_ref(driver->udev);
|
||||
+ struct udev *udev = NULL;
|
||||
g_autoptr(virInterfaceDef) ifacedef = NULL;
|
||||
char *xmlstr = NULL;
|
||||
|
||||
virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
|
||||
|
||||
+ udev = udev_ref(driver->udev);
|
||||
+
|
||||
/* Recursively build up the interface XML based on the requested
|
||||
* interface name
|
||||
*/
|
||||
--
|
||||
2.45.1
|
||||
|
||||
@@ -1,90 +0,0 @@
|
||||
From c120b31f826cd51127d28f8beaa61ac0d5f03048 Mon Sep 17 00:00:00 2001
|
||||
From: Dmitry Frolov <frolov@swemel.ru>
|
||||
Date: Tue, 12 Sep 2023 15:56:47 +0300
|
||||
Subject: [PATCH] interface: fix udev_device_get_sysattr_value return value
|
||||
check
|
||||
|
||||
Reviewing the code I found that return value of function
|
||||
udev_device_get_sysattr_value() is dereferenced without a check.
|
||||
udev_device_get_sysattr_value() may return NULL by number of reasons.
|
||||
|
||||
v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE()
|
||||
v3: More checks added, to skip earlier. More verbose VIR_DEBUG.
|
||||
|
||||
Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
|
||||
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
(cherry picked from commit 2ca94317ac642a70921947150ced8acc674ccdc8)
|
||||
---
|
||||
src/interface/interface_backend_udev.c | 26 +++++++++++++++++++-------
|
||||
1 file changed, 19 insertions(+), 7 deletions(-)
|
||||
|
||||
diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
|
||||
index a0485ddd21..fb6799ed94 100644
|
||||
--- a/src/interface/interface_backend_udev.c
|
||||
+++ b/src/interface/interface_backend_udev.c
|
||||
@@ -23,6 +23,7 @@
|
||||
#include <dirent.h>
|
||||
#include <libudev.h>
|
||||
|
||||
+#include "virlog.h"
|
||||
#include "virerror.h"
|
||||
#include "virfile.h"
|
||||
#include "datatypes.h"
|
||||
@@ -40,6 +41,8 @@
|
||||
|
||||
#define VIR_FROM_THIS VIR_FROM_INTERFACE
|
||||
|
||||
+VIR_LOG_INIT("interface.interface_backend_udev");
|
||||
+
|
||||
struct udev_iface_driver {
|
||||
struct udev *udev;
|
||||
/* pid file FD, ensures two copies of the driver can't use the same root */
|
||||
@@ -354,11 +357,20 @@ udevConnectListAllInterfaces(virConnectPtr conn,
|
||||
const char *macaddr;
|
||||
g_autoptr(virInterfaceDef) def = NULL;
|
||||
|
||||
- path = udev_list_entry_get_name(dev_entry);
|
||||
- dev = udev_device_new_from_syspath(udev, path);
|
||||
- name = udev_device_get_sysname(dev);
|
||||
+ if (!(path = udev_list_entry_get_name(dev_entry))) {
|
||||
+ VIR_DEBUG("Skipping interface, path == NULL");
|
||||
+ continue;
|
||||
+ }
|
||||
+ if (!(dev = udev_device_new_from_syspath(udev, path))) {
|
||||
+ VIR_DEBUG("Skipping interface '%s', dev == NULL", path);
|
||||
+ continue;
|
||||
+ }
|
||||
+ if (!(name = udev_device_get_sysname(dev))) {
|
||||
+ VIR_DEBUG("Skipping interface '%s', name == NULL", path);
|
||||
+ continue;
|
||||
+ }
|
||||
macaddr = udev_device_get_sysattr_value(dev, "address");
|
||||
- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
|
||||
+ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up");
|
||||
|
||||
def = udevGetMinimalDefForDevice(dev);
|
||||
if (!virConnectListAllInterfacesCheckACL(conn, def)) {
|
||||
@@ -964,9 +976,9 @@ udevGetIfaceDef(struct udev *udev, const char *name)
|
||||
|
||||
/* MTU */
|
||||
mtu_str = udev_device_get_sysattr_value(dev, "mtu");
|
||||
- if (virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) {
|
||||
+ if (!mtu_str || virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) {
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR,
|
||||
- _("Could not parse MTU value '%1$s'"), mtu_str);
|
||||
+ _("Could not parse MTU value '%1$s'"), NULLSTR(mtu_str));
|
||||
goto error;
|
||||
}
|
||||
ifacedef->mtu = mtu;
|
||||
@@ -1089,7 +1101,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
|
||||
goto cleanup;
|
||||
|
||||
/* Check if it's active or not */
|
||||
- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
|
||||
+ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up");
|
||||
|
||||
udev_device_unref(dev);
|
||||
|
||||
--
|
||||
2.43.0
|
||||
|
||||
@@ -1,49 +0,0 @@
|
||||
From 98f1cf88fa7e0f992d93f376418fbfb3996a9690 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
|
||||
Date: Fri, 17 May 2024 14:55:24 +0100
|
||||
Subject: [PATCH] rpc: avoid leak of GSource in use for interrupting main loop
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
We never release the reference on the GSource created for
|
||||
interrupting the main loop, nor do we remove it from the
|
||||
main context if our thread is woken up prior to the wakeup
|
||||
callback firing.
|
||||
|
||||
This can result in a leak of GSource objects, along with an
|
||||
ever growing list of GSources attached to the main context,
|
||||
which will gradually slow down execution of the loop, as
|
||||
several operations are O(N) for the number of attached GSource
|
||||
objects.
|
||||
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
---
|
||||
src/rpc/virnetclient.c | 3 ++-
|
||||
1 file changed, 2 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
|
||||
index 147b0d661a..6d424eb599 100644
|
||||
--- a/src/rpc/virnetclient.c
|
||||
+++ b/src/rpc/virnetclient.c
|
||||
@@ -1946,7 +1946,7 @@ static int virNetClientIO(virNetClient *client,
|
||||
/* Check to see if another thread is dispatching */
|
||||
if (client->haveTheBuck) {
|
||||
/* Force other thread to wakeup from poll */
|
||||
- GSource *wakeup = g_idle_source_new();
|
||||
+ g_autoptr(GSource) wakeup = g_idle_source_new();
|
||||
g_source_set_callback(wakeup, virNetClientIOWakeup, client->eventLoop, NULL);
|
||||
g_source_attach(wakeup, client->eventCtx);
|
||||
|
||||
@@ -1968,6 +1968,7 @@ static int virNetClientIO(virNetClient *client,
|
||||
return -1;
|
||||
}
|
||||
|
||||
+ g_source_destroy(wakeup);
|
||||
VIR_DEBUG("Woken up from sleep head=%p call=%p",
|
||||
client->waitDispatch, thiscall);
|
||||
/* Three reasons we can be woken up
|
||||
--
|
||||
2.45.1
|
||||
|
||||
@@ -1,99 +0,0 @@
|
||||
From 8074d64dc2eca846d6a61efe1a9b7428a0ce1dd1 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
|
||||
Date: Tue, 30 Apr 2024 11:51:15 +0100
|
||||
Subject: [PATCH] rpc: ensure temporary GSource is removed from client event
|
||||
loop
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Users are seeing periodic segfaults from libvirt client apps,
|
||||
especially thread heavy ones like virt-manager. A typical
|
||||
stack trace would end up in the virNetClientIOEventFD method,
|
||||
with illegal access to stale stack data. eg
|
||||
|
||||
==238721==ERROR: AddressSanitizer: stack-use-after-return on address 0x75cd18709788 at pc 0x75cd3111f907 bp 0x75cd181ff550 sp 0x75cd181ff548
|
||||
WRITE of size 4 at 0x75cd18709788 thread T11
|
||||
#0 0x75cd3111f906 in virNetClientIOEventFD /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1634:15
|
||||
#1 0x75cd3210d198 (/usr/lib/libglib-2.0.so.0+0x5a198) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
|
||||
#2 0x75cd3216c3be (/usr/lib/libglib-2.0.so.0+0xb93be) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
|
||||
#3 0x75cd3210ddc6 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x5adc6) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
|
||||
#4 0x75cd3111a47c in virNetClientIOEventLoop /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1722:9
|
||||
#5 0x75cd3111a47c in virNetClientIO /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2002:10
|
||||
#6 0x75cd3111a47c in virNetClientSendInternal /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2170:11
|
||||
#7 0x75cd311198a8 in virNetClientSendWithReply /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2198:11
|
||||
#8 0x75cd31111653 in virNetClientProgramCall /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclientprogram.c:318:9
|
||||
#9 0x75cd31241c8f in callFull /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6054:10
|
||||
#10 0x75cd31241c8f in call /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6076:12
|
||||
#11 0x75cd31241c8f in remoteNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/src/remote/remote_client_bodies.h:5959:9
|
||||
#12 0x75cd31410ff7 in virNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/libvirt-network.c:952:15
|
||||
|
||||
The root cause is a bad assumption in the virNetClientIOEventLoop
|
||||
method. This method is run by whichever thread currently owns the
|
||||
buck, and is responsible for handling I/O. Inside a for(;;) loop,
|
||||
this method creates a temporary GSource, adds it to the event loop
|
||||
and runs g_main_loop_run(). When I/O is ready, the GSource callback
|
||||
(virNetClientIOEventFD) will fire and call g_main_loop_quit(), and
|
||||
return G_SOURCE_REMOVE which results in the temporary GSource being
|
||||
destroyed. A g_autoptr() will then remove the last reference.
|
||||
|
||||
What was overlooked, is that a second thread can come along and
|
||||
while it can't enter virNetClientIOEventLoop, it will register an
|
||||
idle source that uses virNetClientIOWakeup to interrupt the
|
||||
original thread's 'g_main_loop_run' call. When this happens the
|
||||
virNetClientIOEventFD callback never runs, and so the temporary
|
||||
GSource is not destroyed. The g_autoptr() will remove a reference,
|
||||
but by virtue of still being attached to the event context, there
|
||||
is an extra reference held causing GSource to be leaked. The
|
||||
next time 'g_main_loop_run' is called, the original GSource will
|
||||
trigger its callback, and access data that was allocated on the
|
||||
stack by the previous thread, and likely SEGV.
|
||||
|
||||
To solve this, the thread calling 'g_main_loop_run' must call
|
||||
g_source_destroy, immediately upon return, to guarantee that
|
||||
the temporary GSource is removed.
|
||||
|
||||
CVE-2024-4418
|
||||
Reviewed-by: Ján Tomko <jtomko@redhat.com>
|
||||
Reported-by: Martin Shirokov <shirokovmartin@gmail.com>
|
||||
Tested-by: Martin Shirokov <shirokovmartin@gmail.com>
|
||||
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
---
|
||||
src/rpc/virnetclient.c | 14 +++++++++++++-
|
||||
1 file changed, 13 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
|
||||
index 68098b1c8d..147b0d661a 100644
|
||||
--- a/src/rpc/virnetclient.c
|
||||
+++ b/src/rpc/virnetclient.c
|
||||
@@ -1657,7 +1657,7 @@ static int virNetClientIOEventLoop(virNetClient *client,
|
||||
#endif /* !WIN32 */
|
||||
int timeout = -1;
|
||||
virNetMessage *msg = NULL;
|
||||
- g_autoptr(GSource) G_GNUC_UNUSED source = NULL;
|
||||
+ g_autoptr(GSource) source = NULL;
|
||||
GIOCondition ev = 0;
|
||||
struct virNetClientIOEventData data = {
|
||||
.client = client,
|
||||
@@ -1721,6 +1721,18 @@ static int virNetClientIOEventLoop(virNetClient *client,
|
||||
|
||||
g_main_loop_run(client->eventLoop);
|
||||
|
||||
+ /*
|
||||
+ * If virNetClientIOEventFD ran, this GSource will already be
|
||||
+ * destroyed due to G_SOURCE_REMOVE. It is harmless to re-destroy
|
||||
+ * it, since we still own a reference.
|
||||
+ *
|
||||
+ * If virNetClientIOWakeup ran, it will have interrupted the
|
||||
+ * g_main_loop_run call, before virNetClientIOEventFD could
|
||||
+ * run, and thus the GSource is still registered, and we need
|
||||
+ * to destroy it since it is referencing stack memory for 'data'
|
||||
+ */
|
||||
+ g_source_destroy(source);
|
||||
+
|
||||
#ifndef WIN32
|
||||
ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
|
||||
#endif /* !WIN32 */
|
||||
--
|
||||
2.45.1
|
||||
|
||||
@@ -1,76 +0,0 @@
|
||||
From 7cb03e6a28e465c49f0cabe8fe2e7d21edb5aadf Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
|
||||
Date: Mon, 18 Dec 2023 12:17:18 +0000
|
||||
Subject: [PATCH] rpc: fix race in waking up client event loop
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
The first thread to issue a client RPC request will own the event
|
||||
loop execution, sitting in the virNetClientIOEventLoop function.
|
||||
|
||||
It releases the client lock while running:
|
||||
|
||||
virNetClientUnlock()
|
||||
g_main_loop_run()
|
||||
virNetClientLock()
|
||||
|
||||
If a second thread arrives with an RPC request, it will queue it
|
||||
for the first thread to process. To inform the first thread that
|
||||
there's a new request it calls g_main_loop_quit() to break it out
|
||||
of the main loop.
|
||||
|
||||
This works if the first thread is in g_main_loop_run() at that
|
||||
time. There is a small window of opportunity, however, where
|
||||
the first thread has released the client lock, but not yet got
|
||||
into g_main_loop_run(). If that happens, the wakeup from the
|
||||
second thread is lost.
|
||||
|
||||
This patch deals with that by changing the way the wakeup is
|
||||
performed. Instead of directly calling g_main_loop_quit(), the
|
||||
second thread creates an idle source to run the quit function
|
||||
from within the first thread. This guarantees that the first
|
||||
thread will see the wakeup.
|
||||
|
||||
Tested by: Fima Shevrin <efim.shevrin@virtuozzo.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
Reviewed-by: Denis V. Lunev <den@openvz.org>
|
||||
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
---
|
||||
src/rpc/virnetclient.c | 13 ++++++++++++-
|
||||
1 file changed, 12 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
|
||||
index 4ab8af68c5..68098b1c8d 100644
|
||||
--- a/src/rpc/virnetclient.c
|
||||
+++ b/src/rpc/virnetclient.c
|
||||
@@ -1848,6 +1848,15 @@ static void virNetClientIOUpdateCallback(virNetClient *client,
|
||||
}
|
||||
|
||||
|
||||
+static gboolean virNetClientIOWakeup(gpointer opaque)
|
||||
+{
|
||||
+ GMainLoop *loop = opaque;
|
||||
+
|
||||
+ g_main_loop_quit(loop);
|
||||
+
|
||||
+ return G_SOURCE_REMOVE;
|
||||
+}
|
||||
+
|
||||
/*
|
||||
* This function sends a message to remote server and awaits a reply
|
||||
*
|
||||
@@ -1925,7 +1934,9 @@ static int virNetClientIO(virNetClient *client,
|
||||
/* Check to see if another thread is dispatching */
|
||||
if (client->haveTheBuck) {
|
||||
/* Force other thread to wakeup from poll */
|
||||
- g_main_loop_quit(client->eventLoop);
|
||||
+ GSource *wakeup = g_idle_source_new();
|
||||
+ g_source_set_callback(wakeup, virNetClientIOWakeup, client->eventLoop, NULL);
|
||||
+ g_source_attach(wakeup, client->eventCtx);
|
||||
|
||||
/* If we are non-blocking, detach the thread and keep the call in the
|
||||
* queue. */
|
||||
--
|
||||
2.43.0
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
From: Martin Kletzander <mkletzan@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 13:56:51 +0200
|
||||
Subject: [PATCH] virarptable: Properly calculate rtattr length
|
||||
Content-type: text/plain
|
||||
|
||||
Use convenience macro which does almost the same thing we were doing,
|
||||
but also pads out the payload length to a multiple of NLMSG_ALIGNTO (4)
|
||||
bytes.
|
||||
|
||||
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Reviewed-by: Laine Stump <laine@redhat.com>
|
||||
---
|
||||
src/util/virarptable.c | 3 +--
|
||||
1 file changed, 1 insertion(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
|
||||
index 299dddd664..d8e41c5a86 100644
|
||||
--- a/src/util/virarptable.c
|
||||
+++ b/src/util/virarptable.c
|
||||
@@ -102,8 +102,7 @@ virArpTableGet(void)
|
||||
return table;
|
||||
|
||||
VIR_WARNINGS_NO_CAST_ALIGN
|
||||
- parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
|
||||
- nh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
|
||||
+ parse_rtattr(tb, NDA_MAX, NDA_RTA(r), NLMSG_PAYLOAD(nh, sizeof(*r)));
|
||||
VIR_WARNINGS_RESET
|
||||
|
||||
if (tb[NDA_DST] == NULL || tb[NDA_LLADDR] == NULL)
|
||||
@@ -0,0 +1,34 @@
|
||||
From: Martin Kletzander <mkletzan@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 13:59:15 +0200
|
||||
Subject: [PATCH] virarptable: Fix check for message length
|
||||
Content-type: text/plain
|
||||
|
||||
The previous check was all wrong since it calculated the how long would
|
||||
the netlink message be if the netlink header was the payload and then
|
||||
subtracted that from the whole message length, a variable that was not
|
||||
used later in the code. This check can fail if there are no additional
|
||||
payloads, struct rtattr in particular, which we are parsing later,
|
||||
however the RTA_OK macro would've caught that anyway.
|
||||
|
||||
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Reviewed-by: Laine Stump <laine@redhat.com>
|
||||
---
|
||||
src/util/virarptable.c | 3 +--
|
||||
1 file changed, 1 insertion(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
|
||||
index d8e41c5a86..45ee76766f 100644
|
||||
--- a/src/util/virarptable.c
|
||||
+++ b/src/util/virarptable.c
|
||||
@@ -81,10 +81,9 @@ virArpTableGet(void)
|
||||
for (; NLMSG_OK(nh, msglen); nh = NLMSG_NEXT(nh, msglen)) {
|
||||
VIR_WARNINGS_RESET
|
||||
struct ndmsg *r = NLMSG_DATA(nh);
|
||||
- int len = nh->nlmsg_len;
|
||||
void *addr;
|
||||
|
||||
- if ((len -= NLMSG_LENGTH(sizeof(*nh))) < 0) {
|
||||
+ if (nh->nlmsg_len < NLMSG_SPACE(sizeof(*r))) {
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||||
_("wrong nlmsg len"));
|
||||
goto cleanup;
|
||||
@@ -0,0 +1,46 @@
|
||||
From: Martin Kletzander <mkletzan@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 14:02:48 +0200
|
||||
Subject: [PATCH] virarptable: End parsing earlier in case of NLMSG_DONE
|
||||
Content-type: text/plain
|
||||
|
||||
Check for the last multipart message right as the first thing. The
|
||||
presumption probably was that the last message might still contain a
|
||||
payload we want to parse. However that cannot be true since that would
|
||||
have to be a type RTM_NEWNEIGH. This was not caught because older
|
||||
kernels were note sending NLMSG_DONE and probably relied on the fact
|
||||
that the parsing just stops after all the messages are walked through,
|
||||
which the NLMSG_OK macro successfully did.
|
||||
|
||||
Resolves: https://issues.redhat.com/browse/RHEL-52449
|
||||
Resolves: https://bugzilla.redhat.com/2302245
|
||||
Fixes: a176d67cdfaf5b8237a7e3a80d8be0e6bdf2d8fd
|
||||
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Reviewed-by: Laine Stump <laine@redhat.com>
|
||||
---
|
||||
src/util/virarptable.c | 6 +++---
|
||||
1 file changed, 3 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
|
||||
index 45ee76766f..20d11f97b0 100644
|
||||
--- a/src/util/virarptable.c
|
||||
+++ b/src/util/virarptable.c
|
||||
@@ -83,6 +83,9 @@ virArpTableGet(void)
|
||||
struct ndmsg *r = NLMSG_DATA(nh);
|
||||
void *addr;
|
||||
|
||||
+ if (nh->nlmsg_type == NLMSG_DONE)
|
||||
+ break;
|
||||
+
|
||||
if (nh->nlmsg_len < NLMSG_SPACE(sizeof(*r))) {
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||||
_("wrong nlmsg len"));
|
||||
@@ -97,9 +100,6 @@ virArpTableGet(void)
|
||||
(!(r->ndm_state == NUD_STALE || r->ndm_state == NUD_REACHABLE)))
|
||||
continue;
|
||||
|
||||
- if (nh->nlmsg_type == NLMSG_DONE)
|
||||
- return table;
|
||||
-
|
||||
VIR_WARNINGS_NO_CAST_ALIGN
|
||||
parse_rtattr(tb, NDA_MAX, NDA_RTA(r), NLMSG_PAYLOAD(nh, sizeof(*r)));
|
||||
VIR_WARNINGS_RESET
|
||||
@@ -0,0 +1,315 @@
|
||||
From 807e2670f2704c41f0a1dca81a5d2f2f9336137c Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:44 -0500
|
||||
Subject: [PATCH 4/9] util: use a single flags arg for virNetDevBandwidthSet(),
|
||||
not multiple bools
|
||||
|
||||
Having two bools in the arg list is on the borderline of being
|
||||
confusing to anyone trying to read the code, but we're about to add a
|
||||
3rd. This patch replaces the two bools with a single flags argument
|
||||
which will instead have one or more bits from virNetDevBandwidthFlags
|
||||
set.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/lxc/lxc_driver.c | 8 ++++++--
|
||||
src/lxc/lxc_process.c | 8 ++++++--
|
||||
src/network/bridge_driver.c | 10 ++++++++--
|
||||
src/qemu/qemu_command.c | 11 ++++++++---
|
||||
src/qemu/qemu_driver.c | 29 ++++++++++++++-------------
|
||||
src/qemu/qemu_hotplug.c | 22 +++++++++++++++------
|
||||
src/util/virnetdevbandwidth.c | 36 ++++++++++++++++++++--------------
|
||||
src/util/virnetdevbandwidth.h | 9 +++++++--
|
||||
tests/virnetdevbandwidthtest.c | 8 +++++++-
|
||||
9 files changed, 94 insertions(+), 47 deletions(-)
|
||||
|
||||
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
|
||||
index 534e257f30..b693980dbb 100644
|
||||
--- a/src/lxc/lxc_driver.c
|
||||
+++ b/src/lxc/lxc_driver.c
|
||||
@@ -3570,8 +3570,12 @@ lxcDomainAttachDeviceNetLive(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(actualType)) {
|
||||
- if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0)
|
||||
+ unsigned int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
|
||||
index f5eb5383ec..0e689fbb70 100644
|
||||
--- a/src/lxc/lxc_process.c
|
||||
+++ b/src/lxc/lxc_process.c
|
||||
@@ -605,8 +605,12 @@ virLXCProcessSetupInterfaces(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(type)) {
|
||||
- if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0)
|
||||
+ unsigned int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
|
||||
index 32572c755f..1c53636450 100644
|
||||
--- a/src/network/bridge_driver.c
|
||||
+++ b/src/network/bridge_driver.c
|
||||
@@ -2058,8 +2058,11 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
|
||||
}
|
||||
}
|
||||
|
||||
- if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0)
|
||||
+ if (virNetDevBandwidthSet(def->bridge, def->bandwidth,
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
|
||||
+ | VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) < 0) {
|
||||
goto error;
|
||||
+ }
|
||||
|
||||
return 0;
|
||||
|
||||
@@ -2141,8 +2144,11 @@ networkStartNetworkBridge(virNetworkObj *obj)
|
||||
* type BRIDGE, is started. On failure, undo anything you've done,
|
||||
* and return -1. On success return 0.
|
||||
*/
|
||||
- if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0)
|
||||
+ if (virNetDevBandwidthSet(def->bridge, def->bandwidth,
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
|
||||
+ | VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) < 0) {
|
||||
goto error;
|
||||
+ }
|
||||
|
||||
if (networkStartHandleMACTableManagerMode(obj) < 0)
|
||||
goto error;
|
||||
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
|
||||
index f15e6bda1e..b4815e5e71 100644
|
||||
--- a/src/qemu/qemu_command.c
|
||||
+++ b/src/qemu/qemu_command.c
|
||||
@@ -8840,9 +8840,14 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
|
||||
def->uuid,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
- } else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0) {
|
||||
- goto cleanup;
|
||||
+ } else {
|
||||
+ unsigned int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
+ goto cleanup;
|
||||
}
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
|
||||
index 736602333e..14929616e5 100644
|
||||
--- a/src/qemu/qemu_driver.c
|
||||
+++ b/src/qemu/qemu_driver.c
|
||||
@@ -9941,21 +9941,22 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
|
||||
virErrorRestore(&orig_err);
|
||||
goto endjob;
|
||||
}
|
||||
- } else if (virNetDevBandwidthSet(net->ifname, newBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0) {
|
||||
- virErrorPtr orig_err;
|
||||
-
|
||||
- virErrorPreserveLast(&orig_err);
|
||||
- ignore_value(virNetDevBandwidthSet(net->ifname,
|
||||
- net->bandwidth,
|
||||
- false,
|
||||
- !virDomainNetTypeSharesHostView(net)));
|
||||
- if (net->bandwidth) {
|
||||
- ignore_value(virDomainNetBandwidthUpdate(net,
|
||||
- net->bandwidth));
|
||||
+ } else {
|
||||
+ unsigned int bwflags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ bwflags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, newBandwidth, bwflags) < 0) {
|
||||
+ virErrorPtr orig_err;
|
||||
+
|
||||
+ virErrorPreserveLast(&orig_err);
|
||||
+ ignore_value(virNetDevBandwidthSet(net->ifname, net->bandwidth, bwflags));
|
||||
+ if (net->bandwidth)
|
||||
+ ignore_value(virDomainNetBandwidthUpdate(net, net->bandwidth));
|
||||
+ virErrorRestore(&orig_err);
|
||||
+ goto endjob;
|
||||
}
|
||||
- virErrorRestore(&orig_err);
|
||||
- goto endjob;
|
||||
}
|
||||
|
||||
/* If the old bandwidth was cleared out, restore qdisc. */
|
||||
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
|
||||
index 7cb1800504..d5e7e99359 100644
|
||||
--- a/src/qemu/qemu_hotplug.c
|
||||
+++ b/src/qemu/qemu_hotplug.c
|
||||
@@ -1279,9 +1279,14 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
|
||||
vm->def->uuid,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
- } else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0) {
|
||||
- goto cleanup;
|
||||
+ } else {
|
||||
+ int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
+ goto cleanup;
|
||||
}
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
@@ -4082,9 +4087,14 @@ qemuDomainChangeNet(virQEMUDriver *driver,
|
||||
vm->def->uuid,
|
||||
!virDomainNetTypeSharesHostView(newdev)) < 0)
|
||||
goto cleanup;
|
||||
- } else if (virNetDevBandwidthSet(newdev->ifname, newb, false,
|
||||
- !virDomainNetTypeSharesHostView(newdev)) < 0) {
|
||||
- goto cleanup;
|
||||
+ } else {
|
||||
+ int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(newdev))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(newdev->ifname, newb, flags) < 0)
|
||||
+ goto cleanup;
|
||||
}
|
||||
} else {
|
||||
if (virDomainInterfaceClearQoS(vm->def, olddev) < 0)
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 2b58c58d3e..1baad849c6 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -173,30 +173,35 @@ virNetDevBandwidthManipulateFilter(const char *ifname,
|
||||
* virNetDevBandwidthSet:
|
||||
* @ifname: on which interface
|
||||
* @bandwidth: rates to set (may be NULL)
|
||||
- * @hierarchical_class: whether to create hierarchical class
|
||||
- * @swapped: true if IN/OUT should be set contrariwise
|
||||
+ * @flags: bits indicating certain optional actions
|
||||
*
|
||||
+
|
||||
* This function enables QoS on specified interface
|
||||
* and set given traffic limits for both, incoming
|
||||
- * and outgoing traffic. Any previous setting get
|
||||
- * overwritten. If @hierarchical_class is TRUE, create
|
||||
- * hierarchical class. It is used to guarantee minimal
|
||||
- * throughput ('floor' attribute in NIC).
|
||||
+ * and outgoing traffic.
|
||||
+ *
|
||||
+ * @flags bits and their meanings:
|
||||
+ *
|
||||
+ * VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
|
||||
+ * whether to create a hierarchical class
|
||||
+ * A hiearchical class structure is used to implement a minimal
|
||||
+ * throughput guarantee ('floor' attribute in NIC).
|
||||
*
|
||||
- * If @swapped is set, the IN part of @bandwidth is set on
|
||||
- * @ifname's TX, and vice versa. If it is not set, IN is set on
|
||||
- * RX and OUT on TX. This is because for some types of interfaces
|
||||
- * domain and the host live on the same side of the interface (so
|
||||
- * domain's RX/TX is host's RX/TX), and for some it's swapped
|
||||
- * (domain's RX/TX is hosts's TX/RX).
|
||||
+ * VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED
|
||||
+ * set if IN/OUT should be set backwards from what's indicated in
|
||||
+ * the bandwidth, i.e. the IN part of @bandwidth is set on
|
||||
+ * @ifname's TX, and the OUT part of @bandwidth is set on
|
||||
+ * @ifname's RX. This is needed because for some types of
|
||||
+ * interfaces the domain and the host live on the same side of the
|
||||
+ * interface (so domain's RX/TX is host's RX/TX), and for some
|
||||
+ * it's swapped (domain's RX/TX is hosts's TX/RX).
|
||||
*
|
||||
* Return 0 on success, -1 otherwise.
|
||||
*/
|
||||
int
|
||||
virNetDevBandwidthSet(const char *ifname,
|
||||
const virNetDevBandwidth *bandwidth,
|
||||
- bool hierarchical_class,
|
||||
- bool swapped)
|
||||
+ unsigned int flags)
|
||||
{
|
||||
int ret = -1;
|
||||
virNetDevBandwidthRate *rx = NULL; /* From domain POV */
|
||||
@@ -205,6 +210,7 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
char *average = NULL;
|
||||
char *peak = NULL;
|
||||
char *burst = NULL;
|
||||
+ bool hierarchical_class = flags & VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;
|
||||
|
||||
if (!bandwidth) {
|
||||
/* nothing to be enabled */
|
||||
@@ -224,7 +230,7 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
return -1;
|
||||
}
|
||||
|
||||
- if (swapped) {
|
||||
+ if (flags & VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) {
|
||||
rx = bandwidth->out;
|
||||
tx = bandwidth->in;
|
||||
} else {
|
||||
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
|
||||
index 6d268fb119..80dc654486 100644
|
||||
--- a/src/util/virnetdevbandwidth.h
|
||||
+++ b/src/util/virnetdevbandwidth.h
|
||||
@@ -39,11 +39,16 @@ void virNetDevBandwidthFree(virNetDevBandwidth *def);
|
||||
|
||||
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevBandwidth, virNetDevBandwidthFree);
|
||||
|
||||
+typedef enum {
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS = (1 << 0),
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED = (1 << 1),
|
||||
+} virNetDevBandwidthSetFlags;
|
||||
+
|
||||
int virNetDevBandwidthSet(const char *ifname,
|
||||
const virNetDevBandwidth *bandwidth,
|
||||
- bool hierarchical_class,
|
||||
- bool swapped)
|
||||
+ unsigned int flags)
|
||||
G_GNUC_WARN_UNUSED_RESULT;
|
||||
+
|
||||
int virNetDevBandwidthClear(const char *ifname);
|
||||
int virNetDevBandwidthCopy(virNetDevBandwidth **dest,
|
||||
const virNetDevBandwidth *src)
|
||||
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
|
||||
index f7c38faa2e..6529ff4026 100644
|
||||
--- a/tests/virnetdevbandwidthtest.c
|
||||
+++ b/tests/virnetdevbandwidthtest.c
|
||||
@@ -82,8 +82,14 @@ testVirNetDevBandwidthSet(const void *data)
|
||||
if (virNetDevOpenvswitchInterfaceSetQos(iface, band, info->uuid, true) < 0)
|
||||
return -1;
|
||||
} else {
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (info->hierarchical_class)
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;
|
||||
+
|
||||
exp_cmd = info->exp_cmd_tc;
|
||||
- if (virNetDevBandwidthSet(iface, band, info->hierarchical_class, true) < 0)
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(iface, band, flags) < 0)
|
||||
return -1;
|
||||
}
|
||||
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,185 @@
|
||||
From 490f58382dca2a415a5f16b6133f298d853bb379 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:45 -0500
|
||||
Subject: [PATCH 5/9] util: make it optional to clear existing tc
|
||||
qdiscs/filters in virNetDevBandwidthSet()
|
||||
|
||||
virNetDevBandwidthSet() always clears all existing qdiscs and their
|
||||
subordinate filters before adding all the new qdiscs/filters. This is
|
||||
normally exactly what we want, but there is one case (the network
|
||||
driver) where the Qdisc added by virNetDevBandwidthSet() may already
|
||||
be in use by the nftables backend (which will add a rule to fix the
|
||||
checksum of dhcp packets); in that case, we *don't* want
|
||||
virNetDevBandwidthSet() to clear out the qdisc that was already added
|
||||
for nftables, and none of the bandwidth filters have been added yet,
|
||||
so there already aren't any "old" filters that need to be removed
|
||||
either - it is safe to just skip virNetDevBandwidthClear() in this
|
||||
case.
|
||||
|
||||
To allow the network driver to set bandwidth without first clearing
|
||||
it, this patch adds the flag VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL to the
|
||||
virNetDevBandwidthSetFlags enum, and recognizes it in
|
||||
virNetDevBandwidthSet() - if the flag is set, then
|
||||
virNetDevBandwidth() will call virNetDevBandwidthClear() just as it
|
||||
always has. But if the flag isn't set it *won't* call
|
||||
virNetDevBandwidthClear().
|
||||
|
||||
As suggested above, VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL is set for all
|
||||
calls to virNetdevBandwidthSet() except for two places in the network
|
||||
driver.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/lxc/lxc_driver.c | 2 +-
|
||||
src/lxc/lxc_process.c | 2 +-
|
||||
src/qemu/qemu_command.c | 2 +-
|
||||
src/qemu/qemu_driver.c | 2 +-
|
||||
src/qemu/qemu_hotplug.c | 4 ++--
|
||||
src/util/virnetdevbandwidth.c | 21 ++++++++++++++++++++-
|
||||
src/util/virnetdevbandwidth.h | 1 +
|
||||
tests/virnetdevbandwidthtest.c | 3 ++-
|
||||
8 files changed, 29 insertions(+), 8 deletions(-)
|
||||
|
||||
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
|
||||
index b693980dbb..81581c74df 100644
|
||||
--- a/src/lxc/lxc_driver.c
|
||||
+++ b/src/lxc/lxc_driver.c
|
||||
@@ -3570,7 +3570,7 @@ lxcDomainAttachDeviceNetLive(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(actualType)) {
|
||||
- unsigned int flags = 0;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
|
||||
index 0e689fbb70..081ce03a57 100644
|
||||
--- a/src/lxc/lxc_process.c
|
||||
+++ b/src/lxc/lxc_process.c
|
||||
@@ -605,7 +605,7 @@ virLXCProcessSetupInterfaces(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(type)) {
|
||||
- unsigned int flags = 0;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
|
||||
index b4815e5e71..ed54fd4c5b 100644
|
||||
--- a/src/qemu/qemu_command.c
|
||||
+++ b/src/qemu/qemu_command.c
|
||||
@@ -8841,7 +8841,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
- unsigned int flags = 0;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
|
||||
index 14929616e5..9549065b1f 100644
|
||||
--- a/src/qemu/qemu_driver.c
|
||||
+++ b/src/qemu/qemu_driver.c
|
||||
@@ -9942,7 +9942,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
|
||||
goto endjob;
|
||||
}
|
||||
} else {
|
||||
- unsigned int bwflags = 0;
|
||||
+ unsigned int bwflags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
bwflags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
|
||||
index d5e7e99359..ceda4119cd 100644
|
||||
--- a/src/qemu/qemu_hotplug.c
|
||||
+++ b/src/qemu/qemu_hotplug.c
|
||||
@@ -1280,7 +1280,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
- int flags = 0;
|
||||
+ int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
@@ -4088,7 +4088,7 @@ qemuDomainChangeNet(virQEMUDriver *driver,
|
||||
!virDomainNetTypeSharesHostView(newdev)) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
- int flags = 0;
|
||||
+ int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(newdev))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 1baad849c6..9c48844c5d 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -196,6 +196,21 @@ virNetDevBandwidthManipulateFilter(const char *ifname,
|
||||
* interface (so domain's RX/TX is host's RX/TX), and for some
|
||||
* it's swapped (domain's RX/TX is hosts's TX/RX).
|
||||
*
|
||||
+ * VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL
|
||||
+ * If VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL is set, then the root
|
||||
+ * qdisc is deleted before adding any new qdisc/class/filter,
|
||||
+ * which causes any pre-existing filters to also be deleted. If
|
||||
+ * not set, then it's assumed that there are no existing rules (or
|
||||
+ * that those already there need to be kept). The caller should
|
||||
+ * set this flag for an existing interface that is having its
|
||||
+ * bandwidth settings modified, but can leave it unset if the
|
||||
+ * interface was newly created and this is the first time
|
||||
+ * bandwidth has been set, but someone else might have already
|
||||
+ * added the qdisc (e.g. this is the case when the network driver
|
||||
+ * is setting bandwidth for a virtual network bridge device - the
|
||||
+ * nftables backend may have already added qdisc handle 1:0 and a
|
||||
+ * filter, and we don't want to delete them)
|
||||
+ *
|
||||
* Return 0 on success, -1 otherwise.
|
||||
*/
|
||||
int
|
||||
@@ -238,7 +253,11 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
tx = bandwidth->out;
|
||||
}
|
||||
|
||||
- virNetDevBandwidthClear(ifname);
|
||||
+ /* Only if the caller requests, clear everything including root
|
||||
+ * qdisc and all filters before adding everything.
|
||||
+ */
|
||||
+ if (flags & VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL)
|
||||
+ virNetDevBandwidthClear(ifname);
|
||||
|
||||
if (tx && tx->average) {
|
||||
average = g_strdup_printf("%llukbps", tx->average);
|
||||
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
|
||||
index 80dc654486..744aa4c826 100644
|
||||
--- a/src/util/virnetdevbandwidth.h
|
||||
+++ b/src/util/virnetdevbandwidth.h
|
||||
@@ -42,6 +42,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevBandwidth, virNetDevBandwidthFree);
|
||||
typedef enum {
|
||||
VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS = (1 << 0),
|
||||
VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED = (1 << 1),
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL = (1 << 2),
|
||||
} virNetDevBandwidthSetFlags;
|
||||
|
||||
int virNetDevBandwidthSet(const char *ifname,
|
||||
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
|
||||
index 6529ff4026..6d5c847ad7 100644
|
||||
--- a/tests/virnetdevbandwidthtest.c
|
||||
+++ b/tests/virnetdevbandwidthtest.c
|
||||
@@ -82,7 +82,8 @@ testVirNetDevBandwidthSet(const void *data)
|
||||
if (virNetDevOpenvswitchInterfaceSetQos(iface, band, info->uuid, true) < 0)
|
||||
return -1;
|
||||
} else {
|
||||
- unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED |
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (info->hierarchical_class)
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
From faebbbbfa3b1bd4120852b3f416c8073ab82d5c5 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:46 -0500
|
||||
Subject: [PATCH 6/9] util: put the command that adds a tx filter qdisc into a
|
||||
separate function
|
||||
|
||||
virNetDevBandwidthSet() adds a queue discipline (qdisc) for each
|
||||
interface that it will need to add tc transmit filters to, and the
|
||||
filters are then attached to the qdisc.
|
||||
|
||||
There are other circumstances where some other function will need to
|
||||
add tc transmit filters to an interface (in particular an upcoming
|
||||
patch to the network driver nftables backend that will use a tc tx
|
||||
filter to fix the checksum of dhcp packets), so that function will
|
||||
also need a qdisc for the tx filter. To assure both always use exactly
|
||||
the same qdisc, this patch puts the command that adds the tx filter
|
||||
qdisc into a separate helper function that can (and will) be called
|
||||
from either place
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/libvirt_private.syms | 1 +
|
||||
src/util/virnetdevbandwidth.c | 30 +++++++++++++++++++++++++-----
|
||||
src/util/virnetdevbandwidth.h | 3 +++
|
||||
3 files changed, 29 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
|
||||
index d15d6a6a9d..0211cee967 100644
|
||||
--- a/src/libvirt_private.syms
|
||||
+++ b/src/libvirt_private.syms
|
||||
@@ -2859,6 +2859,7 @@ virNetDevVFInterfaceStats;
|
||||
|
||||
|
||||
# util/virnetdevbandwidth.h
|
||||
+virNetDevBandWidthAddTxFilterParentQdisc;
|
||||
virNetDevBandwidthClear;
|
||||
virNetDevBandwidthCopy;
|
||||
virNetDevBandwidthEqual;
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 9c48844c5d..90eebe6576 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -266,11 +266,7 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
if (tx->burst)
|
||||
burst = g_strdup_printf("%llukb", tx->burst);
|
||||
|
||||
- cmd = virCommandNew(TC);
|
||||
- virCommandAddArgList(cmd, "qdisc", "add", "dev", ifname, "root",
|
||||
- "handle", "1:", "htb", "default",
|
||||
- hierarchical_class ? "2" : "1", NULL);
|
||||
- if (virCommandRun(cmd, NULL) < 0)
|
||||
+ if (virNetDevBandWidthAddTxFilterParentQdisc(ifname, hierarchical_class) < 0)
|
||||
goto cleanup;
|
||||
|
||||
/* If we are creating a hierarchical class, all non guaranteed traffic
|
||||
@@ -794,3 +790,27 @@ virNetDevBandwidthSetRootQDisc(const char *ifname,
|
||||
|
||||
return 0;
|
||||
}
|
||||
+
|
||||
+/**
|
||||
+ * virNetDevBandwidthAddTxFilterParentQdisc:
|
||||
+ * @ifname: name of interface that needs a qdisc to attach tx filters to
|
||||
+ * @hierarchical_class: true if hierarchical classes will be used on this interface
|
||||
+ *
|
||||
+ * Add a root Qdisc (Queueing Discipline) for attaching Tx filters to
|
||||
+ * @ifname.
|
||||
+ *
|
||||
+ * returns 0 on success, -1 on failure
|
||||
+ */
|
||||
+int
|
||||
+virNetDevBandWidthAddTxFilterParentQdisc(const char *ifname,
|
||||
+ bool hierarchical_class)
|
||||
+{
|
||||
+ g_autoptr(virCommand) cmd = NULL;
|
||||
+
|
||||
+ cmd = virCommandNew(TC);
|
||||
+ virCommandAddArgList(cmd, "qdisc", "add", "dev", ifname, "root",
|
||||
+ "handle", "1:", "htb", "default",
|
||||
+ hierarchical_class ? "2" : "1", NULL);
|
||||
+
|
||||
+ return virCommandRun(cmd, NULL);
|
||||
+}
|
||||
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
|
||||
index 744aa4c826..65c1500637 100644
|
||||
--- a/src/util/virnetdevbandwidth.h
|
||||
+++ b/src/util/virnetdevbandwidth.h
|
||||
@@ -84,3 +84,6 @@ int virNetDevBandwidthUpdateFilter(const char *ifname,
|
||||
int virNetDevBandwidthSetRootQDisc(const char *ifname,
|
||||
const char *qdisc)
|
||||
G_NO_INLINE;
|
||||
+
|
||||
+int virNetDevBandWidthAddTxFilterParentQdisc(const char *ifname,
|
||||
+ bool hierarchical_class);
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
From 73c0fb19ce5b816ee81ede691252855c75391c9a Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:47 -0500
|
||||
Subject: [PATCH 7/9] util: don't re-add the qdisc used for tx filters if it
|
||||
already exists
|
||||
|
||||
There will soon be two separate users of tc on virtual networks, and
|
||||
both will use the "qdisc root handle 1: htb" to add tx filters. One or the
|
||||
other could get the first chance to add the qdisc, and then if at a
|
||||
later time the other decides to use it, we need to prevent the 2nd
|
||||
user from attempting to re-add the qdisc (because that just generates
|
||||
an error).
|
||||
|
||||
We do this by running "tc qdisc show dev $bridge handle 1:" then
|
||||
checking if the output of that command contains both "qdisc" and " 1:
|
||||
".[*] If it does then the qdisc has already been added. If not then we
|
||||
need to add it now.
|
||||
|
||||
[*]As of this writing, the output more exactly starts with "qdisc
|
||||
htb 1: root", but our comparison is made purposefully generous to
|
||||
increase the chances that it will continue to work properly if tc
|
||||
modifies the format of its output.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/util/virnetdevbandwidth.c | 35 ++++++++++++++++++++++++++++------
|
||||
tests/virnetdevbandwidthtest.c | 3 +++
|
||||
2 files changed, 32 insertions(+), 6 deletions(-)
|
||||
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 90eebe6576..5c6a65528c 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -805,12 +805,35 @@ int
|
||||
virNetDevBandWidthAddTxFilterParentQdisc(const char *ifname,
|
||||
bool hierarchical_class)
|
||||
{
|
||||
- g_autoptr(virCommand) cmd = NULL;
|
||||
+ g_autoptr(virCommand) testCmd = NULL;
|
||||
+ g_autofree char *testResult = NULL;
|
||||
|
||||
- cmd = virCommandNew(TC);
|
||||
- virCommandAddArgList(cmd, "qdisc", "add", "dev", ifname, "root",
|
||||
- "handle", "1:", "htb", "default",
|
||||
- hierarchical_class ? "2" : "1", NULL);
|
||||
+ /* first check it the qdisc with handle 1: was already added for
|
||||
+ * this interface by someone else
|
||||
+ */
|
||||
+ testCmd = virCommandNew(TC);
|
||||
+ virCommandAddArgList(testCmd, "qdisc", "show", "dev", ifname,
|
||||
+ "handle", "1:", NULL);
|
||||
+ virCommandSetOutputBuffer(testCmd, &testResult);
|
||||
|
||||
- return virCommandRun(cmd, NULL);
|
||||
+ if (virCommandRun(testCmd, NULL) < 0)
|
||||
+ return -1;
|
||||
+
|
||||
+ /* output will be something like: "qdisc htb 1: root refcnt ..."
|
||||
+ * if the qdisc was already added. We just search for "qdisc" and
|
||||
+ * " 1: " anywhere in the output to allow for tc changing its
|
||||
+ * output format.
|
||||
+ */
|
||||
+ if (!(testResult && strstr(testResult, "qdisc") && strstr(testResult, " 1: "))) {
|
||||
+ /* didn't find qdisc in output, so we need to add one */
|
||||
+ g_autoptr(virCommand) addCmd = virCommandNew(TC);
|
||||
+
|
||||
+ virCommandAddArgList(addCmd, "qdisc", "add", "dev", ifname, "root",
|
||||
+ "handle", "1:", "htb", "default",
|
||||
+ hierarchical_class ? "2" : "1", NULL);
|
||||
+
|
||||
+ return virCommandRun(addCmd, NULL);
|
||||
+ }
|
||||
+
|
||||
+ return 0;
|
||||
}
|
||||
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
|
||||
index 6d5c847ad7..31aa7f469d 100644
|
||||
--- a/tests/virnetdevbandwidthtest.c
|
||||
+++ b/tests/virnetdevbandwidthtest.c
|
||||
@@ -147,6 +147,7 @@ mymain(void)
|
||||
"</bandwidth>",
|
||||
TC " qdisc del dev eth0 root\n"
|
||||
TC " qdisc del dev eth0 ingress\n"
|
||||
+ TC " qdisc show dev eth0 handle 1:\n"
|
||||
TC " qdisc add dev eth0 root handle 1: htb default 1\n"
|
||||
TC " class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps quantum 87\n"
|
||||
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
|
||||
@@ -177,6 +178,7 @@ mymain(void)
|
||||
"</bandwidth>",
|
||||
TC " qdisc del dev eth0 root\n"
|
||||
TC " qdisc del dev eth0 ingress\n"
|
||||
+ TC " qdisc show dev eth0 handle 1:\n"
|
||||
TC " qdisc add dev eth0 root handle 1: htb default 1\n"
|
||||
TC " class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil 2kbps burst 4kb quantum 1\n"
|
||||
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
|
||||
@@ -199,6 +201,7 @@ mymain(void)
|
||||
"</bandwidth>",
|
||||
TC " qdisc del dev eth0 root\n"
|
||||
TC " qdisc del dev eth0 ingress\n"
|
||||
+ TC " qdisc show dev eth0 handle 1:\n"
|
||||
TC " qdisc add dev eth0 root handle 1: htb default 1\n"
|
||||
TC " class add dev eth0 parent 1: classid 1:1 htb rate 4294967295kbps quantum 366503875\n"
|
||||
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,171 @@
|
||||
From dac9cb9030ac03d18f59884864a0a253e3c9f8f1 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:48 -0500
|
||||
Subject: [PATCH 8/9] util: add new "tc" layer for virFirewallCmd objects
|
||||
|
||||
If the layer of a virFirewallCmd is "tc", then the "tc" utility will
|
||||
be executed using the arguments that had been added to the
|
||||
virFirewallCmd
|
||||
|
||||
tc layer doesn't support auto-rollback command creation (any rollback
|
||||
needs to be added manually with virFirewallAddRollbackCmd()), and also
|
||||
tc layer isn't supported by the iptables backend (it would have been
|
||||
straightforward to add, but the iptables backend doesn't need it, and
|
||||
I didn't want to take the chance of causing a regression in that
|
||||
code for no good reason).
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/network/network_nftables.c | 1 +
|
||||
src/util/virfirewall.c | 66 +++++++++++++++++++++-------------
|
||||
src/util/virfirewall.h | 1 +
|
||||
src/util/virfirewalld.c | 1 +
|
||||
4 files changed, 44 insertions(+), 25 deletions(-)
|
||||
|
||||
diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c
|
||||
index 268d1f12ca..cc184105c3 100644
|
||||
--- a/src/network/network_nftables.c
|
||||
+++ b/src/network/network_nftables.c
|
||||
@@ -73,6 +73,7 @@ VIR_ENUM_IMPL(nftablesLayer,
|
||||
"",
|
||||
"ip",
|
||||
"ip6",
|
||||
+ "",
|
||||
);
|
||||
|
||||
|
||||
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
|
||||
index 811b787ecc..9389bcf541 100644
|
||||
--- a/src/util/virfirewall.c
|
||||
+++ b/src/util/virfirewall.c
|
||||
@@ -47,6 +47,7 @@ VIR_ENUM_IMPL(virFirewallLayer,
|
||||
"ethernet",
|
||||
"ipv4",
|
||||
"ipv6",
|
||||
+ "tc",
|
||||
);
|
||||
|
||||
typedef struct _virFirewallGroup virFirewallGroup;
|
||||
@@ -57,6 +58,7 @@ VIR_ENUM_IMPL(virFirewallLayerCommand,
|
||||
EBTABLES,
|
||||
IPTABLES,
|
||||
IP6TABLES,
|
||||
+ TC,
|
||||
);
|
||||
|
||||
struct _virFirewallCmd {
|
||||
@@ -591,6 +593,7 @@ virFirewallCmdIptablesApply(virFirewall *firewall,
|
||||
case VIR_FIREWALL_LAYER_IPV6:
|
||||
virCommandAddArg(cmd, "-w");
|
||||
break;
|
||||
+ case VIR_FIREWALL_LAYER_TC:
|
||||
case VIR_FIREWALL_LAYER_LAST:
|
||||
break;
|
||||
}
|
||||
@@ -672,39 +675,52 @@ virFirewallCmdNftablesApply(virFirewall *firewall G_GNUC_UNUSED,
|
||||
size_t i;
|
||||
int status;
|
||||
|
||||
- cmd = virCommandNew(NFT);
|
||||
+ if (fwCmd->layer == VIR_FIREWALL_LAYER_TC) {
|
||||
|
||||
- if ((virFirewallTransactionGetFlags(firewall) & VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK) &&
|
||||
- fwCmd->argsLen > 1) {
|
||||
- /* skip any leading options to get to command verb */
|
||||
- for (i = 0; i < fwCmd->argsLen - 1; i++) {
|
||||
- if (fwCmd->args[i][0] != '-')
|
||||
- break;
|
||||
- }
|
||||
+ /* for VIR_FIREWALL_LAYER_TC, we run the 'tc' (traffic control) command with
|
||||
+ * the supplied args.
|
||||
+ */
|
||||
+ cmd = virCommandNew(TC);
|
||||
|
||||
- if (i + 1 < fwCmd->argsLen &&
|
||||
- VIR_NFTABLES_ARG_IS_CREATE(fwCmd->args[i])) {
|
||||
+ /* NB: RAW commands don't support auto-rollback command creation */
|
||||
|
||||
- cmdIdx = i;
|
||||
- objectType = fwCmd->args[i + 1];
|
||||
+ } else {
|
||||
|
||||
- /* we currently only handle auto-rollback for rules,
|
||||
- * chains, and tables, and those all can be "rolled
|
||||
- * back" by a delete command using the handle that is
|
||||
- * returned when "-ae" is added to the add/insert
|
||||
- * command.
|
||||
- */
|
||||
- if (STREQ_NULLABLE(objectType, "rule") ||
|
||||
- STREQ_NULLABLE(objectType, "chain") ||
|
||||
- STREQ_NULLABLE(objectType, "table")) {
|
||||
+ cmd = virCommandNew(NFT);
|
||||
|
||||
- needRollback = true;
|
||||
- /* this option to nft instructs it to add the
|
||||
- * "handle" of the created object to stdout
|
||||
+ if ((virFirewallTransactionGetFlags(firewall) & VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK) &&
|
||||
+ fwCmd->argsLen > 1) {
|
||||
+ /* skip any leading options to get to command verb */
|
||||
+ for (i = 0; i < fwCmd->argsLen - 1; i++) {
|
||||
+ if (fwCmd->args[i][0] != '-')
|
||||
+ break;
|
||||
+ }
|
||||
+
|
||||
+ if (i + 1 < fwCmd->argsLen &&
|
||||
+ VIR_NFTABLES_ARG_IS_CREATE(fwCmd->args[i])) {
|
||||
+
|
||||
+ cmdIdx = i;
|
||||
+ objectType = fwCmd->args[i + 1];
|
||||
+
|
||||
+ /* we currently only handle auto-rollback for rules,
|
||||
+ * chains, and tables, and those all can be "rolled
|
||||
+ * back" by a delete command using the handle that is
|
||||
+ * returned when "-ae" is added to the add/insert
|
||||
+ * command.
|
||||
*/
|
||||
- virCommandAddArg(cmd, "-ae");
|
||||
+ if (STREQ_NULLABLE(objectType, "rule") ||
|
||||
+ STREQ_NULLABLE(objectType, "chain") ||
|
||||
+ STREQ_NULLABLE(objectType, "table")) {
|
||||
+
|
||||
+ needRollback = true;
|
||||
+ /* this option to nft instructs it to add the
|
||||
+ * "handle" of the created object to stdout
|
||||
+ */
|
||||
+ virCommandAddArg(cmd, "-ae");
|
||||
+ }
|
||||
}
|
||||
}
|
||||
+
|
||||
}
|
||||
|
||||
for (i = 0; i < fwCmd->argsLen; i++)
|
||||
diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h
|
||||
index bce51259d2..d42e60884b 100644
|
||||
--- a/src/util/virfirewall.h
|
||||
+++ b/src/util/virfirewall.h
|
||||
@@ -39,6 +39,7 @@ typedef enum {
|
||||
VIR_FIREWALL_LAYER_ETHERNET,
|
||||
VIR_FIREWALL_LAYER_IPV4,
|
||||
VIR_FIREWALL_LAYER_IPV6,
|
||||
+ VIR_FIREWALL_LAYER_TC,
|
||||
|
||||
VIR_FIREWALL_LAYER_LAST,
|
||||
} virFirewallLayer;
|
||||
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
|
||||
index 827e201dbb..124523c420 100644
|
||||
--- a/src/util/virfirewalld.c
|
||||
+++ b/src/util/virfirewalld.c
|
||||
@@ -43,6 +43,7 @@ VIR_LOG_INIT("util.firewalld");
|
||||
VIR_ENUM_DECL(virFirewallLayerFirewallD);
|
||||
VIR_ENUM_IMPL(virFirewallLayerFirewallD,
|
||||
VIR_FIREWALL_LAYER_LAST,
|
||||
+ "",
|
||||
"eb",
|
||||
"ipv4",
|
||||
"ipv6",
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,687 @@
|
||||
From b1e2318a0d609fcdff04fcf88953ea87cdd02b95 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:49 -0500
|
||||
Subject: [PATCH 9/9] network: add tc filter rule to nftables backend to fix
|
||||
checksum of DHCP responses
|
||||
|
||||
Please see the commit log for commit v10.9.0-rc1-1-g42ab0148dd for the
|
||||
history and explanation of the problem that this patch is fixing.
|
||||
|
||||
A shorter explanation is that when a guest is connected to a libvirt
|
||||
virtual network using a virtio-net adapter with in-kernel "vhost-net"
|
||||
packet processing enabled, it will fail to acquire an IP address from
|
||||
a DHCP seever running on the host.
|
||||
|
||||
In commit v10.9.0-rc1-1-g42ab0148dd we tried fixing this by *zeroing
|
||||
out* the checksums of these packets with an nftables rule (nftables
|
||||
can't recompute the checksum, but it can set it to 0) . This
|
||||
*appeared* to work initially, but it turned out that zeroing the
|
||||
checksum ends up breaking dhcp packets on *non* virtio/vhost-net guest
|
||||
interfaces. That attempt was reverted in commit v10.9.0-rc2.
|
||||
|
||||
Fortunately, there is an existing way to recompute the checksum of a
|
||||
packet as it leaves an interface - the "tc" (traffic control) utility
|
||||
that libvirt already uses for bandwidth management. This patch uses a
|
||||
tc filter rule to match dhcp response packets on the bridge and
|
||||
recompute their checksum.
|
||||
|
||||
The filter rule must be attached to a tc qdisc, which may also have a
|
||||
filter attached for bandwidth management (in the <bandwidth> element
|
||||
of the network config). Not only must we add the qdisc only once
|
||||
(which was already handled by the patch two prior to this one), but
|
||||
also the filter rule for checksum fixing and the filter rule for
|
||||
bandwidth management must be different priorities so they don't clash;
|
||||
this is solved by adding the checksum-fix filter with "priority 2",
|
||||
while the bandwidth management filter remains "priority 1" (both will
|
||||
always be evaluated anyway, it's just a matter of which is evaluated
|
||||
first).
|
||||
|
||||
So far this method has worked with every different guest we could
|
||||
throw at it, including several that failed with the previous method.
|
||||
|
||||
Fixes: b89c4991daa0ee9371f10937fab3b03c5ffdabc6
|
||||
Reported-by: Rich Jones <rjones@redhat.com>
|
||||
Reported-by: Andrea Bolognani <abologna@redhat.com>
|
||||
Fix-Suggested-by: Eric Garver <egarver@redhat.com>
|
||||
Fix-Suggested-by: Phil Sutter <psutter@redhat.com>
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/network/network_nftables.c | 68 +++++++++++++++++++
|
||||
.../forward-dev-linux.nftables | 40 +++++++++++
|
||||
.../isolated-linux.nftables | 40 +++++++++++
|
||||
.../nat-default-linux.nftables | 40 +++++++++++
|
||||
.../nat-ipv6-linux.nftables | 40 +++++++++++
|
||||
.../nat-ipv6-masquerade-linux.nftables | 40 +++++++++++
|
||||
.../nat-many-ips-linux.nftables | 40 +++++++++++
|
||||
.../nat-no-dhcp-linux.nftables | 40 +++++++++++
|
||||
.../nat-port-range-ipv6-linux.nftables | 40 +++++++++++
|
||||
.../nat-port-range-linux.nftables | 40 +++++++++++
|
||||
.../nat-tftp-linux.nftables | 40 +++++++++++
|
||||
.../route-default-linux.nftables | 40 +++++++++++
|
||||
12 files changed, 508 insertions(+)
|
||||
|
||||
diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c
|
||||
index cc184105c3..748edb0273 100644
|
||||
--- a/src/network/network_nftables.c
|
||||
+++ b/src/network/network_nftables.c
|
||||
@@ -29,6 +29,7 @@
|
||||
|
||||
#include "internal.h"
|
||||
#include "virfirewalld.h"
|
||||
+#include "vircommand.h"
|
||||
#include "virerror.h"
|
||||
#include "virlog.h"
|
||||
#include "virhash.h"
|
||||
@@ -924,6 +925,67 @@ nftablesAddIPSpecificFirewallRules(virFirewall *fw,
|
||||
}
|
||||
|
||||
|
||||
+/**
|
||||
+ * nftablesAddUdpChecksumFixWithTC:
|
||||
+ *
|
||||
+ * Add a tc filter rule to @ifname (the bridge device of this network)
|
||||
+ * that will recompute the checksum of udp packets output from @iface with
|
||||
+ * destination port @port.
|
||||
+ *
|
||||
+ * Normally the checksum should be filled by some part of the basic
|
||||
+ * network stack, but there are cases (e.g. DHCP response packets sent
|
||||
+ * from virtualization host to a QEMU guest when the guest NIC uses
|
||||
+ * vhost-net packet processing) when the host (sender) thinks that
|
||||
+ * packet checksums will be computed elsewhere (and so leaves a
|
||||
+ * partially computed checksum in the packet header) while the guest
|
||||
+ * (receiver) thinks that the checksum has already been fully
|
||||
+ * computed; in the meantime none of the code in between has actually
|
||||
+ * finished computing the checksum.
|
||||
+ *
|
||||
+ * An example of this is DHCP response packets from host to guest. If
|
||||
+ * the checksum of each of these packets isn't properly computed, then
|
||||
+ * many guests (e.g. FreeBSD) will drop them with reason BAD CHECKSUM;
|
||||
+ * this tc filter rule will fix the ip and udp checksums, and the
|
||||
+ * FreeBSD dhcp client will happily accept the packet.
|
||||
+ *
|
||||
+ * (NB: if you're wondering how the tc qdisc and filter are removed
|
||||
+ * when the network is destroyed, the answer is that the kernel
|
||||
+ * automatically (and properly) removes them for us, so we don't need
|
||||
+ * to worry about keeping track/deleting as we do with nftables rules)
|
||||
+ */
|
||||
+static int
|
||||
+nftablesAddUdpChecksumFixWithTC(virFirewall *fw,
|
||||
+ const char *iface,
|
||||
+ int port)
|
||||
+{
|
||||
+ g_autofree char *portstr = g_strdup_printf("%d", port);
|
||||
+
|
||||
+ /* this will add the qdisc (that the filter below is attached to)
|
||||
+ * unless it already exists
|
||||
+ */
|
||||
+ if (virNetDevBandWidthAddTxFilterParentQdisc(iface, true) < 0)
|
||||
+ return -1;
|
||||
+
|
||||
+ /* add a filter to catch all udp packets with dst "port" and
|
||||
+ * recompute their checksum
|
||||
+ */
|
||||
+ virFirewallAddCmd(fw, VIR_FIREWALL_LAYER_TC,
|
||||
+ "filter", "add", "dev", iface,
|
||||
+ "prio", "2", "protocol", "ip", "parent", "1:",
|
||||
+ "u32", "match", "ip", "dport", portstr, "ffff",
|
||||
+ "action", "csum", "ip", "and", "udp",
|
||||
+ NULL);
|
||||
+
|
||||
+ virFirewallAddRollbackCmd(fw, VIR_FIREWALL_LAYER_TC,
|
||||
+ "filter", "del", "dev", iface,
|
||||
+ "prio", "2", "protocol", "ip", "parent", "1:",
|
||||
+ "u32", "match", "ip", "dport", portstr, "ffff",
|
||||
+ "action", "csum", "ip", "and", "udp",
|
||||
+ NULL);
|
||||
+ return 0;
|
||||
+}
|
||||
+
|
||||
+
|
||||
/* nftablesAddFirewallrules:
|
||||
*
|
||||
* @def - the network that needs an nftables firewall added
|
||||
@@ -944,6 +1006,12 @@ nftablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval)
|
||||
|
||||
virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK);
|
||||
|
||||
+ /* add the tc filter rule needed to fixup the checksum of dhcp
|
||||
+ * response packets going from host to guest.
|
||||
+ */
|
||||
+ if (nftablesAddUdpChecksumFixWithTC(fw, def->bridge, 68) < 0)
|
||||
+ return -1;
|
||||
+
|
||||
nftablesAddGeneralFirewallRules(fw, def);
|
||||
|
||||
for (i = 0;
|
||||
diff --git a/tests/networkxml2firewalldata/forward-dev-linux.nftables b/tests/networkxml2firewalldata/forward-dev-linux.nftables
|
||||
index 8badb74beb..6772383b37 100644
|
||||
--- a/tests/networkxml2firewalldata/forward-dev-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/forward-dev-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/isolated-linux.nftables b/tests/networkxml2firewalldata/isolated-linux.nftables
|
||||
index d1b4dac178..546a18b75a 100644
|
||||
--- a/tests/networkxml2firewalldata/isolated-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/isolated-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-default-linux.nftables b/tests/networkxml2firewalldata/nat-default-linux.nftables
|
||||
index 28508292f9..08623c1381 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-default-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-default-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables
|
||||
index d8a9ba706d..3fd6b94eef 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables
|
||||
index a7f09cda59..2811e098d1 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables
|
||||
index b826fe6134..5409d5b552 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables b/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables
|
||||
index d8a9ba706d..3fd6b94eef 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables
|
||||
index ceaed6fa40..d74417cdb3 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-port-range-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-linux.nftables
|
||||
index 1dc37a26ec..b55bb287a9 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-port-range-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-port-range-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-tftp-linux.nftables b/tests/networkxml2firewalldata/nat-tftp-linux.nftables
|
||||
index 28508292f9..08623c1381 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-tftp-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-tftp-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/route-default-linux.nftables b/tests/networkxml2firewalldata/route-default-linux.nftables
|
||||
index 282c9542a5..76d6902517 100644
|
||||
--- a/tests/networkxml2firewalldata/route-default-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/route-default-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
From 114c0ec656e879ab4d67919914bb24cf5993106d Mon Sep 17 00:00:00 2001
|
||||
Message-ID: <114c0ec656e879ab4d67919914bb24cf5993106d.1734201785.git.crobinso@redhat.com>
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 2 Sep 2024 16:13:08 -0400
|
||||
Subject: [PATCH] network: permit <forward mode='open'/> when a network has no
|
||||
IP address
|
||||
Content-type: text/plain
|
||||
|
||||
The whole point of <forward mode='open'/> is to supress libvirt from
|
||||
adding any firewall rules for a network, and someone might want to
|
||||
create a network with no IP address (i.e. they don't want the guests
|
||||
to have connectivity to the host via this interface) and no firewall
|
||||
rules (they don't want any, or they want to add their own). So there's
|
||||
no reason to fail when a network has <forward mode='open'/> and also
|
||||
has no IP address.
|
||||
|
||||
Kind-of-Resolves: https://gitlab.com/libvirt/libvirt/-/issues/588
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Signed-off-by: Cole Robinson <crobinso@redhat.com>
|
||||
---
|
||||
src/conf/network_conf.c | 5 +++--
|
||||
1 file changed, 3 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
|
||||
index 5cf419acf1..320e1b089a 100644
|
||||
--- a/src/conf/network_conf.c
|
||||
+++ b/src/conf/network_conf.c
|
||||
@@ -1789,7 +1789,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt,
|
||||
|
||||
case VIR_NETWORK_FORWARD_ROUTE:
|
||||
case VIR_NETWORK_FORWARD_NAT:
|
||||
- case VIR_NETWORK_FORWARD_OPEN:
|
||||
/* It's pointless to specify L3 forwarding without specifying
|
||||
* the network we're on.
|
||||
*/
|
||||
@@ -1806,8 +1805,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt,
|
||||
def->name);
|
||||
return NULL;
|
||||
}
|
||||
+ break;
|
||||
|
||||
- if (def->forward.type == VIR_NETWORK_FORWARD_OPEN && def->forward.nifs) {
|
||||
+ case VIR_NETWORK_FORWARD_OPEN:
|
||||
+ if (def->forward.nifs) {
|
||||
/* an open network by definition can't place any restrictions
|
||||
* on what traffic is allowed or where it goes, so specifying
|
||||
* a forwarding device is nonsensical.
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
From d51179fa82448f4720f1645f0b7100df80508cc4 Mon Sep 17 00:00:00 2001
|
||||
From: Pavel Hrdina <phrdina@redhat.com>
|
||||
Date: Thu, 9 Jan 2025 16:23:44 +0100
|
||||
Subject: [PATCH] qemu: snapshot: delete disk image only if parent snapshot is
|
||||
external
|
||||
Content-type: text/plain
|
||||
|
||||
When we are deleting external snapshot that is not active we only need
|
||||
to delete overlay disk image of the parent snapshot. This works
|
||||
correctly even if parent snapshot is external and active as it will have
|
||||
another overlay created when user reverted to that snapshot.
|
||||
|
||||
In case the parent snapshot is internal there are no overlay disk images
|
||||
created as everything is stored internally within the disk image. In
|
||||
this case we would delete the actual disk image storing internal
|
||||
snapshots and most likely the original disk image as well resulting in
|
||||
data loss once the VM is shutoff.
|
||||
|
||||
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/734
|
||||
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
|
||||
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
|
||||
---
|
||||
src/qemu/qemu_snapshot.c | 14 ++++++++------
|
||||
1 file changed, 8 insertions(+), 6 deletions(-)
|
||||
|
||||
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
|
||||
index 18b2e478f6..80cd54bf33 100644
|
||||
--- a/src/qemu/qemu_snapshot.c
|
||||
+++ b/src/qemu/qemu_snapshot.c
|
||||
@@ -3144,6 +3144,8 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
|
||||
return -1;
|
||||
}
|
||||
|
||||
+ data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap, data->snapDisk);
|
||||
+
|
||||
if (data->merge) {
|
||||
virStorageSource *snapDiskSrc = NULL;
|
||||
|
||||
@@ -3185,8 +3187,6 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
|
||||
qemuSnapshotGetDisksWithBackingStore(vm, snap, data);
|
||||
}
|
||||
|
||||
- data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap, data->snapDisk);
|
||||
-
|
||||
if (data->parentSnap && !virDomainSnapshotIsExternal(data->parentSnap)) {
|
||||
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
|
||||
_("deleting external snapshot that has internal snapshot as parent not supported"));
|
||||
@@ -3642,10 +3642,12 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
|
||||
if (!data->job)
|
||||
goto error;
|
||||
} else {
|
||||
- if (virStorageSourceInit(data->parentDomDisk->src) < 0 ||
|
||||
- virStorageSourceUnlink(data->parentDomDisk->src) < 0) {
|
||||
- VIR_WARN("Failed to remove snapshot image '%s'",
|
||||
- data->snapDisk->name);
|
||||
+ if (data->parentSnap && virDomainSnapshotIsExternal(data->parentSnap)) {
|
||||
+ if (virStorageSourceInit(data->parentDomDisk->src) < 0 ||
|
||||
+ virStorageSourceUnlink(data->parentDomDisk->src) < 0) {
|
||||
+ VIR_WARN("Failed to remove snapshot image '%s'",
|
||||
+ data->snapDisk->name);
|
||||
+ }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,31 +0,0 @@
|
||||
From c9056e682a8a67dc29e39eb01392fcf8ee978c31 Mon Sep 17 00:00:00 2001
|
||||
From: Jonathan Wright <jonathan@almalinux.org>
|
||||
Date: Wed, 3 Jan 2024 09:26:59 -0600
|
||||
Subject: [PATCH] conf: Restore setting default bus for input devices
|
||||
|
||||
Prior to v9.3.0-rc1~30 we used to set default bus for <input/>
|
||||
devices, during XML parsing. In the commit this code was moved to
|
||||
a post parse callback. But somehow the line that sets the bus in
|
||||
one specific case disappeared. Bring it back.
|
||||
|
||||
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/577
|
||||
Fixes: c4bc4d3b82fbe22e03c986ca896090f481df5c10
|
||||
Signed-off-by: Jonathan Wright <jonathan@almalinux.org>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/conf/domain_postparse.c | 1 +
|
||||
1 file changed, 1 insertion(+)
|
||||
|
||||
diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c
|
||||
index e79913b73f..ee27023f3e 100644
|
||||
--- a/src/conf/domain_postparse.c
|
||||
+++ b/src/conf/domain_postparse.c
|
||||
@@ -657,6 +657,7 @@ virDomainInputDefPostParse(virDomainInputDef *input,
|
||||
if ((input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
|
||||
input->type == VIR_DOMAIN_INPUT_TYPE_KBD) &&
|
||||
(ARCH_IS_X86(def->os.arch) || def->os.arch == VIR_ARCH_NONE)) {
|
||||
+ input->bus = VIR_DOMAIN_INPUT_BUS_PS2;
|
||||
} else if (ARCH_IS_S390(def->os.arch) ||
|
||||
input->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
|
||||
input->bus = VIR_DOMAIN_INPUT_BUS_VIRTIO;
|
||||
--
|
||||
+494
-262
File diff suppressed because it is too large
Load Diff
@@ -1 +1 @@
|
||||
SHA512 (libvirt-9.7.0.tar.xz) = dd771822c0fa0861a32cab9d7f82235b101867fa0a4e8cf9a857ddfb2347e41b625b1e6f8791c4b3543fec836a1a23cae1fac4ce4b40debd51f2097bae46c949
|
||||
SHA512 (libvirt-10.6.0.tar.xz) = edec79e89669d5e9a46be35e0d6334a6ed3bbf32426679549bd998bde24cba52b0378843f41a3abb5d781ad53e2a6a54619a0bad3f168c11fb41736cc6af6568
|
||||
|
||||
Reference in New Issue
Block a user