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

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.