From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id B35445398 for ; Wed, 12 Jul 2017 09:36:43 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Jul 2017 00:36:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,347,1496127600"; d="scan'208";a="1194355727" Received: from jguo15x-mobl3.ccr.corp.intel.com (HELO [10.67.68.66]) ([10.67.68.66]) by fmsmga002.fm.intel.com with ESMTP; 12 Jul 2017 00:36:25 -0700 To: Thomas Monjalon References: <1498648044-57541-2-git-send-email-jia.guo@intel.com> <7030258.umMgtiJvDN@xps> <6683978.dxTV7H3Ttm@xps> Cc: dev@dpdk.org, helin.zhang@intel.com, jingjing.wu@intel.com, bruce.richardson@intel.com, konstantin.ananyev@intel.com, gaetan.rivet@6wind.com, thomas.monjalon@6wind.com, ferruh.yigit@intel.com From: "Guo, Jia" Message-ID: Date: Wed, 12 Jul 2017 15:36:24 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <6683978.dxTV7H3Ttm@xps> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 2/2] net/i40e: add hot plug monitor in i40e 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, 12 Jul 2017 07:36:44 -0000 On 7/10/2017 6:35 AM, Thomas Monjalon wrote: > 07/07/2017 16:08, Guo, Jia: >> On 7/7/2017 6:17 PM, Thomas Monjalon wrote: >>> 07/07/2017 09:56, Thomas Monjalon: >>>> 29/06/2017 07:01, Jeff Guo: >>>>> --- a/drivers/net/i40e/i40e_ethdev.c >>>>> +++ b/drivers/net/i40e/i40e_ethdev.c >>>>> @@ -1283,6 +1283,7 @@ static inline void i40e_GLQF_reg_init(struct i40e_hw *hw) >>>>> >>>>> /* enable uio intr after callback register */ >>>>> rte_intr_enable(intr_handle); >>>>> + >>>>> /* >>>>> * Add an ethertype filter to drop all flow control frames transmitted >>>>> * from VSIs. By doing so, we stop VF from sending out PAUSE or PFC >>>>> @@ -5832,11 +5833,29 @@ struct i40e_vsi * >>>>> { >>>>> struct rte_eth_dev *dev = (struct rte_eth_dev *)param; >>>>> struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); >>>>> + struct rte_uevent event; >>>>> uint32_t icr0; >>>>> + struct rte_pci_device *pci_dev; >>>>> + struct rte_intr_handle *intr_handle; >>>>> + >>>>> + pci_dev = RTE_ETH_DEV_TO_PCI(dev); >>>>> + intr_handle = &pci_dev->intr_handle; >>>>> >>>>> /* Disable interrupt */ >>>>> i40e_pf_disable_irq0(hw); >>>>> >>>>> + /* check device uevent */ >>>>> + if (rte_uevent_get(intr_handle->uevent_fd, &event) == 0) { >>>>> + if (event.subsystem == RTE_UEVENT_SUBSYSTEM_UIO) { >>>>> + if (event.action == RTE_UEVENT_REMOVE) { >>>>> + _rte_eth_dev_callback_process(dev, >>>>> + RTE_ETH_EVENT_INTR_RMV, NULL); >>>>> + return; >>>>> + } >>>>> + }if >>>>> + goto done; >>>>> + } >>>> There is nothing specific to i40e in this patch. >>>> It seems wrong to add such generic code in every drivers. >>> It should be managed at bus layer and not be specific to ethdev only. >> if all could managed at bus layer might also be what i want to see, >> but that would not so economical at currently. because of the event need >> to exposure to driver to use app's callback to handle it by >> detach/attach device. mlx driver also go through this path to show the >> rmv event usege. we just add uevent for pci uio device. Anyway, i think >> the uevent must be useful for future PF/VFIO live migration. if there >> are not other concern about the other patch that added uevent api in >> eal([PATCH v3 1/2] eal: add uevent api for hot plug), i suggest to merge >> it at first. Then we could go next to enhancement it with the 6wind hot >> plug feature. > Sorry it looks wrong to apply half of this patchset, given we are not > sure how the remaining part should be implemented. > Let's take time for a better solution and try to gather more opinions. > It will be highlighted as one of the next priorities, after the bus rework > in progress. some how i agree with your concern, maybe we could find a better option to handle that. 1) about bus layer , as we know , recently the pci uio api be moved out of the eal bus, but not for the eal interrupt. so that would not affect if we add uio api in, just like the exist api "uio_intr_enable" , i could modify the api name into "uio_uevent_get", "uio_uevent_connnect" for better identify. But it still in eal interrupt layer before we decide to modify the eal interrupt layer. 2) about the uevent callback handler, fail-safe driver solution said that fail-safe driver register the callback into device, and device interrupt handler identify the event and callback fail-safe to switch sub-device to process hot plug out case. so if the event process not place in device(mlx driver handle it in device driver), it maybe place in eal. To be generic for all pci uio driver. i could add "uio_uevent_handler" api into eal interrupt. but it will bring some ethdev process into eal. other wise we still need to add the handler in all device driver like this patch here. Any comment about it? 3) the patch set here is about uevent monitoring for pci uio driver. the hot plug remove event processing is depend on the failsave driver. bellow for ref. http://dpdk.org/dev/patchwork/patch/26842/