Re: [dpdk-dev] [PATCH] net/ixgbe: remove hardcoded CRC STRIP config from ixgbe

[Shahaf Shuler <shahafs@mellanox.com>]

Thursday, August 9, 2018 11:32 AM, Ferruh Yigit:
> Subject: Re: [PATCH] net/ixgbe: remove hardcoded CRC STRIP config from
> ixgbe
> 
> On 7/24/2018 3:36 AM, Wei Zhao wrote:
> > There is CRC related ifdefs for ixgbe:
> > CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
> > It is used in VF drivers ixgbevf_dev_configure() functions.
> > VF cannot change the CRC strip behavior and based on what PF
> > configured it needs to response proper to user
> > ixgbevf_dev_configure() request. Right now what PF set is defined by
> > above config options but this method is too static.
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
> <...>
> > @@ -334,6 +334,7 @@ struct rte_eth_rxmode {
> >  	 * structure are allowed to be set.
> >  	 */
> >  	uint64_t offloads;
> > +	uint64_t offloads_disable;
> 
> Do we need a disable flag in ethdev, an offload not enabled is disabled by
> default isn't it. This conflicts with offloads flag and makes is confusing.

+1.

**all** the offloads are disabled by default. 

> 
> For igb/e1000 VF case, VF driver can't change what PF set and VF driver can't
> learn the PF setting dynamically, so this information needs to be passed to VF
> driver by application/user.
> Currently this information passed by compile time config option, my
> suggestion was using devargs.
> 
> In your implementation testpmd parameter added to get this information
> and pass to driver, but this means all applications needs to do this, instead
> adding this support to driver looks better to me.