From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-x232.google.com (mail-oi1-x232.google.com [IPv6:2607:f8b0:4864:20::232]) by mail.toke.dk (Postfix) with ESMTPS id 3FFEE9C7739 for ; Mon, 28 Nov 2022 22:58:33 +0100 (CET) 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=20210112 header.b=pmFNOhjD Received: by mail-oi1-x232.google.com with SMTP id v81so13174086oie.5 for ; Mon, 28 Nov 2022 13:58:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; 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=aLDdh0gmgrkHvlbt0Qk6LQBk7G7QNAMnu/OlUpnhiaY=; b=pmFNOhjDJ3S/9UxkucZH8qV/+mFGOTZRfZfty5m6BnMywRIVs4duQ9zeB4cVW4N2ey AizJNb/312o1Dfy2i1nTZfKnjA2tERK4H8P+KLAkW8kXOoXnBdigBDlfK6l85mvVyeUV sTmIX6LP3FVrbycLqFVHWxUrr+pNUeVIlR8AUCbBKcETp6FEgjAaceyuwpbpPDPYPbip mMzntF4GN4ncVtsGJcq0mD0YB46e6cOwvWgG7bg9xLS+xPGz7ZMz/ga9uaRjgJV9K7rG 0ZWf7lHs4y7o7G71XWvaozlk+kYnDpDdbOEI0l0f/bgePC2UiKbHT+pro1YhydXj4VbD DcjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=aLDdh0gmgrkHvlbt0Qk6LQBk7G7QNAMnu/OlUpnhiaY=; b=nBRwsd1j+odM7+MMpPa6wC5+fvF+UO09a+duiPjbtIoBGaZDeHMMqVPZtX++6BbV5k 2WhjW6Cr+LdnNNj4JgRkpkTVMT2TVW73uQ0vrzWgt4i0OXMgWq3Sqn4c7gFWCbFWoNHW wHBZLEpoK7DkL6H+V20pMvBmnfVHqoy60imgQMN8+UKnRIrUhsWkZxfqBcaMQ/1SjjUH FoE/8RZHe44uLcxouDupe8BYdtx0k7DIOTmnTRq0cMZSYkrYVpuP1F0goLEM3jM4nUDf WiRzTvVIezrd21buX2XyjO16waKmNSK+E8goi2oIPmdYq/mXXD/TYtnBKgSyBissOG46 wcoQ== X-Gm-Message-State: ANoB5pkPb66wRqROMmnu51nPINecWqjYXaQqkKZU07D9PBwL9JjUOkl5 At4AtGVX/YlhOy8mPAEY884SDhnhYtuPKpBUKB654g== X-Google-Smtp-Source: AA0mqf4SyYwhb5aXxQg20tTnSaMO0WzbAk/x8Qc3aJFzEDnvety1XxuMhmJYXqcEeyz8MKX7zmGFTQRCqXi4VTomwsA= X-Received: by 2002:aca:674c:0:b0:35b:79ca:2990 with SMTP id b12-20020aca674c000000b0035b79ca2990mr13366020oiy.125.1669672710977; Mon, 28 Nov 2022 13:58:30 -0800 (PST) MIME-Version: 1.0 References: <20221121182552.2152891-1-sdf@google.com> <20221121182552.2152891-7-sdf@google.com> <874jupviyc.fsf@toke.dk> <20221123111431.7b54668e@kernel.org> <871qptuyie.fsf@toke.dk> <20221123174746.418920e5@kernel.org> <87edts2z8n.fsf@toke.dk> <874jun3m58.fsf@toke.dk> In-Reply-To: <874jun3m58.fsf@toke.dk> From: Stanislav Fomichev Date: Mon, 28 Nov 2022 13:58:19 -0800 Message-ID: To: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: CPMMU5BVYLWKSL3WOOUMS6BAF37JXYMW X-Message-ID-Hash: CPMMU5BVYLWKSL3WOOUMS6BAF37JXYMW 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 , Jakub Kicinski , 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, Tariq Toukan , David Ahern , Willem de Bruijn , Jesper Dangaard Brouer , Anatoly Burakov , Alexander Lobakin , Magnus Karlsson , Maryam Tahhan , xdp-hints@xdp-project.net, netdev@vger.kernel.org X-Mailman-Version: 3.3.7 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next v2 6/8] mlx4: Introduce mlx4_xdp_buff wrapper for 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, Nov 24, 2022 at 4:36 PM Toke H=C3=B8iland-J=C3=B8rgensen wrote: > > Maciej Fijalkowski writes: > > > On Thu, Nov 24, 2022 at 04:17:01PM +0100, Maciej Fijalkowski wrote: > >> On Thu, Nov 24, 2022 at 03:39:20PM +0100, Toke H=C3=B8iland-J=C3=B8rge= nsen wrote: > >> > Jakub Kicinski writes: > >> > > >> > > On Wed, 23 Nov 2022 22:55:21 +0100 Toke H=C3=B8iland-J=C3=B8rgense= n wrote: > >> > >> > Good idea, prototyped below, lmk if it that's not what you had = in mind. > >> > >> > > >> > >> > struct xdp_buff_xsk { > >> > >> > struct xdp_buff xdp; /* 0= 56 */ > >> > >> > u8 cb[16]; /* 56= 16 */ > >> > >> > /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --= - */ > >> > >> > >> > >> As pahole helpfully says here, xdp_buff is actually only 8 bytes = from > >> > >> being a full cache line. I thought about adding a 'cb' field like= this > >> > >> to xdp_buff itself, but figured that since there's only room for = a > >> > >> single pointer, why not just add that and let the driver point it= to > >> > >> where it wants to store the extra context data? > >> > > > >> > > What if the driver wants to store multiple pointers or an integer = or > >> > > whatever else? The single pointer seems quite arbitrary and not > >> > > strictly necessary. > >> > > >> > Well, then you allocate a separate struct and point to that? Like I = did > >> > in mlx5: > >> > > >> > > >> > + struct mlx5_xdp_ctx mlctx =3D { .cqe =3D cqe, .rq =3D rq }; > >> > + struct xdp_buff xdp =3D { .drv_priv =3D &mlctx }; > >> > > >> > but yeah, this does give an extra pointer deref on access. I'm not > >> > really opposed to the cb field either, I just think it's a bit odd t= o > >> > put it in struct xdp_buff_xsk; that basically requires the driver to > >> > keep the layouts in sync. > >> > > >> > Instead, why not but a cb field into xdp_buff itself so it can be us= ed > >> > for both the XSK and the non-XSK paths? Then the driver can just > >> > typecast the xdp_buff into its own struct that has whatever data it > >> > wants in place of the cb field? Agreed, maybe having an explicit cb field in the xdp_buff is a nice compromise (assuming, over time, most devices will use it). > >> Why can't you simply have a pointer to xdp_buff in driver specific > >> xdp_buff container which would point to xdp_buff that is stack based (= or > >> whatever else memory that will back it up - I am about to push a chang= e > >> that makes ice driver embed xdp_buff within a struct that represents R= x > >> ring) for XDP path and for ZC the pointer to xdp_buff that you get fro= m > >> xsk_buff_pool ? This would satisfy both sides I believe and would let = us > >> keep the same container struct. > >> > >> struct mlx4_xdp_buff { > >> struct xdp_buff *xdp; > >> struct mlx4_cqe *cqe; > >> struct mlx4_en_dev *mdev; > >> struct mlx4_en_rx_ring *ring; > >> struct net_device *dev; > >> }; > > > > Nah this won't work from kfunc POV, probably no way to retrieve the > > mlx4_xdp_buff based on xdp_buff ptr that needs to be used as an arg. > > > > Sorry I'll think more about it, in the meantime let's hear more voices > > whether we should keep Stan's original approach + modify xdp_buff_xsk o= r > > go with Toke's proposal. > > OK, so I played around with the mlx5 code a bit more, and I think the > "wrapping struct + cb area" can be made to work without too many ugly > casts; I'll send an updated version of the mlx5 patches with this > incorporated tomorrow, after I've run some tests... I'll probably send a v3 sometime tomorrow (PST), so maybe wait for me to make sure we are working on the same base? Or LMK if you prefer to do it differently..