Re: [PATCH] nwfilter: increase pcap buffer size to be compatible with TPACKET_V3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 26, 2018 at 8:09 AM, Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> wrote:


On Wed, Apr 25, 2018 at 11:25 PM, Laine Stump <laine@xxxxxxxxx> wrote:
When an nwfilter rule sets the parameter CTRL_IP_LEARNING to "dhcp",
this turns on the "dhcpsnoop" thread, which uses libpcap to monitor
traffic on the domain's tap device and extract the IP address from the
DHCP response.

If libpcap on the host is built with TPACKET_V3 defined, the dhcpsnoop
code's initialization of the libpcap socket fails with the following
error:

  virNWFilterSnoopDHCPOpen:1134 : internal error: pcap_setfilter: can't remove kernel filter: Bad file descriptor

It turns out that this was because libpcap with TPACKET_V3 defined
requires a larger buffer size than libvirt was setting (we were
setting it to 128k). Changing the buffer size to 256k eliminates the
error, and the dhcpsnoop thread once again works properly.

Thanks to Christian Ehrhardt <paelzer@xxxxxxxxx> for discovering that
buffer size was the problem.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1547237
Signed-off-by: Laine Stump <laine@xxxxxxxxx>
---
 src/nwfilter/nwfilter_dhcpsnoop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index 6069e70460..62eb617515 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -259,7 +259,7 @@ struct _virNWFilterDHCPDecodeJob {
  * libpcap 1.5 requires a 128kb buffer
  * 128 kb is bigger than (DHCP_PKT_BURST * PCAP_PBUFSIZE / 2)
  */

Tests completed and ok for my small testing scope of these cases:
  Tested-by: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx>

Once you updated the comment as outlined before feel free to also add
  Reviewed-by: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx>

Could you when rewriting also add this line (not required, just if you amend anyway):
I recently see more and more Resolves: instead of "Fixes:" did we change the recommended format for some tools and I missed it?
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux