Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1660592020.git.dxu@dxuuu.xyz> <f850bb7e20950736d9175c61d7e0691098e06182.1660592020.git.dxu@dxuuu.xyz>
 <871qth87r1.fsf@toke.dk> <20220815224011.GA9821@breakpoint.cc>
In-Reply-To: <20220815224011.GA9821@breakpoint.cc>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Mon, 15 Aug 2022 15:47:46 -0700
Message-ID: <CAADnVQLB1SQoYAYEzU_VuJ=q3azeyhBiK-NkU5OZC7rrumi0xQ@mail.gmail.com>
Subject: Re: [PATCH bpf-next 2/3] bpf: Add support for writing to nf_conn:mark
To:     Florian Westphal <fw@strlen.de>
Cc:     =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= <toke@kernel.org>,
        Daniel Xu <dxu@dxuuu.xyz>, bpf <bpf@vger.kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>,
        Pablo Neira Ayuso <pablo@netfilter.org>,
        netfilter-devel <netfilter-devel@vger.kernel.org>,
        Network Development <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Aug 15, 2022 at 3:40 PM Florian Westphal <fw@strlen.de> wrote:
>
> Toke H=C3=B8iland-J=C3=B8rgensen <toke@kernel.org> wrote:
> > > Support direct writes to nf_conn:mark from TC and XDP prog types. Thi=
s
> > > is useful when applications want to store per-connection metadata. Th=
is
> > > is also particularly useful for applications that run both bpf and
> > > iptables/nftables because the latter can trivially access this metada=
ta.
> > >
> > > One example use case would be if a bpf prog is responsible for advanc=
ed
> > > packet classification and iptables/nftables is later used for routing
> > > due to pre-existing/legacy code.
> > >
> > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> >
> > Didn't we agree the last time around that all field access should be
> > using helper kfuncs instead of allowing direct writes to struct nf_conn=
?
>
> I don't see why ct->mark needs special handling.
>
> It might be possible we need to change accesses on nf/tc side to use
> READ/WRITE_ONCE though.

+1
I don't think we need to have a hard rule.
If fields is safe to access directly than it's faster
to let bpf prog read/write it.
There are no backward compat concerns. If conntrack side decides
to make that field special we can disallow direct writes in
the same kernel version. These accesses, just like kfuncs, are unstable.