(I didn't try make syntax-check, but am assuming you have and that it
passes)
On 01/19/2015 11:18 AM, akrowiak@xxxxxxxxxxxxxxxxxx wrote:
> From: Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx>
>
> This patch supplies the funtionality of synchronizing the host macvtap
> device options with the guest device's in response to the
> NIC_RX_FILTER_CHANGED event.
"supplies the functionality of" sounds too busy and doesn't add
anything. Instead maybe say "This patch enables synchronization of the
host macvtap options ...."
>
> The following device options will be synchronized:
> * PROMISC
> * MULTICAST
> * ALLMULTI
>
> Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 92 insertions(+), 0 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5994558..141f91a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4168,6 +4168,93 @@ syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilterPtr guestFilter,
>
>
> static void
> +syncNicRxFilterPromiscMode(char *ifname, virNetDevRxFilterPtr guestFilter,
> + virNetDevRxFilterPtr hostFilter)
> +{
> + bool promisc;
> + bool setpromisc = false;
> +
> + /* Set macvtap promisc mode to true if the guest has vlans defined */
> + /* or synchronize the macvtap promisc mode if different from guest */
> + if (guestFilter->vlan.nTable > 0) {
> + if (!hostFilter->promiscuous) {
> + setpromisc = true;
> + promisc = true;
> + }
> + } else if (hostFilter->promiscuous != guestFilter->promiscuous) {
> + setpromisc = true;
> + promisc = guestFilter->promiscuous;
> + }
> +
> + if (setpromisc) {
> + if (virNetDevSetPromiscuous(ifname, promisc) < 0) {
> + VIR_WARN("Couldn't set PROMISC flag to %s for device %s "
> + "while responding to NIC_RX_FILTER_CHANGED",
> + promisc ? "true" : "false", ifname);
> + }
> + }
> +}
> +
> +
> +static void
> +syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilterPtr guestFilter,
> + virNetDevRxFilterPtr hostFilter)
> +{
> + if (hostFilter->multicast.mode != guestFilter->multicast.mode) {
> + switch (guestFilter->multicast.mode) {
If you typecast the above to virNetDevRxFilterMode, then replace the
"default" below with VIR_NETDEV_RX_FILTER_MODE_NONE, we will get a nice
reminder to add a new case if a new value is ever added to the enum.
> + case VIR_NETDEV_RX_FILTER_MODE_ALL:
> + if (virNetDevSetRcvAllMulti(ifname, true)) {
> +
> + VIR_WARN("Couldn't set allmulticast flag to 'on' for "
> + "device %s while responding to "
> + "NIC_RX_FILTER_CHANGED", ifname);
> + }
> + break;
> +
> + case VIR_NETDEV_RX_FILTER_MODE_NORMAL:
> + if (virNetDevSetRcvMulti(ifname, true)) {
> +
> + VIR_WARN("Couldn't set multicast flag to 'on' for "
> + "device %s while responding to "
> + "NIC_RX_FILTER_CHANGED", ifname);
> + }
> +
> + if (virNetDevSetRcvAllMulti(ifname, false)) {
> + VIR_WARN("Couldn't set allmulticast flag to 'off' for "
> + "device %s while responding to "
> + "NIC_RX_FILTER_CHANGED", ifname);
> + }
> + break;
> +
> + default:
I think this should use VIR_NETDEV_RX_FILTER_MODE_NONE instead of
"default". See above.
> + if (virNetDevSetRcvAllMulti(ifname, false)) {
> + VIR_WARN("Couldn't set allmulticast flag to 'off' for "
> + "device %s while responding to "
> + "NIC_RX_FILTER_CHANGED", ifname);
> + }
> +
> + if (virNetDevSetRcvMulti(ifname, false)) {
> + VIR_WARN("Couldn't set multicast flag to 'off' for "
> + "device %s while responding to "
> + "NIC_RX_FILTER_CHANGED",
> + ifname);
> + }
> + break;
> + }
> + }
> +}
> +
> +
> +static void
> +syncNicRxFilterDeviceOptions(char *ifname, virNetDevRxFilterPtr guestFilter,
> + virNetDevRxFilterPtr hostFilter)
> +{
> + syncNicRxFilterPromiscMode(ifname, guestFilter, hostFilter);
> + syncNicRxFilterMultiMode(ifname, guestFilter, hostFilter);
> +}
> +
> +
> +static void
> syncNicRxFilterMulticast(char *ifname,
> virNetDevRxFilterPtr guestFilter,
> virNetDevRxFilterPtr hostFilter)
> @@ -4250,9 +4337,14 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
> * attributes to match those of the guest network device:
> * - MAC address
> * - Multicast MAC address table
> + * - Device options:
> + * - PROMISC
> + * - MULTICAST
> + * - ALLMULTI
> */
> syncNicRxFilterMacAddr(def->ifname, guestFilter, hostFilter);
> syncNicRxFilterMulticast(def->ifname, guestFilter, hostFilter);
> + syncNicRxFilterDeviceOptions(def->ifname, guestFilter, hostFilter);
> }
>
> endjob:
I think this is otherwise okay.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list