From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 3B1F52647 for ; Tue, 30 May 2017 09:14:08 +0200 (CEST) Received: by mail-wm0-f49.google.com with SMTP id 7so79329091wmo.1 for ; Tue, 30 May 2017 00:14:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Wb4RKQ/MSGhgnKMCk/pUA8czGquiFevQ7MHN7LzDM4M=; b=nzoQg1aZ+Q7m3JwUl4KoEXJ3HP5BZ0W9rBJJbqWnk+H9rLeNqTjxKZEF3fIkXVUplA Cq/S+DFCovLDGjviehLbyiYPQ2rwYN3mu8GRPYr8r83ivkiATgagrwbV6Cp028mHDKKF n/qTKx5JaahU6OOfw7EI7lJEIRvzTp+Dw2D7lqTF1XbEgXD8NxO3YqgsB6MHRyI5JpMv 9ZblvDhylpTQ6niPeAJSStxUVhcgmiXJJ/Xyc1CeWPsBjxCZBvRDRsd1BLOjOt6jab/U GyVwfnuqQMLuzfFrKGsmiCFGku90+skfw9NDfzenrKSdEOQcdV8rcHe/O2yiEOsVmR7I 9cYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Wb4RKQ/MSGhgnKMCk/pUA8czGquiFevQ7MHN7LzDM4M=; b=dPAYUyrRVlHYmvNxeDGo1Qbt+5ap/yG8hTmbb/+G0aFSdwYv+PO0q9hFkd14CLZxIA LEi7NS4C1SMWZAZNEWoIj9uUJ8wBqb1Kd7GArd80rUa6QrX7HiGxdfB6w2+t6dRiEwGo pyskCnDnVPsv9bWjw8YyXMs5HGheltWoAmtLgDEODroBNaf+OhSb3hALwyGwol7oKmZO Ek7QQ9y13EN+LxtdgbrfimEnW8jBtewmQGl5mWnlk6upe4DRxFb6ZxAkyjeJts2rqMR0 TPunSCrFs/FnXei0KLHEgHt1dQbGvUSkIs7RxIrDNRE8uXozmFZZYVYLl4DP5V94SXP3 CnbA== X-Gm-Message-State: AODbwcBHxCbBxsFUKxdlCwEUDB1O+2a5KU0D8sBltCxQi+G9w6s2A6N8 BqKxj2KTMf7laIKp X-Received: by 10.28.111.7 with SMTP id k7mr654664wmc.50.1496128447666; Tue, 30 May 2017 00:14:07 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id h73sm26926374wma.10.2017.05.30.00.14.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 30 May 2017 00:14:06 -0700 (PDT) Date: Tue, 30 May 2017 09:14:00 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Jeff Guo Cc: helin.zhang@intel.com, jingjing.wu@intel.com, bruce.richardson@intel.com, konstantin.ananyev@intel.com, yuanhan.liu@intel.com, dev@dpdk.org Message-ID: <20170530071400.GN14914@bidouze.vm.6wind.com> References: <1495986280-26207-1-git-send-email-jia.guo@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1495986280-26207-1-git-send-email-jia.guo@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [RFC] Add hot plug event in rte eal interrupt and inplement it in i40e driver. 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: Tue, 30 May 2017 07:14:08 -0000 Hi Jeff, A few comments below: On Sun, May 28, 2017 at 11:44:40PM +0800, Jeff Guo wrote: >For HW hotplug feature, we had already got upstream that removal event adding from 6wind as bellow. > >dependency of “add device removal event” patch set: >http://dpdk.org/dev/patchwork/patch/23693/ >[dpdk-dev,v2,1/5] ethdev: introduce device removal event >http://dpdk.org/dev/patchwork/patch/23694/ >[dpdk-dev,v2,2/5] net/mlx4: device removal event support >http://dpdk.org/dev/patchwork/patch/23695/ >[dpdk-dev,v2,3/5] app/testpmd: generic event handler >http://dpdk.org/dev/patchwork/patch/23696/ >[dpdk-dev,v2,4/5] app/testpmd: request link status interrupt >http://dpdk.org/dev/patchwork/patch/23697/ >[dpdk-dev,v2,5/5] app/testpmd: request device removal interrupt > >>From the patches, we can see a new event type “RTE_ETH_DEV_INTR_RMV” has been added into the ethdev, and the event has been implemented in mlx4 driver, and Testpmd be use for testing purposes and as a practical usage example for how to use these event. The progress is use the mlx4 driver to register interrupt callback function to rte eal interrupt source, and when rte epolling detect the IBV_EVENT_DEVICE_FATAL , which is identify the device remove behavior, it will callback into the driver’s interrupt handle to handle it, and then callback to the user app, such as testpmd, to detach the pci device. > >So far, except the mlx4 driver, other driver like i40, that not have the remove interrupt from hw, will not be able to monitoring the remove behavior, so in order to expand the hot plug feature for all driver cases, something must be done ot detect the remove event at the kernel level and offer a new line of interrupt to the userland. The idea is coming as below. > It's nice that this event is extended to other drivers. >Use I40e as example, we know that the rmv interrupt is not added in hw, but we could monitor the uio file descriptor to catch the device remove event as bellow. > >The info of uevent form FD monitoring: >remove@/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/uio/uio2 >ACTION=remove >DEVPATH=/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/uio/uio2 >SUBSYSTEM=uio >MAJOR=243 >MINOR=2 >DEVNAME=uio2 >SEQNUM=11366 > >Firstly, in order to monitor the uio file descriptor, we need to create socket to monitor the uio fd, that is defined as “hotplug_fd”, and then add the uio fd into the epoll fd list, rte eal could epoll all of the interrupt event from hw interrupt and also include the uevent from hotplug_fd. > >Secondly, in order to read out the uevent that monitoring, we need to add uevent API in rte layer. We plan add 2 , rte_uevent_connect and rte_get_uevent. All driver interrupt handler could use these API to enable the uevent monitoring, and read out the uevent type , then corresponding to handle these uevent, such as detach the device when get the remove type. > I find having a generic uevent API interesting. However, all specifics pertaining to UIO use (hotplug_fd, subsystem enum) should stay in UIO specific code (eal_pci_uio.c?). I am currently moving the PCI bus out of the EAL. EAL subsystems should not rely on PCI specifics, as they won't be available afterward. It should also allow you to clean up your API. Exposing hotplug_fd and requiring PMDs to link it can be avoided and should result in a simpler API. >Signed-off-by: Jeff Guo >--- > drivers/net/i40e/i40e_ethdev.c | 15 +++ > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 146 ++++++++++++++++++++- > .../linuxapp/eal/include/exec-env/rte_interrupts.h | 32 +++++ > 3 files changed, 191 insertions(+), 2 deletions(-) > >diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c >index 4c49673..0336f82 100644 >--- a/drivers/net/i40e/i40e_ethdev.c >+++ b/drivers/net/i40e/i40e_ethdev.c >@@ -66,6 +66,8 @@ > #include "i40e_pf.h" > #include "i40e_regs.h" > >+extern int hotplug_fd; >+ > #define ETH_I40E_FLOATING_VEB_ARG "enable_floating_veb" > #define ETH_I40E_FLOATING_VEB_LIST_ARG "floating_veb_list" > >@@ -5808,8 +5810,21 @@ i40e_dev_interrupt_handler(void *param) > { > 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; > >+ /* check device uevent */ >+ while (rte_get_uevent(hotplug_fd, &event) > 0) { >+ if (event.subsystem == 1) { >+ if (event.action == RTE_UEVENT_ADD) { >+ //do nothing here >+ } else if (event.action == RTE_UEVENT_REMOVE) { >+ _rte_eth_dev_callback_process(dev, >+ RTE_ETH_EVENT_INTR_RMV, NULL); >+ } >+ } >+ } >+ > /* Disable interrupt */ > i40e_pf_disable_irq0(hw); > >diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c >index 2e3bd12..873ab5f 100644 >--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c >+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c >@@ -65,6 +65,10 @@ > #include > #include > >+#include >+#include >+#include >+ > #include "eal_private.h" > #include "eal_vfio.h" > #include "eal_thread.h" >@@ -74,6 +78,11 @@ > > static RTE_DEFINE_PER_LCORE(int, _epfd) = -1; /**< epoll fd per thread */ > >+#define RTE_UEVENT_MSG_LEN 4096 >+#define RTE_UEVENT_SUBSYSTEM_UIO 1 >+ >+int hotplug_fd = -1; >+ > /** > * union for pipe fds. > */ >@@ -669,10 +678,13 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds) > RTE_SET_USED(r); > return -1; > } >+ > rte_spinlock_lock(&intr_lock); > TAILQ_FOREACH(src, &intr_sources, next) >- if (src->intr_handle.fd == >- events[n].data.fd) >+ if ((src->intr_handle.fd == >+ events[n].data.fd) || >+ (hotplug_fd == >+ events[n].data.fd)) > break; > if (src == NULL){ > rte_spinlock_unlock(&intr_lock); >@@ -858,7 +870,24 @@ eal_intr_thread_main(__rte_unused void *arg) > } > else > numfds++; >+ >+ /** >+ * add device uevent file descriptor >+ * into wait list for hot plug. >+ */ >+ ev.events = EPOLLIN | EPOLLPRI | EPOLLRDHUP | EPOLLHUP; >+ ev.data.fd = hotplug_fd; >+ if (epoll_ctl(pfd, EPOLL_CTL_ADD, >+ hotplug_fd, &ev) < 0){ >+ rte_panic("Error adding hotplug_fd %d epoll_ctl, %s\n", >+ hotplug_fd, strerror(errno)); >+ } >+ else >+ numfds++; >+ > } >+ >+ > rte_spinlock_unlock(&intr_lock); > /* serve the interrupt */ > eal_intr_handle_interrupts(pfd, numfds); >@@ -877,6 +906,9 @@ rte_eal_intr_init(void) > int ret = 0, ret_1 = 0; > char thread_name[RTE_MAX_THREAD_NAME_LEN]; > >+ /* connect to monitor device uevent */ >+ rte_uevent_connect(); >+ > /* init the global interrupt source head */ > TAILQ_INIT(&intr_sources); > >@@ -1255,3 +1287,113 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle) > > return 0; > } >+ >+int >+rte_uevent_connect(void) >+{ >+ struct sockaddr_nl addr; >+ int ret; >+ int netlink_fd; >+ int size = 64 * 1024; >+ int nonblock = 1; >+ memset(&addr, 0, sizeof(addr)); >+ addr.nl_family = AF_NETLINK; >+ addr.nl_pid = getpid(); >+ addr.nl_groups = 0xffffffff; >+ >+ netlink_fd = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_KOBJECT_UEVENT); >+ if (netlink_fd < 0) >+ return -1; >+ >+ setsockopt(netlink_fd, SOL_SOCKET, SO_RCVBUFFORCE, &size, sizeof(size)); >+ >+ ret = ioctl(netlink_fd, FIONBIO, &nonblock); >+ if (ret != 0) { >+ RTE_LOG(ERR, EAL, >+ "ioctl(FIONBIO) failed\n"); >+ close(netlink_fd); >+ return -1; >+ } >+ >+ if (bind(netlink_fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { >+ close(netlink_fd); >+ return -1; >+ } >+ >+ hotplug_fd = netlink_fd; >+ >+ return netlink_fd; >+} >+ >+static int >+parse_event(const char *buf, struct rte_uevent *event) >+{ >+ char action[RTE_UEVENT_MSG_LEN]; >+ char subsystem[RTE_UEVENT_MSG_LEN]; >+ char dev_path[RTE_UEVENT_MSG_LEN]; >+ >+ memset(action, 0, RTE_UEVENT_MSG_LEN); >+ memset(subsystem, 0, RTE_UEVENT_MSG_LEN); >+ memset(dev_path, 0, RTE_UEVENT_MSG_LEN); >+ >+ while (*buf) { >+ if (!strncmp(buf, "ACTION=", 7)) { >+ buf += 7; >+ snprintf(action, sizeof(action), "%s", buf); >+ } else if (!strncmp(buf, "DEVPATH=", 8)) { >+ buf += 8; >+ snprintf(dev_path, sizeof(dev_path), "%s", buf); >+ } else if (!strncmp(buf, "SUBSYSTEM=", 10)) { >+ buf += 10; >+ snprintf(subsystem, sizeof(subsystem), "%s", buf); >+ } >+ while (*buf++) >+ ; >+ } >+ >+ if (!strncmp(subsystem, "uio", 3)) { >+ >+ event->subsystem = RTE_UEVENT_SUBSYSTEM_UIO; >+ if (!strncmp(action, "add", 3)) { >+ event->action = RTE_UEVENT_ADD; >+ } >+ if (!strncmp(action, "remove", 6)) { >+ event->action = RTE_UEVENT_REMOVE; >+ } >+ return 1; >+ } >+ >+ return -1; >+} >+ >+int >+rte_get_uevent(int fd, struct rte_uevent *uevent) >+{ >+ int ret; >+ char buf[RTE_UEVENT_MSG_LEN]; >+ >+ memset(uevent, 0, sizeof(struct rte_uevent)); >+ memset(buf, 0, RTE_UEVENT_MSG_LEN); >+ >+ ret = recv(fd, buf, RTE_UEVENT_MSG_LEN - 1, MSG_DONTWAIT); >+ if (ret > 0) { >+ return parse_event(buf, uevent); >+ } >+ >+ if (ret < 0) { >+ if (errno == EAGAIN || errno == EWOULDBLOCK) { >+ return 0; >+ } else { >+ RTE_LOG(ERR, EAL, >+ "Socket read error(%d): %s\n", >+ errno, strerror(errno)); >+ } >+ } >+ >+ /* connection closed */ >+ if (ret == 0) { >+ return -1; >+ } >+ >+ return 0; >+} >diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h >index 6daffeb..d32ba01 100644 >--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h >+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h >@@ -99,6 +99,16 @@ struct rte_intr_handle { > int *intr_vec; /**< intr vector number array */ > }; > >+enum rte_uevent_action { >+ RTE_UEVENT_ADD = 0, /**< uevent type of device add */ >+ RTE_UEVENT_REMOVE = 1, /**< uevent type of device remove*/ >+}; >+ >+struct rte_uevent { >+ enum rte_uevent_action action; /**< uevent action type */ >+ int subsystem; /**< subsystem id */ >+}; >+ > #define RTE_EPOLL_PER_THREAD -1 /**< to hint using per thread epfd */ > > /** >@@ -236,4 +246,26 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle); > int > rte_intr_cap_multiple(struct rte_intr_handle *intr_handle); > >+/** >+ * It read out the uevent from the specific file descriptor. >+ * >+ * @param fd >+ * The fd which the uevent associated to >+ * @param uevent >+ * Pointer to the uevent which read from the monitoring fd. >+ * @return >+ * - On success, one. >+ * - On failure, zeor or a negative value. >+ */ >+int >+rte_get_uevent(int fd, struct rte_uevent *uevent); >+ >+/** >+ * Connect to the device uevent file descriptor. >+ * @return >+ * return the connected uevent fd. >+ */ >+int >+rte_uevent_connect(void); >+ > #endif /* _RTE_LINUXAPP_INTERRUPTS_H_ */ >-- >2.7.4 > -- Gaëtan Rivet 6WIND