On 15/04/17 06:33, Ben Pfaff wrote:
On Wed, Mar 22, 2017 at 11:10:23AM +0100, Eelco Chaudron wrote:
The "ovs-appctl dpif-netdev/pmd-rxq-show" and "ovs-appctl
dpif-netdev/pmd-stats-show" commands show their output per core_id,
sorted on the hash location. My OCD was kicking in when using these
commands, hence this change to display them in natural core_id order.

In addition I had to change a test case that would fail if the cores
where not in order in the hash list. This is due to OVS assigning
queues to cores based on the order in the hash list. The test case now
checks if any core has the set of queues in the given order.

Manually tested this on my setup, and ran clang-analyze.

Signed-off-by: Eelco Chaudron <echau...@redhat.com>
This should be helpful, thanks.

sorted_poll_thread_list() worries me.  It calls cmap_count() and then
uses CMAP_FOR_EACH to iterate the list and assumes (asserts, even) that
the number of pmds does not change during iteration.  Is this safe?
If it is, then a clearer comment would be helpful, and it would be wise
to have an assertion for the exact number at the end.

Thanks,

Ben.

Hi Ben,

To be honest I copied it from a couple of functions above, sorted_poll_list(),
without giving it the proper attention.

So when looking at it more in depth, I see it needs to be handled differently. Creation/deletion of the threads is protected with the dp->port_mutex, however the show commands are only protected by the dp_netdev_mutex. So in theory both activities can happen in parallel. So to avoid an assert, I guess just printing
data in transition is safer.

I guess the following change should fix it:

    CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
-       ovs_assert(k < n_pmds);
+       if (k >= n_pmds) {
+           break;
+       }
        pmd_list[k++] = pmd;
    }

In addition I also changed the calling function, and made it safe in case the
ltist shrunk:

  sorted_poll_thread_list(dp, &pmd_list, &n);
    for (i = 0; i < n; i++) {
        pmd = pmd_list[i];
+       if (!pmd) {
+           break;
+       }

        if (type == PMD_INFO_SHOW_RXQ) {


Let me know if you want another patch?

Cheers,

Eelco


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to