From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by mail.toke.dk (Postfix) with ESMTPS id 7F678A28BD4 for ; Thu, 7 Sep 2023 18:33:28 +0200 (CEST) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20221208 header.b=U/Ck58iW Received: by mail-pj1-x102f.google.com with SMTP id 98e67ed59e1d1-26d54d3d984so838905a91.1 for ; Thu, 07 Sep 2023 09:33:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1694104406; x=1694709206; darn=xdp-project.net; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=HFTyNpM7yAvE7umrFRNXGK4VqMg2wT15/J42PJIZjWk=; b=U/Ck58iWQ6hXncrY3CXrXDSVIfr6Kgjo0HH6RgRn2bWUcCW/TQ9BHT4LyLqj0LQLLI WBXMweHDNYbiSAkT5voYCCoQa6I7zONkDcfyotfKc9oZ3aPGVWgtM7izEm+ytEwKoc9r bAEVPZeSNVsBIudc/0jtxQkxav0z8NlcooHB4Yn3Q+nxMc0HGYICiwcTT4/PcUxPgYLb GenqxPBmtjlk2UC+F39u9AsQrCY1Uwlfu0PL2dZycLO8QZLR2U1mT7KrjMGyabLMMl0R TeHsd84zMC6ipIUR/Z4cV8MiAcDT/b5eWnoDwe0ETmT9xXzUubFYRP4tXVsrsic35ehB GrEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694104406; x=1694709206; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HFTyNpM7yAvE7umrFRNXGK4VqMg2wT15/J42PJIZjWk=; b=EDoDTa0r6HAmzM0BRweURK961unAfQOeQiGCuDwATe/gBYaXg/zEWPbd38YfxC1Eow MvtKU95RZsoipS49PcAI7noF1D+WJwjSHErZStTaoFBdHDmSNOM6BvWjkEmevK2COXMt I3byRjk9h8NB4wEHAZslUDWKatYRWlnOc4I7A3KA73H/scvHJozx1QlKyRyOM0s2lIQ4 2X6LAJs0LJGje+QMbCFH36jJ3ujb/rWKx0CRFNDxtuSn+Ty11p9CeNT2iEaUrRwAVXwk MyUnGLDO3NRmoLAOyX5WyUFVf4bsJLs47xSrs3/ZoL+RXuxT+aL/nYTPHcwU7zDuJ3M5 meSA== X-Gm-Message-State: AOJu0YwECZhI1JO5KAI2JD2mfxXkOWmzDLAS8gb/OlsujpqufAO8jI/p YWlDrHtoLD2DV5uDY/421NNrEFiNxAj1rsZ398x+wg== X-Google-Smtp-Source: AGHT+IFVXK1irbxzXO8CRqUEV4PhxNxRhGrZ3jC0Oo8gFa/nAhDym68ODzwQwY9xK229KAYNL+iNYAgidGBtiQ/rr0U= X-Received: by 2002:a17:90a:b013:b0:26f:5d72:8de3 with SMTP id x19-20020a17090ab01300b0026f5d728de3mr119822pjq.20.1694104405601; Thu, 07 Sep 2023 09:33:25 -0700 (PDT) MIME-Version: 1.0 References: <20230824192703.712881-1-larysa.zaremba@intel.com> <20230824192703.712881-6-larysa.zaremba@intel.com> In-Reply-To: From: Stanislav Fomichev Date: Thu, 7 Sep 2023 09:33:14 -0700 Message-ID: To: Larysa Zaremba Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: P3ILO4POP56B42V4WWJUIBJSJ642DQQY X-Message-ID-Hash: P3ILO4POP56B42V4WWJUIBJSJ642DQQY X-MailFrom: sdf@google.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Maciej Fijalkowski , bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, haoluo@google.com, jolsa@kernel.org, David Ahern , Jakub Kicinski , Willem de Bruijn , Jesper Dangaard Brouer , Anatoly Burakov , Alexander Lobakin , Magnus Karlsson , Maryam Tahhan , xdp-hints@xdp-project.net, netdev@vger.kernel.org, Willem de Bruijn , Alexei Starovoitov , Simon Horman , Tariq Toukan , Saeed Mahameed X-Mailman-Version: 3.3.8 Precedence: list Subject: [xdp-hints] Re: [RFC bpf-next 05/23] ice: Introduce ice_xdp_buff List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, Sep 7, 2023 at 7:27=E2=80=AFAM Larysa Zaremba wrote: > > On Tue, Sep 05, 2023 at 07:53:03PM +0200, Maciej Fijalkowski wrote: > > On Mon, Sep 04, 2023 at 08:11:09PM +0200, Larysa Zaremba wrote: > > > On Mon, Sep 04, 2023 at 05:32:14PM +0200, Maciej Fijalkowski wrote: > > > > On Thu, Aug 24, 2023 at 09:26:44PM +0200, Larysa Zaremba wrote: > > > > > In order to use XDP hints via kfuncs we need to put > > > > > RX descriptor and ring pointers just next to xdp_buff. > > > > > Same as in hints implementations in other drivers, we achieve > > > > > this through putting xdp_buff into a child structure. > > > > > > > > Don't you mean a parent struct? xdp_buff will be 'child' of ice_xdp= _buff > > > > if i'm reading this right. > > > > > > > > > > ice_xdp_buff is a child in terms of inheritance (pointer to ice_xdp_b= uff could > > > replace pointer to xdp_buff, but not in reverse). > > > > > > > > > > > > > Currently, xdp_buff is stored in the ring structure, > > > > > so replace it with union that includes child structure. > > > > > This way enough memory is available while existing XDP code > > > > > remains isolated from hints. > > > > > > > > > > Minimum size of the new child structure (ice_xdp_buff) is exactly > > > > > 64 bytes (single cache line). To place it at the start of a cache= line, > > > > > move 'next' field from CL1 to CL3, as it isn't used often. This s= till > > > > > leaves 128 bits available in CL3 for packet context extensions. > > > > > > > > I believe ice_xdp_buff will be beefed up in later patches, so what = is the > > > > point of moving 'next' ? We won't be able to keep ice_xdp_buff in a= single > > > > CL anyway. > > > > > > > > > > It is to at least keep xdp_buff and descriptor pointer (used for ever= y hint) in > > > a single CL, other fields are situational. > > > > Right, something must be moved...still, would be good to see perf > > before/after :) > > > > > > > > > > > > > > > Signed-off-by: Larysa Zaremba > > > > > --- > > > > > drivers/net/ethernet/intel/ice/ice_txrx.c | 7 +++-- > > > > > drivers/net/ethernet/intel/ice/ice_txrx.h | 26 +++++++++++++= +++--- > > > > > drivers/net/ethernet/intel/ice/ice_txrx_lib.h | 10 +++++++ > > > > > 3 files changed, 38 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/= net/ethernet/intel/ice/ice_txrx.c > > > > > index 40f2f6dabb81..4e6546d9cf85 100644 > > > > > --- a/drivers/net/ethernet/intel/ice/ice_txrx.c > > > > > +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c > > > > > @@ -557,13 +557,14 @@ ice_rx_frame_truesize(struct ice_rx_ring *r= x_ring, const unsigned int size) > > > > > * @xdp_prog: XDP program to run > > > > > * @xdp_ring: ring to be used for XDP_TX action > > > > > * @rx_buf: Rx buffer to store the XDP action > > > > > + * @eop_desc: Last descriptor in packet to read metadata from > > > > > * > > > > > * Returns any of ICE_XDP_{PASS, CONSUMED, TX, REDIR} > > > > > */ > > > > > static void > > > > > ice_run_xdp(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, > > > > > struct bpf_prog *xdp_prog, struct ice_tx_ring *xdp_ri= ng, > > > > > - struct ice_rx_buf *rx_buf) > > > > > + struct ice_rx_buf *rx_buf, union ice_32b_rx_flex_desc= *eop_desc) > > > > > { > > > > > unsigned int ret =3D ICE_XDP_PASS; > > > > > u32 act; > > > > > @@ -571,6 +572,8 @@ ice_run_xdp(struct ice_rx_ring *rx_ring, stru= ct xdp_buff *xdp, > > > > > if (!xdp_prog) > > > > > goto exit; > > > > > > > > > > + ice_xdp_meta_set_desc(xdp, eop_desc); > > > > > > > > I am currently not sure if for multi-buffer case HW repeats all the > > > > necessary info within each descriptor for every frag? IOW shouldn't= you be > > > > using the ice_rx_ring::first_desc? > > > > > > > > Would be good to test hints for mbuf case for sure. > > > > > > > > > > In the skb path, we take metadata from the last descriptor only, so t= his should > > > be fine. Really worth testing with mbuf though. > > I retract my promise to test this with mbuf, as for now hints and mbuf ar= e not > supposed to go together [0]. Hm, I don't think it's intentional. I don't see why mbuf and hints can't coexist. Anything pops into your mind? Otherwise, can change that mask to be ~(BPF_F_XDP_DEV_BOUND_ONLY|BPF_F_XDP_HAS_FRAGS) as part of the series (or separately, up to you). > Making sure they can co-exist peacefully can be a topic for another serie= s. > For now I just can just say with high confidence that in case of multi-bu= ffer > frames, we do have all the supported metadata in the EoP descriptor. > > [0] https://elixir.bootlin.com/linux/v6.5.2/source/kernel/bpf/offload.c#L= 234 > > > > > Ok, thanks! > >