From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id B2227727F for ; Mon, 2 Apr 2018 07:49:23 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Apr 2018 22:49:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,394,1517904000"; d="scan'208";a="216864060" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga005.fm.intel.com with ESMTP; 01 Apr 2018 22:49:21 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 1 Apr 2018 22:49:21 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 1 Apr 2018 22:49:20 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.235]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.226]) with mapi id 14.03.0319.002; Mon, 2 Apr 2018 13:49:19 +0800 From: "Wu, Jingjing" To: "Guo, Jia" , "stephen@networkplumber.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Ananyev, Konstantin" , "gaetan.rivet@6wind.com" , "thomas@monjalon.net" , "motih@mellanox.com" , "Van Haaren, Harry" , "Tan, Jianfeng" CC: "jblunck@infradead.org" , "shreyansh.jain@nxp.com" , "dev@dpdk.org" , "Zhang, Helin" Thread-Topic: [PATCH V17 4/4] app/testpmd: enable device hotplug monitoring Thread-Index: AQHTx3c/O66yQn5xeEqI4KdAvf0M4aPs9Ayg Date: Mon, 2 Apr 2018 05:49:18 +0000 Message-ID: <9BB6961774997848B5B42BEC655768F810FCF3B8@SHSMSX103.ccr.corp.intel.com> References: <1522063256-3997-5-git-send-email-jia.guo@intel.com> <1522339205-27698-1-git-send-email-jia.guo@intel.com> <1522339205-27698-5-git-send-email-jia.guo@intel.com> In-Reply-To: <1522339205-27698-5-git-send-email-jia.guo@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDZmNDc1ZDgtN2M2Ni00NTVmLTgyODktZWI1YzA4ZmRlMTY3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Im8yUm10SEViMGdNanhrV1wvQmM1bHpuWjRzM1dtQXQ2WXp4S1E3dUZvM1JZPSJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action 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 V17 4/4] app/testpmd: enable device hotplug monitoring 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: Mon, 02 Apr 2018 05:49:25 -0000 >=20 > +static int > +eth_dev_event_callback_register(portid_t port_id) > +{ > + int diag; > + char *device_name; > + > + /* if port id equal -1, unregister all device event callbacks */ > + if (port_id =3D=3D (portid_t)HOT_PLUG_FOR_ALL_DEVICE) { > + device_name =3D NULL; > + } else { > + device_name =3D strdup(rte_eth_devices[port_id].device->name); > + if (!device_name) { > + free(device_name); If device_name is NULL, no memory allocated, why free? > + return -1; > + } > + } > + /* register the dev_event callback */ > + diag =3D rte_dev_event_callback_register(device_name, > + eth_dev_event_callback, (void *)(intptr_t)port_id); > + if (diag) { > + printf("Failed to setup dev_event callback\n"); > + return -1; > + } > + > + free(device_name); > + return 0; > +} > + > + > +static int > +eth_dev_event_callback_unregister(portid_t port_id) > +{ > + int diag; > + char *device_name; > + > + /* if port id equal -1, unregister all device event callbacks */ > + if (port_id =3D=3D (portid_t)HOT_PLUG_FOR_ALL_DEVICE) { > + device_name =3D NULL; > + } else { > + device_name =3D strdup(rte_eth_devices[port_id].device->name); > + if (!device_name) { > + free(device_name); Same as above. > + return -1; > + } > + } > + > + /* unregister the dev_event callback */ > + diag =3D rte_dev_event_callback_unregister(device_name, > + eth_dev_event_callback, (void *)(intptr_t)port_id); > + if (diag) { > + printf("Failed to setup dev_event callback\n"); > + return -1; > + } > + > + free(device_name); > + return 0; > +} > + > void > attach_port(char *identifier) > { > @@ -1869,6 +1941,8 @@ attach_port(char *identifier) > if (rte_eth_dev_attach(identifier, &pi)) > return; >=20 > + eth_dev_event_callback_register(pi); > + > socket_id =3D (unsigned)rte_eth_dev_socket_id(pi); > /* if socket_id is invalid, set to 0 */ > if (check_socket_id(socket_id) < 0) > @@ -1880,6 +1954,12 @@ attach_port(char *identifier) >=20 > ports[pi].port_status =3D RTE_PORT_STOPPED; >=20 > + if (hot_plug) { > + hotplug_list_add(rte_eth_devices[pi].device, > + rte_eth_devices[pi].data->kdrv); > + eth_dev_event_callback_register(pi); > + } > + > printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports); > printf("Done\n"); > } > @@ -1906,6 +1986,12 @@ detach_port(portid_t port_id) >=20 > nb_ports =3D rte_eth_dev_count(); >=20 > + if (hot_plug) { > + hotplug_list_add(rte_eth_devices[port_id].device, > + rte_eth_devices[port_id].data->kdrv); > + eth_dev_event_callback_register(port_id); > + } > + > printf("Port '%s' is detached. Now total ports is %d\n", > name, nb_ports); > printf("Done\n"); > @@ -1916,6 +2002,7 @@ void > pmd_test_exit(void) > { > portid_t pt_id; > + int ret; >=20 > if (test_done =3D=3D 0) > stop_packet_forwarding(); > @@ -1929,6 +2016,19 @@ pmd_test_exit(void) > close_port(pt_id); > } > } > + Need to check if hot_plug is enabled? > + ret =3D rte_dev_event_monitor_stop(); > + if (ret) { > + RTE_LOG(ERR, EAL, "fail to stop device event monitor."); > + return; > + } > + > + ret =3D eth_dev_event_callback_unregister(HOT_PLUG_FOR_ALL_DEVICE); > + if (ret) { > + RTE_LOG(ERR, EAL, "fail to unregister all event callbacks."); > + return; > + } <...> > +static void > +add_dev_event_callback(void *arg) > +{ > + char *dev_name =3D (char *)arg; > + > + rte_eal_alarm_cancel(add_dev_event_callback, arg); > + > + if (!in_hotplug_list(dev_name)) Remove "!" in the check > + return; > + > + RTE_LOG(ERR, EAL, "add device: %s\n", dev_name); It is not ERR, please make the log aligned with remove device. > + attach_port(dev_name); > +} > + <...> > + > +/* This function is used by the interrupt thread */ > +static int > +eth_dev_event_callback(char *device_name, enum rte_dev_event_type type, > + void *arg) > +{ > + static const char * const event_desc[] =3D { > + [RTE_DEV_EVENT_ADD] =3D "add", > + [RTE_DEV_EVENT_REMOVE] =3D "remove", > + }; > + char *dev_name =3D malloc(strlen(device_name) + 1); > + > + strcpy(dev_name, device_name); Why not use strdup as above? > + if (type >=3D RTE_DEV_EVENT_MAX) { > + fprintf(stderr, "%s called upon invalid event %d\n", > + __func__, type); > + fflush(stderr); > + } else if (event_print_mask & (UINT32_C(1) << type)) { > + printf("%s event\n", > + event_desc[type]); > + fflush(stdout); > + } > + > + switch (type) { > + case RTE_DEV_EVENT_REMOVE: > + if (rte_eal_alarm_set(100000, > + rmv_dev_event_callback, arg)) > + fprintf(stderr, > + "Could not set up deferred device removal\n"); > + break; > + case RTE_DEV_EVENT_ADD: > + if (rte_eal_alarm_set(100000, > + add_dev_event_callback, dev_name)) > + fprintf(stderr, > + "Could not set up deferred device add\n"); > + break; > + default: > + break; > + } > + return 0; Always 0, even alarm set fails? Thanks Jingjing