From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 1762311C5 for ; Thu, 29 Sep 2016 15:41:45 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP; 29 Sep 2016 06:41:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,415,1470726000"; d="scan'208";a="174560009" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by fmsmga004.fm.intel.com with ESMTP; 29 Sep 2016 06:41:43 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.198]) by irsmsx110.ger.corp.intel.com ([163.33.3.25]) with mapi id 14.03.0248.002; Thu, 29 Sep 2016 14:41:42 +0100 From: "Kerlin, MarcinX" To: "Pattan, Reshma" , "dev@dpdk.org" CC: "De Lara Guarch, Pablo" , "thomas.monjalon@6wind.com" Thread-Topic: [dpdk-dev] [PATCH v4 1/2] librte_ether: add protection against overwrite device data Thread-Index: AQHSGZEmIwsNxZ9j4U+cy4KCWJU2xaCQZyLQ Date: Thu, 29 Sep 2016 13:41:42 +0000 Message-ID: <68D830D942438745AD09BAFA99E33E812BD873@IRSMSX102.ger.corp.intel.com> References: <1474901586-8706-2-git-send-email-marcinx.kerlin@intel.com> <1474974783-4861-1-git-send-email-marcinx.kerlin@intel.com> <1474974783-4861-2-git-send-email-marcinx.kerlin@intel.com> <3AEA2BF9852C6F48A459DA490692831F010A91F9@IRSMSX109.ger.corp.intel.com> In-Reply-To: <3AEA2BF9852C6F48A459DA490692831F010A91F9@IRSMSX109.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 1/2] librte_ether: add protection against overwrite device data X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Sep 2016 13:41:46 -0000 Hi Reshma, > -----Original Message----- > From: Pattan, Reshma > Sent: Wednesday, September 28, 2016 4:04 PM > To: Kerlin, MarcinX ; dev@dpdk.org > Cc: De Lara Guarch, Pablo ; > thomas.monjalon@6wind.com; Kerlin, MarcinX > Subject: RE: [dpdk-dev] [PATCH v4 1/2] librte_ether: add protection again= st > overwrite device data >=20 >=20 >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Marcin Kerlin > > Sent: Tuesday, September 27, 2016 12:13 PM > > To: dev@dpdk.org > > Cc: De Lara Guarch, Pablo ; > > thomas.monjalon@6wind.com; Kerlin, MarcinX > > Subject: [dpdk-dev] [PATCH v4 1/2] librte_ether: add protection > > against overwrite device data > > > > +int > > +rte_eth_dev_release_dev_data(uint8_t port_id) { > > + char device[RTE_ETH_NAME_MAX_LEN]; > > + struct rte_eth_dev_data *eth_dev_data =3D NULL; > > + > > + > > @@ -631,6 +691,8 @@ int > > rte_eth_dev_detach(uint8_t port_id, char *name) { > > struct rte_pci_addr addr; > > + struct rte_eth_dev_data *eth_dev_data =3D NULL; > > + char device[RTE_ETH_NAME_MAX_LEN]; > > int ret =3D -1; > > > > if (name =3D=3D NULL) { > > @@ -642,6 +704,15 @@ rte_eth_dev_detach(uint8_t port_id, char *name) > > if (rte_eth_dev_is_detachable(port_id)) > > goto err; > > > > + /* get device name by port id */ > > + if (rte_eth_dev_get_name_by_port(port_id, device)) > > + goto err; > > + > > + /* look for an entry in the shared device data */ > > + eth_dev_data =3D rte_eth_dev_get_dev_data_by_name(device); > > + if (eth_dev_data =3D=3D NULL) > > + goto err; > > + > > if (rte_eth_dev_get_device_type(port_id) =3D=3D RTE_ETH_DEV_PCI) { > > ret =3D rte_eth_dev_get_addr_by_port(port_id, &addr); > > if (ret < 0) > > @@ -661,6 +732,9 @@ rte_eth_dev_detach(uint8_t port_id, char *name) > > goto err; > > } > > > > + /* clear an entry in the shared device data */ > > + memset(eth_dev_data, 0, sizeof(struct rte_eth_dev_data)); > > + > > return 0; > > >=20 > In this function, the new code chunks together is nothing but the functi= on " > rte_eth_dev_release_dev_data()". > So u can call the function itself rather than a duplicate code. It was intentional, reason: If I call function in place: (1) beginning: then I lose device name for function below rte_eth_dev_detac= h_vdev (1.1): a) this is important for drivers that hold name in shared rte_eth_dev_dat= a[] b) not important for drivers that prepare own rte_eth_dev_data e.g pcap (r= te_eth_pcap.c, line 816) (2) end: then I lose device name for my function rte_eth_dev_release_dev_da= ta, because in the above function=20 rte_eth_dev_detach_vdev (1.1) for e.g pcap is call rte_free(eth_dev->data) = which removes me a pointer to the name (rte_eth_pcap.c, line 1079). rte_eth_dev_detach (uint8_t port_id, char *name){ ... (1) rte_eth_dev_release_dev_data(port_id); if (rte_eth_dev_get_device_type(port_id) =3D=3D RTE_ETH_DEV_PCI) { ret =3D rte_eth_dev_get_addr_by_port(port_id, &addr); if (ret < 0) goto err; ret =3D rte_eth_dev_detach_pdev(port_id, &addr); if (ret < 0) goto err; snprintf(name, RTE_ETH_NAME_MAX_LEN, "%04x:%02x:%02x.%d", addr.domain, addr.bus, addr.devid, addr.function); } else { (1.1) ret =3D rte_eth_dev_detach_vdev(port_id, name); if (ret < 0) goto err; } (2) rte_eth_dev_release_dev_data(port_id); ... } This is reason why I keep name at the beginning but I release the name at t= he end function after detach. At this point I do not see how the code directly replace by one function ca= ll. Regards, Marcin >=20 > Thanks, > Reshma