This patch adds test cases for parsing of parameters with
multiple occurrances of the same name.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
This patch modifies the NWFilter parameter parser to support multiple
elements with the same name and to internally build a list of items.
An example of the XML looks like this:
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
<parameter name='TEST' value='10.1.1.1'/>
The list of values is then stored in the newly introduced data type
virNWFilterVarValue.
The XML formatter is also adapted to print out all items in alphabetical
order sorted by 'name'.
This patch also fixes a bug in the XML schema on the way.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
This patch extends the NWFilter driver for Linux (ebiptables) to create
rules for each member of a previously introduced list. If for example
an attribute value (internally) looks like this:
IP = [10.0.0.1, 10.0.0.2, 10.0.0.3]
then 3 rules will be generated for a rule accessing the variable 'IP',
one for each member of the list. The effect of this is that this now
allows for filtering for multiple values in one field. This can then be
used to support for filtering/allowing of multiple IP addresses per
interface.
An iterator is introduced that extracts each member of a list and
puts it into a hash table which then is passed to the function creating
a rule. For the above example the iterator would cause 3 loops.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
NWFilters can be provided name-value pairs using the following
XML notation:
<filterref filter='xyz'>
<parameter name='PORT' value='80'/>
<parameter name='VAL' value='abc'/>
</filterref>
The internal representation currently is so that a name is stored as a
string and the value as well. This patch now addresses the value part of it
and introduces a data structure for storing a value either as a simple
value or as an array for later support of lists.
This patch adjusts all code that was handling the values in hash tables
and makes it use the new data type.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
This patch adds several aspects of documentation about the network filtering
system:
- chains, chains' priorities and chains' default priorities
- talks about lists of elements, i.e., a variable assigned multiple values
(part of already ACK-ed series)
- already mentions the vlan, stp and mac chains added later on
(https://www.redhat.com/archives/libvir-list/2011-October/msg01238.html)
- mentions limitations of vlan filtering (when sent by VM) on Linux systems
The previous patch extends the priority of filtering rules into negative
numbers. We now use this possibility to interleave the jumping into
chains with filtering rules to for example create the 'root' table of
an interface with the following sequence of rules:
Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
-p IPv4 -j I-vnet0-ipv4
-p ARP -j I-vnet0-arp
-p ARP -j ACCEPT
-p 0x8035 -j I-vnet0-rarp
-p 0x835 -j ACCEPT
-j DROP
The '-p ARP -j ACCEPT' rule now appears between the jumps.
Since the 'arp' chain has been assigned priority -700 and the 'rarp'
chain -600, the above ordering can now be achieved with the following
rule:
<rule action='accept' direction='out' priority='-650'>
<mac protocolid='arp'/>
</rule>
This patch now sorts the commands generating the above shown jumps into
chains and interleaves their execution with those for generating rules.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
So far rules' priorities have only been valid in the range [0,1000].
Now I am extending their priority into the range [-1000, 1000] for subsequently
being able to sort rules and the access of (jumps into) chains following
priorities.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
This patch enables chains that have a known prefix in their name.
Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All prefixes
are also protocols that can be evaluated on the ebtables level.
Following the prefix they will be automatically connected to an interface's
'root' chain and jumped into following the protocol they evaluate, i.e.,
a table 'arp-xyz' will be accessed from the root table using
ebtables -t nat -A <iface root table> -p arp -j I-<ifname>-arp-xyz
thus generating a 'root' chain like this one here:
Bridge chain: libvirt-O-vnet0, entries: 5, policy: ACCEPT
-p IPv4 -j O-vnet0-ipv4
-p ARP -j O-vnet0-arp
-p 0x8035 -j O-vnet0-rarp
-p ARP -j O-vnet0-arp-xyz
-j DROP
where the chain 'arp-xyz' is accessed for filtering of ARP packets.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
This patch extends the filter XML to support priorities of chains
in the XML. An example would be:
<filter name='allow-arpxyz' chain='arp-xyz' priority='200'>
[...]
</filter>
The permitted values for priorities are [-1000, 1000].
By setting the priority of a chain the order in which it is accessed
from the interface root chain can be influenced.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Use the name of the chain rather than its type index (enum).
This pushes the later enablement of chains with user-given names
into the XML parser. For now we still only allow those names that
are well known ('root', 'arp', 'rarp', 'ipv4' and 'ipv6').
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Use scripts for the renaming and cleaning up of chains. This allows us to get
rid of some of the code that is only capable of renaming and removing chains
whose names are hardcoded.
A shell function 'collect_chains' is introduced that is given the name
of an ebtables chain and then recursively determines the names of all
chains that are accessed from this chain and its sub-chains using 'jumps'.
The resulting list of chain names is then used to delete all the found
chains by first flushing and then deleting them.
The same function is also used for renaming temporary filters to their final
names.
I tested this with the bash and dash as script interpreters.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Use the previously introduced chain priorities to sort the chains for access
from an interface's 'root' table and have them created in the proper order.
This gets rid of a lot of code that was previously creating the chains in a
more hardcoded way.
To determine what protocol a filter is used for evaluation do prefix-
matching, i.e., the filter 'arp' is used to filter for the 'arp' protocol,
'ipv4' for the 'ipv4' protocol and 'arp-xyz' will also be used to filter
for the 'arp' protocol following the prefix 'arp' in its name.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
For better handling of the sorting of chains introduce an internally used
priority. Use a lookup table to store the priorities. For now their actual
values do not matter just that the values cause the chains to be properly
sorted through changes in the following patches. However, the values are
chosen as negative so that once they are sorted along with filtering rules
(whose priority may only be positive for now) they will always be instantiated
before them (lower values cause instantiation before higher values). This
is done to maintain backwards compatibility.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Add a function to the virHashTable for getting an array of the hash table's
key-value pairs and have the keys (optionally) sorted.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Replacing the strchr call with two variables through a strstr call.
Calling strchr with two variables triggers a gcc 4.3/4.4
bug when used in combination with -Wlogical-op and at least -O1.
This patch fixes the bug shown in bugzilla 738778. It's not an nwfilter problem but a connection sharing / closure issue.
https://bugzilla.redhat.com/show_bug.cgi?id=738778
Depending on the speed / #CPUs of the machine you are using you may not see this bug all the time.
Get rid of the #if __linux__ check in virPidFileReadPathIfAlive that
was preventing a check of a symbolic link in /proc/<pid>/exe on
non-linux platforms against an expected executable. Replace
this with a run-time check testing whether the /proc/<pid>/exe is a
symbolic link and if so call the function doing the comparison
against the expected file the link is supposed to point to.
This is in response to bugzilla 664629
https://bugzilla.redhat.com/show_bug.cgi?id=664629
The patch below returns an appropriate error message if the chain of
nwfilters is found to contain unresolvable variables and therefore
cannot be instantiated.
Example: The following XMl added to a domain:
<interface type='bridge'>
<mac address='52:54:00:9f:80:45'/>
<source bridge='virbr0'/>
<model type='virtio'/>
<filterref filter='test'/>
</interface>
that references the following filter
<filter name='test' chain='root'>
<filterref filter='clean-traffic'/>
<filterref filter='allow-dhcp-server'/>
</filter>
now displays upon 'virsh start mydomain'
error: Failed to start domain mydomain
error: internal error Cannot instantiate filter due to unresolvable variable: DHCPSERVER
'DHPCSERVER' is contained in allow-dhcp-server.
This patch fixes the compilation of netlink.c and interface.c on those
systems missing either libnl or that have an older linux/if_link.h
include file not supporting macvtap or VF_PORTS.
WITH_MACVTAP is '1' if newer include files were detected, '0' otherwise.
IFLA_PORT_MAX is defined in linux/if_link.h if yet more functionality is
supported.
In a second cleanup step this patch makes several interface functions from macvtap.c commonly available by moving them into interface.c and prefixing their names with 'iface'. Those functions taking Linux-specific structures as parameters are only visible on Linux.
ifaceRestoreMacAddress returns the return code from the ifaceSetMacAddr call and display an error message if setting the MAC address did not work. The caller is unchanged and still ignores the return code (which is ok).
In a first cleanup step, make nlComm from macvtap.c commonly available
for other code to use. Since nlComm uses Linux-specific structures as
parameters it's prototype is only visible on Linux.
The below patch decreases the response time of libvirt to errors reported by Qemu upon startup by checking whether the qemu process is still alive while polling for the local socket to show up.
This patch also introduces a special handling of signal for the Win32 part of virKillProcess.
This patch reorders the locks for the nwfilter updates and the access
the nwfilter objects. In the case that the IP address learning thread
was instantiating filters while an update happened, the previous order
lead to a deadlock.
This patch enables filtering of gratuitous ARP packets using the following XML:
<rule action='accept' direction='in' priority='425'>
<arp gratuitous='true'/>
</rule>
Due to differences in /proc/cpuinfo the parsing of the cpu data is
different between architectures. On PPC /proc/cpuinfo looks like this:
[original formatting with tabs]
processor : 0
cpu : PPC970MP, altivec supported
clock : 2297.700000MHz
revision : 1.1 (pvr 0044 0101)
processor : 1
cpu : PPC970MP, altivec supported
clock : 2297.700000MHz
revision : 1.1 (pvr 0044 0101)
[..]
timebase : 14318000
platform : pSeries
model : IBM,8844-AC1
machine : CHRP IBM,8844-AC1
The patch adapts the parsing of the data found in /proc/cpuinfo.
/sys/devices/system/cpu/cpuX/topology/physical_package_id also
always returns -1. Check for it on ppc and make it '0' if found negative.
This patch enables the migration of Qemu VMs between hosts of different endianess. I tested this by migrating a i686 VM between a x86 and ppc64 host.
I am converting the 'int's in the VM's state header to uint32_t assuming this doesn't break compatibility with existing deployments other than Linux.
This patch adds support for the evaluation of TCP flags in nwfilters.
It adds documentation to the web page and extends the tests as well.
Also, the nwfilter schema is extended.
The following are some example for rules using the tcp flags:
<rule action='accept' direction='in'>
<tcp state='NONE' flags='SYN/ALL' dsptportstart='80'/>
</rule>
<rule action='drop' direction='in'>
<tcp state='NONE' flags='SYN/ALL'/>
</rule>
This patch adds the possibility to not just drop packets, but to also have them rejected where iptables at least sends an ICMP msg back to the originator. On ebtables this again maps into dropping packets since rejecting is not supported.
I am adding 'since 0.8.9' to the docs assuming this will be the next version of libvirt.
This patch reorders the connlimit and comment match extensions relative to the state match (-m state); connlimit being most useful if found after a -m state --state NEW and not before it.
This patch fixes 2 occurrences of nla_put expression with a '!' in
front of them that basically prevented the detection that the buffer
is too small. However, code further below would then detect that the
buffer is too small when further parts are added to the netlink message.
While doing some testing with Qemu and creating huge logfiles I encountered the case where the VM could not start anymore due to the lseek() to the end of the Qemu VM's log file failing. The patch below fixes the problem by replacing the previously used 'int' with 'off_t'.
To reproduce this error, you could do the following:
dd if=/dev/zero of=/var/log/libvirt/qemu/<name of VM>.log bs=1024 count=$((1024*2048))
and you should get an error like this:
error: Failed to start domain <name of VM>
error: Unable to seek to -2147482651 in /var/log/libvirt/qemu/<name of VM>.log: Success
This patch introduces the usage of the pre-associate state of the IEEE 802.1Qbg standard on incoming VM migration on the target host. It is in response to bugzilla entry 632750.
https://bugzilla.redhat.com/show_bug.cgi?id=632750
For being able to differentiate the exact reason as to why a macvtap device is being created, either due to a VM creation or an incoming VM migration, I needed to pass that reason as a parameter from wherever qemudStartVMDaemon is being called in order to determine whether to send an ASSOCIATE (VM creation) or a PRE-ASSOCIATE (incoming VM migration) towards lldpad.
I am also fixing a problem with the virsh domainxml-to-native call on the way.
Gerhard successfully tested the patch with a recent blade network 802.1Qbg-compliant switch.
The patch should not have any side-effects on the 802.1Qbh support in libvirt, but Roopa (cc'ed) may want to verify this.
The IP address learning thread was causing a deadlock when it instantiated a filter while a filter update/change was ongoing. The reason for this was the ordering of locks due to the following calls
virNWFilterUnlockFilterUpdates()
virNWFilterPoolObjFindByName()
The below patch now puts the order of the locks in the above shown order when instantiating the filter from the IP address learning thread.
I am replacing the last instances of close() I found with VIR_CLOSE() / VIR_FORCE_CLOSE respectively.
The first part patches virsh, which I missed out on previously.
The 2nd patch I had left out intentionally to look at it more carefully:
The 'closed' variable could be easily removed since it wasn't used anywhere else. The possible race condition that could result from the filedescriptor being closed and not set to -1 (and possibly let us write into 'something' totally different if the fd was allocated by another thread) seems to be prevented by the qemuMonitorLock() already placed around the code that reads from or writes to the fd. So the change of this code as shown in the patch should not have any side-effects.
Rather than only cleaning any remaining ebtables rules, also clean those applied to iptables and ip6tables when detecting the IP address of an interface. Previous applied iptables rules may hinder DHCP packets.
Similarly to deprecating close(), I am now deprecating fclose() and
introduce VIR_FORCE_FCLOSE() and VIR_FCLOSE(). Also, fdopen() is replaced with
VIR_FDOPEN().
Most of the files are opened in read-only mode, so usage of
VIR_FORCE_CLOSE() seemed appropriate. Others that are opened in write
mode already had the fclose()< 0 check and I converted those to
VIR_FCLOSE()< 0.
I did not find occurrences of possible double-closed files on the way.
In a first step I am converting the netlink message construction in
macvtap code to use libnl. It's pretty much a 1:1 conversion except that
now the message needs to be allocated and deallocated.
Using automated replacement with sed and editing I have now replaced all
occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
course. Some replacements were straight forward, others I needed to pay
attention. I hope I payed attention in all the right places... Please
have a look. This should have at least solved one more double-close
error.
During a shutdown/restart cycle libvirtd forgot the macvtap device name that it had created on behalf of a VM so that a stale macvtap device remained on the host when the VM terminated. Libvirtd has to actively tear down a macvtap device and it uses its name for identifying which device to tear down.
The solution is to not blank out the <target dev='...'/> completely, but only blank it out on VMs that are not active. So, if a VM is active, the device name makes it into the XML and is also being parsed. If a VM is not active, the device name is discarded.
The 2nd and 3rd hunk show the only double-closed file descriptor code part that I found while trying to clean up close(). The first hunk seems a harmless cleanup in that same file.
In the table built for traffic coming from the VM going to the host make the following changes:
- don't ACCEPT the packets but do a 'RETURN' and let the host-specific firewall rules in subsequent rules evaluate whether the traffic is allowed to enter
- use the '-m state' in the rules as everywhere else
Since bugs due to double-closed file descriptors are difficult to track down in a multi-threaded system, I am introducing the VIR_CLOSE(fd) macro to help avoid mistakes here.
There are lots of places where close() is being used. In this patch I am only cleaning up usage of close() in src/conf where the problems were.
I also dare to declare close() as being deprecated in libvirt code base (HACKING).
The following filter transition from a filter allowing incoming TCP connections
<rule action='accept' direction='in' priority='401'>
<tcp/>
</rule>
<rule action='accept' direction='out' priority='500'>
<tcp/>
</rule>
to one that does not allow them
<rule action='drop' direction='in' priority='401'>
<tcp/>
</rule>
<rule action='accept' direction='out' priority='500'>
<tcp/>
</rule>
did previously not cut off existing (ssh) connections but only prevented newly initiated ones. The attached patch allows to cut off existing connections as well, thus enforcing what the filter is showing.
I had only tested with a configuration where the physical interface is connected to the bridge where the filters are applied. This patch now also solves a filtering problem where the physical interface is not connected to the bridge, but the bridge is given an IP address and the host routes between bridge and physical interface. Here the filters drop non-allowed traffic on the outgoing side on the host.
This is from a bug report and conversation on IRC where Soren reported that while a filter update is occurring on one or more VMs (due to a rule having been edited for example), a deadlock can occur when a VM referencing a filter is started.
The problem is caused by the two locking sequences of
qemu driver, qemu domain, filter # for the VM start operation
filter, qemu_driver, qemu_domain # for the filter update operation
that obviously don't lock in the same order. The problem is the 2nd lock sequence. Here the qemu_driver lock is being grabbed in qemu_driver:qemudVMFilterRebuild()
The following solution is based on the idea of trying to re-arrange the 2nd sequence of locks as follows:
qemu_driver, filter, qemu_driver, qemu_domain
and making the qemu driver recursively lockable so that a second lock can occur, this would then lead to the following net-locking sequence
qemu_driver, filter, qemu_domain
where the 2nd qemu_driver lock has been ( logically ) eliminated.
The 2nd part of the idea is that the sequence of locks (filter, qemu_domain) and (qemu_domain, filter) becomes interchangeable if all code paths where filter AND qemu_domain are locked have a preceding qemu_domain lock that basically blocks their concurrent execution
So, the following code paths exist towards qemu_driver:qemudVMFilterRebuild where we now want to put a qemu_driver lock in front of the filter lock.
-> nwfilterUndefine() [ locks the filter ]
-> virNWFilterTestUnassignDef()
-> virNWFilterTriggerVMFilterRebuild()
-> qemudVMFilterRebuild()
-> nwfilterDefine()
-> virNWFilterPoolAssignDef() [ locks the filter ]
-> virNWFilterTriggerVMFilterRebuild()
-> qemudVMFilterRebuild()
-> nwfilterDriverReload()
-> virNWFilterPoolLoadAllConfigs()
->virNWFilterPoolObjLoad()
-> virNWFilterPoolAssignDef() [ locks the filter ]
-> virNWFilterTriggerVMFilterRebuild()
-> qemudVMFilterRebuild()
-> nwfilterDriverStartup()
-> virNWFilterPoolLoadAllConfigs()
->virNWFilterPoolObjLoad()
-> virNWFilterPoolAssignDef() [ locks the filter ]
-> virNWFilterTriggerVMFilterRebuild()
-> qemudVMFilterRebuild()
Qemu is not the only driver using the nwfilter driver, but also the UML driver calls into it. Therefore qemuVMFilterRebuild() can be exchanged with umlVMFilterRebuild() along with the driver lock of qemu_driver that can now be a uml_driver. Further, since UML and Qemu domains can be running on the same machine, the triggering of a rebuild of the filter can touch both types of drivers and their domains.
In the patch below I am now extending each nwfilter callback driver with functions for locking and unlocking the (VM) driver (UML, QEMU) and introduce new functions for locking all registered callback drivers and unlocking them. Then I am distributing the lock-all-cbdrivers/unlock-all-cbdrivers call into the above call paths. The last shown callpath starting with nwfilterDriverStart() is problematic since it is initialize before the Qemu and UML drives are and thus a lock in the path would result in a NULL pointer attempted to be locked -- the call to virNWFilterTriggerVMFilterRebuild() is never called, so we never lock either the qemu_driver or the uml_driver in that path. Therefore, only the first 3 paths now receive calls to lock and unlock all callback drivers. Now that the locks are distributed where it matters I can remove the qemu_driver and uml_driver lock from qemudVMFilterRebuild() and umlVMFilterRebuild() and not requiring the recursive locks.
For now I want to put this out as an RFC patch. I have tested it by 'stretching' the critical section after the define/undefine functions each lock the filter so I can (easily) concurrently execute another VM operation (suspend,start). That code is in this patch and if you want you can de-activate it. It seems to work ok and operations are being blocked while the update is being done.
I still also want to verify the other assumption above that locking filter and qemu_domain always has a preceding qemu_driver lock.
This patch adds another example to the nwfilter html page and provides 2 solutions for how to write a filter meeting the given requirements using newly added features.
This patch adds a test case for testing the XML parser's and instantiator's
support of the state attribute. The other test case tests existing
capabilities. Both test cases will be used in TCK again.
In this patch I am extending the rule instantiator to create the state
match according to the state attribute in the XML. Only one iptables
rule in the incoming or outgoing direction will be created for a rule
in direction 'in' or 'out' respectively. A rule in direction 'inout' does
get iptables rules in both directions.
In this patch I am extending the rule instantiator to create the comment
node where supported, which is the case for iptables and ip6tables.
Since commands are written in the format
cmd='iptables ...-m comment --comment \"\" '
certain characters ('`) in the comment need to be escaped to
prevent comments from becoming commands themselves or cause other
forms of (bash) substitutions. I have tested this with various input and in
my tests the input made it straight into the comment. A test case for TCK
will be provided separately that tests this.
In this patch I am reworking the logic around detecting virtual port support and requiring the libnl dependency.
- It requires --with-macvtap and displays an error in case of --without-macvtap --with-virtualport.
- It tests for availability of certain data in include files and displays an error in case the include file is not at the correct level and --with-virtualport was chosen
- displays 'checking' messages for macvtap and virtualport support and results
- libnl support is required when macvtap is found or requested; if libnl is not there, please supply without-macvtap
The patch below reports a warning in the log if the generated ip(6)tables rules would not be effective due to the proc filesystem entries
/proc/sys/net/bridge/bridge-nf-call-iptables
/proc/sys/net/bridge/bridge-nf-call-ip6tables
containing a '0'. The warning tells the user what to do. I am rate-limiting the warning message to appear only every 10 seconds.
In this patch I am extending and fixing the nwfilter module's reload support to stop all ongoing threads (for learning IP addresses of interfaces) and rebuild the filtering rules of all interfaces of all VMs when libvirt is started. Now libvirtd rebuilds the filters upon the SIGHUP signal and libvirtd restart.
About the patch: The nwfilter functions require a virConnectPtr. Therefore I am opening a connection in qemudStartup, which later on needs to be closed outside where the driver lock is held since otherwise it ends up in a deadlock due to virConnectClose() trying to lock the driver as well.
I have tested this now for a while with several machines running and needing the IP address learner thread(s). The rebuilding of the firewall rules seems to work fine following libvirtd restart or a SIGHUP. Also the termination of libvirtd worked fine.
When sniffing the network traffic, discard class D and E IP addresses when sniffing traffic. This was a reason why filters were not correctly rebuilt on VMs on the local 192.* network when libvirt was restarted and those VMs did not use a DHCP request to get its IP address.
While testing the SIGHUP handling and reloading of the nwfilter driver, I found that when the filters are rebuilt and mutlipe threads handled the individual interfaces, concurrently running multiple external bash scripts causes strange failures even though the executed ebtables commands are working on different tables for different interfaces. I cannot say for sure where the concurrency problems are caused, but introducing this lock definitely helps.
This patch fixes a couple of complaints from valgrind when tickling libvirtd with SIGHUP.
The first two files contain fixes for memory leaks. The 3rd one initializes an uninitialized variable. The 4th one is another memory leak.
Following Daniel Berrange's multiple helpful suggestions for improving
this patch and introducing another driver interface, I now wrote the
below patch where the nwfilter driver registers the functions to
instantiate and teardown the nwfilters with a function in
conf/domain_nwfilter.c called virDomainConfNWFilterRegister. Previous
helper functions that were called from qemu_driver.c and qemu_conf.c
were move into conf/domain_nwfilter.h with slight renaming done for
consistency. Those functions now call the function expored by
domain_nwfilter.c, which in turn call the functions of the new driver
interface, if available.
This patch adds an optional XML attribute to a nwfilter rule to give the user control over whether the rule is supposed to be using the iptables state match or not. A rule may now look like shown in the XML below with the statematch attribute either having value '0' or 'false' (case-insensitive).
[...]
<rule action='accept' direction='in' statematch='false'>
<tcp srcmacaddr='1:2:3:4:5:6'
srcipaddr='10.1.2.3' srcipmask='32'
dscp='33'
srcportstart='20' srcportend='21'
dstportstart='100' dstportend='1111'/>
</rule>
[...]
I am also extending the nwfilter schema and add this attribute to a test case.
This patch works around a recent extension of the netlink driver I had made use of when building the netlink messages. Unfortunately older kernels don't accept IFLA_IFNAME + name of interface as a replacement for the interface's index, so this patch now gets the interface index ifindex if it's not provided (ifindex <= 0).
This patch that adds support for configuring 802.1Qbg and 802.1Qbh
switches. The 802.1Qbh part has been successfully tested with real
hardware. The 802.1Qbg part has only been tested with a (dummy)
server that 'behaves' similarly to how we expect lldpad to 'behave'.
The following changes were made during the development of this patch:
- Merging Scott's v13-pre1 patch
- Fixing endptr related bug while using virStrToLong_ui() pointed out
by Jim Meyering
- Addressing Jim Meyering's comments to v11
- requiring mac address to the vpDisassociateProfileId() function to
pass it further to the 802.1Qbg disassociate part (802.1Qbh untouched)
- determining pid of lldpad daemon by reading it from /var/run/libvirt.pid
(hardcode as is hardcode alson in lldpad sources)
- merging netlink send code for kernel target and user space target
(lldpad) using one function nlComm() to send the messages
- adding a select() after the sending and before the reading of the
netlink response in case lldpad doesn't respond and so we don't hang
- when reading the port status, in case of 802.1Qbg, no status may be
received while things are 'in progress' and only at the end a status
will be there.
- when reading the port status, use the given instanceId and vf to pick
the right IFLA_VF_PORT among those nested under IFLA_VF_PORTS.
- never sending nor parsing IFLA_PORT_SELF type of messages in the
802.1Qbg case
- iterating over the elements in a IFLA_VF_PORTS to pick the right
IFLA_VF_PORT by either IFLA_PORT_PROFILE and given profileId
(802.1Qbh) or IFLA_PORT_INSTANCE_UUID and given instanceId (802.1Qbg)
and reading the current status in IFLA_PORT_RESPONSE.
- recycling a previous patch that adds functionality to interface.c to
- get the vlan identifier on an interface
- get the flags of an interface and some convenience function to
check whether an interface is 'up' or not (not currently used here)
- adding function to determine the root physical interface of an
interface. For example if a macvtap is linked to eth0.100, it will
find eth0. Also adding a function that finds the vlan on the 'way to
the root physical interface'
- conveying the root physical interface name and index in case of 802.1Qbg
- conveying mac address of macvlan device and vlan identifier in
IFLA_VFINFO_LIST[ IFLA_VF_INFO[ IFLA_VF_MAC(mac), IFLA_VF_VLAN(vlan) ] ]
to (future) lldpad via netlink
- To enable build with --without-macvtap rename the
[dis|]associatePortProfileId functions, prepend 'vp' before their
name and make them non-static functions.
- Renaming variable multicast to nltarget_kernel and inverting
the logic
- Addressing Jim Meyering's comments; this also touches existing
code for example for correcting indentation of break statements or
simplification of switch statements.
- Renamed occurrencvirVirtualPortProfileDef to virVirtualPortProfileParamses
- 802.1Qbg part prepared for sending a RTM_SETLINK and getting
processing status back plus a subsequent RTM_GETLINK to
get IFLA_PORT_RESPONSE.
Note: This interface for 802.1Qbg may still change
- [David Allan] move getPhysfn inside IFLA_VF_PORT_MAX to avoid
compiler
warning when latest if_link.h isn't available
- move from Stefan's 802.1Qb{g|h} XML v8 to v9
- move hostuuid and vf index calcs to inside doPortProfileOp8021Qbh
- remove debug fprintfs
- use virGetHostUUID (thanks Stefan!)
- fix compile issue when latest if_link.h isn't available
- change poll timeout to 10s, at 1/8 intervals
- if polling times out, log msg and return -ETIMEDOUT
- Add Stefan's code for getPortProfileStatus
- Poll for up to 2 secs for port-profile status, at 1/8 sec intervals:
- if status indicates error, abort openMacvtapTap
- if status indicates success, exit polling
- if status is "in-progress" after 2 secs of polling, exit
polling loop silently, without error
My patch finishes out the 802.1Qbh parts, which Stefan had mostly complete.
I've tested using the recent kernel updates for VF_PORT netlink msgs and
enic for Cisco's 10G Ethernet NIC. I tested many VMs, each with several
direct interfaces, each configured with a port-profile per the XML. VM-to-VM,
and VM-to-external work as expected. VM-to-VM on same host (using same NIC)
works same as VM-to-VM where VMs are on diff hosts. I'm able to change
settings on the port-profile while the VM is running to change the virtual
port behaviour. For example, adjusting a QoS setting like rate limit. All
VMs with interfaces using that port-profile immediatly see the effect of the
change to the port-profile.
I don't have a SR-IOV device to test so source dev is a non-SR-IOV device,
but most of the code paths include support for specifing the source dev and
VF index. We'll need to complete this by discovering the PF given the VF
linkdev. Once we have the PF, we'll also have the VF index. All this info-
mation is available from sysfs.
Since the macvtap device needs active tear-down and the teardown logic
is based on the interface name, it can happen that if for example 1 out
of 3 interfaces was successfully created, that during the failure path
the macvtap's target device name is used to tear down an interface that
is doesn't own (owned by another VM).
So, in this patch, the target interface name is reset so that there is
no target interface name and the interface name is always cleared after
a tear down.
This patch parses the following two XML descriptions, one for
802.1Qbg and one for 802.1Qbh, and stores the data internally.
The actual triggering of the switch setup protocol has not been
implemented here but the relevant code to do that should go into
the functions associatePortProfileId() and disassociatePortProfileId().
<interface type='direct'>
<source dev='eth0.100' mode='vepa'/>
<model type='virtio'/>
<virtualport type='802.1Qbg'>
<parameters managerid='12' typeid='0x123456' typeidversion='1'
instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f'/>
</virtualport>
<filterref filter='clean-traffic'/>
</interface>
<interface type='direct'>
<source dev='eth0.100' mode='vepa'/>
<model type='virtio'/>
<virtualport type='802.1Qbh'>
<parameters profileid='my_profile'/>
</virtualport>
</interface>
I'd suggest to use this patch as a base for triggering the setup
protocol with the 802.1Qb{g|h} switch.
Several rounds of changes were made to this patch. The
following is a list of these changes.
- Renamed structure virVirtualPortProfileDef to virVirtualPortProfileParams
as per Daniel Berrange's request
- Addressing Daniel Berrange's comments:
- removing macvtap.h's dependency on domain_conf.h by
moving the virVirtualPortProfileDef structure into macvtap.h
and not passing virtDomainNetDefPtr to any functions in
macvtap.c
- Addressed most of Chris Wright's comments:
- indicating error in case virtualport XML node cannot be parsed
properly
- parsing hex and decimal numbers using virStrToLong_ui() with
parameter '0' for base
- tgifname (target interface name) variable wasn't necessary
to pass to openMacvtapTap function anymore
- assigning the virtual port data structure to the virDomainNetDef
only if it was previously parsed
- make sure that the error code returned by openMacvtapTap() is a negative n
in case the associatePortProfileId() function failed.
- renaming vsi in the XML to virtualport
- replace all occurrences of vsi in the source as well
- removing mode and MAC address parameters from the functions that
will communicate with the hareware diretctly or indirectly
- moving the associate and disassociate functions to the end of the
file for subsequent patches to easier make them generally available
for export
- passing the macvtap interface name rather than the link device since
this otherwise gives funny side effects when using netlink messages
where IFLA_IFNAME and IFLA_ADDRESS are specified and the link dev
all of a sudden gets the MAC address of the macvtap interface.
- Removing rc = -1 error indications in the case of 802.1Qbg|h setup in case
we wanted to use hook scripts for the setup and so the setup doesn't fail
here.
- if instance ID UUID is not supplied it will automatically be generated
- adapted schema to make instance ID UUID optional
- added test case
- parser and XML generator have been separated into their own
functions so they can be re-used elsewhere (passthrough case
for example)
- Adapted XML parser and generator support the above shown type
(802.1Qbg, 802.1Qbh).
- Adapted schema to above XML
- Adapted test XML to above XML
- Passing through the VM's UUID which seems to be necessary for
802.1Qbh -- sorry no host UUID
- adding virtual function ID to association function, in case it's
necessary to use (for SR-IOV)
This patch introduces a dependency on libnl, which subsequent patches
will then use.
Changes from V1 to V2:
- added diffstats
- following changes in tree
Add ebtables,iptables & iptables-ipv6 dependency to rpm.
Changes from V1 to V2:
-passing --without-libpcap to configure script, if libpcap is not to be used
When a filter is updated, only those interfaces must have their old
rules cleared that either reference the filter directly or indirectly
through another filter. Remember between the different steps of the
instantiation of the filters which interfaces must be skipped. I am
using a hash map to remember the names of the interfaces and store a
bogus pointer to ~0 into it that need not be freed.
For the decision on whether to instantiate the rules, the check for a
pending IP address learn request is not sufficient since then only the
thread could instantiate the rules. So, a boolean needs to be passed
when the thread instantiates the filter rules late and the IP address
learn request is still pending in order to override the check for the
pending learn request. If the rules are to be updated while the thread
is active, this will not be done immediately but the thread will do that
later on.
Introduce a function to notify the IP address learning
thread to terminate and thus release the lock on the interface.
Notify the thread before grabbing the lock on the interface
and tearing down the rules. This prevents a 'virsh destroy' to
tear down the rules that the IP address learning thread has
applied.
The functions invoked by the IP address learning thread
that apply some basic filtering rules did not clean up
any previous filtering rules that may still be there
(due to a libvirt restart for example). With the
patch below all the rules are cleaned up first.
Also, I am introducing a function to drop all traffic
in case the IP address learning thread could not apply
the rules.
The local DHCP server on virtbr0 sends DHCP ACK messages when a VM is
started and requests an IP address while the initial DHCP lease on the
VM's MAC address hasn't expired. So, also pick the IP address of the VM
if that type of message is seen.
Thanks to Gerhard Stenzel for providing a test case for this.
Changes from V1 to V2:
- cleanup: replacing DHCP option numbers through constants
Stefan Berger
Stefan Berger