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: <fed6ef09-0f5b-8c3d-0484-bb0995d09282@redhat.com> Date: Mon, 22 May 2023 13:41:43 +0200 From: Jesper Dangaard Brouer <jbrouer@...hat.com> To: Alexander Lobakin <aleksander.lobakin@...el.com>, Jesper Dangaard Brouer <jbrouer@...hat.com>, Daniel Borkmann <daniel@...earbox.net> Cc: brouer@...hat.com, Larysa Zaremba <larysa.zaremba@...el.com>, bpf@...r.kernel.org, Stanislav Fomichev <sdf@...gle.com>, Alexei Starovoitov <ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>, Jakub Kicinski <kuba@...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>, Jiri Olsa <jolsa@...nel.org>, Jesse Brandeburg <jesse.brandeburg@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.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, netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH RESEND bpf-next 14/15] net, xdp: allow metadata > 32 On 19/05/2023 18.35, Alexander Lobakin wrote: > From: Jesper Dangaard Brouer <jbrouer@...hat.com> > Date: Tue, 16 May 2023 17:35:27 +0200 > >> >> On 16/05/2023 14.37, Alexander Lobakin wrote: >>> From: Larysa Zaremba<larysa.zaremba@...el.com> >>> Date: Mon, 15 May 2023 19:08:39 +0200 >>> >>>> On Mon, May 15, 2023 at 06:17:02PM +0200, Jesper Dangaard Brouer wrote: >>>>> >>>>> On 12/05/2023 17.26, Larysa Zaremba wrote: >>>>>> From: Aleksander Lobakin<aleksander.lobakin@...el.com> >>>>>> >>>>>> When using XDP hints, metadata sometimes has to be much bigger >>>>>> than 32 bytes. Relax the restriction, allow metadata larger than 32 >>>>>> bytes >>>>>> and make __skb_metadata_differs() work with bigger lengths. >>>>>> >>>>>> Now size of metadata is only limited by the fact it is stored as u8 >>>>>> in skb_shared_info, so maximum possible value is 255. >>>>> >>>>> I'm confused, IIRC the metadata area isn't stored "in skb_shared_info". >>>>> The maximum possible size is limited by the XDP headroom, which is also >>>>> shared/limited with/by xdp_frame. I must be reading the sentence >>>>> wrong, >>>>> somehow. >>> >>> skb_shared_info::meta_size is u8. Since metadata gets carried from >>> xdp_buff to skb, this check is needed (it's compile-time constant >>> anyway). >>> Check for headroom is done separately already (two sentences below). >>> >> >> Damn, argh, for SKBs the "meta_len" is stored in skb_shared_info, which >> is located on another cacheline. >> That is a sure way to KILL performance! :-( > > Have you read the code? I use type_max(typeof_member(shinfo, meta_len)), > what performance are you talking about? > Not talking about your changes (in this patch). I'm realizing that SKBs using metadata area will have a performance hit due to accessing another cacheline (the meta_len in skb_shared_info). IIRC Daniel complained about this performance hit (in the past), I guess this explains it. IIRC Cilium changed to use percpu variables/datastore to workaround this. > The whole xdp_metalen_invalid() gets expanded into: > > return (metalen % 4) || metalen > 255; > > at compile-time. All those typeof shenanigans are only to not open-code > meta_len's type/size/max. > >> >> But only use for SKBs that gets created from xdp with metadata, right? >> Normal netstack processing actually access this skb_shinfo->meta_len in gro_list_prepare(). As the caller dev_gro_receive() later access other memory in skb_shared_info, then the GRO code path already takes this hit to begin with. --Jesper
Powered by blists - more mailing lists