Bug 1547237 - TPACKET_V3 is enabled, causing failure of libvirt's use of libpcap
Summary: TPACKET_V3 is enabled, causing failure of libvirt's use of libpcap
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: libvirt
Version: 27
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Libvirt Maintainers
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-02-20 20:03 UTC by Laine Stump
Modified: 2018-08-01 17:54 UTC (History)
15 users (show)

Fixed In Version: libvirt-3.7.0-6.fc27
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1556935 (view as bug list)
Environment:
Last Closed: 2018-08-01 17:54:31 UTC
Type: Bug


Attachments (Terms of Use)
Libpcap patch that logs debug information and bypasses the problem (1.53 KB, patch)
2018-03-27 10:27 UTC, Martin Sehnoutka
no flags Details | Diff

Description Laine Stump 2018-02-20 20:03:52 UTC
Starting with F27, the Fedora-only patch that disabled TPACKET_V3 support was removed with the comment:

   Drop TPACKET_V3 patch as it should be fixed in kernel by now


This is apparently not the case since, on a host with F27 packages, including libpcap-1.8.1-6 and kernel 4.15.3-300, the previously-functional libvirt code that uses libpcap to watch for DHCP traffic now fails when pcap_setfilter() returns EBADF.

Here is the excerpt of libvirt code (from the file src/nwfilter/nwfilter_dhcpsnoop.c):

    [...]
    handle = pcap_create(ifname, pcap_errbuf);

    if (handle == NULL) {
        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                       _("pcap_create failed"));
        goto cleanup_nohandle;
    }

    if (pcap_set_snaplen(handle, PCAP_PBUFSIZE) < 0 ||
        pcap_set_buffer_size(handle, PCAP_BUFFERSIZE) < 0 ||
        pcap_activate(handle) < 0) {
        virReportError(VIR_ERR_INTERNAL_ERROR,
                       _("setup of pcap handle failed: %s"),
                       pcap_geterr(handle));
        goto cleanup;
    }

    if (pcap_compile(handle, &fp, ext_filter, 1, PCAP_NETMASK_UNKNOWN) != 0) {
        virReportError(VIR_ERR_INTERNAL_ERROR,
                       _("pcap_compile: %s"), pcap_geterr(handle));
        goto cleanup;
    }

    if (pcap_setfilter(handle, &fp) != 0) {  <=== FAILURE HERE
        virReportError(VIR_ERR_INTERNAL_ERROR,
                       _("pcap_setfilter: %s"), pcap_geterr(handle));
        goto cleanup_freecode;
    }
    [...]

If I add the patch titled "pcap-linux: don't use TPACKETV3 for memory mmapped
 capture" back to the F27 build of libpcap (built locally with fedpkg) and install the resulting rpm, the same code magically begins to work.

I haven't checked rawhide, but I assume the behavior is the same.

I think Fedora needs to re-disable TPACKET_V3

Comment 1 Martin Sehnoutka 2018-02-22 13:27:19 UTC
TPACKET_V3 was enable because it is a vital feature for network packet analyzers. I don't want to disable it again, because it will trigger other bugs (usually it is about packet drops).

Comment 2 Laine Stump 2018-02-22 14:24:58 UTC
Can you recommend how we should modify the libvirt code that uses pcap to avoid this regression in pcap's behavior? (while maintaining compatibility with other/older distros that haven't enabled TPACKET_V3)

Comment 3 Michal Ruprich 🐧 2018-02-26 11:33:43 UTC
Switching the needinfo to the assignee(myself).

Comment 5 Martin Sehnoutka 2018-03-22 08:36:38 UTC
Could you please provide us with some steps to reproduce? Something about how to compile/run the code in order to trigger this bug, so that we can properly examine it.

Comment 6 Laine Stump 2018-03-26 14:09:49 UTC
If you have a system with libvirt and virt-manager installed, you can just create a virtual machine, then edit its configuration with "virsh edit" to att the following "<filterref>" section to its <interface>, then attempt to start the virtual machine:

    <interface type='network'>
      <source network='default'/>
      <filterref filter='clean-traffic'>
        <parameter name='CTRL_IP_LEARNING' value='dhcp'/>
        <parameter name='DHCPSERVER' value='192.168.122.1'/>
      </filterref>
    </interface>

