Date: Tue, 16 Aug 2016 17:41:28 -0700 From: Mark Johnston <markj@FreeBSD.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r303646 - head/sys/ofed/drivers/infiniband/ulp/ipoib Message-ID: <20160817004128.GB94062@wkstn-mjohnston.west.isilon.com> In-Reply-To: <20160817000953.GB1069@FreeBSD.org> References: <201608012222.u71MMB4E018482@repo.freebsd.org> <20160817000953.GB1069@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Aug 16, 2016 at 05:09:53PM -0700, Gleb Smirnoff wrote: > Mark, > > On Mon, Aug 01, 2016 at 10:22:11PM +0000, Mark Johnston wrote: > M> Author: markj > M> Date: Mon Aug 1 22:22:11 2016 > M> New Revision: 303646 > M> URL: https://svnweb.freebsd.org/changeset/base/303646 > M> > M> Log: > M> ipoib: Bound the number of egress mbufs buffered during pathrec lookups. > M> > M> In pathological situations where the master subnet manager becomes > M> unresponsive for an extended period, we may otherwise end up queuing all > M> of the system's mbufs while waiting for a response to a path record lookup. > M> > M> This addresses the same issue as commit 1e85b806f9 in Linux. > M> > M> Reviewed by: cem, ngie > M> MFC after: 2 weeks > M> Sponsored by: EMC / Isilon Storage Division > M> > M> Modified: > M> head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c > M> > M> Modified: head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c > M> ============================================================================== > M> --- head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c Mon Aug 1 22:19:23 2016 (r303645) > M> +++ head/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c Mon Aug 1 22:22:11 2016 (r303646) > M> @@ -660,7 +660,13 @@ ipoib_unicast_send(struct mbuf *mb, stru > M> new_path = 1; > M> } > M> if (path) { > M> - _IF_ENQUEUE(&path->queue, mb); > M> + if (_IF_QLEN(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) > M> + _IF_ENQUEUE(&path->queue, mb); > M> + else { > M> + if_inc_counter(priv->dev, IFCOUNTER_OERRORS, 1); > M> + m_freem(mb); > M> + } > > Shouldn't that be IFCOUNTER_ODROPS? I'm not sure. I used IFCOUNTER_OERRORS to be consistent with other error cases in this function. This error case doesn't represent the normal source of outbound packet drops but rather indicates that a key routing agent on the network is not responding. OQDROPS seems like it's specifically for the case that we can't buffer packets because the transmitter isn't keeping up.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160817004128.GB94062>