From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 0305A1BB29 for ; Wed, 4 Apr 2018 05:15:33 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Apr 2018 20:15:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,404,1517904000"; d="scan'208";a="34079863" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga002.fm.intel.com with ESMTP; 03 Apr 2018 20:15:32 -0700 Received: from fmsmsx126.amr.corp.intel.com (10.18.125.43) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 3 Apr 2018 20:15:31 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX126.amr.corp.intel.com (10.18.125.43) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 3 Apr 2018 20:15:31 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.241]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.88]) with mapi id 14.03.0319.002; Wed, 4 Apr 2018 11:15:29 +0800 From: "Tan, Jianfeng" To: "Guo, Jia" , "stephen@networkplumber.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "gaetan.rivet@6wind.com" , "Wu, Jingjing" , "thomas@monjalon.net" , "motih@mellanox.com" , "Van Haaren, Harry" CC: "jblunck@infradead.org" , "shreyansh.jain@nxp.com" , "dev@dpdk.org" , "Zhang, Helin" Thread-Topic: [PATCH V18 3/4] eal/linux: uevent parse and process Thread-Index: AQHTyzd1+TKpwN9ls0itrmNJVNG4faPv6eBw Date: Wed, 4 Apr 2018 03:15:29 +0000 Message-ID: References: <1522339205-27698-5-git-send-email-jia.guo@intel.com> <1522751639-2315-1-git-send-email-jia.guo@intel.com> <1522751639-2315-4-git-send-email-jia.guo@intel.com> In-Reply-To: <1522751639-2315-4-git-send-email-jia.guo@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH V18 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: Wed, 04 Apr 2018 03:15:34 -0000 Hi Jeff, Looks much better now, but still have some issues to address. > -----Original Message----- > From: Guo, Jia > Sent: Tuesday, April 3, 2018 6:34 PM > To: stephen@networkplumber.org; Richardson, Bruce; Yigit, Ferruh; > Ananyev, Konstantin; gaetan.rivet@6wind.com; Wu, Jingjing; > thomas@monjalon.net; motih@mellanox.com; Van Haaren, Harry; Tan, > Jianfeng > Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Guo, > Jia; Zhang, Helin > Subject: [PATCH V18 3/4] eal/linux: uevent parse and process >=20 > 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. >=20 > Signed-off-by: Jeff Guo > --- > v18->v17: > refine socket configuration. > --- > lib/librte_eal/linuxapp/eal/eal_dev.c | 178 > +++++++++++++++++++++++++++++++++- > 1 file changed, 176 insertions(+), 2 deletions(-) >=20 > diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c > b/lib/librte_eal/linuxapp/eal/eal_dev.c > index 9c8d1a0..9f2ee40 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_dev.c > +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c > @@ -2,21 +2,195 @@ > * Copyright(c) 2018 Intel Corporation > */ >=20 > +#include > +#include > +#include > +#include > + > #include > #include > #include > +#include > +#include > + > +#include "eal_private.h" > + > +static struct rte_intr_handle intr_handle =3D {.fd =3D -1 }; > +static bool monitor_started; > + > +#define EAL_UEV_MSG_LEN 4096 > +#define EAL_UEV_MSG_ELEM_LEN 128 > + > +/* identify the system layer which event exposure from */ Reword it a little bit:=20 /* 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 =3D 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 =3D AF_NETLINK; > + addr.nl_pid =3D 0; > + addr.nl_groups =3D 0xffffffff; > + > + ret =3D bind(intr_handle.fd, (struct sockaddr *) &addr, sizeof(addr)); > + if (ret < 0) { > + RTE_LOG(ERR, EAL, "Failed to bind socket for netlink fd.\n"); Reword it a little bit so that we can understand it's a log related to hotp= lug: Failed to bind uevent socket > + goto err; > + } > + > + return 0; > +err: > + close(intr_handle.fd); Then: intr_handle.fd =3D -1? > + return ret; > +} > + > +static void > +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 =3D 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++; > + } > + if (!strncmp(buf, "ACTION=3D", 7)) { > + buf +=3D 7; > + i +=3D 7; > + snprintf(action, sizeof(action), "%s", buf); > + } else if (!strncmp(buf, "SUBSYSTEM=3D", 10)) { > + buf +=3D 10; > + i +=3D 10; > + snprintf(subsystem, sizeof(subsystem), "%s", buf); > + } else if (!strncmp(buf, "PCI_SLOT_NAME=3D", 14)) { > + buf +=3D 14; > + i +=3D 14; > + snprintf(pci_slot_name, sizeof(subsystem), "%s", buf); > + event->devname =3D strdup(pci_slot_name); > + } > + for (; i < length; i++) { > + if (*buf =3D=3D '\0') > + break; > + buf++; > + } > + } > + > + if (!strncmp(subsystem, "uio", 3)) > + event->subsystem =3D EAL_DEV_EVENT_SUBSYSTEM_UIO; > + else if (!strncmp(subsystem, "pci", 3)) > + event->subsystem =3D EAL_DEV_EVENT_SUBSYSTEM_PCI; > + else if (!strncmp(subsystem, "vfio", 4)) How can we indicate it is an event with subsystem that we will not handle? > + event->subsystem =3D EAL_DEV_EVENT_SUBSYSTEM_VFIO; > + if (!strncmp(action, "add", 3)) > + event->type =3D RTE_DEV_EVENT_ADD; > + if (!strncmp(action, "remove", 6)) > + event->type =3D RTE_DEV_EVENT_REMOVE; How can we indicate it is an event with type that we will not handle? My suggestion is to define a return value for that: - EVENT_VALID returned for an event that we will handle later. - EVENT_INVALID returned for any unknown events. > +} > + > +static int > +dev_uev_receive(int fd, 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 =3D recv(fd, buf, EAL_UEV_MSG_LEN, MSG_DONTWAIT); > + if (ret < 0) { > + RTE_LOG(ERR, EAL, > + "Socket read error(%d): %s.\n", > + errno, strerror(errno)); > + return -1; > + } else if (ret =3D=3D 0) > + /* connection closed */ > + return -1; > + > + dev_uev_parse(buf, uevent, EAL_UEV_MSG_LEN); > + > + return 0; > +} > + > +static void > +dev_uev_process(__rte_unused void *param) > +{ > + struct rte_dev_event uevent; > + > + if (dev_uev_receive(intr_handle.fd, &uevent)) If error happens, we shall start an alarm task to remove the callback of in= terrupt thread. > + return; You may want to add a log here for debugging, showing what event comes for = which device. >=20 > + if (uevent.devname) You can also filter this kind of events using the way I suggested above. > + dev_callback_process(uevent.devname, uevent.type); > +} >=20 > int __rte_experimental > rte_dev_event_monitor_start(void) > { > - /* TODO: start uevent monitor for linux */ > + int ret; > + > + if (monitor_started) > + return 0; > + > + ret =3D dev_uev_socket_fd_create(); > + if (ret) { > + RTE_LOG(ERR, EAL, "error create device event fd.\n"); > + return -1; > + } > + > + intr_handle.type =3D RTE_INTR_HANDLE_DEV_EVENT; > + ret =3D rte_intr_callback_register(&intr_handle, dev_uev_process, > NULL); > + > + if (ret) { > + RTE_LOG(ERR, EAL, "fail to register uevent callback.\n"); > + return -1; > + } > + > + monitor_started =3D true; > + > return 0; > } >=20 > int __rte_experimental > rte_dev_event_monitor_stop(void) > { > - /* TODO: stop uevent monitor for linux */ > + int ret; > + > + if (!monitor_started) > + return 0; > + > + ret =3D rte_intr_callback_unregister(&intr_handle, dev_uev_process, > + (void *)-1); > + if (ret < 0) { > + RTE_LOG(ERR, EAL, "fail to unregister uevent callback.\n"); > + return ret; > + } > + > + close(intr_handle.fd); > + intr_handle.fd =3D -1; > + monitor_started =3D false; > return 0; > } > -- > 2.7.4