Hi Kevin, My 2c below..
/Billy > -----Original Message----- > From: [email protected] [mailto:ovs-dev- > [email protected]] On Behalf Of Kevin Traynor > Sent: Wednesday, November 22, 2017 7:11 PM > To: [email protected]; [email protected] > 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 <[email protected]> > Signed-off-by: Kevin Traynor <[email protected]> > --- > > 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). 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. > - if (total_qa >= total_qb) { > - return -1; > + if (total_qa > total_qb) { > + winner = -1; > + } else if (total_qa == total_qb) { > + /* Cycles are the same. Tiebreak on port/queue id. */ > + if (qb->port->port_no > qa->port->port_no) { > + winner = -1; > + } else if (qa->port->port_no == qb->port->port_no) { > + winner = netdev_rxq_get_queue_id(qa->rx) > + - netdev_rxq_get_queue_id(qb->rx); > + } > } > - return 1; > + return winner; > } > > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
