All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <kafai@fb.com>
To: Tanner Love <tannerlove.kernel@gmail.com>
Cc: <netdev@vger.kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Willem de Bruijn <willemb@google.com>,
	Petar Penkov <ppenkov@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Tanner Love <tannerlove@google.com>,
	Stanislav Fomichev <sdf@google.com>
Subject: Re: [PATCH net-next v5 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr
Date: Fri, 11 Jun 2021 18:13:42 -0700	[thread overview]
Message-ID: <20210612011342.2aywi36zfe6a5qh5@kafai-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAHrNZNgDfpWEdVK19cZ5CLK5T3RbKQSWPVx20e5S-_+zGJ7n=w@mail.gmail.com>

On Fri, Jun 11, 2021 at 02:07:10PM -0700, Tanner Love wrote:
> > A nit. It is a good chance to move the new BTF_ID_LIST_SINGLE
> > and most of the check_flow_keys_access() to filter.c.
> > Take a look at check_sock_access().
> >
> 
> It's not clear to me why it's preferable to move most of the
> check_flow_keys_access() to filter.c. In particular, the part of
> your comment that I don't understand is the "most of" part. Why
> would we want to separate the flow-keys-access-checking logic
> into two separate functions? Thanks
Right, actually, the whole function can be moved.
I found it easier to follow from flow_dissector_is_valid_access()
to flow_keys's access check without jumping around between two
different files.

> 
> Additionally, it seems that we don't actually need the
> BTF_ID_LIST_SINGLE. Can we not, instead, set
> regs[value_regno].btf_id to
> btf_find_by_name_kind(btf_vmlinux, "virtio_net_hdr", BTF_KIND_STRUCT)
> ? (And we'll check that that value is not <= 0.)
BTF_ID_LIST_SINGLE is resolved during compilation.
btf_find_by_name_kind() will be repeatedly finding the btf_id during runtime.
It is not like a killer but still unnecessary.

  parent reply	other threads:[~2021-06-12  1:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 18:38 [PATCH net-next v5 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
2021-06-10 18:38 ` [PATCH net-next v5 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr Tanner Love
2021-06-11  6:49   ` Martin KaFai Lau
     [not found]     ` <CAHrNZNgDfpWEdVK19cZ5CLK5T3RbKQSWPVx20e5S-_+zGJ7n=w@mail.gmail.com>
2021-06-12  1:13       ` Martin KaFai Lau [this message]
2021-06-12  1:31         ` Martin KaFai Lau
2021-06-10 18:38 ` [PATCH net-next v5 2/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb Tanner Love
2021-06-10 18:38 ` [PATCH net-next v5 3/3] selftests/net: amend bpf flow dissector prog to do vnet hdr validation Tanner Love

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210612011342.2aywi36zfe6a5qh5@kafai-mbp.dhcp.thefacebook.com \
    --to=kafai@fb.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jasowang@redhat.com \
    --cc=kuba@kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=ppenkov@google.com \
    --cc=sdf@google.com \
    --cc=tannerlove.kernel@gmail.com \
    --cc=tannerlove@google.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.