Re: [dpdk-dev] [PATCH v4 0/6] ethdev: introduce shared Rx queue

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 10/12/21 1:55 PM, Xueming(Steven) Li wrote:
> On Tue, 2021-10-12 at 11:48 +0300, Andrew Rybchenko wrote:
>> On 10/12/21 9:37 AM, Xueming(Steven) Li wrote:
>>> On Mon, 2021-10-11 at 23:11 +0800, Xueming Li wrote:
>>>> On Mon, 2021-10-11 at 14:49 +0300, Andrew Rybchenko wrote:
>>>>> Hi Xueming,
>>>>>
>>>>> On 9/30/21 5:55 PM, Xueming Li wrote:
>>>>>> In current DPDK framework, all RX queues is pre-loaded with mbufs for
>>>>>> incoming packets. When number of representors scale out in a switch
>>>>>> domain, the memory consumption became significant. Further more,
>>>>>> polling all ports leads to high cache miss, high latency and low
>>>>>> throughputs.
>>>>>>  
>>>>>> This patch introduces shared RX queue. PF and representors with same
>>>>>> configuration in same switch domain could share RX queue set by
>>>>>> specifying shared Rx queue offloading flag and sharing group.
>>>>>>
>>>>>> All ports that Shared Rx queue actually shares One Rx queue and only
>>>>>> pre-load mbufs to one Rx queue, memory is saved.
>>>>>>
>>>>>> Polling any queue using same shared RX queue receives packets from all
>>>>>> member ports. Source port is identified by mbuf->port.
>>>>>>
>>>>>> Multiple groups is supported by group ID. Port queue number in a shared
>>>>>> group should be identical. Queue index is 1:1 mapped in shared group.
>>>>>> An example of polling two share groups:
>>>>>>   core	group	queue
>>>>>>   0	0	0
>>>>>>   1	0	1
>>>>>>   2	0	2
>>>>>>   3	0	3
>>>>>>   4	1	0
>>>>>>   5	1	1
>>>>>>   6	1	2
>>>>>>   7	1	3
>>>>>>
>>>>>> Shared RX queue must be polled on single thread or core. If both PF0 and
>>>>>> representor0 joined same share group, can't poll pf0rxq0 on core1 and
>>>>>> rep0rxq0 on core2. Actually, polling one port within share group is
>>>>>> sufficient since polling any port in group will return packets for any
>>>>>> port in group.
>>>>>
>>>>> I apologize that I jump in into the review process that late.
>>>>
>>>> Appreciate the bold suggestion, never too late :)
>>>>
>>>>>
>>>>> Frankly speaking I doubt that it is the best design to solve
>>>>> the problem. Yes, I confirm that the problem exists, but I
>>>>> think there is better and simpler way to solve it.
>>>>>
>>>>> The problem of the suggested solution is that it puts all
>>>>> the headache about consistency to application and PMDs
>>>>> without any help from ethdev layer to guarantee the
>>>>> consistency. As the result I believe it will be either
>>>>> missing/lost consistency checks or huge duplication in
>>>>> each PMD which supports the feature. Shared RxQs must be
>>>>> equally configured including number of queues, offloads
>>>>> (taking device level Rx offloads into account), RSS
>>>>> settings etc. So, applications must care about it and
>>>>> PMDs (or ethdev layer) must check it.
>>>>
>>>> The name might be confusing, here is my understanding:
>>>> 1. NIC  shares the buffer supply HW Q between shared RxQs - for memory
>>>> 2. PMD polls one shared RxQ - for latency and performance
>>>> 3. Most per queue features like offloads and RSS not impacted. That's
>>>> why this not mentioned. Some offloading might not being supported due
>>>> to PMD or hw limitation, need to add check in PMD case by case.
>>>> 4. Multiple group is defined for service level flexibility. For
>>>> example, PF and VIP customer's load distributed via queues and dedicate
>>>> cores. Low priority customers share one core with one shared queue.
>>>> multiple groups enables more combination.
>>>> 5. One port could assign queues to different group for polling
>>>> flexibility. For example first 4 queues in group 0 and next 4 queues in
>>>> group1, each group have other member ports with 4 queues, so the port
>>>> with 8 queues could be polled with 8 cores w/o non-shared rxq penalty,
>>>> in other words, each core only poll one shared RxQ.
>>>>
>>>>>
>>>>> The advantage of the solution is that any device may
>>>>> create group and subsequent devices join. Absence of
>>>>> primary device is nice. But do we really need it?
>>>>> Will the design work if some representors are configured
>>>>> to use shared RxQ, but some do not? Theoretically it
>>>>> is possible, but could require extra non-trivial code
>>>>> on fast path.
>>>>
>>>> If multiple groups, any device could be hot-unplugged.
>>>>
>>>> Mixed configuration is supported, the only difference is how to set
>>>> mbuf->port. Since group is per queue, mixed is better to be supported,
>>>> didn't see any difficulty here.
>>>>
>>>> PDM could select to support only group 0, same settings for each rxq,
>>>> that fits most scenario.
>>>>
>>>>>
>>>>> Also looking at the first two patch I don't understand
>>>>> how application will find out which devices may share
>>>>> RxQs. E.g. if we have two difference NICs which support
>>>>> sharing, we can try to setup only one group 0, but
>>>>> finally will have two devices (not one) which must be
>>>>> polled.
>>>>>
>>>>> 1. We need extra flag in dev_info->dev_capa
>>>>>    RTE_ETH_DEV_CAPA_RX_SHARE to advertise that
>>>>>    the device supports Rx sharing.
>>>>
>>>> dev_info->rx_queue_offload_capa could be used here, no?
>>
>> It depends. But we definitely need a flag which
>> says that below rx_domain makes sense. It could be
>> either RTE_ETH_DEV_CAPA_RX_SHARE or an Rx offload
>> capability.
>>
>> The question is if it is really an offload. The offload is
>> when something could be done by HW/FW and result is provided
>> to SW. May be it is just a nit picking...
>>
>> May be we don't need an offload at all. Just have
>> RTE_ETH_DEV_CAPA_RXQ_SHARE and use non-zero group ID
>> as a flag that an RxQ should be shared (zero - default,
>> no sharing). ethdev layer may check consistency on
>> its layer to ensure that the device capability is
>> reported if non-zero group is specified on queue setup.
>>
>>>>
>>>>>
>>>>> 2. I think we need "rx_domain" in device info
>>>>>    (which should be treated in boundaries of the
>>>>>    switch_domain) if and only if
>>>>>    RTE_ETH_DEV_CAPA_RX_SHARE is advertised.
>>>>>    Otherwise rx_domain value does not make sense.
>>>>
>>>> I see, this will give flexibility of different hw, will add it.
>>>>
>>>>>
>>>>> (1) and (2) will allow application to find out which
>>>>> devices can share Rx.
>>>>>
>>>>> 3. Primary device (representors backing device) should
>>>>>    advertise shared RxQ offload. Enabling of the offload
>>>>>    tells the device to provide packets to all device in
>>>>>    the Rx domain with mbuf->port filled in appropriately.
>>>>>    Also it allows app to identify primary device in the
>>>>>    Rx domain. When application enables the offload, it
>>>>>    must ensure that it does not treat used port_id as an
>>>>>    input port_id, but always check mbuf->port for each
>>>>>    packet.
>>>>>
>>>>> 4. A new Rx mode should be introduced for secondary
>>>>>    devices. It should not allow to configure RSS, specify
>>>>>    any Rx offloads etc. ethdev must ensure it.
>>>>>    It is an open question right now if it should require
>>>>>    to provide primary port_id. In theory representors
>>>>>    have it. However, may be it is nice for consistency
>>>>>    to ensure that application knows that it does.
>>>>>    If shared Rx mode is specified for device, application
>>>>>    does not need to setup RxQs and attempts to do it
>>>>>    should be discarded in ethdev.
>>>>>    For consistency it is better to ensure that number of
>>>>>    queues match.
>>>>
>>>> RSS and Rx offloads should be supported as individual, PMD needs to
>>>> check if not supported.
>>
>> Thinking a bit more about it I agree that RSS settings could
>> be individual. Offload could be individual as well, but I'm
>> not sure about all offloads. E.g. Rx scatter which is related
>> to Rx buffer size (which is shared since Rx mempool is shared)
>> vs MTU. May be it is acceptable. We just must define rules
>> what should happen if offloads contradict to each other.
>> It should be highlighted in the description including
>> driver callback to ensure that PMD maintainers are responsible
>> for consistency checks.
>>
>>>>
>>>>>    It is an interesting question what should happen if
>>>>>    primary device is reconfigured and shared Rx is
>>>>>    disabled on reconfiguration.
>>>>
>>>> I feel better no primary port/queue assumption in configuration, all
>>>> members are equally treated, each queue can join or quit share group,
>>>> that's important to support multiple groups.
>>
>> I agree. The problem of many flexible solutions is
>> complexity to support. We'll see how it goes.
>>
>>>>
>>>>>
>>>>> 5. If so, in theory implementation of the Rx burst
>>>>>    in the secondary could simply call Rx burst on
>>>>>    primary device.
>>>>>
>>>>> Andrew.
>>>>
>>>
>>> Hi Andrew,
>>>
>>> I realized that we are talking different things, this feature
>>> introduced 2 RxQ share:
>>> 1. Share mempool to save memory
>>> 2. Share polling to save latency
>>>
>>> What you suggested is reuse all RxQ configuration IIUC, maybe we should
>>> break the flag into 3, so application could learn PMD capability and
>>> configure accordingly, how do you think?
>>> RTE_ETH_RX_OFFLOAD_RXQ_SHARE_POOL
>>
>> Not sure that I understand. Just specify the same mempool
>> on Rx queue setup. Isn't it sufficient?
>>
>>> RTE_ETH_RX_OFFLOAD_RXQ_SHARE_POLL
>>
>> It implies pool sharing if I'm not mistaken. Of course,
>> we can pool many different HW queues in one poll, but it
>> hardly makes sense to care specially about it.
>> IMHO RxQ sharing is a sharing of the underlying HW Rx queue.
>>
>>> RTE_ETH_RX_OFFLOAD_RXQ_SHARE_CFG //implies POOL and POLL
>>
>> It is hardly a feature. Rather a possible limitation.
> 
> Thanks, then I'd drop this suggestion then.
> 
> Here is the TODO list, let me know if anything missing:
> 1. change offload flag to RTE_ETH_DEV_CAPA_RX_SHARE

RTE_ETH_DEV_CAPA_RXQ_SHARE since it is not sharing of
entire Rx, but just some queues.

> 2. RxQ share group check in ethdev
> 3. add rx_domain into device info
> 
>>
>> Andrew.
>