From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id AC6FB1B2AB for ; Wed, 3 Oct 2018 19:59:12 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Oct 2018 10:59:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,336,1534834800"; d="scan'208";a="79607455" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.49]) ([10.237.221.49]) by orsmga006.jf.intel.com with ESMTP; 03 Oct 2018 10:59:09 -0700 To: Raslan Darawsheh , "keith.wiles@intel.com" Cc: Thomas Monjalon , "dev@dpdk.org" , Shahaf Shuler , Ori Kam References: <1538047196-13789-2-git-send-email-rasland@mellanox.com> <1538476438-20891-1-git-send-email-rasland@mellanox.com> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: Date: Wed, 3 Oct 2018 18:59:09 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <1538476438-20891-1-git-send-email-rasland@mellanox.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4 1/2] net/tap: change queue fd to be pointers to process private X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Oct 2018 17:59:13 -0000 On 10/2/2018 11:34 AM, Raslan Darawsheh wrote: > change the fds for the queues to be pointers and add new process private > structure and make the queue fds point to it. > > Signed-off-by: Raslan Darawsheh > --- > drivers/net/tap/rte_eth_tap.c | 63 ++++++++++++++++++++++++------------------- > drivers/net/tap/rte_eth_tap.h | 9 +++++-- > drivers/net/tap/tap_intr.c | 4 +-- > 3 files changed, 44 insertions(+), 32 deletions(-) > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > index ad5ae98..8cc4552 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -64,6 +64,7 @@ > > static struct rte_vdev_driver pmd_tap_drv; > static struct rte_vdev_driver pmd_tun_drv; > +static struct pmd_process_private *process_private; This is the handlers for eth_dev queues, this should be _per eth_dev_, you can't have a single global variable for this, we will see the problems below. <...> > @@ -1633,6 +1634,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name, > goto error_exit_nodev; > } > > + process_private = (struct pmd_process_private *) > + rte_zmalloc_socket(tap_name, sizeof(struct pmd_process_private), > + RTE_CACHE_LINE_SIZE, dev->device->numa_node); For each tap device, you overwrite the "process_private", - So it has the address of last created tap device, we will come back this one - and problem is how to free this back if an eth_dev removed? We lost references except last one. > + > pmd = dev->data->dev_private; > pmd->dev = dev; > snprintf(pmd->name, sizeof(pmd->name), "%s", tap_name); > @@ -1669,8 +1674,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name, > /* Presetup the fds to -1 as being not valid */ > pmd->ka_fd = -1; > for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) { > - pmd->rxq[i].fd = -1; > - pmd->txq[i].fd = -1; > + process_private->rxq_fds[i] = -1; > + process_private->txq_fds[i] = -1; > + pmd->rxq[i].fd = &process_private->rxq_fds[i]; > + pmd->txq[i].fd = &process_private->txq_fds[i]; for eth_dev, dev->data->dev_private->rxq[i].fd points to a memory address, and remember "dev->data->dev_private" is shared between primary and secondary, when secondary updates its rxq[i].fd to point its memory block, won't it corrupt the primary??? I think redirection is not helping here, I am not sure to the this within the shared memory, you need a not shared area.