netdev - Re: [PATCH bpf-next] bpf: devmap: move drop error path to devmap for 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:   Wed, 17 Feb 2021 20:15:25 +0000
From:   Edward Cree <ecree.xilinx@...il.com>
To:     Lorenzo Bianconi <lorenzo@...nel.org>, bpf@...r.kernel.org
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        ast@...nel.org, daniel@...earbox.net, lorenzo.bianconi@...hat.com,
        brouer@...hat.com, toke@...hat.com, freysteinn.alfredsson@....se
Subject: Re: [PATCH bpf-next] bpf: devmap: move drop error path to devmap for
 XDP_REDIRECT

On 17/02/2021 13:56, Lorenzo Bianconi wrote:
> We want to change the current ndo_xdp_xmit drop semantics because
> it will allow us to implement better queue overflow handling.
> This is working towards the larger goal of a XDP TX queue-hook.
> Move XDP_REDIRECT error path handling from each XDP ethernet driver to
> devmap code. According to the new APIs, the driver running the
> ndo_xdp_xmit pointer, will break tx loop whenever the hw reports a tx
> error and it will just return to devmap caller the number of successfully
> transmitted frames. It will be devmap responsability to free dropped frames.
> Move each XDP ndo_xdp_xmit capable driver to the new API<snip>> diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
> index 1665529a7271..0c6650d2e239 100644
> --- a/drivers/net/ethernet/sfc/tx.c
> +++ b/drivers/net/ethernet/sfc/tx.c
> @@ -412,14 +412,6 @@ netdev_tx_t __efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb
>  	return NETDEV_TX_OK;
>  }
>  
> -static void efx_xdp_return_frames(int n,  struct xdp_frame **xdpfs)
> -{
> -	int i;
> -
> -	for (i = 0; i < n; i++)
> -		xdp_return_frame_rx_napi(xdpfs[i]);
> -}
> -
>  /* Transmit a packet from an XDP buffer
>   *
>   * Returns number of packets sent on success, error code otherwise.
> @@ -492,12 +484,7 @@ int efx_xdp_tx_buffers(struct efx_nic *efx, int n, struct xdp_frame **xdpfs,
>  	if (flush && i > 0)
>  		efx_nic_push_buffers(tx_queue);
>  
> -	if (i == 0)
> -		return -EIO;
> -
> -	efx_xdp_return_frames(n - i, xdpfs + i);
> -
> -	return i;
> +	return i == 0 ? -EIO : i;
>  }

Could this be "return i ?: -EIO;"?  (I'm undecided on whether that would
 actually be better.)

Either way, have an
Acked-by: Edward Cree <ecree.xilinx@...il.com>
for the sfc part.