netdev - Re: [PATCH net-next 2/2] virtio_net: Implement XDP_FLAGS_NO

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: <pj41zlpn2cpukf.fsf@u68c7b5b1d2d758.ant.amazon.com>
Date:   Sun, 10 Jan 2021 19:31:44 +0200
From:   Shay Agroskin <shayagr@...zon.com>
To:     Charlie Somerville <charlie@...rlie.bz>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <mst@...hat.com>,
        <jasowang@...hat.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] virtio_net: Implement XDP_FLAGS_NO_TX support


Charlie Somerville <charlie@...rlie.bz> writes:

> No send queues will be allocated for XDP filters. Attempts to 
> transmit
> packets when no XDP send queues exist will fail with EOPNOTSUPP.
>
> Signed-off-by: Charlie Somerville <charlie@...rlie.bz>
> ---
>  drivers/net/virtio_net.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 508408fbe78f..ed08998765e0 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -485,6 +485,10 @@ static struct send_queue 
> *virtnet_xdp_sq(struct virtnet_info *vi)
>  {
>  	unsigned int qp;
>  
> +	/* If no queue pairs are allocated for XDP use, return 
> NULL */
> +	if (vi->xdp_queue_pairs == 0)
> +		return NULL;
> +
>  	qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + 
>  smp_processor_id();
>  	return &vi->sq[qp];
>  }
> @@ -514,6 +518,11 @@ static int virtnet_xdp_xmit(struct 
> net_device *dev,
>  
>  	sq = virtnet_xdp_sq(vi);
>  
> +	/* No send queue exists if program was attached with 
> XDP_NO_TX */
> +	if (unlikely(!sq)) {
> +		return -EOPNOTSUPP;
> +	}
> +
>  	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
>  		ret = -EINVAL;
>  		drops = n;
> @@ -1464,7 +1473,7 @@ static int virtnet_poll(struct napi_struct 
> *napi, int budget)
>  
>  	if (xdp_xmit & VIRTIO_XDP_TX) {
>  		sq = virtnet_xdp_sq(vi);
> -		if (virtqueue_kick_prepare(sq->vq) && 
> virtqueue_notify(sq->vq)) {
> +		if (sq && virtqueue_kick_prepare(sq->vq) && 
> virtqueue_notify(sq->vq)) {

Is this addition needed ? Seems like we don't set VIRTIO_XDP_TX 
bit in case of virtnet_xdp_xmit() failure, so the surrounding 'if' 
won't be taken.

>  			u64_stats_update_begin(&sq->stats.syncp);
>  			sq->stats.kicks++;
>  			u64_stats_update_end(&sq->stats.syncp);
> @@ -2388,7 +2397,7 @@ static int 
> virtnet_restore_guest_offloads(struct virtnet_info *vi)
...
>