[PATCH v12 11/12] net/tap: do not mark queue full as error

[Ferruh Yigit]

On 5/2/2024 10:31 PM, Stephen Hemminger wrote:
> If tap device in kernel returns EAGAIN that means it is full.
> That is not an error.
> 
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
> ---
>  drivers/net/tap/rte_eth_tap.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 2484a82ccb..485bd35912 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -542,7 +542,6 @@ tap_write_mbufs(struct tx_queue *txq, uint16_t num_mbufs,
>  		struct rte_mbuf *seg = mbuf;
>  		uint64_t l4_ol_flags;
>  		int proto;
> -		int n;
>  		int j;
>  		int k; /* current index in iovecs for copying segments */
>  
> @@ -647,14 +646,18 @@ tap_write_mbufs(struct tx_queue *txq, uint16_t num_mbufs,
>  		}
>  
>  		/* copy the tx frame data */
> -		n = writev(process_private->fds[txq->queue_id], iovecs, k);
> -		if (n <= 0)
> -			return -1;
> +		if (unlikely(writev(process_private->fds[txq->queue_id], iovecs, k) < 0)) {
> +			TAP_LOG(DEBUG, "writev (qid=%u fd=%d) %s",
> +				txq->queue_id, process_private->fds[txq->queue_id],
> +				strerror(errno));
>

Do we really want logging in datapath?

> +			return (errno == EAGAIN) ? 0 : -1;
>

Nack.

Returning '0' will cause 'pmd_tx_burst()' to continue and increase
'num_tx', this will mislead as packet sent successfully.

We should have three return values from 'tap_write_mbufs()':
<0 -> Error, 'pmd_tx_burst()' should break, stats.errors updated.
 0 ->  'pmd_tx_burst()' should break, valid num_tx returned
>0 -> 'pmd_tx_burst()' work as it is.

> +		}
>  
>  		(*num_packets)++;
>  		(*num_tx_bytes) += rte_pktmbuf_pkt_len(mbuf);
>  	}
> -	return 0;
> +
> +	return 1;
> 

Why '1', wouldn't it be better to return number of packets written
successfully.