Re: [dpdk-dev] [PATCH v5 1/3] ethdev: introduce Rx queue offloads API

[Shahaf Shuler <shahafs@mellanox.com>]

Hi Ferruh,

Tuesday, October 3, 2017 3:32 AM, Ferruh Yigit:
> On 9/28/2017 7:54 PM, Shahaf Shuler wrote:
> > Introduce a new API to configure Rx offloads.
> >
> > In the new API, offloads are divided into per-port and per-queue
> > offloads. The PMD reports capability for each of them.
> > Offloads are enabled using the existing DEV_RX_OFFLOAD_* flags.
> > To enable per-port offload, the offload should be set on both device
> > configuration and queue configuration. To enable per-queue offload,
> > the offloads can be set only on queue configuration.
> >
> > Applications should set the ignore_offload_bitfield bit on rxmode
> > structure in order to move to the new API.
> >
> > The old Rx offloads API is kept for the meanwhile, in order to enable
> > a smooth transition for PMDs and application to the new API.
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> 
> <...>
> 
> > @@ -1102,8 +1193,18 @@ rte_eth_rx_queue_setup(uint8_t port_id,
> uint16_t rx_queue_id,
> >  	if (rx_conf == NULL)
> >  		rx_conf = &dev_info.default_rxconf;
> >
> > +	local_conf = *rx_conf;
> > +	if (dev->data->dev_conf.rxmode.ignore_offload_bitfield == 0) {
> > +		/**
> > +		 * Reflect port offloads to queue offloads in order for
> > +		 * offloads to not be discarded.
> > +		 */
> > +		rte_eth_convert_rx_offload_bitfield(&dev->data-
> >dev_conf.rxmode,
> > +						    &local_conf.offloads);
> > +	}
> 
> If an application switches to the new method, it will set "offloads" and if
> underlying PMD doesn't support the new method it will just do nothing with
> "offloads" variable but problem is application won't know PMD just ignored
> them, it may think per queue offloads set.
> 
> Does it make sense to notify application that PMD doesn't understand that
> new "offloads" flag?

I don't think it is needed. In the new API the per-queue Rx offloads caps are reported using a new rx_queue_offload_capa field. Old PMD will not set it, therefore application which use the new API will see that the underlying PMD is supporting only per-port Rx offloads. 
This should be enough for it to understand that the per-queue offloads won't be set. 

> 
> > +
> >  	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id,
> nb_rx_desc,
> > -					      socket_id, rx_conf, mp);
> > +					      socket_id, &local_conf, mp);
> >  	if (!ret) {
> >  		if (!dev->data->min_rx_buf_size ||
> >  		    dev->data->min_rx_buf_size > mbp_buf_size)
> 
> <...>
> 
> >  /**
> > @@ -691,6 +712,12 @@ struct rte_eth_rxconf {
> >  	uint16_t rx_free_thresh; /**< Drives the freeing of RX descriptors.
> */
> >  	uint8_t rx_drop_en; /**< Drop packets if no descriptors are
> available. */
> >  	uint8_t rx_deferred_start; /**< Do not start queue with
> > rte_eth_dev_start(). */
> > +	/**
> > +	 * Per-queue Rx offloads to be set using DEV_RX_OFFLOAD_* flags.
> > +	 * Only offloads set on rx_queue_offload_capa or rx_offload_capa
> > +	 * fields on rte_eth_dev_info structure are allowed to be set.
> > +	 */
> 
> How application will use above "capa" flags to decide what to set? Since
> "rx_queue_offload_capa" is new field introduced with this patch no PMD
> implemented it yet, does it means no application will be able to use per
> queue offloads yet?

Yes.
Application which use the new offloads API should query the device info and look into the rx_offloads_capa and rx_queue_offloads_capa.
According to those 2 caps it will decide how to set the offloads. 
Per-queue Rx offloads is a new functionality introduced in this series. Of course old PMD will not support it, and this will be reflected on the rx_queue_offlaods_capa.  


> 
> > +	uint64_t offloads;
> >  };
> >
> 
> <...>