[ovs-dev] [PATCH] Extends the existing mirror configuration parameters

[Maxime Coquelin]

Hi Gaetan,

On 5/24/21 1:26 AM, Gaëtan Rivet wrote:
> On Wed, May 19, 2021, at 09:55, Maxime Coquelin wrote:
>> Hi Liang-min,
>>
>> When replying inline, please do not prefix with ">>" as it is handled as
>> quoted text. There is no need to prefix.
>>
>> On 5/18/21 8:00 PM, Wang, Liang-min wrote:
>>>> -----Original Message-----
>>>> From: Maxime Coquelin <maxime.coquelin at redhat.com>
>>>> Sent: Tuesday, May 18, 2021 12:15 PM
>>>> To: Miskell, Timothy <timothy.miskell at intel.com>; dev at openvswitch.org
>>>> Cc: Wang, Liang-min <liang-min.wang at intel.com>
>>>> Subject: Re: [PATCH] Extends the existing mirror configuration parameters
>>>>
>> [...]
>>>>> +            pkt_buf[match_count]->vlan_tci = dst_vlan_id;
>>>>> +            rte_mbuf_refcnt_update(pkt_buf[match_count], 1);
>>>>
>>>>
>>>>
>>>>> +            match_count++;
>>>>> +        }
>>>>> +    }
>>>>> +
>>>>> +    dst_qidx = (data->n_dst_queue > qidx)?qidx:(data->n_dst_queue -1);
>>>>
>>>> Wouldn't it scale better with:
>>>> dst_qidx = qidx % data->n_dst_queue
>>>> ?
>>>>
>>>>> We tried to avoid using "%" operator. We could add "unlikely" and the suggested "%" to make improvement
>>
>> Not sure adding 'unlikely' is really necessary. The cost of the modulo
>> operation is nothing compared to all we do in this path.
>>
> 
> Hi,
> 
> Although the modulo might well be nothing compared to the rest,
> an alternative is to use Lemire's fastrange: https://github.com/lemire/fastrange
> Here is the uint32_t version:
> 
> /*
> * Given a value "word", produces an integer in [0,p) without division.
> * The function is as fair as possible in the sense that if you iterate
> * through all possible values of "word", then you will generate all
> * possible outputs as uniformly as possible.
> */
> static inline uint32_t
> fastrange32(uint32_t word, uint32_t p) {
>     return (uint32_t)(((uint64_t)word * (uint64_t)p) >> 32);
> }
> 
> It should be as fair as the modulo.

Interresting, note that modulo is used to distribute on Vhost Tx queues:
https://github.com/openvswitch/ovs/blob/13c0eaa7b4fc2694a8c6cc8e6487ec6538c607e4/lib/netdev-dpdk.c#L2601

Maxime

> Kind regards,
>