Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 Sep 2021 09:43:06 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        Gleb Smirnoff <glebius@freebsd.org>
Cc:        kp@freebsd.org, src-committers@freebsd.org,  dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: 5091ca26507b - main - pf: save on branching in the common case in pf_test
Message-ID:  <CAGudoHHrxk7=tYF66Q6DHFkwxmKb=QEKu%2BAYiDK9sfT5OGeqoQ@mail.gmail.com>
In-Reply-To: <YTrtUWbpWxxzviSQ@FreeBSD.org>
References:  <202108171959.17HJx2lL069856@gitrepo.freebsd.org> <YTrtUWbpWxxzviSQ@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 9/10/21, Gleb Smirnoff <glebius@freebsd.org> wrote:
>   Mateusz,
>
> On Tue, Aug 17, 2021 at 07:59:02PM +0000, Mateusz Guzik wrote:
> M> The branch main has been updated by mjg:
> M>
> M> URL:
> https://cgit.FreeBSD.org/src/commit/?id=5091ca26507b83ebd5ce677dd26bd10e4bad4153
> M>
> M> commit 5091ca26507b83ebd5ce677dd26bd10e4bad4153
> M> Author:     Mateusz Guzik <mjg@FreeBSD.org>
> M> AuthorDate: 2021-08-17 18:04:31 +0000
> M> Commit:     Mateusz Guzik <mjg@FreeBSD.org>
> M> CommitDate: 2021-08-17 19:56:33 +0000
> M>
> M>     pf: save on branching in the common case in pf_test
> M>
> M>     Reviewed by:    kp
> M>     Sponsored by:   Rubicon Communications, LLC ("Netgate")
> M> ---
> M>  sys/netpfil/pf/pf.c | 6 +++---
> M>  1 file changed, 3 insertions(+), 3 deletions(-)
> M>
> M> diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
> M> index e2dd3eb7c0de..add76c7b98d4 100644
> M> --- a/sys/netpfil/pf/pf.c
> M> +++ b/sys/netpfil/pf/pf.c
> M> @@ -6151,7 +6151,7 @@ pf_test(int dir, int pflags, struct ifnet *ifp,
> struct mbuf **m0, struct inpcb *
> M>
> M>  	PF_RULES_RLOCK();
> M>
> M> -	if (ip_divert_ptr != NULL &&
> M> +	if (__predict_false(ip_divert_ptr != NULL) &&
>
> This is an optimization for a setup without divert(4) at cost of
> pessimization
> for a setup with divert(4). IMHO, __predict_false() predicate should guard
> against error paths and other unusual events, not favor one setup over
> other.
>

divert is non-standard and comes with tons of overhead, so I think
this is justified.

The real fix would be to either implement hot patchable branches or
otherwise generate pf_test always (or never) doing divert without
having to check for it, but that's far in the future.

-- 
Mateusz Guzik <mjguzik gmail.com>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHHrxk7=tYF66Q6DHFkwxmKb=QEKu%2BAYiDK9sfT5OGeqoQ>