[ovs-dev] [PATCH] dpif-netdev: proper tx queue id

Daniele Di Proietto diproiettod at vmware.com
Wed Aug 5 16:26:32 UTC 2015



On 05/08/2015 16:42, "Ilya Maximets" <i.maximets at samsung.com> wrote:

>Sorry, I agree that example is incorrect. It is really not true, because
>of using ovs_numa_get_n_cores() to call netdev_set_multiq().
>No, I didn't actually observe a bug.
>
>But there is another example:
>
>	same configuration(2 pmd threads with 1 port,
>	2 rxqs per port and pmd-cpu-mask = 00000014).
>
>	pmd_1->tx_qid = 2, pmd_2->tx_qid = 4,
>	txq_needs_locking = true (if device hasn't ovs_numa_get_n_cores() queues)
>
>	Lets netdev->real_n_txq = 2; (device has 2 queues)
>
>	In that case, after truncating in netdev_dpdk_send__()
>		'qid = qid % dev->real_n_txq;'
>	pmd_1: qid = 2 % 2 = 0
>	pmd_2: qid = 4 % 2 = 0
>
>	So, both threads will call dpdk_queue_pkts() with same qid = 0.
>	This is unexpected behavior if there is 2 tx queues in device.
>	Queue #1 will not be used and both threads will lock queue #0
>	on each send.

Yes, that is true.  In general it is hard to properly distribute
transmission queues because potentially every pmd thread can send
a packet to any netdev.

I agree that we can do better than this. Currently we create a txq
for every core (not for every pmd thread), because we don't want
to reconfigure every device when a new thread is added (I rembember
discussing this with Alex, perhaps he can provide more insight).
We can always change the code to create one txq for every pmd
thread (I'd appreciate other opinions on this).

>
>About your example:
>	2 pmd threads can't call netdev_send() with same tx_qid,
>	because pmd->tx_qid = pmd->core_id and there is only one thread
>	with core_id = 0. See dp_netdev_configure_pmd().
>
>	So,
>	pmd1 will call netdev_send(netdev=dpdk0, tx_qid= *pmd1->core_id* )
>	pmd2 will call netdev_send(netdev=dpdk0, tx_qid= *pmd2->core_id* )

I agree, on current master they can't. I meant with this patch applied.

>
>	
>On 05.08.2015 17:54, Daniele Di Proietto wrote:
>> 
>> 
>> On 05/08/2015 13:28, "Ilya Maximets" <i.maximets at samsung.com> wrote:
>> 
>>> Currently tx_qid is equal to pmd->core_id. This leads to wrong
>>> behavior if pmd-cpu-mask different from '/(0*)(1|3|7)?(f*)/',
>>> e.g. if core_ids are not sequential, or doesn't start from 0, or both.
>>>
>>> Example(just one of possible wrong scenarios):
>>>
>>> 	starting 2 pmd threads with 1 port, 2 rxqs per port and
>>> 	pmd-cpu-mask = 00000014.
>>>
>>> 	It that case pmd_1->tx_qid = 2, pmd_2->tx_qid = 4 and
>>> 	txq_needs_locking = false (if device has 2 queues).
>>>
>>> 	While netdev_dpdk_send__() qid will not be truncated and
>>> 	dpdk_queue_pkts() will be called for nonexistent queues (2 and 4).
>> 
>> This shouldn't be possible, because the datapath requests one txq
>> for each core in the system, not for each core in pmd-cpu-mask
>> (see the calls to netdev_set_multiq() in dpif-netdev.c).
>> 
>> Did you actually observe a bug or an unexpected behaviour?
>> 
>> I didn't read the patch carefully (I want to understand the problem
>>first),
>> but it appears that two pmd threads could call netdev_send on the same
>> port, with the same tx_qid concurrently.  Example:
>> 
>>     pmd1 is processing dpdk0 with rx_qid 0, pmd2 is processing dpdk1
>>with
>>     rx_qid 0.
>> 
>>     The flow table is configured to send everything to dpdk0.
>> 
>>     pmd1 will call netdev_send(netdev=dpdk0, tx_qid=0)
>>     pmd2 will call netdev_send(netdev=dpdk0, tx_qid=0)
>>     
>>     these calls can happen concurrently
>> 
>>>
>>> Fix that by calculating tx_qid from rxq indexes for each rxq
>>>separately.
>>> 'rxq_poll' structure supplemented by tx_qid and renamed to 'q_poll'.
>>> 'poll_list' moved inside dp_netdev_pmd_thread structure to be able
>>> to get proper tx_qid for current port while calling netdev_send().
>>> Also, information about queues of each thread added to log.
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
>>> ---
>>> lib/dpif-netdev.c | 102
>>> ++++++++++++++++++++++++++----------------------------
>>> lib/netdev.c      |   6 ++++
>>> lib/netdev.h      |   1 +
>>> 3 files changed, 57 insertions(+), 52 deletions(-)




More information about the dev mailing list