netdev - Re: [bpf-next V5 PATCH 05/15] xdp: introduce a new xdp

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]

Date:   Fri, 23 Mar 2018 10:11:53 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Netdev <netdev@...r.kernel.org>,
        BjörnTöpel <bjorn.topel@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Eugenia Emantayev <eugenia@...lanox.com>,
        Jason Wang <jasowang@...hat.com>,
        John Fastabend <john.fastabend@...il.com>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Gal Pressman <galp@...lanox.com>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Re: [bpf-next V5 PATCH 05/15] xdp: introduce a new xdp_frame type

On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer
<brouer@...hat.com> wrote:
> This is needed to convert drivers tuntap and virtio_net.
>
> This is a generalization of what is done inside cpumap, which will be
> converted later.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>
> ---
>  include/net/xdp.h |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 1ee154fe0be6..13f71a15c79f 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -59,6 +59,46 @@ struct xdp_buff {
>         struct xdp_rxq_info *rxq;
>  };
>
> +struct xdp_frame {
> +       void *data;
> +       u16 len;
> +       u16 headroom;
> +       u16 metasize;
> +       /* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,
> +        * while mem info is valid on remote CPU.
> +        */
> +       struct xdp_mem_info mem;
> +};
> +
> +/* Convert xdp_buff to xdp_frame */
> +static inline
> +struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
> +{
> +       struct xdp_frame *xdp_frame;
> +       int metasize;
> +       int headroom;
> +
> +       /* Assure headroom is available for storing info */
> +       headroom = xdp->data - xdp->data_hard_start;
> +       metasize = xdp->data - xdp->data_meta;
> +       metasize = metasize > 0 ? metasize : 0;
> +       if (unlikely((headroom - metasize) < sizeof(*xdp_frame)))
> +               return NULL;
> +
> +       /* Store info in top of packet */
> +       xdp_frame = xdp->data_hard_start;
> +

I thought the point of this stuff was supposed to be performance. This
is going to hurt. You are adding yet another cacheline to the number
of lines accessed to process the frame.

I would like to know how of a slow down is being added after you apply
this to the Tx path of ixgbe. I suspect there should be a noticeable
difference for your basic xmit loopback test.

> +       xdp_frame->data = xdp->data;
> +       xdp_frame->len  = xdp->data_end - xdp->data;
> +       xdp_frame->headroom = headroom - sizeof(*xdp_frame);
> +       xdp_frame->metasize = metasize;
> +
> +       /* rxq only valid until napi_schedule ends, convert to xdp_mem_info */
> +       xdp_frame->mem = xdp->rxq->mem;
> +
> +       return xdp_frame;
> +}
> +
>  static inline
>  void xdp_return_frame(void *data, struct xdp_mem_info *mem)
>  {
>