Re: [dpdk-dev] Survey for final decision about per-port offload API

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/24/2018 11:00 PM, Thomas Monjalon wrote:
> Hi,
> 
> First, this is my summary after the survey answers and comments:
> 
> 1/ allow "forgetting" port offloads in queue offloads setup
> 
> 2/ update documentation, applications and remove checks in PMDs for 18.05-rc2
> 
> 3/ an offload enabled at port level, cannot be disabled at queue level
> 
> 4/ The queue capabilities must be a subset of port capabilities,
> i.e. every queue capabilities must be reported as port capabilities.
> But the port capabilities should be reported at queue level
> only if it can be enabled on queue when it is disabled on port level.
> 
> 
> 24/04/2018 12:39, Ferruh Yigit:
>> On 3/30/2018 2:47 PM, Thomas Monjalon wrote:
>>> There are some discussions about a specific part of the offload API:
>>> 	"To enable per-port offload, the offload should be set on both
>>> 	device configuration and queue setup."
>>>
>>> It means the application must repeat the port offload flags
>>> in rte_eth_conf.[rt]xmode.offloads and rte_eth_[rt]xconf.offloads,
>>> when calling respectively rte_eth_dev_configure() and
>>> rte_eth_[rt]x_queue_setup for each queue.
>>>
>>> The PMD must check if there is mismatch, i.e. a port offload not
>>> repeated in queue setup.
>>> There is a proposal to do this check at ethdev level:
>>> 	http://dpdk.org/ml/archives/dev/2018-March/094023.html
>>>
>>> It was also proposed to relax the API and allow "forgetting" port
>>> offloads in queue offloads:
>>> 	http://dpdk.org/ml/archives/dev/2018-March/092978.html
>>>
>>> It would mean the offloads applied to a queue result of OR operation:
>>> 	rte_eth_conf.[rt]xmode.offloads | rte_eth_[rt]xconf.offloads
>>>
>>> 1/ Do you agree with above API change?
>>
>> There is a detail of ability to disabling queue level offloads in queue_setup()
>> function, I want to discuss here.
>>
>> Prolog:
>> port level offload: An offload only can be applied port level, to all queues.
>> queue level offload: An offload can be applied into individual queues of the port
>>
>> PMD reports port offload capability: port level offload + queue level offload
>> PMD reports queue offload capability: queue level offload
>>
>>
>> Above suggested change to API:
>> - Application will be limited in configure() to set only an offload within "port
>> offload capability"
> 
> "limited" is not the right word, given port offload capability reports also
> queue level offload capability.
> 
>> - Application will be limited in queue_setup() to set only an offload within
>> "queue offload capability"
> 
> Yes
> 
>> This doesn't say much about disabling an offload in queue_setup(), as a rule:
>> - An "port level offload" can't be disabled in queue_setup()
> 
> Yes
> 
>> There are two cases of disable:
>> 1- Disabling a "queue level offload" enabled queue_setup() previously
>> 2- Disabling a "queue level offload" enabled in configure()
>>
>> If second is not supported, to disable the offload, applications should
>> stop->re-configure()->re-queue_setup()->start the port. But having this
>> capability makes the offloading parameters more confusing for applications.
> 
> I don't understand the last sentence.
> 
>> I suggest adding disable support to fist one but not second one.
> 
> Yes, it is the item 3 of the survey.

Yes indeed.

> 
>> According this,
>> application:
>> - In configure() set offload within "port offload capability"
>> - In queue_setup() set offload within "queue offload capability". Offloads are
>> incremental to ones in configure()
>>
>> PMDs:
>> - In configure() verify the offload against "port offload capability"
>> - In queue_setup() verify the offload against "queue offload capability"
> 
> At ethdev level, we should filter out the offloads already enabled at port level,
> before calling the queue setup op.

Above two steps can be moved to ethdev layer, agreed. But I guess you are
talking about something else, can you please detail?

> 
>> - In queue_setup() if requested offload is not enabled already, enable it for queue
>> - In queue_setup() if an offload value cleared in requested offload that is set
>> in port_offload, return error.
> 
> No
> The item 1 of the survey is about allow "forgetting" port offloads.
> If offload is enabled at port level, and not repeated in queue setup,
> nothing happen. It stays enabled at port level.

Right.

> 
>> - In queue_setup() if an offload value cleared in requested offload that is not
>> set in port_offload but set in queue_offload, disable it for that queue.
> 
> Yes

OK, good to agree on this, this was the main topic of this email.

> 
> 
>> Samples according initial suggestion + disable support:
>>
>> Sample 1:
>> port level offload: A, B
>> queue level offload: C, D
>> port offload capability: A, B, C, D
>> queue offload capability: C, D
>>
>> configure(A,C): Q1:A,C  Q2:A,C [queue_setup() can't disable A,C after this]
>> queue_setup(Q1, B): --> Error [Can't enable port level offload in queue_setup()]
>> queue_setup(Q1, D): Q1:A,C,D
>> queue_setup(Q1, ""): Q1:A,C [Disabled D]
> 
> Yes we can disable a queue offload.
> 
>> queue_setup(Q2, "C,D"): Q2:A,C,D
>> queue_setup(Q2, ""): Q2:A,C
> 
> Yes we cannot disable a port offload.
> 
>> queue_setup(Q2, A): --> Error [A is port_level offload]
> 
> No, it is the same as queue_setup(Q2, "C,D").
> We can repeat an already enabled port offload in queue setup.

You are right, no need to return error here, since it is already enabled for queue.

> 
> 
>> Sample 2:
>> port level offload: A, B, C
>> queue level offload: ""
>> port offload capability: A, B, C
>> queue offload capability: ""   [no way to change offloads in queue level]
>>
>> configure(A,C): Q1:A,C  Q2:A,C
>> queue_setup(Q1, B): --> Error
>> queue_setup(Q1, A): --> Error
> 
> No, we can repeat an already enabled port offload in queue setup.

Right.

> 
>> queue_setup(Q2, ""): Q2:A,C
>> queue_setup(Q1, ""): Q1:A,C
>>
>>
>> Sample 3:
>> port level offload: ""
>> queue level offload: A, B, C, D
>> port offload capability: A, B, C, D
>> queue offload capability: A, B, C, D
>>
>> configure(A): Q1:A  Q2:A
>> queue_setup(Q1, A): Q1:A
>> queue_setup(Q1, ""): Q1:A
>> queue_setup(Q1, A,B,C,D): Q1:A,B,C,D
>> queue_setup(Q1, B): Q1:A,B [Disable C,D]
> 
> Yes
> 
>> queue_setup(Q2, C): Q2:A,C
>> queue_setup(Q1, ""): Q1:A [Disable B]
>> queue_setup(Q2, ""): Q2:A [Disable C]
> 
> Yes
> 
> 
>