From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ie0-f180.google.com (mail-ie0-f180.google.com [209.85.223.180]) by dpdk.org (Postfix) with ESMTP id F41BE58EF for ; Fri, 13 Mar 2015 01:10:46 +0100 (CET) Received: by iegc3 with SMTP id c3so71402867ieg.3 for ; Thu, 12 Mar 2015 17:10:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=/HFOyU18mYWKZmtxOF/mhiIjlua6plL7WsPpC3QfkNw=; b=fXf+wSiIb7pHfF2JmprxBdd5T4QF+o89x8L3lyVhTJbPSi2qkseFbPDPG9hA7cgkGr IuE3pWHJ8g0C742RT+xW5976Tq9Yq2sQL1TjBtEJMmHvhQj0V6WU1/B1s+RB5QN+5QQH O6t7jJnj24L3W1LIAv/YfcqtOmDN8zMwmbHmkhf0/OSijGmkD8+hCUUqgKQfFCps2otj lQqDLwOJBvzQwvmhE7hVkXYqfmQnsosIbWdz6bga4UBdfjovlyZYVEgAbqOJB1dGqAvE C7aHuCK7sFx0rCbijMCIpU16JB5PiCXM5nsXNXXVX5+zK0KSN5lVmgPHrrVnAhiFozcR YLLQ== X-Gm-Message-State: ALoCoQkzsEp4u2hnNRQnDlgHtsN50G/FTtdpoYclob+qqA+IZL9ac0b48fVCure00A0sX9ZG+HuW MIME-Version: 1.0 X-Received: by 10.107.16.31 with SMTP id y31mr13603280ioi.53.1426205446319; Thu, 12 Mar 2015 17:10:46 -0700 (PDT) Received: by 10.64.15.5 with HTTP; Thu, 12 Mar 2015 17:10:46 -0700 (PDT) In-Reply-To: <8CEF83825BEC744B83065625E567D7C2049F26C7@IRSMSX108.ger.corp.intel.com> References: <1426012584-3614-1-git-send-email-linville@tuxdriver.com> <8CEF83825BEC744B83065625E567D7C2049F26C7@IRSMSX108.ger.corp.intel.com> Date: Fri, 13 Mar 2015 09:10:46 +0900 Message-ID: From: Tetsuya Mukawa To: "Iremonger, Bernard" Content-Type: text/plain; charset=UTF-8 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [RFC] af_packet: support port hotplug X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Mar 2015 00:10:47 -0000 2015-03-13 2:05 GMT+09:00 Iremonger, Bernard : > > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of John W. Linville >> Sent: Tuesday, March 10, 2015 6:36 PM >> To: dev@dpdk.org >> Subject: [dpdk-dev] [RFC] af_packet: support port hotplug >> >> This patch adds finalization code to free resources allocated by the PMD. This is based on earlier >> patches for other PMDs by Tetsuya Mukawa , with corrections related to data- >> >name. >> >> Signed-off-by: John W. Linville >> Cc: Tetsuya Mukawa >> --- >> lib/librte_pmd_af_packet/rte_eth_af_packet.c | 56 ++++++++++++++++++++++++++++ >> 1 file changed, 56 insertions(+) >> >> diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c >> b/lib/librte_pmd_af_packet/rte_eth_af_packet.c >> index 80e9bdf7f819..57998ab19c96 100644 >> --- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c >> +++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c >> @@ -399,6 +399,13 @@ static struct eth_dev_ops ops = { >> .stats_reset = eth_stats_reset, >> }; >> >> +static struct eth_driver rte_af_packet_pmd = { >> + .pci_drv = { >> + .name = "rte_af_packet_pmd", >> + .drv_flags = RTE_PCI_DRV_DETACHABLE, >> + }, >> +}; >> + >> /* >> * Opens an AF_PACKET socket >> */ >> @@ -653,6 +660,10 @@ rte_pmd_init_internals(const char *name, >> if (*eth_dev == NULL) >> goto error; >> >> + /* check length of device name */ >> + if ((strlen((*eth_dev)->data->name) + 1) > sizeof(data->name)) >> + goto error; >> + >> /* >> * now put it all together >> * - store queue data in internals, >> @@ -669,12 +680,14 @@ rte_pmd_init_internals(const char *name, >> data->nb_tx_queues = (uint16_t)nb_queues; >> data->dev_link = pmd_link; >> data->mac_addrs = &(*internals)->eth_addr; >> + strncpy(data->name, (*eth_dev)->data->name, >> +strlen((*eth_dev)->data->name)); >> >> pci_dev->numa_node = numa_node; >> >> (*eth_dev)->data = data; >> (*eth_dev)->dev_ops = &ops; >> (*eth_dev)->pci_dev = pci_dev; >> + (*eth_dev)->driver = &rte_af_packet_pmd; >> >> return 0; >> >> @@ -835,10 +848,53 @@ rte_pmd_af_packet_devinit(const char *name, const char *params) >> return 0; >> } >> >> +static int >> +rte_pmd_af_packet_devuninit(const char *name) { >> + struct rte_eth_dev *eth_dev = NULL; >> + struct pmd_internals *internals; >> + struct tpacket_req req; >> + >> + unsigned q; >> + >> + RTE_LOG(INFO, PMD, "Closing AF_PACKET ethdev on numa socket %u\n", >> + rte_socket_id()); >> + >> + if (name == NULL) >> + return -1; > > Hi Tetsuya, John, > > Before detaching a port, the port must be stopped and closed. > The stop and close are only allowed for RTE_PROC_PRIMARY. > Should there be a check for process_type here? > > if (rte_eal_process_type() != RTE_PROC_PRIMARY) > return -EPERM; > > Regards, > > Bernard > Hi Bernard, I agree with stop() and close() are only called by primary process, but it may not need to add like above. Could you please check rte_ethdev.c? - struct rte_eth_dev_data *rte_eth_dev_data; This array is shared between processes. So we need to initialize of finalize carefully like you said. - struct rte_eth_dev rte_eth_devices[] This array is per process. And 'data' variable of this structure indicates a pointer of rte_eth_dev_data. All PMDs for physical NIC allocates like above when PMDs are initialized. (Even when a process is secondary, initialization function of PMDs will be called) But virtual device PMDs allocate rte_eth_dev_data and overwrite 'data' variable of rte_eth_devices while initialization. As a result, primary and secondary process has their own 'rte_eth_dev_data' for a virtual device. So I guess all processes need to free it not to leak memory. Thanks, Tetsuya >> + >> + /* retrieve ethdev entry */ >> + eth_dev = rte_eth_dev_allocated(name); >> + if (eth_dev == NULL) >> + return -1; >> + >> + internals = eth_dev->data->dev_private; >> + req = internals->req; >> + >> + for (q = 0; q < internals->nb_queues; q++) { >> + munmap(internals->rx_queue[q].map, >> + 2 * req.tp_block_size * req.tp_block_nr); >> + if (internals->rx_queue[q].rd) >> + rte_free(internals->rx_queue[q].rd); >> + if (internals->tx_queue[q].rd) >> + rte_free(internals->tx_queue[q].rd); >> + } >> + >> + rte_free(internals); >> + rte_free(eth_dev->data); >> + rte_free(eth_dev->pci_dev); >> + >> + rte_eth_dev_release_port(eth_dev); >> + >> + >> + return 0; >> +} >> + >> static struct rte_driver pmd_af_packet_drv = { >> .name = "eth_af_packet", >> .type = PMD_VDEV, >> .init = rte_pmd_af_packet_devinit, >> + .uninit = rte_pmd_af_packet_devuninit, >> }; >> >> PMD_REGISTER_DRIVER(pmd_af_packet_drv); >> -- >> 2.1.0 >