On 10/02/2014 10:19 AM, Laine Stump wrote:
On 10/01/2014 03:36 PM, Tony Krowiak wrote:
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.
Sure. I would prefer using netlink over /proc, but I have yet to figure
out the code to retrieve the multicast list in that manner (see my above
mention of iproute's "bridge" command - interesting that two different
parts of the same package retrieve the info in different ways).
I spent some time back when I first started looking at this trying to figure
out how to use netlink to retrieve the data, but was unable to find much
doc on how to do it, so I ultimately settled on /proc. I think the code
can be structured in such a way that the /proc code can eventually be
replaced with netlink.
It would be very useful if virNetDevGetRxFilter() called a function for
each one of the components (e.g. virNetDevGetMcastList(),
virNetDevGetPromiscuous(), virNetDevAddMcast(), virNetDevDelMCast(),
etc) so that the implementation of each of these functions could be
easily changed to use netlink (or whatever was appropriate for any given
platform) based on some configure-time conditionals.
That is how my code is currently structured. I just need to change it
to use your structure.
I'm about to post V2 of my patches, which have minor changes, so you may
want to use those as a base instead of the originals.
Will do.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list