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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