On 04/21/2015 08:22 AM, Michal Privoznik wrote:
> Because packets going through the egress from a bridge (where our
> bandwidth limiting takes place) have no information about which
> interface they came from, the QoS rules that we create instead
> use the source MAC address of the packets to make their decisions
> about which QDisc the packet should be in.
>
> One flaw in this is that when a guest changed the MAC address it
> used, packets from the guest would no longer be put into the
> correct QDisc, but would instead be put in an "unprivileged"
> class, resulting in the bandwidth "floor" (minimum guaranteed)
> being no longer honored.
>
> Now that libvirt has infrastructure to capture and respond to
> RX_FILTER_CHANGE events from qemu (sent whenever a guest
> interface modifies its MAC address, among other things), we can
> notice when a guest MAC address changes, and update the QoS rules
> accordingly, so that bandwidth floor is honored even after a
> guest MAC address change.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6fc9696..6209754b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4377,6 +4377,18 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
> syncNicRxFilterDeviceOptions(def->ifname, guestFilter, hostFilter);
> }
>
> + if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_NETWORK) {
> + const char *brname = virDomainNetGetActualBridgeName(def);
> +
> + /* For libivrt network connections, set the following TUN/TAP network
> + * device attributes to match those of the guest network device:
> + * - QoS filters (which are based on MAC address)
> + */
Don't you want to check to make sure this interface actually has
bandwidth settings before trying to update them? Maybe by prepending "if
(virDomainNetGetActualBandwidth(def)) to this following if clause.
> + if (virNetDevBandwidthUpdateFilter(brname, &guestFilter->mac,
> + def->data.network.actual->class_id) < 0)
Just to be defensive, you should check that def->data.network.actual !=
NULL too (it *should* always point to something when the domain is
active, but just in case)
> + goto endjob;
> + }
> +
> endjob:
> qemuDomainObjEndJob(driver, vm);
>
ACK with those two changes.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list