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 919AC1B8C0 for ; Tue, 26 Jun 2018 17:12:32 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2018 08:12:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,274,1526367600"; d="scan'208";a="67430226" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.237.220.28]) ([10.237.220.28]) by fmsmga001.fm.intel.com with ESMTP; 26 Jun 2018 08:12:28 -0700 To: "Zhang, Qi Z" , "thomas@monjalon.net" Cc: "Ananyev, Konstantin" , "dev@dpdk.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Shelton, Benjamin H" , "Vangati, Narender" References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180626070832.3055-1-qi.z.zhang@intel.com> <20180626070832.3055-7-qi.z.zhang@intel.com> <969f0cb7-4186-d05c-442f-6341f1c44e93@intel.com> <039ED4275CED7440929022BC67E706115323E71A@SHSMSX103.ccr.corp.intel.com> <30b1c93c-1d7c-07d9-d41a-76e84f63d6a1@intel.com> <039ED4275CED7440929022BC67E706115323E7A5@SHSMSX103.ccr.corp.intel.com> <365fd549-14c6-ae72-84be-3ec7389eaa84@intel.com> <039ED4275CED7440929022BC67E706115323E7E2@SHSMSX103.ccr.corp.intel.com> <53fe3a3e-ae37-0765-f83d-c88a279028a1@intel.com> <039ED4275CED7440929022BC67E706115323E8B9@SHSMSX103.ccr.corp.intel.com> From: "Burakov, Anatoly" Message-ID: <0b7fffc1-9637-dcd8-e0a1-e8e6c73621b5@intel.com> Date: Tue, 26 Jun 2018 16:12:27 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <039ED4275CED7440929022BC67E706115323E8B9@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4 06/24] ethdev: enable hotplug on multi-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: Tue, 26 Jun 2018 15:12:34 -0000 On 26-Jun-18 3:24 PM, Zhang, Qi Z wrote: > > >> -----Original Message----- >> From: Burakov, Anatoly >> Sent: Tuesday, June 26, 2018 9:46 PM >> To: Zhang, Qi Z ; thomas@monjalon.net >> Cc: Ananyev, Konstantin ; dev@dpdk.org; >> Richardson, Bruce ; Yigit, Ferruh >> ; Shelton, Benjamin H >> ; Vangati, Narender >> >> Subject: Re: [PATCH v4 06/24] ethdev: enable hotplug on multi-process >> >> On 26-Jun-18 2:25 PM, Zhang, Qi Z wrote: >>> >>> >>>> -----Original Message----- >>>> From: Burakov, Anatoly >>>> Sent: Tuesday, June 26, 2018 9:21 PM >>>> To: Zhang, Qi Z ; thomas@monjalon.net >>>> Cc: Ananyev, Konstantin ; dev@dpdk.org; >>>> Richardson, Bruce ; Yigit, Ferruh >>>> ; Shelton, Benjamin H >>>> ; Vangati, Narender >>>> >>>> Subject: Re: [PATCH v4 06/24] ethdev: enable hotplug on multi-process >>>> >>>> On 26-Jun-18 1:58 PM, Zhang, Qi Z wrote: >>>>> >>>>> my understand is peer is identified by a string (or filename) what I >>>>> mean is clone the content of the buffer that peer point to , So I >>>>> don't need to worry if the original peer be used to point to some >>>>> other data >>>>> >>>> >>>> As far as the application is concerned, peer is an opaque pointer, >>>> and should be treated as such. Peeking behind a void pointer that is >>>> not designed for this purpose is not a good idea, even if technically you >> know what's in there. >>> >>> We can expose a clone interface, like MP_PEER_CLONE, so we don't need to >> know what's inside, just need to know that it can be used on another thread? >>> >> >> Well, that can probably work. Feels like a hacky workaround though. >> >> Another way to do the same thing would be to store peer information right in >> the message, as opposed to providing it separately. Still a hack though, and will >> require far more changes, but it could be a better solution as (if done right) it >> would allow identifying which reply came from which peer. >> >> Of course, an even better approach would be to devise some kind of >> addressing scheme (uuid?), so that peer addresses are no longer opaque >> pointers but rather are valid data types. >> >> Thoughts? > > I may not give insight comment from the IPC implementation, but from user's view, what required is a unique token, > it can be used for reply at anywhere and at any time, to me, currently implementation looks like missing some mapping management between an abstract token to its real data. > Well, the idea was to not provide that :) We wanted to keep it simple and actively discourage any attempts to know which peer you're communicating with, as allowing that would imply some kind of addressing scheme, peer discovery and things like that, which we don't want to bother with. If you want replies, you use callbacks, which provide you with a peer address. On the one hand, i understand the frustration of being forced to deal with deliberate simplicity of IPC API and to create workarounds like doing sendmsg() and storing requests that you're waiting on inside the application and launching interrupt threads, all for things that should Just Work in an IPC API. Believe me, i know about its deficiencies more than most people :) On the other hand, we don't want to introduce hacks to solve a specific problem. If we are to solve this in IPC, we should do it properly. I don't think adding a workaround with "cloning" peer address is the solution. A proper solution would've been to implement addressing scheme, so that any response could be submitted at any time. The hard route would be to add peer discovery, etc. The easy route would be to just change the peer address to something typed, something that we can use to reconstruct the original peer id. A great candidate for this is the uuid, but it's an external dependency. A workaround for this would be to just use a uint64 value obtained through some magic that is part of socket path. For example, replace "/var/run/dpdk/rte/mp_socket_pid_rdtsc" with something just as (likely) unique, but something that can be put into a uint64. Without significantly changing the API and the internals, i think this is the best we will be able to do. > Thanks > Qi. > >> >> -- >> Thanks, >> Anatoly -- Thanks, Anatoly