[libvirt] [PATCH 0/3] nwfilter: Fix some issues in DHCP Snooping code
The following 3 patches fix some issues in the DHCP Snooping code. Stefan Berger (3): nwfilter: Cap the poll timeout in the DHCP Snooping code nwfilter: Display the pcap errror message nwfilter: Increase buffer size for libpcap src/nwfilter/nwfilter_dhcpsnoop.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) -- 1.8.1.4
From: Stefan Berger <stefanb@linux.vnet.ibm.com> Cap the poll timeout in the DHCP Snooping code to a max. of 10 seconds to not hold up the libvirt shutdown longer than this. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index a96a790..de9c33b 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -265,6 +265,7 @@ struct _virNWFilterSnoopRateLimitConf { const unsigned int burstRate; const unsigned int burstInterval; }; +#define SNOOP_POLL_MAX_TIMEOUT_MS (10 * 1000) /* milliseconds */ typedef struct _virNWFilterSnoopPcapConf virNWFilterSnoopPcapConf; typedef virNWFilterSnoopPcapConf *virNWFilterSnoopPcapConfPtr; @@ -1419,6 +1420,10 @@ virNWFilterDHCPSnoopThread(void *req0) break; } + /* cap pollTo so we don't hold up the join for too long */ + if (pollTo < 0 || pollTo > SNOOP_POLL_MAX_TIMEOUT_MS) + pollTo = SNOOP_POLL_MAX_TIMEOUT_MS; + n = poll(fds, ARRAY_CARDINALITY(fds), pollTo); if (n < 0) { -- 1.8.1.4
On 03/02/2014 09:34 PM, Stefan Berger wrote:
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cap the poll timeout in the DHCP Snooping code to a max. of 10 seconds to not hold up the libvirt shutdown longer than this.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 5 +++++ 1 file changed, 5 insertions(+)
I'm assuming you've tested that the code gracefully handles a timeout failure? ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake <eblake@redhat.com> wrote on 03/03/2014 02:42:57 PM:
From: Eric Blake <eblake@redhat.com> To: Stefan Berger/Watson/IBM@IBMUS, libvir-list@redhat.com, Cc: laine@laine.org Date: 03/03/2014 02:43 PM Subject: Re: [libvirt] [PATCH 1/3] nwfilter: Cap the poll timeout in the DHCP Snooping code
On 03/02/2014 09:34 PM, Stefan Berger wrote:
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cap the poll timeout in the DHCP Snooping code to a max. of 10 seconds to not hold up the libvirt shutdown longer than this.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 5 +++++ 1 file changed, 5 insertions(+)
I'm assuming you've tested that the code gracefully handles a timeout failure?
Timeout failer = returns with return code '0' from poll(). This is gracefully handled by the code. Stefan
From: Stefan Berger <stefanb@linux.vnet.ibm.com> Display the pcap error message in the log. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index de9c33b..7cb0ac0 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1113,8 +1113,9 @@ 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, "%s", - _("setup of pcap handle failed")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("setup of pcap handle failed: %s"), + pcap_geterr(handle)); goto cleanup; } -- 1.8.1.4
On 03/02/2014 09:34 PM, Stefan Berger wrote:
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
Display the pcap error message in the log.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: Stefan Berger <stefanb@linux.vnet.ibm.com> Libpcap 1.5 requires a larger buffer than previous pcap versions. Adjust the size of the buffer to 128kb. This patch should address symptoms in BZ 1071181 and BZ 731059 Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 7cb0ac0..ab9bf09 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -250,7 +250,11 @@ struct _virNWFilterDHCPDecodeJob { # define DHCP_PKT_BURST 50 /* pkts/sec */ # define DHCP_BURST_INTERVAL_S 10 /* sec */ -# define PCAP_BUFFERSIZE (DHCP_PKT_BURST * PCAP_PBUFSIZE / 2) +/* + * libpcap 1.5 requires a 128kb buffer + * 128 kb is bigger than (DHCP_PKT_BURST * PCAP_PBUFSIZE / 2) + */ +# define PCAP_BUFFERSIZE (128 * 1024) # define MAX_QUEUED_JOBS (DHCP_PKT_BURST + 2 * DHCP_PKT_RATE) -- 1.8.1.4
On 03/02/2014 09:34 PM, Stefan Berger wrote:
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
Libpcap 1.5 requires a larger buffer than previous pcap versions. Adjust the size of the buffer to 128kb.
This patch should address symptoms in BZ 1071181 and BZ 731059
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: Eric Blake <eblake@redhat.com> To: Stefan Berger/Watson/IBM@IBMUS, libvir-list@redhat.com, Cc: laine@laine.org Date: 03/03/2014 02:44 PM Subject: Re: [libvirt] [PATCH 3/3] nwfilter: Increase buffer size for
Eric Blake <eblake@redhat.com> wrote on 03/03/2014 02:43:56 PM: libpcap
On 03/02/2014 09:34 PM, Stefan Berger wrote:
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
Libpcap 1.5 requires a larger buffer than previous pcap versions. Adjust the size of the buffer to 128kb.
This patch should address symptoms in BZ 1071181 and BZ 731059
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
ACK.
Pushed.
participants (2)
-
Eric Blake -
Stefan Berger