7:58 a.m.
v2 of:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/ED...
diff to v1:
- Validate in validation phase of XML parsing, or since network XML
doesn't have validation phase, do it during network start (just like
other stuff)
Michal Prívozník (2):
conf: Introduce virNetDevBandwidthValidate()
conf: Validate QoS values
src/conf/domain_validate.c | 9 +++++++
src/conf/netdev_bandwidth_conf.c | 42 ++++++++++++++++++++++++++++++
src/conf/netdev_bandwidth_conf.h | 2 ++
src/conf/schemas/networkcommon.rng | 3 ++-
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 4 +++
6 files changed, 60 insertions(+), 1 deletion(-)
--
2.44.2
7:58 a.m.
New subject: [PATCH v2 1/2] conf: Introduce virNetDevBandwidthValidate()
This function validates whether parsed limits are within range as
defined by 'tc' sources (since we use tc to set QoS; or OVS which
then uses tc too). The 'tc' program stores speeds in 64bit
integers (unit is bytes per second) and sizes in uints (unit is
bytes). We use different units: kilobytes per second and
kibibytes and therefore we can parse values larger than 'tc' can
handle and thus need a function to check if values still fit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/netdev_bandwidth_conf.c | 42 ++++++++++++++++++++++++++++++++
src/conf/netdev_bandwidth_conf.h | 2 ++
src/libvirt_private.syms | 1 +
3 files changed, 45 insertions(+)
diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
index 9faa46a27f..43a2c62240 100644
--- a/src/conf/netdev_bandwidth_conf.c
+++ b/src/conf/netdev_bandwidth_conf.c
@@ -200,6 +200,48 @@ virNetDevBandwidthFormat(const virNetDevBandwidth *def,
}
+#define CHECK_LIMIT(val, limit, name) \
+ do { \
+ if ((val) > (limit)) { \
+ virReportError(VIR_ERR_OVERFLOW, \
+ _("value '%1$llu' is too big for '%2$s'
parameter, maximum is '%3$llu'"), \
+ val, name, (unsigned long long) limit); \
+ return false; \
+ } \
+ } while (0)
+
+static bool
+virNetDevBandwidthRateValidate(const virNetDevBandwidthRate *rate)
+{
+ const unsigned long long speedLimit = 1ULL << 54;
+ const unsigned int sizeLimit = UINT_MAX >> 10;
+
+ /* These limits are taken straight from 'tc' sources. */
+
+ if (!rate)
+ return true;
+
+ CHECK_LIMIT(rate->average, speedLimit, "average");
+ CHECK_LIMIT(rate->peak, speedLimit, "peak");
+ CHECK_LIMIT(rate->burst, sizeLimit, "burst");
+ CHECK_LIMIT(rate->floor, speedLimit, "floor");
+
+ return true;
+}
+
+#undef CHECK_LIMIT
+
+bool
+virNetDevBandwidthValidate(const virNetDevBandwidth *def)
+{
+ if (!def)
+ return true;
+
+ return virNetDevBandwidthRateValidate(def->in) &&
+ virNetDevBandwidthRateValidate(def->out);
+}
+
+
bool virNetDevSupportsBandwidth(virDomainNetType type)
{
switch ((virDomainNetType) type) {
diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h
index b679b0f51f..6dbe0298f6 100644
--- a/src/conf/netdev_bandwidth_conf.h
+++ b/src/conf/netdev_bandwidth_conf.h
@@ -34,6 +34,8 @@ int virNetDevBandwidthFormat(const virNetDevBandwidth *def,
unsigned int class_id,
virBuffer *buf);
+bool virNetDevBandwidthValidate(const virNetDevBandwidth *def);
+
bool virNetDevSupportsBandwidth(virDomainNetType type);
bool virNetDevBandwidthHasFloor(const virNetDevBandwidth *b);
bool virNetDevBandwidthSupportsFloor(virNetworkForwardType type);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0accca442a..7a5df5a6a4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -825,6 +825,7 @@ virNetDevBandwidthFormat;
virNetDevBandwidthHasFloor;
virNetDevBandwidthParse;
virNetDevBandwidthSupportsFloor;
+virNetDevBandwidthValidate;
virNetDevSupportsBandwidth;
--
2.44.2
7:59 a.m.
New subject: [PATCH v2 2/2] conf: Validate QoS values
Since we use 'tc' to set QoS, or we instruct OVS which then uses
'tc', we have to make sure values are within range acceptable to
'tc'.
Resolves: https://issues.redhat.com/browse/RHEL-45200
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_validate.c | 9 +++++++++
src/conf/schemas/networkcommon.rng | 3 ++-
src/network/bridge_driver.c | 4 ++++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 39b8d67928..ab1caadc7a 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -22,6 +22,7 @@
#include "domain_validate.h"
#include "domain_conf.h"
+#include "netdev_bandwidth_conf.h"
#include "vircgroup.h"
#include "virconftypes.h"
#include "virlog.h"
@@ -2068,6 +2069,10 @@ virDomainActualNetDefValidate(const virDomainNetDef *net)
return -1;
}
+ if (!virNetDevBandwidthValidate(bandwidth)) {
+ return -1;
+ }
+
if (virDomainNetDefValidatePortOptions(macstr, actualType, vport,
virDomainNetGetActualPortOptionsIsolated(net))
< 0) {
return -1;
@@ -2143,6 +2148,10 @@ virDomainNetDefValidate(const virDomainNetDef *net)
return -1;
}
+ if (!virNetDevBandwidthValidate(net->bandwidth)) {
+ return -1;
+ }
+
switch (net->type) {
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
if (!virDomainNetIsVirtioModel(net)) {
diff --git a/src/conf/schemas/networkcommon.rng b/src/conf/schemas/networkcommon.rng
index 6df6d43f54..28424f9abd 100644
--- a/src/conf/schemas/networkcommon.rng
+++ b/src/conf/schemas/networkcommon.rng
@@ -180,9 +180,10 @@
</define>
<define name="speed">
- <data type="unsignedInt">
+ <data type="unsignedLong">
<param name="pattern">[0-9]+</param>
<param name="minInclusive">1</param>
+ <param name="maxInclusive">18014398509481984</param>
</data>
</define>
<define name="BurstSize">
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 32572c755f..915211d1b5 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2838,6 +2838,10 @@ networkValidate(virNetworkDriverState *driver,
return -1;
}
+ if (!virNetDevBandwidthValidate(def->bandwidth)) {
+ return -1;
+ }
+
/* we support configs with a single PF defined:
* <pf dev='eth0'/>
* or with a list of netdev names:
--
2.44.2
9:11 a.m.
On Tue, Aug 13, 2024 at 14:58:58 +0200, Michal Privoznik wrote:
v2 of:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/ED...
diff to v1:
- Validate in validation phase of XML parsing, or since network XML
doesn't have validation phase, do it during network start (just like
other stuff)
The latter makes sense. We even re-validate the definition of VMs when
starting up.
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
100
days inactive
100
days old
3 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa