On 11/24/2014 12:48 PM, Laine Stump wrote:
> In order for the kernel to be able to promiscuous mode on as many
> ports of a bridge as possible, at most one attached device can have
> learning and unicast_flood enabled (in practice, this one device is
> always the physical device that connects the bridge to the real
> world). If more than one device has those settings enabled, the kernel
> cannot enable promiscuous mode. Since both settings default to
> enabled, we need to turn them both off (using
> virNetDevBridgeSetLearning() and virNetDevBridgeSetUnicastFlood()) for
> every tap device plugged into a bridge that has promiscLinks='no'.
>
> If there is only one device with learning/unicast_flood enabled, then
> that device has promiscuous mode disabled. If there are *no* devices
> with learning/unicast_flood enabled (e.g. for a libvirt "route",
> "nat", or isolated network that has no physical device attached), then
> all devices will have promiscuous mode disabled.
>
> Once we have disabled learning and flooding, any packet that has a
> destination MAC address not present in the forwarding database (fdb)
> will be dropped by the bridge, and libvirt is responsible for adding
> entries to the bridge fdb for the MAC address of each guest
> interface. That is done with virNetDevBridgeFDBAdd().
>
> None of this has any effect for kernels prior to 3.15 (upstream kernel
> commit 2796d0c648c940b4796f84384fbcfb0a2399db84 "bridge: Automatically
> manage port promiscuous mode"). Even after that, until kernel 3.17
> (upstream commit 5be5a2df40f005ea7fb7e280e87bbbcfcf1c2fc0 "bridge: Add
> filtering support for default_pvid") the following workaround is
> required in order for untagged traffic to pass (vlan tagged traffic
> will in any case currently not pass with promiscLinks='no')
> ---
> src/qemu/qemu_command.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index cbdef9c..d3d129a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -35,6 +35,7 @@
> #include "virerror.h"
> #include "virfile.h"
> #include "virnetdev.h"
> +#include "virnetdevbridge.h"
> #include "virstring.h"
> #include "virtime.h"
> #include "viruuid.h"
> @@ -350,6 +351,21 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
> virDomainAuditNetDevice(def, net, tunpath, false);
> goto cleanup;
> }
> + if (virDomainNetGetActualPromiscLinks(net) == VIR_TRISTATE_BOOL_NO) {
> + /* In order to make as many as possible of the links to a
> + * bridge promiscuous/no-flood, we need to turn off
> + * learning and unicast_flood, and add an fdb entry to the
> + * bridge for this new device.
> + */
In patch 6 we explicitly check vlan filtering being enabled - that's not
done here, does it need to be?
> + if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0)
> + goto cleanup;
> + if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false) < 0)
> + goto cleanup;
> + if (virNetDevBridgeFDBAdd(&net->mac, net->ifname,
> + VIR_NETDEVBRIDGE_FDB_FLAG_MASTER |
> + VIR_NETDEVBRIDGE_FDB_FLAG_TEMP) < 0)
> + goto cleanup;
I'd probably have similar "concerns" as patch 6 with issues that could
occur if the default for promiscLinks changed and the error path logic.
Although I see now that restart logic doesn't mean much (hey, it's late
in the day).
ACK to what's here though. Obviously if you changed the name from my
patch 3 suggestion, then the BOOL_NO above would change as well.
John
> + }
> } else {
> if (qemuCreateInBridgePortWithHelper(cfg, brname,
> &net->ifname,
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list