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

[Daniel P. Berrangé]

On Thu, Apr 26, 2018 at 08:09:47AM +0200, Christian Ehrhardt wrote:
> On Wed, Apr 25, 2018 at 11:25 PM, Laine Stump <laine at laine.org> 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 at gmail.com> for discovering that
> > buffer size was the problem.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1547237
> > Signed-off-by: Laine Stump <laine at laine.org>
> > ---
> >  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)
> >   */
> >
> 
> I just started building with the change for a few tests on this - no
> results yet.
> 
> But we are all puzzled/unsure enough on the size that I'd already ask to
> modify the comment above to explain the new size.
> 
> Maybe we should explain:
> - why 128 isn't enough
> - why you chose "only" 256
> - why the default size might be too big
> - your size considerations for many guest scenarios
> 
> That will help the next one stumbling over this code.

FWIW, having just checked libpcap git history, the current 2 MB default
size has been there since 2008 !   I'm guessing we don't use that as we
don't want to consume lots of RAM when many guests are running.



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 :|