[libvirt] [tck PATCH 0/2] Fix no-mac-broadcast test

It's surprising that nobody ever noticed in the past that this test was either failing, or wasn't testing what it intended to test. An extra bonus to fixing the test is that we are now testing virNWFilterDefineXML(), and while writing it I discovered that define_nwfilter() has been missing from the perl bindings since the very beginning (libvirt-0.8.0, nearly 7 years ago!). (Since nobody complained before now, I guess anyone using nwfilter via perl was only using pre-define filter rules). Laine Stump (2): cleanup all nwfilters beginning with ^tck Fix no-mac-broadcast test lib/Sys/Virt/TCK.pm | 16 ++++++++++++++++ scripts/nwfilter/230-no-mac-broadcast.t | 29 ++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) -- 2.14.3

Just as we do with domains, network, etc, do a pre-test check for any existing nwfilters that start with "tck" (the test will be aborted in that case unless "--force" is added to the commandline), and remove same during the cleanup at the end. Signed-off-by: Laine Stump <laine@laine.org> --- lib/Sys/Virt/TCK.pm | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index f9d9f30..e7ff71b 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -130,6 +130,11 @@ sub sanity_check { die "there is/are " . int(@nets) . " pre-existing inactive network(s) in this driver"; } + my @nwfilters = grep { $_->get_name =~ /^tck/ } $conn->list_nwfilters; + if (@nwfilters) { + die "there is/are " . int(@nwfilters) . " pre-existing nwfilter(s) in this driver"; + } + my @pools = grep { $_->get_name =~ /^tck/ } $conn->list_storage_pools; if (@pools) { die "there is/are " . int(@pools) . " pre-existing active storage_pool(s) in this driver"; @@ -188,6 +193,16 @@ sub reset_networks { } } +sub reset_nwfilters { + my $self = shift; + my $conn = shift; + + my @nwfilters = grep { $_->get_name =~ /^tck/ } $conn->list_nwfilters; + foreach my $nwfilter (@nwfilters) { + $nwfilter->undefine; + } +} + sub reset_storage_pools { my $self = shift; my $conn = shift; @@ -217,6 +232,7 @@ sub reset { $self->reset_domains($conn); $self->reset_networks($conn); + $self->reset_nwfilters($conn); $self->reset_storage_pools($conn); } -- 2.14.3

On Tue, Mar 06, 2018 at 12:56:46PM -0500, Laine Stump wrote:
Just as we do with domains, network, etc, do a pre-test check for any existing nwfilters that start with "tck" (the test will be aborted in that case unless "--force" is added to the commandline), and remove same during the cleanup at the end.
Signed-off-by: Laine Stump <laine@laine.org> --- lib/Sys/Virt/TCK.pm | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

This test is supposed to test that the no-mac-broadcast nwfilter properly blocks all outgoing traffic with the MAC broadcast address as its destination. When the no-mac-broadcast filter is used by itself, though, it blocks even DHCP and ARP requests, meaning that the network connection to the guest isn't even enough to allow the test script to ssh in to do its work. This patch solves the problem by temporarily creating a new nwfilter that precedes the no-mac-broadcast rule with clean-traffic (which will allow dhcp requests and responses) and allow-arp (as the name states). This gives us enough network connection to get into the guest, attempt a broadcast ping, and see that it fails. (I'm not sure how this test ever reported success in the past. If it did, it was only because something else was broken). Signed-off-by: Laine Stump <laine@laine.org> --- scripts/nwfilter/230-no-mac-broadcast.t | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/scripts/nwfilter/230-no-mac-broadcast.t b/scripts/nwfilter/230-no-mac-broadcast.t index 08695ae..ee2d43f 100644 --- a/scripts/nwfilter/230-no-mac-broadcast.t +++ b/scripts/nwfilter/230-no-mac-broadcast.t @@ -34,6 +34,7 @@ use Test::Exception; use Net::OpenSSH; use File::Spec::Functions qw(catfile catdir rootdir); +my $nwfilter; my $tck = Sys::Virt::TCK->new(); my $conn = eval { $tck->setup(); }; BAIL_OUT "failed to setup test harness: $@" if $@; @@ -42,13 +43,39 @@ END { } my $networkip = get_network_ip($conn, "default"); +my $networkipaddr = $networkip->addr(); my $networkipbroadcast = $networkip->broadcast()->addr(); diag "network ip is $networkip, broadcast address is $networkipbroadcast"; +# we are testing the no-mac-broadcast filter, but that filter by +# itself makes for a completely unusable network connection. In order +# to have enough networking to properly run the test, we need to allow +# dhcp and arp broadcast traffic, which is done via the clean-traffic +# and allow-arp filters; the no-mac-broadcast filter then forbids any +# other packets with the broadcast address for destination. +# +my $nwfilter_xml = <<EOF; +<filter name='tck-test-broadcast'> + <filterref filter='clean-traffic'/> + <filterref filter='allow-arp'/> + <filterref filter='no-mac-broadcast'/> +</filter> +EOF + +# define_nwfilter() was missing from perl bindings until libvirt 4.2.0, +# so we go in the back door when it's not there. +$nwfilter = $conn->can("define_nwfilter") + ? $conn->define_nwfilter($nwfilter_xml) + : Sys::Virt::NWFilter->_new(connection => $conn, xml => $nwfilter_xml); + # create first domain and start it my $xml = $tck->generic_domain(name => "tck", fullos => 1, netmode => "network", - filterref => "no-mac-broadcast")->as_xml(); + filterref => "tck-test-broadcast", + filterparams => { + CTRL_IP_LEARNING => "dhcp", + DHCPSERVER => $networkipaddr + })->as_xml(); my $dom; ok_domain(sub { $dom = $conn->define_domain($xml) }, "created persistent domain object"); -- 2.14.3

On Tue, Mar 06, 2018 at 12:56:47PM -0500, Laine Stump wrote:
This test is supposed to test that the no-mac-broadcast nwfilter properly blocks all outgoing traffic with the MAC broadcast address as its destination. When the no-mac-broadcast filter is used by itself, though, it blocks even DHCP and ARP requests, meaning that the network connection to the guest isn't even enough to allow the test script to ssh in to do its work.
This patch solves the problem by temporarily creating a new nwfilter that precedes the no-mac-broadcast rule with clean-traffic (which will allow dhcp requests and responses) and allow-arp (as the name states). This gives us enough network connection to get into the guest, attempt a broadcast ping, and see that it fails.
(I'm not sure how this test ever reported success in the past. If it did, it was only because something else was broken).
Signed-off-by: Laine Stump <laine@laine.org> --- scripts/nwfilter/230-no-mac-broadcast.t | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé
-
Laine Stump