I sent a v3 that uses an alternative mapping computed when the
pmd thread is created. I hope it's clearer
Thanks!
On 02/06/2015 18:33, "Flavio Leitner" <f...@sysclose.org> wrote:
>On Fri, May 29, 2015 at 12:42:00PM +0100, Mark D. Gray wrote:
>> From: Daniele Di Proietto <diproiet...@vmware.com>
>>
>> Non pmd threads have a core_id == UINT32_MAX, while queue ids used by
>> netdevs range from 0 to the number of CPUs. Therefore core ids cannot
>> be used directly to select a queue.
>>
>> This commit introduces a simple mapping to fix the problem: non pmd
>> threads use queue 0, pmd threads on core 0 to N use queues 1 to N+1
>>
>> Fixes: d5c199ea7ff7 ("netdev-dpdk: Properly support non pmd threads.")
>>
>> Reported-by: 차은호 <eunho....@atto-research.com
>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>> Signed-off-by: Mark D. Gray <mark.d.g...@intel.com>
>> ---
>> lib/dpif-netdev.c | 20 ++++++++++++++++----
>> lib/netdev-dpdk.c | 10 ++++++----
>> 2 files changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 76d1003..de700f7 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -1068,7 +1068,7 @@ do_add_port(struct dp_netdev *dp, const char
>>*devname, const char *type,
>> }
>> /* There can only be ovs_numa_get_n_cores() pmd threads,
>> * so creates a txq for each. */
>> - error = netdev_set_multiq(netdev, n_cores, dp->n_dpdk_rxqs);
>> + error = netdev_set_multiq(netdev, n_cores + 1,
>>dp->n_dpdk_rxqs);
>> if (error && (error != EOPNOTSUPP)) {
>> VLOG_ERR("%s, cannot set multiq", devname);
>> return errno;
>> @@ -2402,7 +2402,8 @@ dpif_netdev_pmd_set(struct dpif *dpif, unsigned
>>int n_rxqs, const char *cmask)
>> }
>>
>> /* Sets the new rx queue config. */
>> - err = netdev_set_multiq(port->netdev,
>>ovs_numa_get_n_cores(),
>> + err = netdev_set_multiq(port->netdev,
>> + ovs_numa_get_n_cores() + 1,
>> n_rxqs);
>> if (err && (err != EOPNOTSUPP)) {
>> VLOG_ERR("Failed to set dpdk interface %s rx_queue
>>to:"
>> @@ -3328,8 +3329,18 @@ dpif_netdev_register_upcall_cb(struct dpif
>>*dpif, upcall_callback *cb,
>> dp->upcall_cb = cb;
>> }
>>
>> +static int
>> +core_id_to_qid(unsigned core_id)
>> +{
>> + if (core_id != NON_PMD_CORE_ID) {
>> + return core_id + 1;
>> + } else {
>> + return 0;
>> + }
>> +}
>> +
>> static void
>> -dp_netdev_drop_packets(struct dp_packet ** packets, int cnt, bool
>>may_steal)
>> +dp_netdev_drop_packets(struct dp_packet **packets, int cnt, bool
>>may_steal)
>> {
>> if (may_steal) {
>> int i;
>> @@ -3387,7 +3398,8 @@ 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->core_id, packets, cnt,
>>may_steal);
>> + netdev_send(p->netdev, core_id_to_qid(pmd->core_id),
>>packets, cnt,
>> + may_steal);
>> return;
>> }
>> break;
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 63243d8..1d20d98 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -892,9 +892,11 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,
>>struct dp_packet **packets,
>> int nb_rx;
>>
>> /* There is only one tx queue for this core. Do not flush other
>> - * queueus. */
>> + * queues. */
>> if (rxq_->queue_id == rte_lcore_id()) {
>> - dpdk_queue_flush(dev, rxq_->queue_id);
>> + /* TX queue 0 is the nonpmd queue. Each core owns TX queue
>> + * number = core_id + 1 */
>> + dpdk_queue_flush(dev, rxq_->queue_id + 1);
>
>I found this patch a bit hard to follow without the commit log, so
>maybe move all the '+ 1' to functions close to each other and add
>a comment explaining how the mapping works? The core_id_to_qid()
>is a good example.
>
>
>> }
>>
>> nb_rx = rte_eth_rx_burst(rx->port_id, rxq_->queue_id,
>> @@ -1070,8 +1072,8 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
>>struct dp_packet **pkts,
>> if (dev->type == DPDK_DEV_VHOST) {
>> __netdev_dpdk_vhost_send(netdev, (struct dp_packet **) mbufs,
>>newcnt, true);
>> } else {
>> - dpdk_queue_pkts(dev, qid, mbufs, newcnt);
>> - dpdk_queue_flush(dev, qid);
>> + dpdk_queue_pkts(dev, qid, mbufs, newcnt);
>> + dpdk_queue_flush(dev, qid);
>
>This doesn't look right.
>
>fbl
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Sm
>B5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=Qxr44wWSHJSBA2l4BIzOF19CPqPFmY
>fhaDgtvjxdPpw&s=5_AWwzzWC5KIs8l8hqVMTp47xQZvQjez7-R3-ZTfEg0&e=
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
