netdev - Re: [PATCH net-next v2] lan966x: Don't use xdp_frame when action is 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 21 Apr 2023 15:34:37 +0200
From:   Alexander Lobakin <aleksander.lobakin@...el.com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.com>
CC:     <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <bpf@...r.kernel.org>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <ast@...nel.org>, <daniel@...earbox.net>, <hawk@...nel.org>,
        <john.fastabend@...il.com>, <richardcochran@...il.com>,
        <UNGLinuxDriver@...rochip.com>, <alexandr.lobakin@...el.com>,
        <maciej.fijalkowski@...el.com>
Subject: Re: [PATCH net-next v2] lan966x: Don't use xdp_frame when action is
 XDP_TX

From: Horatiu Vultur <horatiu.vultur@...rochip.com>
Date: Fri, 21 Apr 2023 15:14:22 +0200

> When the action of an xdp program was XDP_TX, lan966x was creating
> a xdp_frame and use this one to send the frame back. But it is also
> possible to send back the frame without needing a xdp_frame, because
> it is possible to send it back using the page.
> And then once the frame is transmitted is possible to use directly
> page_pool_recycle_direct as lan966x is using page pools.
> This would save some CPU usage on this path, which results in higher
> number of transmitted frames. Bellow are the statistics:
> Frame size:    Improvement:
> 64                ~8%
> 256              ~11%
> 512               ~8%

Nice bump, esp. for 256 }:>

> 1000              ~0%
> 1500              ~0%

[...]

> @@ -699,15 +701,14 @@ static void lan966x_fdma_tx_start(struct lan966x_tx *tx, int next_to_use)
>  	tx->last_in_use = next_to_use;
>  }
>  
> -int lan966x_fdma_xmit_xdpf(struct lan966x_port *port,
> -			   struct xdp_frame *xdpf,
> -			   struct page *page,
> -			   bool dma_map)
> +int lan966x_fdma_xmit_xdpf(struct lan966x_port *port, void *ptr, u32 len)
>  {
>  	struct lan966x *lan966x = port->lan966x;
>  	struct lan966x_tx_dcb_buf *next_dcb_buf;
>  	struct lan966x_tx *tx = &lan966x->tx;
> +	struct xdp_frame *xdpf;
>  	dma_addr_t dma_addr;
> +	struct page *page;
>  	int next_to_use;
>  	__be32 *ifh;
>  	int ret = 0;
> @@ -722,8 +723,19 @@ int lan966x_fdma_xmit_xdpf(struct lan966x_port *port,
>  		goto out;
>  	}
>  
> +	/* Fill up the buffer */
> +	next_dcb_buf = &tx->dcbs_buf[next_to_use];
> +	next_dcb_buf->use_skb = false;
> +	next_dcb_buf->xdp_ndo = !len;
> +	next_dcb_buf->len = len + IFH_LEN_BYTES;

Is it intended that for .ndo_xdp_xmit cases this field will equal just
%IFH_LEN_BYTES as @len is zero?

> +	next_dcb_buf->used = true;
> +	next_dcb_buf->ptp = false;
> +	next_dcb_buf->dev = port->dev;
> +
>  	/* Generate new IFH */
> -	if (dma_map) {
> +	if (!len) {
> +		xdpf = ptr;
> +
>  		if (xdpf->headroom < IFH_LEN_BYTES) {
>  			ret = NETDEV_TX_OK;
>  			goto out;
[...]

Thanks,
Olek