From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id A52A41CAC5 for ; Thu, 5 Apr 2018 13:05:25 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Apr 2018 04:05:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,410,1517904000"; d="scan'208";a="39571052" Received: from tanjianf-mobl.ccr.corp.intel.com (HELO [10.255.27.107]) ([10.255.27.107]) by FMSMGA003.fm.intel.com with ESMTP; 05 Apr 2018 04:05:21 -0700 To: Jeff Guo , stephen@networkplumber.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, gaetan.rivet@6wind.com, jingjing.wu@intel.com, thomas@monjalon.net, motih@mellanox.com, harry.van.haaren@intel.com References: <1522751639-2315-5-git-send-email-jia.guo@intel.com> <1522918968-15290-1-git-send-email-jia.guo@intel.com> <1522918968-15290-4-git-send-email-jia.guo@intel.com> Cc: jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, helin.zhang@intel.com From: "Tan, Jianfeng" Message-ID: <1a90253f-fe66-9388-769c-1d0dbf1f3ecd@intel.com> Date: Thu, 5 Apr 2018 19:05:21 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1522918968-15290-4-git-send-email-jia.guo@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH V19 3/4] eal/linux: uevent parse and process 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: Thu, 05 Apr 2018 11:05:26 -0000 On 4/5/2018 5:02 PM, Jeff Guo wrote: > In order to handle the uevent which has been detected from the kernel > side, add uevent parse and process function to translate the uevent into > device event, which user has subscribed to monitor. > > Signed-off-by: Jeff Guo > --- > v19->18: > fix some misunderstanding part > --- > lib/librte_eal/linuxapp/eal/eal_dev.c | 196 +++++++++++++++++++++++++++++++++- > 1 file changed, 194 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c > index 9c8d1a0..4686c41 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_dev.c > +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c > @@ -2,21 +2,213 @@ > * Copyright(c) 2018 Intel Corporation > */ > > +#include > +#include > +#include > +#include > + > #include > #include > #include > +#include > +#include > + > +#include "eal_private.h" > + > +static struct rte_intr_handle intr_handle = {.fd = -1 }; > +static bool monitor_started; > + > +#define EAL_UEV_MSG_LEN 4096 > +#define EAL_UEV_MSG_ELEM_LEN 128 > + > +/* identify the system layer which reports this event. */ > +enum eal_dev_event_subsystem { > + EAL_DEV_EVENT_SUBSYSTEM_PCI, /* PCI bus device event */ > + EAL_DEV_EVENT_SUBSYSTEM_UIO, /* UIO driver device event */ > + EAL_DEV_EVENT_SUBSYSTEM_VFIO, /* VFIO driver device event */ > + EAL_DEV_EVENT_SUBSYSTEM_MAX > +}; > + > +static int > +dev_uev_socket_fd_create(void) > +{ > + struct sockaddr_nl addr; > + int ret; > + > + intr_handle.fd = socket(PF_NETLINK, SOCK_RAW | SOCK_CLOEXEC | > + SOCK_NONBLOCK, > + NETLINK_KOBJECT_UEVENT); > + if (intr_handle.fd < 0) { > + RTE_LOG(ERR, EAL, "create uevent fd failed.\n"); > + return -1; > + } > + > + memset(&addr, 0, sizeof(addr)); > + addr.nl_family = AF_NETLINK; > + addr.nl_pid = 0; > + addr.nl_groups = 0xffffffff; > + > + ret = bind(intr_handle.fd, (struct sockaddr *) &addr, sizeof(addr)); > + if (ret < 0) { > + RTE_LOG(ERR, EAL, "Failed to bind uevent socket.\n"); > + goto err; > + } > + > + return 0; > +err: > + close(intr_handle.fd); > + intr_handle.fd = -1; > + return ret; > +} > + > +static int > +dev_uev_parse(const char *buf, struct rte_dev_event *event, int length) > +{ > + char action[EAL_UEV_MSG_ELEM_LEN]; > + char subsystem[EAL_UEV_MSG_ELEM_LEN]; > + char pci_slot_name[EAL_UEV_MSG_ELEM_LEN]; > + int i = 0, ret = 0; > + > + memset(action, 0, EAL_UEV_MSG_ELEM_LEN); > + memset(subsystem, 0, EAL_UEV_MSG_ELEM_LEN); > + memset(pci_slot_name, 0, EAL_UEV_MSG_ELEM_LEN); > + > + while (i < length) { > + for (; i < length; i++) { > + if (*buf) > + break; > + buf++; > + } > + /** > + * check device uevent from kernel side, no need to check > + * uevent from udev. > + */ > + if (!strncmp(buf, "libudev", 7)) { > + buf += 7; > + i += 7; > + return -1; > + } > + if (!strncmp(buf, "ACTION=", 7)) { > + buf += 7; > + i += 7; > + snprintf(action, sizeof(action), "%s", buf); > + } else if (!strncmp(buf, "SUBSYSTEM=", 10)) { > + buf += 10; > + i += 10; > + snprintf(subsystem, sizeof(subsystem), "%s", buf); > + } else if (!strncmp(buf, "PCI_SLOT_NAME=", 14)) { > + buf += 14; > + i += 14; > + snprintf(pci_slot_name, sizeof(subsystem), "%s", buf); > + event->devname = strdup(pci_slot_name); > + } > + for (; i < length; i++) { > + if (*buf == '\0') > + break; > + buf++; > + } > + } > + > + /* parse the subsystem layer */ > + if (!strncmp(subsystem, "uio", 3)) > + event->subsystem = EAL_DEV_EVENT_SUBSYSTEM_UIO; > + else if (!strncmp(subsystem, "pci", 3)) > + event->subsystem = EAL_DEV_EVENT_SUBSYSTEM_PCI; > + else if (!strncmp(subsystem, "vfio", 4)) > + event->subsystem = EAL_DEV_EVENT_SUBSYSTEM_VFIO; > + else > + ret = -1; We can just return -1 here. > > + /* parse the action type */ > + if (!strncmp(action, "add", 3)) > + event->type = RTE_DEV_EVENT_ADD; > + else if (!strncmp(action, "remove", 6)) > + event->type = RTE_DEV_EVENT_REMOVE; > + else > + ret = -1; We can just return -1 here. > + return ret; return 0 here. > +} > + > +static void > +dev_uev_handler(__rte_unused void *param) > +{ > + struct rte_dev_event uevent; > + int ret; > + char buf[EAL_UEV_MSG_LEN]; > + > + memset(&uevent, 0, sizeof(struct rte_dev_event)); > + memset(buf, 0, EAL_UEV_MSG_LEN); > + > + ret = recv(intr_handle.fd, buf, EAL_UEV_MSG_LEN, MSG_DONTWAIT); > + if (ret == 0 || (ret < 0 && errno != EAGAIN)) { > + /* connection is closed or broken, can not up again. */ > + RTE_LOG(ERR, EAL, "uevent socket connection is broken.\n"); Again, we need an alarm to unregister the callback from intr thread. > + return; > + } else if (ret < 0) { > + RTE_LOG(ERR, EAL, > + "uevent socket read error(%d): %s.\n", > + errno, strerror(errno)); > + return; > + } I think the above code can be adjusted as: if (ret == 0 || (ret < 0 && errno == EAGAIN)) return; else if (ret < 0) { RTE_LOG(ERR, EAL, "..."); set_alarm_to_unregister(); } > + > + ret = dev_uev_parse(buf, &uevent, EAL_UEV_MSG_LEN); > + if (ret < 0) { > + RTE_LOG(ERR, EAL, "It is not an valid event " s/ERR/DEBUG, or there are too many logs. > + "that need to be handle.\n"); > + return; > + } > + > + RTE_LOG(DEBUG, EAL, "receive uevent(name:%s, type:%d, subsystem:%d)\n", > + uevent.devname, uevent.type, uevent.subsystem); > + > + if (uevent.devname) > + dev_callback_process(uevent.devname, uevent.type); > +} > > int __rte_experimental > rte_dev_event_monitor_start(void) > { > - /* TODO: start uevent monitor for linux */ > + int ret; > + > + if (monitor_started) > + return 0; > + > + ret = dev_uev_socket_fd_create(); > + if (ret) { > + RTE_LOG(ERR, EAL, "error create device event fd.\n"); > + return -1; > + } > + > + intr_handle.type = RTE_INTR_HANDLE_DEV_EVENT; > + ret = rte_intr_callback_register(&intr_handle, dev_uev_handler, NULL); > + > + if (ret) { > + RTE_LOG(ERR, EAL, "fail to register uevent callback.\n"); > + return -1; > + } > + > + monitor_started = true; > + > return 0; > } > > int __rte_experimental > rte_dev_event_monitor_stop(void) > { > - /* TODO: stop uevent monitor for linux */ > + int ret; > + > + if (!monitor_started) > + return 0; > + > + ret = rte_intr_callback_unregister(&intr_handle, dev_uev_handler, > + (void *)-1); > + if (ret < 0) { > + RTE_LOG(ERR, EAL, "fail to unregister uevent callback.\n"); > + return ret; > + } > + > + close(intr_handle.fd); > + intr_handle.fd = -1; > + monitor_started = false; > return 0; > }