[libvirt] [PATCH 0/6] handle NIC_RX_FILTER_CHANGED events from qemu

[Tony Krowiak]

On 09/30/2014 03:47 PM, Laine Stump wrote:
> On 09/30/2014 02:28 PM, Tony Krowiak wrote:
>> On 09/24/2014 05:50 AM, Laine Stump wrote:
>>> These patches set up an event handler for qemu's NIC_RX_FILTER_CHANGED
>>> event, which is sent whenever a guest makes a change to a network
>>> device's unicast/multicast filter, vlan table, or MAC address.
>>>
>>> The handler checks if it is appropriate to respond to the
>>> NIC_RX_FILTER_CHANGED event (based on device type and configuration)
>>> and takes appropriate action. Currently it checks if the guest
>>> interface has been configured with trustGuestRxFilters='yes' (defaults
>>> to 'no' for security reasons), and if the host side device is
>>> macvtap. If so, and the MAC address on the guest has changed, the MAC
>>> address of the macvtap device is changed to match.
>>>
>>> The result of this is that networking from the guest will continue to
>>> work if the mac address of a macvtap-connected network device is
>>> changed from within the guest, as long as trustGuestRxFilters='yes'
>>> (previously changing the MAC address in the guest would break
>>> networking).
>>>
>>> I still need to add code to compare the old and new unicast and
>>> multicast lists and program the filters in the macvtap to match the
>>> guest, and to check for a non-empty vlan table and handle that
>>> (currently that means just setting promiscuous mode on the macvtap),
>>> but that can come in a followup series.
>> I was very interested in this patch set because I developed a set of
>> patches to respond to the NIC_RX_FILTER_CHANGED event.  I completed
>> the patch set several weeks ago and have been awaiting completion of
>> our internal review before submitting them to this mailing list.
>> Apparently you beat me to the punch.  I have code that compares
>> the old and new multicast lists and synchronizes the macvtap filters
>> with the guest's.  I can modify my patches to integrate this function
>> into what you have provided with this patch set.  Would that be
>> agreeable?
> Since I've just started working on exactly that, yes :-)
>
> What I'd started was a function virNetDevGetRxFilter(ifname, filter) in
> virnetdev.c which would do for the host-side tap/macvtap device what
> qemuMonitorQueryRxFilter() does for the guest's interface - retrieve the
> current unicast/multicast/vlan tables & modes (using code from
> iproute2's "bridge" command as a guide to write equivalent libnl-based
> code) and return them in a fully-populated  virNetDevRxFilter object
> (that's why I defined that struct in virnetdev.h even though I've so far
> used it only in the qemu driver), which would be called from the event
> handler; the event handler would compare the two virNetDevRxFilter
> objects (the one from the host-side device and the one from the
> guest-side device) and issue the necessary commands to make everything
> match (well, actually what I've been told is that in the case of vlans,
> if the guest has a non-empty vlan table we currently have to just set
> the macvtap to promiscuous mode).
>
> It sounds like you're only interested in the multicast list, so if you
> wanted to fill in enough to make it do that, your code could be used as
> a model to do the unicast list (which seems to be empty most of the time
> anyway; as I usually operate above that layer, I'm truthfully not
> exactly sure when it's even used).
I am interested in a complete solution, however; I chose to ignore the 
unicast
list for the time being for reasons similar to yours.  I wrote code to
synchronize the VLAN table, but subsequently learned that there were 
problems
with VLAN on macvtap, so I took that code out of the patch set.

I used the "ip maddr show" command in iproute2 as a model for acquiring the
multicast list.  This command reads the /proc/net/dev_mcast file on the 
host to
get the current multicast MAC address list for the macvtap device. To
synchronize the macvtap device's multicast list, I compared the list from
/proc/net/dev_mcast with that returned from the query_rx_filter command 
and used
the SIOCADDMULTI and SIOCDELMULTI ioctl's add and delete multicast MAC 
addresses
as needed.  I did not do anything with the mode (state) values as I have yet
to figure out how to do that.

I also wrote code to synchronize the following interface flags:
* promiscuous
* multicast
* allmulti
* broadcast

I can integrate what I have with your infrastructure including:
* Create a virNetDevGetRxFilter(ifname, filter) function in virnetdev.c 
that will
   populate the following fields in the filter:
     * name
     * mac
     * promiscuous
     * broadcastAllowed
     * multicast.table
     * multicast.nTable
     * vlan.table
     * vlan.nTable
* Implement the event handler code to:
     * compare the two virNetDevRxFilter objects and issue the necessary 
commands to
       synchronize the multicast MAC address lists
     * set the promiscuous mode if the guest has a non-empty vlan table
     * compare the device flags and synchronize their values

If you think this would be a worthwhile endeavor, I've got your patches 
installed and
can proceed.
>
> In the future, this same infrastructure will be used to monitor MAC
> address and filter changes for interfaces connected to host bridges via
> normal tap devices, so that the bridge can be taken out of "learning"
> mode (we then need to manually update the bridge's MAC tables based on
> the info from query-rx-filter); this can apparently yield a significant
> performance increase.
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>