On Thu, Jan 30, 2020 at 03:43:06PM +0100, Erik Skultety wrote:
> Our nwfilter code doesn't set any timeout on the pcap paket buffer which
> means that when DHCP snooping is enabled on a guest interface and
> libvirt is trying to learn the IP address from guest's DHCP traffic, it
> takes up to 4x longer to ping a guest successfully compared to a case
> where nwfilter isn't enabled at all or libvirt uses the cached nwfilter
> leases to populate the corresponding rules to ebtables.
> With the pcap filter and rate limiting already in place, we should be
> able to afford enabling the immediate paket delivery, FWIW immediate
> mode was actually the default prior libpcap-1.5.0 (CentOS 6) regardless
> of whether a buffer was requested.
>
> The lack of any kind of timeout on the pcap buffer messed with the
> libvirt TCK test suite which, even with a generous timeout in place,
> timeouts every single time simply because it takes a while until
> guest actually starts producing any kind of traffic to fill up
> the buffer in place (appart from the DHCP traffic which happens fairly
> early on).
>
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
>
> An alternative I've been also looking into is to use pcap_set_timeout before
> activating the pcap handle. The question is what should an appropriate timeout
> look like in that case (e.g. I tried with 500ms), but since prior
> libpcap < 1.5.0 the capture devices were always in the immediate mode on Linux,
> I'd go down the same road again, quoting the man page:
>
> "in 1.5.0 and later, they are, by default, not in immediate mode, so if
> pcap_set_immediate_mode() is available, it should be used"
Yeah, I don't see a serious downside to immediate mode and it
was historical default, so this looks fine, especially because
it solves the annoying buffsize problem.
>
> src/nwfilter/nwfilter_dhcpsnoop.c | 24 +-----------------------
> 1 file changed, 1 insertion(+), 23 deletions(-)
>
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
> index 10567e9cd3..a1c0c0189e 100644
> --- a/src/nwfilter/nwfilter_dhcpsnoop.c
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -242,23 +242,6 @@ struct _virNWFilterDHCPDecodeJob {
> # define DHCP_PKT_BURST 50 /* pkts/sec */
> # define DHCP_BURST_INTERVAL_S 10 /* sec */
>
> -/*
> - * NB: Any libpcap built with HAVE_TPACKET3 will require
> - * PCAP_BUFFERSIZE to be at least 262144 (although
> - * pcap_set_buffer_size() with a lower value will succeed, and the
> - * error will only show up later when pcap_setfilter() is called).
> - *
> - * It is possible that in the future libpcap could increase the
> - * minimum size even further, but due to the fact that each guest
> - * using dhcp snooping keeps 2 pcap sockets open (and thus 2 buffers
> - * allocated) for the life of the guest, we want to minimize the
> - * length of the buffer, so instead of leaving it at the default size
> - * (2MB), we are setting it to the minimum viable size and including
> - * this clue in the source to help quickly resolve the problem when/if
> - * it reoccurs.
> - */
> -# define PCAP_BUFFERSIZE (256 * 1024)
> -
> # define MAX_QUEUED_JOBS (DHCP_PKT_BURST + 2 * DHCP_PKT_RATE)
>
> typedef struct _virNWFilterSnoopRateLimitConf virNWFilterSnoopRateLimitConf;
> @@ -1098,13 +1081,8 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac,
> goto cleanup_nohandle;
> }
>
> - /* IMPORTANT: If there is any failure of *any* pcap_* function
> - * during setup of the socket, look to the comment where
> - * PCAP_BUFFERSIZE is defined. It may be too small, even if the
> - * generated error doesn't imply that.
> - */
> if (pcap_set_snaplen(handle, PCAP_PBUFSIZE) < 0 ||
> - pcap_set_buffer_size(handle, PCAP_BUFFERSIZE) < 0 ||
> + pcap_set_immediate_mode(handle, 1) < 0 ||
> pcap_activate(handle) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("setup of pcap handle failed: %s"),
Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
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 :|
