Re: [dpdk-dev] [PATCH V5 2/2] net/tap: use new Rx offloads API

[Shahaf Shuler <shahafs@mellanox.com>]

Monday, March 12, 2018 9:05 PM, Ferruh Yigit:
> On 3/12/2018 5:58 PM, Shahaf Shuler wrote:
> > Monday, March 12, 2018 7:00 PM, Ferruh Yigit:
> >> There are some devices supports queue level offloads and there are
> >> some devices supports port level offloads.
> >>
> >> Values queue offload = 0x0 and port offload = 0x1000, for the device
> >> that support queue level offloads may mean disabling all offloads for
> >> that specific queue and this is valid value. For the device that
> >> support port level offloads this is an error.
> >
> > device which don't support port level offloads should not be configured
> with port offloads.
> > Well implemented PMDs will fail the configuration with port offload =
> 0x1000.
> 
> For this particular error in tap:
> Rx queue offloads = 0x0,
> requested port offloads = 0x1000,
> supported offloads = 0x300e
> 
> Since supported offloads reported, and requested is subset of it I think port
> level offloads are OK, problem is in queue level offloads.
> 
> >
> >>
> >> And there should be some restrictions on offloading values:
> >> 1- requested port offloads should be subset of supported port
> >> offloads
> >> 2- supported queue offloads should be subset of supported port
> >> offloads
> >> 3- requested queue offloads should be subset of supported queue
> >> offloads
> >
> > This is correct.
> >
> >>
> >> And since these information is part of dev_info, these can be managed
> >> in the ethdev layer, instead of checked in each PMD (as done in tap).
> >>
> >>
> >> According above, would you mind walk-trough how application can set
> >> offloads:
> >>
> >> A) New application that implements new offloading APIs:
> >> - Get dev_info
> >> - Configure Rx/Tx offloads based on rx_offload_capa / tx_offload_capa
> >> - If rx_queue_offload_capa / tx_queue_offload_capa is other than 0,
> >> setup RxQ / TxQ offloads based on these values.
> >> - If rx_queue_offload_capa / tx_queue_offload_capa is 0, queue level
> >> offlaods are not supported by this device, ignore offloads during RxQ
> >> / TxQ setup.
> >
> > With the current API it is not correct. queue offloads should be at least the
> port offloads. If the application wants to enable more queue specific offloads
> it can as long as those are supported.
> 
> So above statement 2 is wrong?

If you mean this one:
> >> 2- supported queue offloads should be subset of supported port
> >> offloads

It is correct. every queue offload can be a port offload. Since the simple case of enabling the offload on each of the queues is exactly the same as enabling it for the port. 

> 
> Just to confirm, a queue can not disable an offload configured for port but
> can enable more offloads?

Yes. the check in the Tap PMD verifies it. 

> 
> > The following pseudo code demonstrate above:
> >
> > Dev_configure(port, port_offloads)
> > Rx_queue_offloads = port_offloads
> > If (rx_queue_offload_capa != 0)
> > 	Rx_queue_offloads |= rx_queue_offload_capa
> 
> Again to confirm, a device that supports port level offloads free to ignore TxQ
> / RxQ offload values, right?

you actually have 2 types of devices:
1. device which supports only port offloads
2. device which support both port and queue offloads. 

Device of type #1 *may* ignore the queue offloads, and also will not need to verify each port offload is set also on the queue.
The expectation from application is to always follow the API (enable port offload on both port and queue) regardless how the PMD is implemented. 

 So why we need "Rx_queue_offloads =
> port_offloads", will following be true?
> 
> Dev_configure(port, port_offloads)
> If (rx_queue_offload_capa != 0)
>         Rx_queue_offloads = port_offloads | rx_queue_offload_capa

Again - the application should follow the API which currently dictates how to set port offload. It is not depends on the rx_queue_offloads capabilities. 
For example, PMD which don't support queue offloads can still have verification for the API that each port offload is set also on the queue offloads. 

> 
> >
> >>
> >> All look OK here.
> >>
> >>
> >> B) Old application with old offloading API
> >> - Get dev_info, which provides only rx_offload_capa, tx_offload_capa
> >> and txq_flags
> >> - set rte_eth_rxmode->bitfield_values  ==> ethdev will convert them
> >> to port level Rx offloads.
> >> - port level offloads are empty!!
> >> - ethdev will set queue level Rx offloads to be same as port level Rx
> offloads.
> >> - ethdev will set txq_flags values for Tx offloads to queue level Tx
> offloads.
> >>
> >> Things should work well for PMDs with old offloading API.
> >>
> >> For the PMDs that support new offloading API, port level Tx offload
> >> values are missing and Queue level and Port level Tx offloads
> >> mismatch. Am I missing something here, if not how can we solve this issue
> in PMDs?
> >
> > Those PMDs (new PMD for old application) can use the
> ETH_TXQ_FLAGS_IGNORE which must be set for application which uses the
> new API..
> > see snipped code from mlx5 PMD:
> >
> > [1]
> > /*
> >  * Don't verify port offloads for application which
> >  * use the old API.
> >  */
> > if (!!(conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) &&
> >     !priv_is_tx_queue_offloads_allowed(priv, conf->offloads)) {
> >         ret = ENOTSUP;
> >         ERROR("%p: Tx queue offloads 0x%" PRIx64 " don't match port "
> >               "offloads 0x%" PRIx64 " or supported offloads 0x%" PRIx64,
> >               (void *)dev, conf->offloads,
> >               dev->data->dev_conf.txmode.offloads,
> >               mlx5_priv_get_tx_port_offloads(priv));
> >         goto out;
> > }
> 
> What this code does is: "if new offload API is used and queue offload is not
> valid, return error", this is completely different than what I say.

It also says if old API is used for this new PMD ((!!(conf->txq_flags & ETH_TXQ_FLAGS_IGNORE)  is 0 ) don't do verification between the port and the queues offloads on the Tx side. 

> 
> My concern is how new PMD will handle old application because of missing
> port level Tx offloads.
> 
> I guess each time PMD needs to use a Tx offload, it needs to check
> ETH_TXQ_FLAGS_IGNORE. If IGNORE is set PMD will use  txmode->offloads,
> if not it will use txq->offloads. Do you think this solves the issue?

Old application will know to set only Tx queue offloads (since it will set only the TXQ flags). Before the ethdev offloads rework every Tx offload was queue offload, so compatibility wise it is OK. 
Meaning the old PMD which was able to run with the old application should configure only Tx *queue* offloads. 
Considering that, the new PMD should consider only the txq->offloads as the queue offloads and the txmode->offloads should be 0. This perfectly match the offloads API.