[dpdk-dev] [PATCH v3 7/7] net/mlx4: remove empty Tx segment support

Adrien Mazarguil adrien.mazarguil at 6wind.com
Mon Oct 30 15:24:03 CET 2017


On Mon, Oct 30, 2017 at 10:07:29AM +0000, Matan Azrad wrote:
> Move empty segment case processing to debug mode.
> 
> Signed-off-by: Matan Azrad <matan at mellanox.com>

Whoa, I think there's a misunderstanding here. Nothing prevents applications
from attempting to send zero-length segments, and the PMD must survive this
somehow.

I think this commit should be dropped, more below.

> ---
>  drivers/net/mlx4/mlx4_rxtx.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c
> index 482c399..c005a41 100644
> --- a/drivers/net/mlx4/mlx4_rxtx.c
> +++ b/drivers/net/mlx4/mlx4_rxtx.c
> @@ -305,15 +305,18 @@ static int handle_multi_segs(struct rte_mbuf *buf,
>  			return -1;
>  		}
>  #endif /* NDEBUG */
> -		if (likely(sbuf->data_len)) {
> -			byte_count = rte_cpu_to_be_32(sbuf->data_len);
> -		} else {
> +		byte_count = rte_cpu_to_be_32(sbuf->data_len);
> +#ifndef NDEBUG
> +		if (unlikely(!sbuf->data_len)) {
> +			DEBUG("%p: Empty segment is not allowed",
> +					(void *)txq);
>  			/*
>  			 * Zero length segment is treated as inline segment
>  			 * with zero data.
>  			 */
>  			byte_count = RTE_BE32(0x80000000);
>  		}
> +#endif /* NDEBUG */

This change means outside of debug mode and according to PRM, a zero-length
segment is interpreted as containing 2 GiB worth of data, which guarantees
some sort of crash.

To properly enforce such a limitation, you'd need a check (possibly
unlikely()) to reject the packet and stop the TX function at this point
anyway. Such a check negates any kind of optimization brought by this
commit, as small as it is.

You'd better leave the existing code unmodified in my opinion.

-- 
Adrien Mazarguil
6WIND


More information about the dev mailing list