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
http://openvswitch.org/mailman/listinfo/dev

Reply via email to