From owner-svn-src-stable@freebsd.org Mon Dec 31 10:59:32 2018 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CCD60142F570; Mon, 31 Dec 2018 10:59:31 +0000 (UTC) (envelope-from vmaffione@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 6A25994329; Mon, 31 Dec 2018 10:59:31 +0000 (UTC) (envelope-from vmaffione@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 103599831; Mon, 31 Dec 2018 10:59:31 +0000 (UTC) (envelope-from vmaffione@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id wBVAxUes060531; Mon, 31 Dec 2018 10:59:30 GMT (envelope-from vmaffione@FreeBSD.org) Received: (from vmaffione@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id wBVAxUQd060530; Mon, 31 Dec 2018 10:59:30 GMT (envelope-from vmaffione@FreeBSD.org) Message-Id: <201812311059.wBVAxUQd060530@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: vmaffione set sender to vmaffione@FreeBSD.org using -f From: Vincenzo Maffione Date: Mon, 31 Dec 2018 10:59:30 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r342646 - stable/12/sys/dev/netmap X-SVN-Group: stable-12 X-SVN-Commit-Author: vmaffione X-SVN-Commit-Paths: stable/12/sys/dev/netmap X-SVN-Commit-Revision: 342646 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 6A25994329 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.97 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.98)[-0.976,0]; NEURAL_HAM_LONG(-1.00)[-0.998,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Dec 2018 10:59:32 -0000 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