Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC

[Shahaf Shuler <shahafs@mellanox.com>]

Hi Ferruh,

Tuesday, June 19, 2018 9:03 PM, Ferruh Yigit
> Subject: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC
> 

[...]


> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index de3f869ed..28cf168aa 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -388,6 +388,9 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev
> *dev)
> 
>  	if (config->hw_fcs_strip)
>  		offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
> +	else
> +		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
> +

I think it should be:
if (config->hw_fcs_strip) {
	offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
	offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
}

The hw_fcs_strip is the capability from device which allows the PMD to toggle the CRC stripping.

>  	if (config->hw_csum)
>  		offloads |= (DEV_RX_OFFLOAD_IPV4_CKSUM |
>  			     DEV_RX_OFFLOAD_UDP_CKSUM |
> @@ -1419,17 +1422,17 @@ mlx5_rxq_new(struct rte_eth_dev *dev,
> uint16_t idx, uint16_t desc,
>  	/* Configure VLAN stripping. */
>  	tmpl->rxq.vlan_strip = !!(offloads &
> DEV_RX_OFFLOAD_VLAN_STRIP);
>  	/* By default, FCS (CRC) is stripped by hardware. */
> -	if (offloads & DEV_RX_OFFLOAD_CRC_STRIP) {
> -		tmpl->rxq.crc_present = 0;
> -	} else if (config->hw_fcs_strip) {
> -		tmpl->rxq.crc_present = 1;
> -	} else {
> -		DRV_LOG(WARNING,
> -			"port %u CRC stripping has been disabled but will"
> -			" still be performed by hardware, make sure
> MLNX_OFED"
> -			" and firmware are up to date",
> -			dev->data->port_id);
> -		tmpl->rxq.crc_present = 0;
> +	tmpl->rxq.crc_present = 0;
> +	if (rte_eth_dev_is_keep_crc(offloads)) {
> +		if (config->hw_fcs_strip) {
> +			tmpl->rxq.crc_present = 1;
> +		} else {
> +			DRV_LOG(WARNING,
> +				"port %u CRC stripping has been disabled but
> will"
> +				" still be performed by hardware, make sure
> MLNX_OFED"
> +				" and firmware are up to date",
> +				dev->data->port_id);
> +		}
>  	}
>  	DRV_LOG(DEBUG,
>  		"port %u CRC stripping is %s, %u bytes will be subtracted
> from"