We want all threads to be set as workers or to have a job assigned to
them, which can easily be achieved in virThreadCreate wrapper to
pthread_create. Let's make sure we always use the wrapper.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The 'virDomainNetType' is unused in every impl of the
virNWFilterRuleCreateInstance driver method. Remove it
from the code to avoid the dependancy on the external
enum.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virNWFilterTechDriver struct is nothing to do with the nwfilter
XML configuration. It stores data specific to the driver implementation
so should be in a header in the driver directory instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
'const fooPtr' is the same as 'foo * const' (the pointer won't
change, but it's contents can). But in general, if an interface
is trying to be const-correct, it should be using 'const foo *'
(the pointer is to data that can't be changed).
Fix up offenders in nwfilter code.
This patch does nothing about the stupidity evident in having
__virNWFilterInstantiateFilter, _virNWFilterInstantiateFilter,
and virNWFilterInstantiateFilter, which differ only by leading
underscores, and which infringes on the namespace reserved to
the implementation - that would need to be a separate cleanup.
* src/nwfilter/nwfilter_dhcpsnoop.h (virNWFilterDHCPSnoopReq): Use
intended type.
* src/nwfilter/nwfilter_gentech_driver.h
(virNWFilterInstantiateFilter)
(virNWFilterUpdateInstantiateFilter)
(virNWFilterInstantiataeFilterLate, virNWFilterTeardownFilter)
(virNWFilterCreateVarHashmap): Likewise.
* src/nwfilter/nwfilter_learnipaddr.h (virNWFilterLearnIPAddress):
Likewise.
* src/conf/nwfilter_conf.h (virNWFilterApplyBasicRules)
(virNWFilterApplyDHCPOnlyRules): Likewise.
(virNWFilterDefFormat): Make const-correct.
* src/conf/nwfilter_params.h (virNWFilterVarValueCopy)
(virNWFilterVarValueGetSimple, virNWFilterVarValueGetCardinality)
(virNWFilterVarValueEqual, virNWFilterVarAccessEqual)
(virNWFilterVarAccessGetVarName, virNWFilterVarAccessGetType)
(virNWFilterVarAccessGetIterId, virNWFilterVarAccessGetIndex)
(virNWFilterVarAccessIsAvailable)
(virNWFilterVarCombIterGetVarValue): Use intended type.
(virNWFilterVarValueGetNthValue): Make const-correct.
* src/nwfilter/nwfilter_dhcpsnoop.c (virNWFilterSnoopReqLeaseDel)
(virNWFilterSnoopIFKeyFMT, virNWFilterDHCPSnoopReq)
(virNWFilterSnoopPruneIter, virNWFilterSnoopRemAllReqIter)
(virNWFilterDHCPSnoopReq): Fix fallout.
* src/nwfilter/nwfilter_gentech_driver.c
(virNWFilterVarHashmapAddStdValues, virNWFilterCreateVarHashmap)
(virNWFilterInstantiate, __virNWFilterInstantiateFilter)
(_virNWFilterInstantiateFilter, virNWFilterInstantiateFilterLate)
(virNWFilterInstantiateFilter)
(virNWFilterUpdateInstantiateFilter)
(virNWFilterRollbackUpdateFilter, virNWFilterTeardownFilter):
Likewise.
* src/nwfilter/nwfilter_learnipaddr.c (virNWFilterLearnIPAddress):
Likewise.
* src/conf/nwfilter_params.c (virNWFilterVarValueCopy)
(virNWFilterVarValueGetSimple)
(virNWFilterVarValueGetCardinality, virNWFilterVarValueEqual)
(virNWFilterVarCombIterAddVariable)
(virNWFilterVarCombIterGetVarValue, virNWFilterVarValueCompare)
(virNWFilterFormatParamAttributes, virNWFilterVarAccessEqual)
(virNWFilterVarAccessGetVarName, virNWFilterVarAccessGetType)
(virNWFilterVarAccessGetIterId, virNWFilterVarAccessGetIndex)
(virNWFilterVarAccessGetIntIterId)
(virNWFilterVarAccessIsAvailable)
(virNWFilterVarValueGetNthValue): Likewise.
* src/nwfilter/nwfilter_ebiptables_driver.c (ebtablesApplyBasicRules)
(ebtablesApplyDHCPOnlyRules, ebiptablesRuleOrderSort)
(ebiptablesRuleOrderSortPtr): Likewise.
* src/conf/nwfilter_conf.c (virNWFilterDefEqual)
(virNWFilterDefFormat): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.
* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/; If/. If/
FreeBSD and OpenBSD have a <net/if.h> that is not self-contained;
and mingw lacks the header altogether. But gnulib has just taken
care of that for us, so we might as well simplify our code. In
the process, I got a syntax-check failure if we don't also take
the gnulib execinfo module.
* .gnulib: Update to latest, for execinfo and net_if.
* bootstrap.conf (gnulib_modules): Add execinfo and net_if modules.
* configure.ac: Let gnulib check for headers. Simplify check for
'struct ifreq', while also including enough prereq headers.
* src/internal.h (IF_NAMESIZE): Drop, now that gnulib guarantees it.
* src/nwfilter/nwfilter_learnipaddr.h: Use correct header for
IF_NAMESIZE.
* src/util/virnetdev.c (includes): Assume <net/if.h> exists.
* src/util/virnetdevbridge.c (includes): Likewise.
* src/util/virnetdevtap.c (includes): Likewise.
* src/util/logging.c (includes): Assume <execinfo.h> exists.
(virLogStackTraceToFd): Handle gnulib's fallback implementation.
Per the FSF address could be changed from time to time, and GNU
recommends the following now: (http://www.gnu.org/licenses/gpl-howto.html)
You should have received a copy of the GNU General Public License
along with Foobar. If not, see <http://www.gnu.org/licenses/>.
This patch removes the explicit FSF address, and uses above instead
(of course, with inserting 'Lesser' before 'General').
Except a bunch of files for security driver, all others are changed
automatically, the copyright for securify files are not complete,
that's why to do it manually:
src/security/security_selinux.h
src/security/security_driver.h
src/security/security_selinux.c
src/security/security_apparmor.h
src/security/security_apparmor.c
src/security/security_driver.c
Introduce new members in the virMacAddr 'class'
- virMacAddrSet: set virMacAddr from a virMacAddr
- virMacAddrSetRaw: setting virMacAddr from raw 6 byte MAC address buffer
- virMacAddrGetRaw: writing virMacAddr into raw 6 byte MAC address buffer
- virMacAddrCmp: comparing two virMacAddr
- virMacAddrCmpRaw: comparing a virMacAddr with a raw 6 byte MAC address buffer
then replace raw MAC addresses by replacing
- 'unsigned char *' with virMacAddrPtr
- 'unsigned char ... [VIR_MAC_BUFLEN]' with virMacAddr
and introduce usage of above functions where necessary.
The goal of this patch is to prepare for support for multiple IP
addresses per interface in the DHCP snooping code.
Move the code for the IP address map that maps interface names to
IP addresses into their own file. Rename the functions on the way
but otherwise leave the code as-is. Initialize this new layer
separately before dependent layers (iplearning, dhcpsnooping)
and shut it down after them.
In preparation of DHCP Snooping and the detection of multiple IP
addresses per interface:
The hash table that is used to collect the detected IP address of an
interface can so far only handle one IP address per interface. With
this patch we extend this to allow it to handle a list of IP addresses.
Above changes the returned variable type of virNWFilterGetIpAddrForIfname()
from char * to virNWFilterVarValuePtr; adapt all existing functions calling
this function.
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.
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.
- using INT_BUFSIZE_BOUND() to determine the length of the buffersize
for printing and integer into
- not explicitly initializing static var threadsTerminate to false
anymore, since that's done automatically
Changes after V2:
- removed while looks in case of OOM error
- removed on ifaceDown() call
- preceding one ifaceDown() call with an ifaceCheck() call
Since the name of an interface can be the same between stops and starts
of different VMs I have to switch the IP address learning thread to use
the index of the interface to determine whether an interface is still
available or not - in the case of macvtap the thread needs to listen for
traffic on the physical interface, thus having to time out periodically
to check whether the VM's macvtap device is still there as an indication
that the VM is still alive. Previously the following sequence of 2 VMs
with macvtap device
virsh start testvm1; virsh destroy testvm1 ; virsh start testvm2
would not terminate the thread upon testvm1's destroy since the name of
the interface on the host could be the same (i.e, macvtap0) on testvm1
and testvm2, thus it was easily race-able. The thread would then
determine the IP address parameter for testvm2 but apply the rule set
for testvm1. :-(
I am also introducing a lock for the interface (by name) that the thread
must hold while it listens for the traffic and releases when it
terminates upon VM termination or 0.5 second thereafter. Thus, the new
thread for a newly started VM with the same interface name will not
start while the old one still holds the lock. The only other code that I
see that also needs to grab the lock to serialize operation is the one
that tears down the firewall that were established on behalf of an
interface.
I am moving the code applying the 'basic' firewall rules during the IP
address learning phase inside the thread but won't start the thread
unless it is ensured that the firewall driver has the ability to apply
the 'basic' firewall rules.
I am moving some of the eb/iptables related functions into the interface
of the firewall driver and am making them only accessible via the driver's
interface. Otherwise exsiting code is adapted where needed. I am adding one
new function to the interface that checks whether the 'basic' rules can be
applied, which will then be used by a subsequent patch.
This patch implements support for learning a VM's IP address. It uses
the pcap library to listen on the VM's backend network interface (tap)
or the physical ethernet device (macvtap) and tries to capture packets
with source or destination MAC address of the VM and learn from DHCP
Offers, ARP traffic, or first-sent IPv4 packet what the IP address of
the VM's interface is. This then allows to instantiate the network
traffic filtering rules without the user having to provide the IP
parameter somewhere in the filter description or in the interface
description as a parameter. This only supports to detect the parameter
IP, which is for the assumed single IPv4 address of a VM. There is not
support for interfaces that may have multiple IP addresses (IP
aliasing) or IPv6 that may then require more than one valid IP address
to be detected. A VM can have multiple independent interfaces that each
uses a different IP address and in that case it will be attempted to
detect each one of the address independently.
So, when for example an interface description in the domain XML has
looked like this up to now:
<interface type='bridge'>
<source bridge='mybridge'/>
<model type='virtio'/>
<filterref filter='clean-traffic'>
<parameter name='IP' value='10.2.3.4'/>
</filterref>
</interface>
you may omit the IP parameter:
<interface type='bridge'>
<source bridge='mybridge'/>
<model type='virtio'/>
<filterref filter='clean-traffic'/>
</interface>
Internally I am walking the 'tree' of a VM's referenced network filters
and determine with the given variables which variables are missing. Now,
the above IP parameter may be missing and this causes a libvirt-internal
thread to be started that uses the pcap library's API to listen to the
backend interface (in case of macvtap to the physical interface) in an
attempt to determine the missing IP parameter. If the backend interface
disappears the thread terminates assuming the VM was brought down. In
case of a macvtap device a timeout is being used to wait for packets
from the given VM (filtering by VM's interface MAC address). If the VM's
macvtap device disappeared the thread also terminates. In all other
cases it tries to determine the IP address of the VM and will then apply
the rules late on the given interface, which would have happened
immediately if the IP parameter had been explicitly given. In case an
error happens while the firewall rules are applied, the VM's backend
interface is 'down'ed preventing it to communicate. Reasons for failure
for applying the network firewall rules may that an ebtables/iptables
command failes or OOM errors. Essentially the same failure reasons may
occur as when the firewall rules are applied immediately on VM start,
except that due to the late application of the filtering rules the VM
now is already running and cannot be hindered anymore from starting.
Bringing down the whole VM would probably be considered too drastic.
While a VM's IP address is attempted to be determined only limited
updates to network filters are allowed. In particular it is prevented
that filters are modified in such a way that they would introduce new
variables.
A caveat: The algorithm does not know which one is the appropriate IP
address of a VM. If the VM spoofs an IP address in its first ARP traffic
or IPv4 packets its filtering rules will be instantiated for this IP
address, thus 'locking' it to the found IP address. So, it's still
'safer' to explicitly provide the IP address of a VM's interface in the
filter description if it is known beforehand.
* configure.ac: detect libpcap
* libvirt.spec.in: require libpcap[-devel] if qemu is built
* src/internal.h: add the new ATTRIBUTE_PACKED define
* src/Makefile.am src/libvirt_private.syms: add the new modules and symbols
* src/nwfilter/nwfilter_learnipaddr.[ch]: new module being added
* src/nwfilter/nwfilter_driver.c src/conf/nwfilter_conf.[ch]
src/nwfilter/nwfilter_ebiptables_driver.[ch]
src/nwfilter/nwfilter_gentech_driver.[ch]: plu the new functionality in
* tests/nwfilterxml2xmltest: extend testing