PERFORCE change 131478 for review

[Robert Watson]

http://perforce.freebsd.org/chv.cgi?CH=131478

Change 131478 by rwatson at rwatson_cinnamon on 2007/12/23 16:48:44

	Rework setup portion of zero-copy support for libpcap:
	
	- Test environmental variable only once when creating descriptor.
	- Rename environmental variable to BPF_ZEROCOPY to match kernel
	  option
	- If the environmental variable is present, try to set the
	  zero-copy buffer mode, and if that fails, fall back to normal
	  operation (which is what the comment suggests it does; instead
	  it was failing descriptor allocation if the setmode ioctl
	  failed).
	- When mmaping of shared buffers fails, follow the normal error
	  path.
	- Clean up the free path for zero-copy buffers.

Affected files ...

.. //depot/projects/zcopybpf/src/contrib/libpcap/pcap-bpf.c#10 edit
.. //depot/projects/zcopybpf/src/contrib/libpcap/pcap-int.h#6 edit

Differences ...

==== //depot/projects/zcopybpf/src/contrib/libpcap/pcap-bpf.c#10 (text+ko) ====

@@ -788,14 +788,10 @@
 	 * attach to, so we do that here also.
 	 */
 #ifdef BIOCSETBUFMODE
-	if (getenv("BPF_ZERO_COPY")) {
-		bufmode = BPF_BUFMODE_ZBUF;
-		if (ioctl(fd, BIOCSETBUFMODE, (caddr_t)&bufmode) < 0) {
-			snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSETBUFMODE: %s",
-			    pcap_strerror(errno));
-			goto bad;
-		}
-
+	bufmode = BPF_BUFMODE_ZBUF;
+	if (getenv("BPF_ZEROCOPY") &&
+	    ioctl(fd, BIOCSETBUFMODE, (caddr_t)&bufmode) == 0) {
+		p->zerocopy = 1;
 		if (ioctl(fd, BIOCGETZMAX, (caddr_t)&zbufmax) < 0) {
 			snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCGETZMAX: %s",
 			    pcap_strerror(errno));
@@ -806,8 +802,6 @@
 		 * XXXRW: This logic should be revisited.
 		 */
 		p->zbufsize = 32768;
-		if (p->zbufsize % getpagesize() != 0)
-			p->zbufsize = getpagesize();
 		if (p->zbufsize > zbufmax)
 			p->zbufsize = zbufmax;
 
@@ -816,12 +810,9 @@
 		p->zbuf2 = mmap(NULL, p->zbufsize, PROT_READ | PROT_WRITE,
 		    MAP_ANON, -1, 0);
 		if (p->zbuf1 == MAP_FAILED || p->zbuf2 == MAP_FAILED) {
-			if (p->zbuf1 != MAP_FAILED)
-				munmap(p->zbuf1, p->zbufsize);
-			if (p->zbuf2 != MAP_FAILED)
-				munmap(p->zbuf1, p->zbufsize);
 			snprintf(ebuf, PCAP_ERRBUF_SIZE, "mmap: %s",
 			    pcap_strerror(errno));
+			goto bad;
 		}
 
 		bzero(&bz, sizeof(bz));
@@ -1059,7 +1050,7 @@
 #endif
 	/* set timeout */
 	p->to_ms = to_ms;
-	if (to_ms != 0 && getenv("BPF_ZERO_COPY") == NULL) {
+	if (to_ms != 0 && !p->zerocopy) {
 		/*
 		 * XXX - is this seconds/nanoseconds in AIX?
 		 * (Treating it as such doesn't fix the timeout
@@ -1252,21 +1243,19 @@
 
 	(void)close(fd);
 #ifdef BIOCSETBUFMODE
-	if (p->zbuf1 != NULL)
-		munmap(p->zbuf1, v);
-	if (p->zbuf2 != NULL)
-		munmap(p->zbuf2, v);
 	/*
-	 * If we are using zerocopy, the packet buffer will be referencing
-	 * an address in one of the shared pages, if any.  In which case
-	 * we will not free it.
+	 * In zero-copy mode, p->buffer is just a pointer into one of the two
+	 * memory-mapped buffers, so no need to free it.
 	 */
-	if (getenv("BPF_ZERO_COPY") == NULL && p->buffer != NULL)
-		free(p->buffer);
-#else
+	if (p->zerocopy) {
+		if (p->zbuf1 != MAP_FAILED && p->zbuf1 != NULL)
+			munmap(p->zbuf1, p->zbufsize);
+		if (p->zbuf2 != MAP_FAILED && p->zbuf1 != NULL)
+			munmap(p->zbuf1, p->zbufsize);
+	} else
+#endif
 	if (p->buffer != NULL)
 		free(p->buffer);
-#endif
 	if (p->dlt_list != NULL)
 		free(p->dlt_list);
 	free(p);

==== //depot/projects/zcopybpf/src/contrib/libpcap/pcap-int.h#6 (text+ko) ====

@@ -189,6 +189,7 @@
 	u_char *zbuf1, *zbuf2;
 	u_int zbufsize;
 	u_int timeout;
+	u_int zerocopy;
 
 	/*
 	 * If there's currently a buffer being actively processed, then it is