Date: Wed, 13 Feb 2019 13:03:51 -0500 From: Randall Stewart <rrs@netflix.com> To: John Baldwin <jhb@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r344099 - head/sys/net Message-ID: <DC8566A0-7C0E-41E9-AA6A-7CDCE58976B4@netflix.com> In-Reply-To: <80314D46-6FEC-462D-8EC5-FCE1ECFF81EF@netflix.com> References: <201902131457.x1DEvx9V051533@repo.freebsd.org> <99453977-0f52-6050-3f40-e0fd7ea43d7f@FreeBSD.org> <80314D46-6FEC-462D-8EC5-FCE1ECFF81EF@netflix.com>
next in thread | previous in thread | raw e-mail | index | archive | help
oh and one other thing.. It was *not* a random IFP.. it was the IFP to the lagg. I.e. an alloc() was done to the lagg.. and the free was done back to the same IFP (that provided the allocate). R > On Feb 13, 2019, at 1:02 PM, Randall Stewart <rrs@netflix.com> wrote: >=20 > I disagree. If you define an alloc it is only > reciprocal that you should define a free. >=20 > The code in question that hit this was changed (its in a version > of rack that has the rate-limit and TLS code).. but I think these > things *should* be balanced.. if you provide an Allocate, you > should also provide a Free=E2=80=A6=20 >=20 > R >=20 >=20 >> On Feb 13, 2019, at 12:09 PM, John Baldwin <jhb@freebsd.org> wrote: >>=20 >> On 2/13/19 6:57 AM, Randall Stewart wrote: >>> Author: rrs >>> Date: Wed Feb 13 14:57:59 2019 >>> New Revision: 344099 >>> URL: https://svnweb.freebsd.org/changeset/base/344099 >>>=20 >>> Log: >>> This commit adds the missing release mechanism for the >>> ratelimiting code. The two modules (lagg and vlan) did have >>> allocation routines, and even though they are indirect (and >>> vector down to the underlying interfaces) they both need to >>> have a free routine (that also vectors down to the actual = interface). >>>=20 >>> Sponsored by: Netflix Inc. >>> Differential Revision: https://reviews.freebsd.org/D19032 >>=20 >> Hmm, I don't understand why you'd ever invoke if_snd_tag_free from = anything >> but 'tag->ifp' rather than some other ifp. What if the route for a = connection >> moves so that a tag allocated on cc0 is now on a route that goes over = em0? >> You can't expect em0 to have an if_snd_tag_free routine that will = know to >> go invoke cxgbe's snd_tag_free. I think you should always be using >> 'tag->ifp->if_snd_tag_free' to free tags and never using any other = ifp. >>=20 >> That is, I think this should be reverted and that instead you need to = fix >> the code invoking if_snd_tag_free to invoke it on the tag's ifp = instead of >> some random other ifp. >>=20 >> --=20 >> John Baldwin >>=20 >>=20 >=20 > ------ > Randall Stewart > rrs@netflix.com >=20 >=20 >=20 ------ Randall Stewart rrs@netflix.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?DC8566A0-7C0E-41E9-AA6A-7CDCE58976B4>