If you've installed the debuginfo for libvirt, you can set a breakpoint on the function virNWFilterSNoopDHCPOpen() (which is the source of the bit of code I posted above).

I can provide more detailed step-by-step instructions if you need it, but didn't want to pollute the BZ with a long treatise on installing virt stuff if you already had it on.

If you need any assistance while doing this, you can find me (and several other libvirt developers) on IRC in the #virt channel on irc.oftc.net.

Comment 7 Martin Sehnoutka 2018-03-27 10:27:09 UTC
Created attachment 1413663 [details]
Libpcap patch that logs debug information and bypasses the problem

Thanks for the reproducer. Just for the record, it is necessary to install libvirt-daemon-config-nwfilter package before this starts to work. Then you can attach to running libvirtd using gdb, set the breakpoint and try to start the virtual machine.

The real name of the function is: virNWFilterSnoopDHCPOpen , it took me a while to notice the capital N in SNoop :)

The bug itself is triggered in the function:
pcap_setfilter_linux ->
pcap_setfilter_linux_common ->
reset_kernel_filter

where setsockopt is called with handle->fd as an argument, which is -1 in this case. I am not sure yet, where the -1 came from in this file descriptor, but surrounding this function with a condition bypasses the problem and my VM starts just fine. Though I don't know if the libvirt filter works, because I don't know what it is supposed to do. I attach a patch for libpcap, that can be used for debugging. It is not meant as a fix, rather for other maintainers to try it.

Comment 8 Laine Stump 2018-03-27 11:09:09 UTC
(In reply to Martin Sehnoutka from comment #7)
> Created attachment 1413663 [details]
> Libpcap patch that logs debug information and bypasses the problem
> 
> Thanks for the reproducer. Just for the record, it is necessary to install
> libvirt-daemon-config-nwfilter package before this starts to work.

Interesting. That should have been installed as a dependency. I'll look into why it's missing. Was your test platform F28, or something else?

> 
> The real name of the function is: virNWFilterSnoopDHCPOpen , it took me a
> while to notice the capital N in SNoop :)

Oops! Sorry about that - that will teach me to type instead of paste :-)

> I attach a
> patch for libpcap, that can be used for debugging. It is not meant as a fix,
> rather for other maintainers to try it.

