From 347ba9bb9b7a8e053ced54016f903e749ebecb7f Mon Sep 17 00:00:00 2001 From: Ilya Maximets Date: Tue, 26 Jan 2016 09:12:34 +0300 Subject: [PATCH] dpif-netdev: Unique and sequential tx_qids. Currently tx_qid is equal to pmd->core_id. This leads to unexpected 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: starting 2 pmd threads with 1 port, 2 rxqs per port, pmd-cpu-mask = 00000014 and let dev->real_n_txq = 2 It that case pmd_1->tx_qid = 2, pmd_2->tx_qid = 4 and txq_needs_locking = true (if device hasn't ovs_numa_get_n_cores()+1 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. Fix that by using sequential tx_qids. Signed-off-by: Ilya Maximets Signed-off-by: Daniele Di Proietto --- lib/dpif-netdev.c | 56 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 8c87c0532..ad6b2024d 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -434,7 +434,7 @@ struct dp_netdev_pmd_thread { /* threads on same numa node. */ unsigned core_id; /* CPU core id of this pmd thread. */ int numa_id; /* numa node id of this pmd thread. */ - int tx_qid; /* Queue id used by this pmd thread to + atomic_int tx_qid; /* Queue id used by this pmd thread to * send packets on all netdevs */ struct ovs_mutex poll_mutex; /* Mutex for poll_list. */ @@ -1284,6 +1284,13 @@ get_port_by_name(struct dp_netdev *dp, return ENOENT; } +static int +get_n_pmd_threads(struct dp_netdev *dp) +{ + /* There is one non pmd thread in dp->poll_threads */ + return cmap_count(&dp->poll_threads) - 1; +} + static int get_n_pmd_threads_on_numa(struct dp_netdev *dp, int numa_id) { @@ -2820,16 +2827,6 @@ dp_netdev_pmd_get_next(struct dp_netdev *dp, struct cmap_position *pos) return next; } -static int -core_id_to_qid(unsigned core_id) -{ - if (core_id != NON_PMD_CORE_ID) { - return core_id; - } else { - return ovs_numa_get_n_cores(); - } -} - /* Configures the 'pmd' based on the input argument. */ static void dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, @@ -2838,10 +2835,14 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, pmd->dp = dp; pmd->index = index; pmd->core_id = core_id; - pmd->tx_qid = core_id_to_qid(core_id); pmd->numa_id = numa_id; pmd->poll_cnt = 0; + atomic_init(&pmd->tx_qid, + (core_id == NON_PMD_CORE_ID) + ? ovs_numa_get_n_cores() + : get_n_pmd_threads(dp)); + ovs_refcount_init(&pmd->ref_cnt); latch_init(&pmd->exit_latch); atomic_init(&pmd->change_seq, PMD_INITIAL_SEQ); @@ -2919,17 +2920,40 @@ dp_netdev_destroy_all_pmds(struct dp_netdev *dp) } } -/* Deletes all pmd threads on numa node 'numa_id'. */ +/* Deletes all pmd threads on numa node 'numa_id' and + * fixes tx_qids of other threads to keep them sequential. */ static void dp_netdev_del_pmds_on_numa(struct dp_netdev *dp, int numa_id) { struct dp_netdev_pmd_thread *pmd; + int n_pmds_on_numa, n_pmds; + int *free_idx, k = 0; + + n_pmds_on_numa = get_n_pmd_threads_on_numa(dp, numa_id); + free_idx = xmalloc(n_pmds_on_numa * sizeof *free_idx); CMAP_FOR_EACH (pmd, node, &dp->poll_threads) { if (pmd->numa_id == numa_id) { + atomic_read_relaxed(&pmd->tx_qid, &free_idx[k]); + k++; dp_netdev_del_pmd(dp, pmd); } } + + n_pmds = get_n_pmd_threads(dp); + CMAP_FOR_EACH (pmd, node, &dp->poll_threads) { + int old_tx_qid; + + atomic_read_relaxed(&pmd->tx_qid, &old_tx_qid); + + if (old_tx_qid >= n_pmds) { + int new_tx_qid = free_idx[--k]; + + atomic_store_relaxed(&pmd->tx_qid, new_tx_qid); + } + } + + free(free_idx); } /* Returns PMD thread from this numa node with fewer rx queues to poll. @@ -3571,7 +3595,11 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt, case OVS_ACTION_ATTR_OUTPUT: p = dp_netdev_lookup_port(dp, u32_to_odp(nl_attr_get_u32(a))); if (OVS_LIKELY(p)) { - netdev_send(p->netdev, pmd->tx_qid, packets, cnt, may_steal); + int tx_qid; + + atomic_read_relaxed(&pmd->tx_qid, &tx_qid); + + netdev_send(p->netdev, tx_qid, packets, cnt, may_steal); return; } break; -- 2.20.1