From owner-freebsd-net@freebsd.org Sat Jul 18 17:52:59 2020 Return-Path: Delivered-To: freebsd-net@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id A093A36FBA9 for ; Sat, 18 Jul 2020 17:52:59 +0000 (UTC) (envelope-from jmg@gold.funkthat.com) Received: from gold.funkthat.com (gate2.funkthat.com [208.87.223.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "gate2.funkthat.com", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4B8FvZ3tNNz4YLg for ; Sat, 18 Jul 2020 17:52:57 +0000 (UTC) (envelope-from jmg@gold.funkthat.com) Received: from gold.funkthat.com (localhost [127.0.0.1]) by gold.funkthat.com (8.15.2/8.15.2) with ESMTPS id 06IHqial033947 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sat, 18 Jul 2020 10:52:44 -0700 (PDT) (envelope-from jmg@gold.funkthat.com) Received: (from jmg@localhost) by gold.funkthat.com (8.15.2/8.15.2/Submit) id 06IHqhIv033946; Sat, 18 Jul 2020 10:52:43 -0700 (PDT) (envelope-from jmg) Date: Sat, 18 Jul 2020 10:52:43 -0700 From: John-Mark Gurney To: Marko Zec Cc: freebsd-net@FreeBSD.org Subject: Re: IF_DRV_PREPEND unlocked? Message-ID: <20200718175243.GZ4213@funkthat.com> Mail-Followup-To: Marko Zec , freebsd-net@FreeBSD.org References: <20200715232624.GR4213@funkthat.com> <20200716072622.5fa35ba2@x23> <20200716074917.04445daa@x23> <20200716185629.GT4213@funkthat.com> <20200717120311.59377e0d@x23> <20200717185609.GX4213@funkthat.com> <20200717235438.1fee733b@x23> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200717235438.1fee733b@x23> X-Operating-System: FreeBSD 11.3-STABLE amd64 X-PGP-Fingerprint: D87A 235F FB71 1F3F 55B7 ED9B D5FF 5A51 C0AC 3D65 X-Files: The truth is out there X-URL: https://www.funkthat.com/ X-Resume: https://www.funkthat.com/~jmg/resume.html X-TipJar: bitcoin:13Qmb6AeTgQecazTWph4XasEsP7nGRbAPE X-to-the-FBI-CIA-and-NSA: HI! HOW YA DOIN? can i haz chizburger? User-Agent: Mutt/1.6.1 (2016-04-27) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (gold.funkthat.com [127.0.0.1]); Sat, 18 Jul 2020 10:52:44 -0700 (PDT) X-Rspamd-Queue-Id: 4B8FvZ3tNNz4YLg X-Spamd-Bar: +++ Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=none (mx1.freebsd.org: domain of jmg@gold.funkthat.com has no SPF policy when checking 208.87.223.18) smtp.mailfrom=jmg@gold.funkthat.com X-Spamd-Result: default: False [3.23 / 15.00]; RCVD_TLS_ALL(0.00)[]; ARC_NA(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; NEURAL_SPAM_SHORT(0.41)[0.414]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[funkthat.com]; AUTH_NA(1.00)[]; NEURAL_SPAM_MEDIUM(0.57)[0.568]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; NEURAL_SPAM_LONG(0.05)[0.045]; R_SPF_NA(0.00)[no SPF record]; FORGED_SENDER(0.30)[jmg@funkthat.com,jmg@gold.funkthat.com]; SUBJECT_ENDS_QUESTION(1.00)[]; R_DKIM_NA(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; MIME_TRACE(0.00)[0:+]; FROM_NEQ_ENVFROM(0.00)[jmg@funkthat.com,jmg@gold.funkthat.com]; ASN(0.00)[asn:32354, ipnet:208.87.216.0/21, country:US] X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 Jul 2020 17:52:59 -0000 Marko Zec wrote this message on Fri, Jul 17, 2020 at 23:54 +0200: > On Fri, 17 Jul 2020 11:56:09 -0700 > John-Mark Gurney wrote: > > Marko Zec wrote this message on Fri, Jul 17, 2020 at 12:03 +0200: > ... > > > #define IFQ_DRV_IS_EMPTY(ifq) \ > > > (((ifq)->ifq_drv_len == 0) && ((ifq)->ifq_len == 0)) > > > > > > So, if per altq(9) the contract is that with IFQ_DRV_* the ifq_drv_* > > > fields should be protected by some caller-provided mechanism, while > > > the other ifq_* fields will be implictly protected by ifq_mtx, how > > > can accessing ifw_len without holding ifq_mtx in the above example > > > be safe? > > > > Reading is safe when you aren't modifying it, and only using it to > > inform if you should recheck w/ a lock... > > > > This way a driver can do: > > if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) { > > mtx_lock(sc->sc_mtx); > > for (;;) { > > IFQ_DRV_DEQUEUE(&ifp->if_snd, m); > > if (m == NULL) > > break; > > sendpkt(m); > > } > > mtx_unlock(sc->sc_mtx); > > } > > > > which saves an expensive lock/unlock op when there are no packets > > in the queue... > > The above snippet is fine even if IFQ_DRV_IS_EMPTY() returns 0 during a > race with another thread: per altq(9) the subsequent (then properly > locked) IFQ_DRV_DEQUEUE() might find out that the queue is actually > empty, and bail out if (m == NULL). > > But what if IFQ_DRV_IS_EMPTY() returns 1 due to a race with another > thread which has just executed IFQ_ENQUEUE() (invisible to the first > thread due to lack of synchronization), and therefore leaves the mbuf in > the queue, instead of dequeuing and processing it? This is where the _OACTIVE flag comes into play.. IFQ_HANDOFF_ADJ checks the flag, and calls if_start if it's not set.. (hmm, do we need a barrier here to ensure that enqueue completes before we check the _OACTIVE flag, as this check of the flag is unlocked?) As long the driver has not set the _OACTIVE flag, the if_start routine will be called for the enqueued packet... It's only a problem if the driver does an unlocked check for the queue and the driver has set the _OACTIVE flag, AND the driver will not do a future check of the queue, like it's suppose to when it finishes a packet TX and clears the _OACTIVE flag... -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."