netdev - Re: [PATCH v5 bpf-next 06/14] net: mvneta: add multi buffer support to 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]

Message-ID: <pj41zlh7ohpz6h.fsf@u68c7b5b1d2d758.ant.amazon.com>
Date:   Sat, 19 Dec 2020 17:56:22 +0200
From:   Shay Agroskin <shayagr@...zon.com>
To:     Lorenzo Bianconi <lorenzo@...nel.org>
CC:     <bpf@...r.kernel.org>, <netdev@...r.kernel.org>,
        <davem@...emloft.net>, <kuba@...nel.org>, <ast@...nel.org>,
        <daniel@...earbox.net>, <sameehj@...zon.com>,
        <john.fastabend@...il.com>, <dsahern@...nel.org>,
        <brouer@...hat.com>, <echaudro@...hat.com>,
        <lorenzo.bianconi@...hat.com>, <jasowang@...hat.com>
Subject: Re: [PATCH v5 bpf-next 06/14] net: mvneta: add multi buffer support
 to XDP_TX


Lorenzo Bianconi <lorenzo@...nel.org> writes:

> Introduce the capability to map non-linear xdp buffer running
> mvneta_xdp_submit_frame() for XDP_TX and XDP_REDIRECT
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 94 
>  ++++++++++++++++-----------
>  1 file changed, 56 insertions(+), 38 deletions(-)
[...]
>  			if (napi && buf->type == 
>  MVNETA_TYPE_XDP_TX)
>  				xdp_return_frame_rx_napi(buf->xdpf);
>  			else
> @@ -2054,45 +2054,64 @@ mvneta_xdp_put_buff(struct mvneta_port 
> *pp, struct mvneta_rx_queue *rxq,
>  
>  static int
>  mvneta_xdp_submit_frame(struct mvneta_port *pp, struct 
>  mvneta_tx_queue *txq,
> -			struct xdp_frame *xdpf, bool dma_map)
> +			struct xdp_frame *xdpf, int *nxmit_byte, 
> bool dma_map)
>  {
> -	struct mvneta_tx_desc *tx_desc;
> -	struct mvneta_tx_buf *buf;
> -	dma_addr_t dma_addr;
> +	struct xdp_shared_info *xdp_sinfo = 
> xdp_get_shared_info_from_frame(xdpf);
> +	int i, num_frames = xdpf->mb ? xdp_sinfo->nr_frags + 1 : 
> 1;
> +	struct mvneta_tx_desc *tx_desc = NULL;
> +	struct page *page;
>  
> -	if (txq->count >= txq->tx_stop_threshold)
> +	if (txq->count + num_frames >= txq->size)
>  		return MVNETA_XDP_DROPPED;
>  
> -	tx_desc = mvneta_txq_next_desc_get(txq);
> +	for (i = 0; i < num_frames; i++) {
> +		struct mvneta_tx_buf *buf = 
> &txq->buf[txq->txq_put_index];
> +		skb_frag_t *frag = i ? &xdp_sinfo->frags[i - 1] : 
> NULL;
> +		int len = frag ? xdp_get_frag_size(frag) : 
> xdpf->len;

nit, from branch prediction point of view, maybe it would be 
better to write
     int len = i ? xdp_get_frag_size(frag) : xdpf->len;

since the value of i is checked one line above
Disclaimer: I'm far from a compiler expert, and don't know whether 
the compiler would know to group these two assignments together 
into a single branch prediction decision, but it feels like using 
'i' would make this decision easier for it.

Thanks,
Shay

[...]