svn commit: r342646 - stable/12/sys/dev/netmap

[Vincenzo Maffione]

Author: vmaffione
Date: Mon Dec 31 10:59:30 2018
New Revision: 342646
URL: https://svnweb.freebsd.org/changeset/base/342646

Log:
  MFC r342368, r342369
  
  netmap: fix bug in netmap_poll() optimization
  
  The bug was introduced by r339639, although it is present in the upstream
  netmap code since 2015. It is due to resetting the want_rx variable to
  POLLIN, rather than resetting it to POLLIN|POLLRDNORM.
  It only affects select(), which uses POLLRDNORM. poll() is not affected,
  because it uses POLLIN.
  Also, it only affects FreeBSD, because Linux skips the optimization
  implemented by the piece of code where the bug occurs.
  To check if txsync can be skipped, it is necessary to look for
  unseen TX space. However, this means comparing ring->cur
  against ring->tail, rather than ring->head against ring->tail
  (like nm_ring_empty() does).
  
  Sponsored by:	Sunny Valley Networks

Modified:
  stable/12/sys/dev/netmap/netmap.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/dev/netmap/netmap.c
==============================================================================
--- stable/12/sys/dev/netmap/netmap.c	Mon Dec 31 10:17:42 2018	(r342645)
+++ stable/12/sys/dev/netmap/netmap.c	Mon Dec 31 10:59:30 2018	(r342646)
@@ -3292,28 +3292,38 @@ netmap_poll(struct netmap_priv_d *priv, int events, NM
 	 * that we must call nm_os_selrecord() unconditionally.
 	 */
 	if (want_tx) {
-		enum txrx t = NR_TX;
-		for (i = priv->np_qfirst[t]; want[t] && i < priv->np_qlast[t]; i++) {
+		const enum txrx t = NR_TX;
+		for (i = priv->np_qfirst[t]; i < priv->np_qlast[t]; i++) {
 			kring = NMR(na, t)[i];
-			/* XXX compare ring->cur and kring->tail */
-			if (!nm_ring_empty(kring->ring)) {
+			if (kring->ring->cur != kring->ring->tail) {
+				/* Some unseen TX space is available, so what
+				 * we don't need to run txsync. */
 				revents |= want[t];
-				want[t] = 0;	/* also breaks the loop */
+				want[t] = 0;
+				break;
 			}
 		}
 	}
 	if (want_rx) {
-		enum txrx t = NR_RX;
-		want_rx = 0; /* look for a reason to run the handlers */
+		const enum txrx t = NR_RX;
+		int rxsync_needed = 0;
+
 		for (i = priv->np_qfirst[t]; i < priv->np_qlast[t]; i++) {
 			kring = NMR(na, t)[i];
-			if (kring->ring->cur == kring->ring->tail /* try fetch new buffers */
-			    || kring->rhead != kring->ring->head /* release buffers */) {
-				want_rx = 1;
+			if (kring->ring->cur == kring->ring->tail
+				|| kring->rhead != kring->ring->head) {
+				/* There are no unseen packets on this ring,
+				 * or there are some buffers to be returned
+				 * to the netmap port. We therefore go ahead
+				 * and run rxsync. */
+				rxsync_needed = 1;
+				break;
 			}
 		}
-		if (!want_rx)
-			revents |= events & (POLLIN | POLLRDNORM); /* we have data */
+		if (!rxsync_needed) {
+			revents |= want_rx;
+			want_rx = 0;
+		}
 	}
 #endif