[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <PH0PR11MB5830DD3BA9F6CBDA648F5AF8D89D9@PH0PR11MB5830.namprd11.prod.outlook.com>
Date: Tue, 18 Apr 2023 04:34:52 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: "Brouer, Jesper" <brouer@...hat.com>,
"bpf@...r.kernel.org" <bpf@...r.kernel.org>,
Stanislav Fomichev <sdf@...gle.com>,
Toke Høiland-Jørgensen <toke@...hat.com>
CC: "Brouer, Jesper" <brouer@...hat.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"martin.lau@...nel.org" <martin.lau@...nel.org>,
"ast@...nel.org" <ast@...nel.org>,
"daniel@...earbox.net" <daniel@...earbox.net>,
"Lobakin, Aleksander" <aleksander.lobakin@...el.com>,
"Zaremba, Larysa" <larysa.zaremba@...el.com>,
"xdp-hints@...-project.net" <xdp-hints@...-project.net>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"john.fastabend@...il.com" <john.fastabend@...il.com>,
"hawk@...nel.org" <hawk@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>
Subject: RE: [PATCH bpf-next V1 2/5] igc: add igc_xdp_buff wrapper for
xdp_buff in driver
On Monday, April 17, 2023 10:57 PM, Jesper Dangaard Brouer <brouer@...hat.com> wrote:
>Driver specific metadata data for XDP-hints kfuncs are propagated via tail
>extending the struct xdp_buff with a locally scoped driver struct.
>
>Zero-Copy AF_XDP/XSK does similar tricks via struct xdp_buff_xsk. This
>xdp_buff_xsk struct contains a CB area (24 bytes) that can be used for extending
>the locally scoped driver into. The XSK_CHECK_PRIV_TYPE define catch size
>violations build time.
>
Since the main purpose of this patch is to introduce igc_xdp_buff, and
you have another two patches for timestamp and hash,
thus, suggest to move timestamp and hash related code into respective patches.
>Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>
>---
> drivers/net/ethernet/intel/igc/igc.h | 6 ++++++
> drivers/net/ethernet/intel/igc/igc_main.c | 30 ++++++++++++++++++++++-------
> 2 files changed, 29 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/igc/igc.h
>b/drivers/net/ethernet/intel/igc/igc.h
>index f7f9e217e7b4..c609a2e648f8 100644
>--- a/drivers/net/ethernet/intel/igc/igc.h
>+++ b/drivers/net/ethernet/intel/igc/igc.h
>@@ -499,6 +499,12 @@ struct igc_rx_buffer {
> };
> };
>
>+/* context wrapper around xdp_buff to provide access to descriptor
>+metadata */ struct igc_xdp_buff {
>+ struct xdp_buff xdp;
>+ union igc_adv_rx_desc *rx_desc;
Move rx_desc to 4th patch (Rx hash patch)
>+};
>+
> struct igc_q_vector {
> struct igc_adapter *adapter; /* backlink */
> void __iomem *itr_register;
>diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
>b/drivers/net/ethernet/intel/igc/igc_main.c
>index bfa9768d447f..3a844cf5be3f 100644
>--- a/drivers/net/ethernet/intel/igc/igc_main.c
>+++ b/drivers/net/ethernet/intel/igc/igc_main.c
>@@ -2236,6 +2236,8 @@ static bool igc_alloc_rx_buffers_zc(struct igc_ring
>*ring, u16 count)
> if (!count)
> return ok;
>
>+ XSK_CHECK_PRIV_TYPE(struct igc_xdp_buff);
>+
> desc = IGC_RX_DESC(ring, i);
> bi = &ring->rx_buffer_info[i];
> i -= ring->count;
>@@ -2520,8 +2522,8 @@ static int igc_clean_rx_irq(struct igc_q_vector
>*q_vector, const int budget)
> union igc_adv_rx_desc *rx_desc;
> struct igc_rx_buffer *rx_buffer;
> unsigned int size, truesize;
>+ struct igc_xdp_buff ctx;
> ktime_t timestamp = 0;
>- struct xdp_buff xdp;
> int pkt_offset = 0;
> void *pktbuf;
>
>@@ -2555,13 +2557,14 @@ static int igc_clean_rx_irq(struct igc_q_vector
>*q_vector, const int budget)
> }
>
> if (!skb) {
>- xdp_init_buff(&xdp, truesize, &rx_ring->xdp_rxq);
>- xdp_prepare_buff(&xdp, pktbuf - igc_rx_offset(rx_ring),
>+ xdp_init_buff(&ctx.xdp, truesize, &rx_ring->xdp_rxq);
>+ xdp_prepare_buff(&ctx.xdp, pktbuf - igc_rx_offset(rx_ring),
> igc_rx_offset(rx_ring) + pkt_offset,
> size, true);
>- xdp_buff_clear_frags_flag(&xdp);
>+ xdp_buff_clear_frags_flag(&ctx.xdp);
>+ ctx.rx_desc = rx_desc;
Move rx_desc to 4th patch (Rx hash patch)
>
>- skb = igc_xdp_run_prog(adapter, &xdp);
>+ skb = igc_xdp_run_prog(adapter, &ctx.xdp);
> }
>
> if (IS_ERR(skb)) {
>@@ -2583,9 +2586,9 @@ static int igc_clean_rx_irq(struct igc_q_vector
>*q_vector, const int budget)
> } else if (skb)
> igc_add_rx_frag(rx_ring, rx_buffer, skb, size);
> else if (ring_uses_build_skb(rx_ring))
>- skb = igc_build_skb(rx_ring, rx_buffer, &xdp);
>+ skb = igc_build_skb(rx_ring, rx_buffer, &ctx.xdp);
> else
>- skb = igc_construct_skb(rx_ring, rx_buffer, &xdp,
>+ skb = igc_construct_skb(rx_ring, rx_buffer, &ctx.xdp,
> timestamp);
>
> /* exit if we failed to retrieve a buffer */ @@ -2686,6 +2689,15
>@@ static void igc_dispatch_skb_zc(struct igc_q_vector *q_vector,
> napi_gro_receive(&q_vector->napi, skb); }
>
>+static struct igc_xdp_buff *xsk_buff_to_igc_ctx(struct xdp_buff *xdp) {
>+ /* xdp_buff pointer used by ZC code path is alloc as xdp_buff_xsk. The
>+ * igc_xdp_buff shares its layout with xdp_buff_xsk and private
>+ * igc_xdp_buff fields fall into xdp_buff_xsk->cb
>+ */
>+ return (struct igc_xdp_buff *)xdp; }
>+
Move xsk_buff_to_igc_ctx to 3th patch (timestamp patch), which is first patch
adding xdp_metadata_ops support to igc.
> static int igc_clean_rx_irq_zc(struct igc_q_vector *q_vector, const int budget) {
> struct igc_adapter *adapter = q_vector->adapter; @@ -2704,6 +2716,7
>@@ static int igc_clean_rx_irq_zc(struct igc_q_vector *q_vector, const int
>budget)
> while (likely(total_packets < budget)) {
> union igc_adv_rx_desc *desc;
> struct igc_rx_buffer *bi;
>+ struct igc_xdp_buff *ctx;
> ktime_t timestamp = 0;
> unsigned int size;
> int res;
>@@ -2721,6 +2734,9 @@ static int igc_clean_rx_irq_zc(struct igc_q_vector
>*q_vector, const int budget)
>
> bi = &ring->rx_buffer_info[ntc];
>
>+ ctx = xsk_buff_to_igc_ctx(bi->xdp);
Move xsk_buff_to_igc_ctx to 3th patch (timestamp patch), which is first patch
adding xdp_metadata_ops support to igc.
>+ ctx->rx_desc = desc;
Move rx_desc to 4th patch (Rx hash patch)
Thanks & Regards
Siang
>+
> if (igc_test_staterr(desc, IGC_RXDADV_STAT_TSIP)) {
> timestamp = igc_ptp_rx_pktstamp(q_vector->adapter,
> bi->xdp->data);
>
Powered by blists - more mailing lists