lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 9 Jun 2021 23:15:25 -0400 From: Willem de Bruijn <willemdebruijn.kernel@...il.com> To: Jason Wang <jasowang@...hat.com> Cc: Tanner Love <tannerlove.kernel@...il.com>, Network Development <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Petar Penkov <ppenkov@...gle.com>, Jakub Kicinski <kuba@...nel.org>, "Michael S . Tsirkin" <mst@...hat.com>, Tanner Love <tannerlove@...gle.com> Subject: Re: [PATCH net-next v4 2/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb On Wed, Jun 9, 2021 at 11:09 PM Jason Wang <jasowang@...hat.com> wrote: > > > 在 2021/6/9 上午1:02, Tanner Love 写道: > > retry: > > - if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys, > > + /* only if flow dissection not already done */ > > + if (!static_branch_unlikely(&sysctl_flow_dissect_vnet_hdr_key) && > > + !skb_flow_dissect_flow_keys_basic(NULL, skb, &keys, > > NULL, 0, 0, 0, > > 0)) { > > > So I still wonder the benefit we could gain from reusing the bpf flow > dissector here. Consider the only context we need is the flow keys, Maybe I misunderstand the comment, but the goal is not just to access the flow keys, but to correlate data in the packet payload with the fields of the virtio_net_header. E.g., to parse a packet payload to verify that it matches the gso type and header length. I don't see how we could make that two separate programs, one to parse a packet (flow dissector) and one to validate the vnet_hdr. > we > had two choices > > a1) embed the vnet header checking inside bpf flow dissector > a2) introduce a dedicated eBPF type for doing that > > And we have two ways to access the vnet header > > b1) via pesudo __sk_buff > b2) introduce bpf helpers > > I second for a2 and b2. The main motivation is to hide the vnet header > details from the bpf subsystem. b2 assumes that we might have many different variants of vnet_hdr, but that all will be able to back a functional interface like "return the header length"? Historically, the struct has not seen such a rapid rate of change that I think would warrant introducing this level of indirection. The little_endian field does demonstrate one form of how the struct can be context sensitive -- and not self describing. I think we can capture multiple variants of the struct, if it ever comes to that, with a versioning field. We did not add that right away, because that can be introduced later, when a new version arrives. But we have a plan for the eventuality. > > Thanks >
Powered by blists - more mailing lists