> >> -----Original Message----- > >> From: Stokes, Ian [mailto:ian.sto...@intel.com] > >> Sent: Thursday, November 23, 2017 3:30 PM > >> To: aserd...@ovn.org; 'Kevin Traynor' <ktray...@redhat.com>; O > >> Mahony, Billy <billy.o.mah...@intel.com>; d...@openvswitch.org > >> Subject: RE: [ovs-dev] [RFC PATCH v2] dpif-netdev: Add port/queue > >> tiebreaker to rxq_cycle_sort. > >> > >>>> -----Original Message----- > >>>> From: Kevin Traynor [mailto:ktray...@redhat.com] > >>>> Sent: Thursday, November 23, 2017 1:05 PM > >>>> To: O Mahony, Billy <billy.o.mah...@intel.com>; > >>>> d...@openvswitch.org; aserd...@ovn.org > >>>> Subject: Re: [ovs-dev] [RFC PATCH v2] dpif-netdev: Add port/queue > >>>> tiebreaker to rxq_cycle_sort. > >>>> > >>>> On 11/23/2017 10:42 AM, O Mahony, Billy wrote: > >>>>> Hi Kevin, > >>>>> > >>>>> My 2c below.. > >>>>> > >>>>> /Billy > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > >>>>>> boun...@openvswitch.org] On Behalf Of Kevin Traynor > >>>>>> Sent: Wednesday, November 22, 2017 7:11 PM > >>>>>> To: d...@openvswitch.org; aserd...@ovn.org > >>>>>> Subject: [ovs-dev] [RFC PATCH v2] dpif-netdev: Add port/queue > >>>>>> tiebreaker to rxq_cycle_sort. > >>>>>> > >>>>>> rxq_cycle_sort is used to sort the rx queues by their measured > >>>>>> number of > >>>> cycles. > >>>>>> In the event that they are equal 0 could be returned. > >>>>>> However, it is observed that returning 0 results in a different > >>>>>> sort order on Windows/Linux. This is ok in practice but it causes > >>>>>> a unit test failure for > >>>>>> "1007: PMD - pmd-cpu-mask/distribution of rx queues" on Windows. > >>>>>> > >>>>>> In order to have a consistent sort result, introduce a tiebreaker > >>>>>> of > >>>> port/queue. > >>>>>> > >>>>>> Fixes: 655856ef39b9 ("dpif-netdev: Change rxq_scheduling to use > >>>>>> rxq processing > >>>>>> cycles.") > >>>>>> Reported-by: Alin Gabriel Serdean <aserd...@ovn.org> > >>>>>> Signed-off-by: Kevin Traynor <ktray...@redhat.com> > >>>>>> --- > >>>>>> > >>>>>> v2: Inadvertently reversed the order for non-tiebreak cases in v1. > >>> Fix that. > >>>>>> > >>>>>> lib/dpif-netdev.c | 15 ++++++++++++--- > >>>>>> 1 file changed, 12 insertions(+), 3 deletions(-) > >>>>>> > >>>>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index > >>>>>> 0a62630..57451e9 100644 > >>>>>> --- a/lib/dpif-netdev.c > >>>>>> +++ b/lib/dpif-netdev.c > >>>>>> @@ -3452,4 +3452,5 @@ rxq_cycle_sort(const void *a, const void *b) > >>>>>> uint64_t total_qa, total_qb; > >>>>>> unsigned i; > >>>>>> + int winner = 1; > >>>>>> > >>>>>> qa = *(struct dp_netdev_rxq **) a; @@ -3464,8 +3465,16 @@ > >>>>>> rxq_cycle_sort(const void *a, const void *b) > >>>>>> dp_netdev_rxq_set_cycles(qb, RXQ_CYCLES_PROC_HIST, > >>>>>> total_qb); > >>>>>> > >>>>> [[BO'M]] > >>>>> I think it's worth adding a comment to state this compare function > >>>>> never > >>>> return's 0 and that the reason for this (the sort order difference > >>>> on different OSs). > >>>>> > >>>> > >>>> Sure, I will add this for PATCH version. > >>>> > >>>>> Also the function should probably be called rxq_cycle_compare as > >>>>> its really > >>>> comparing entries rather than sorting a list of entries And it's > >>>> used as the compar arg to qsort. > >>>>> > >>>> > >>>> yeah, I guess it would be a better name. It would be a separate > >>>> patch as it's an unrelated change, but I might submit at same time > >>>> as making > >>> this PATCH. > >>>> > >>>> thanks, > >>>> Kevin. > >>>> > >>> [Alin Serdean] I applied the patch and the test passes now. Thanks a > >>> lot Kevin. > >> > >> Thanks for testing on windows Alin, I'll add a Tested-by tag for you > >> on the PATCH version at commit if that's ok with you? > > Sure 😊. > > Alin. > > Thanks for testing Alin. I've added the Tested-by: tag in the PATCH > version just sent. > > I've moved these return value changes back into the other balance related > patchset that I split them out from, as now we know it fixes the Windows > unit test and as independent patches for master they cause conflicts with > each other. Grouping them together means I could rebase the series so they > will all apply in order.
OK > > Kevin. > > >> > >> Ian > > > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev