From owner-svn-src-head@freebsd.org Wed Feb 13 18:02:42 2019 Return-Path: Delivered-To: svn-src-head@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 958E614D3EC5 for ; Wed, 13 Feb 2019 18:02:42 +0000 (UTC) (envelope-from rrs@netflix.com) Received: from mail-qt1-x841.google.com (mail-qt1-x841.google.com [IPv6:2607:f8b0:4864:20::841]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 24D026E0D4 for ; Wed, 13 Feb 2019 18:02:42 +0000 (UTC) (envelope-from rrs@netflix.com) Received: by mail-qt1-x841.google.com with SMTP id o6so3647439qtk.6 for ; Wed, 13 Feb 2019 10:02:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=uiKX2qMENUw/ulv0QYq9Wzkap5tYQ63VQnMsYxqp9L0=; b=lY1k6a5vb4G4nPM17DlKF9LZirX/1PVqhE8/YwZhWQGLE2QBYuhOrpDrFa7kfqJUjG GqXhpkwbtGDZm+vGgM223G+KUqrbiLosqus5cyv8bWNL2okS4zNCmMYwVV1VbIm4fr5x q3kBNvzdzxCp8aolO/M1ah+sQk3I4vwk2G4uJtw2tG0FRYG9+227oQHmfdYEyhu3F9nf 3+OXgpNjQ1ZuxLx7mgh536Cfwur//GNU3X3BjIknrZZ3jjTfBJsPYrnDGupsqNaQaDQR n2AZP8jCiE7yTaPK7iU3EDHs2sL+Zh5Dhstl/9Nv5K/EyEqEWP6Pppng0DqPMKS46Pxa w7gg== X-Gm-Message-State: AHQUAuZQ40N3GbBvoYvUfzm2GLggbJZWbhmp+UVbngX/7Tu8Rol5QQ+Y EiuB75zdNDnIMBGvW9Vy6EMeXoagS3hmgg== X-Google-Smtp-Source: AHgI3IbgV+C3+XTNrA2Egu4racrOL/NsNce5DDLJF1ZD4Y5A7ggE3z8wT4sLFGU5lmPX5Vv4Osv5PQ== X-Received: by 2002:ac8:346c:: with SMTP id v41mr1426530qtb.231.1550080960160; Wed, 13 Feb 2019 10:02:40 -0800 (PST) Received: from ?IPv6:2607:fb10:7061:7fd::8524? ([2607:fb10:7061:7fd::8524]) by smtp.gmail.com with ESMTPSA id f19sm23251744qtf.1.2019.02.13.10.02.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 13 Feb 2019 10:02:39 -0800 (PST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: Re: svn commit: r344099 - head/sys/net From: Randall Stewart In-Reply-To: <99453977-0f52-6050-3f40-e0fd7ea43d7f@FreeBSD.org> Date: Wed, 13 Feb 2019 13:02:37 -0500 Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <80314D46-6FEC-462D-8EC5-FCE1ECFF81EF@netflix.com> References: <201902131457.x1DEvx9V051533@repo.freebsd.org> <99453977-0f52-6050-3f40-e0fd7ea43d7f@FreeBSD.org> To: John Baldwin X-Mailer: Apple Mail (2.3445.9.1) X-Rspamd-Queue-Id: 24D026E0D4 X-Spamd-Bar: ------ X-Spamd-Result: default: False [-6.99 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; NEURAL_HAM_SHORT(-0.99)[-0.992,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Feb 2019 18:02:42 -0000 I disagree. If you define an alloc it is only reciprocal that you should define a free. 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 R > On Feb 13, 2019, at 12:09 PM, John Baldwin 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 ------ Randall Stewart rrs@netflix.com