For the sake of gathering information, I'll build with this patch applied and try it to see if libvirt's functionality is still there (what's supposed to happen is that libvirt will monitor all DHCP traffic, and grab the IP address of the guest out of the DHCP response, then use that IP to build additional iptables rules).

Comment 9 Martin Sehnoutka 2018-03-27 12:04:41 UTC
(In reply to Laine Stump from comment #8)
> Was your test platform F28, or something else?
> 

Fedora 27 Workstation

Comment 10 Christian Ehrhardt 2018-04-25 08:06:30 UTC
Hi,
I wanted to let you know that this problem is not only affecting Fedora.
I happened to debug [1] leading the same conclusions on the fd being -1 breaking the latter setsockopt.

I asked on #virt and fortunately danpb was aware of this bug and pointed me here.
Ubuntu and Debian have no disabled TPACKET_V3 (never had) so it seems about all releases I could test are affected by this bug.

Glad I found this bug to participate here and to be able to track your further insights as well.

[1]: https://bugs.launchpad.net/libvirt/+bug/1758037

Comment 11 Christian Ehrhardt 2018-04-25 10:04:35 UTC
I realized that the path without:
  <parameter name='CTRL_IP_LEARNING' value='dhcp'/>
works but is totally different.
It uses pcap_open_live

So I compared our code in virNWFilterSnoopDHCPOpen with pcap_open_live.
We use different buffer sizes and don't set promisc and timeout.
But the minimal change I found that seems to make it work was:

--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -1115,7 +1115,6 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac,
     }

     if (pcap_set_snaplen(handle, PCAP_PBUFSIZE) < 0 ||
- pcap_set_buffer_size(handle, PCAP_BUFFERSIZE) < 0 ||
         pcap_activate(handle) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("setup of pcap handle failed: %s"),


The size for this was defined as:
  /*
   * libpcap 1.5 requires a 128kb buffer
   * 128 kb is bigger than (DHCP_PKT_BURST * PCAP_PBUFSIZE / 2)
   */
  # define PCAP_BUFFERSIZE (128 * 1024)

This is from [1], does all that from a libpcap experts POV make sense?
Would it be reasonable to drop this call these days or change the size?

[1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=49b59a151f60b0a178b023b727bac30f80bd6000

Comment 12 Daniel Berrangé 2018-04-25 15:29:17 UTC
Looking at source the default size if you don't call pcap_set_buffer_size is *massively* larger than the PCAP_BUFFERSIZE constant libvirt has defined:

$ grep -r  'buffer_size =' *
pcap.c:	p->opt.buffer_size = 0;		/* use the platform's default */
pcap.c:	p->opt.buffer_size = buffer_size;
pcap-linux.c:	if (handle->opt.buffer_size == 0) {
pcap-linux.c:		handle->opt.buffer_size = 2*1024*1024;
pcap-win32.c:	 	if (p->opt.buffer_size == 0)
pcap-win32.c:	 		p->opt.buffer_size = WIN32_DEFAULT_KERNEL_BUFFER_SIZE;

$ grep -r WIN32_DEFAULT_KERNEL_BUFFER_SIZE *
pcap-win32.c:#define	WIN32_DEFAULT_KERNEL_BUFFER_SIZE 1000000
pcap-win32.c:		 * WIN32_DEFAULT_KERNEL_BUFFER_SIZE.
pcap-win32.c:	 		p->opt.buffer_size = WIN32_DEFAULT_KERNEL_BUFFER_SIZE;


So libpcap uses  2 MB on Linux, or 1 MB on Windows.

We need to consider what maximum oldest libpcap we support to support is, to decide whether we can safely drop the pcap_Set_buffer_size call, vs increase its value.

Comment 13 Laine Stump 2018-04-25 21:30:10 UTC
I just tried setting the buffer size to 256k instead of 128k, and everything works. I posted a patch:

 https://www.redhat.com/archives/libvir-list/2018-April/msg02459.html

This should be safer than not setting the buffer size when using older libpcap versions that might have a very small default buffer size, and should place too much of a burden on hosts that use a new libpcap (even 1000 guests with CTRL_IP_LEARNING turned on would only use 128MB more memory than they do now).

Does this seem reasonable?

Comment 14 Laine Stump 2018-04-27 21:58:13 UTC
After discussion on-list, we decided that although we may need to increase the buffer size again sometime in the future, due to memory usage concerns it is still best to just set the buffer size to 256k, and include a lot of verbiage explaining the choice (and pointing out the probable source of failure if it happens again) in the code and in the git log. (See the follow-up comments to the V12 patch listed above).


To that end, I just pushed this patch upstream:

https://www.redhat.com/archives/libvir-list/2018-April/msg02635.html

commit ce5aebeacd10a1c15cb3ee46a59c8b5ff235589e
Author: Laine Stump <laine@laine.org>
Date:   Wed Apr 25 17:12:03 2018 -0400

    nwfilter: increase pcap buffer size to be compatible with TPACKET_V3
    
I also pushed that to the v3.7-maint branch so that it will automatically go into the next maintenance release of libvirt 3.7 (used by Fedora 27), and would have done the same for v4.1-maint (which will be used by F28) but that branch hasn't been created yet.

Comment 15 Laine Stump 2018-04-27 22:00:11 UTC
(forgot to switch the component to libvirt)

Comment 16 Laine Stump 2018-06-21 14:36:16 UTC
Because it isn't clear from Comment 14, I wanted to point out that the patch referenced there is not present upstream until libvirt-4.3.0. I had pushed it to the v4.1-maint branch at the end of April so that it will be included in an F28 build, but the Fedora builds have lately been based on the upstream mainline releases rather than the maintenance branches, so this bug is still present in F28 (and also F27)

Comment 17 Fedora Update System 2018-07-03 17:18:12 UTC
libvirt-3.7.0-6.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-2b053454a4

Comment 18 Fedora Update System 2018-07-04 16:22:24 UTC
libvirt-3.7.0-6.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-2b053454a4

Comment 19 Fedora Update System 2018-08-01 17:54:31 UTC
libvirt-3.7.0-6.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.