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
| ||
|
Message-ID: <ZNEOf3vTu6pmNG1J@lincoln> Date: Mon, 7 Aug 2023 17:32:15 +0200 From: Larysa Zaremba <larysa.zaremba@...el.com> To: Alexei Starovoitov <alexei.starovoitov@...il.com>, Jesper Dangaard Brouer <hawk@...nel.org>, Stanislav Fomichev <sdf@...gle.com> CC: Willem de Bruijn <willemdebruijn.kernel@...il.com>, bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, David Ahern <dsahern@...il.com>, Jakub Kicinski <kuba@...nel.org>, Willem de Bruijn <willemb@...gle.com>, Jesper Dangaard Brouer <brouer@...hat.com>, "Anatoly Burakov" <anatoly.burakov@...el.com>, Alexander Lobakin <alexandr.lobakin@...el.com>, Magnus Karlsson <magnus.karlsson@...il.com>, Maryam Tahhan <mtahhan@...hat.com>, <xdp-hints@...-project.net>, "Network Development" <netdev@...r.kernel.org>, Simon Horman <simon.horman@...igine.com> Subject: Re: [PATCH bpf-next v4 12/21] xdp: Add checksum hint On Mon, Jul 31, 2023 at 06:03:26PM -0700, Alexei Starovoitov wrote: > On Mon, Jul 31, 2023 at 3:56 AM Larysa Zaremba <larysa.zaremba@...el.com> wrote: > > > > On Sun, Jul 30, 2023 at 09:13:02AM -0400, Willem de Bruijn wrote: > > > Alexei Starovoitov wrote: > > > > On Sat, Jul 29, 2023 at 9:15 AM Willem de Bruijn > > > > <willemdebruijn.kernel@...il.com> wrote: > > > > > > > > > > Alexei Starovoitov wrote: > > > > > > On Fri, Jul 28, 2023 at 07:39:14PM +0200, Larysa Zaremba wrote: > > > > > > > > > > > > > > +union xdp_csum_info { > > > > > > > + /* Checksum referred to by ``csum_start + csum_offset`` is considered > > > > > > > + * valid, but was never calculated, TX device has to do this, > > > > > > > + * starting from csum_start packet byte. > > > > > > > + * Any preceding checksums are also considered valid. > > > > > > > + * Available, if ``status == XDP_CHECKSUM_PARTIAL``. > > > > > > > + */ > > > > > > > + struct { > > > > > > > + u16 csum_start; > > > > > > > + u16 csum_offset; > > > > > > > + }; > > > > > > > + > > > > > > > > > > > > CHECKSUM_PARTIAL makes sense on TX, but this RX. I don't see in the above. > > > > > > > > > > It can be observed on RX when packets are looped. > > > > > > > > > > This may be observed even in XDP on veth. > > > > > > > > veth and XDP is a broken combination. GSO packets coming out of containers > > > > cannot be parsed properly by XDP. > > > > It was added mainly for testing. Just like "generic XDP". > > > > bpf progs at skb layer is much better fit for veth. > > > > > > Ok. Still, seems forward looking and little cost to define the > > > constant? > > > > > > > +1 > > CHECKSUM_PARTIAL is mostly for testing and removing/adding it doesn't change > > anything from the perspective of the user that does not use it, so I think it is > > worth having. > > "little cost to define the constant". > Not really. A constant in UAPI is a heavy burden. Sorry for the delayed response. I still do not comprehend the problem fully for this particular case, considering it shouldn't block any future changes to the API by itself. But, I personally have no reason to push hard the veth-supporting changes (aside from wanting the tests to look nicer). Still, before removing this in v5, I would like to get some additional feedback on this, preferably from Jesper (who, if I remember correctly, takes an interest in XDP on veth) or Stanislav. If instead of union xdp_csum_info we will have just checksum as a second argument, there will be no going back for this particular kfunc, so I want to be sure nobody will ever need such feature. [...]
Powered by blists - more mailing lists