From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 30AF71B239 for ; Tue, 19 Dec 2017 21:44:37 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Dec 2017 12:44:36 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,428,1508828400"; d="scan'208";a="4034593" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.241.225.69]) ([10.241.225.69]) by orsmga007.jf.intel.com with ESMTP; 19 Dec 2017 12:44:36 -0800 To: Adrien Mazarguil Cc: dev@dpdk.org, Stephen Hemminger References: <20171124172132.GW4062@6wind.com> <20171218162443.12971-1-adrien.mazarguil@6wind.com> <20171218162443.12971-3-adrien.mazarguil@6wind.com> <29ee4328-363b-19e2-fcc5-c1af57395277@intel.com> <20171219150605.GG5377@6wind.com> From: Ferruh Yigit Message-ID: <1f1dbbf9-d1b4-504a-8cbd-e2717ee804d7@intel.com> Date: Tue, 19 Dec 2017 12:44:35 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171219150605.GG5377@6wind.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v1 2/3] net/hyperv: implement core functionality 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, 19 Dec 2017 20:44:38 -0000 On 12/19/2017 7:06 AM, Adrien Mazarguil wrote: > On Mon, Dec 18, 2017 at 05:54:45PM -0800, Ferruh Yigit wrote: >> On 12/18/2017 8:46 AM, Adrien Mazarguil wrote: >>> As described in more details in the attached documentation (see patch >>> contents), this virtual device driver manages NetVSC interfaces in virtual >>> machines hosted by Hyper-V/Azure platforms. >>> >>> This driver does not manage traffic nor Ethernet devices directly; it acts >>> as a thin configuration layer that automatically instantiates and controls >>> fail-safe PMD instances combining tap and PCI sub-devices, so that each >>> NetVSC interface is exposed as a single consolidated port to DPDK >>> applications. >>> >>> PCI sub-devices being hot-pluggable (e.g. during VM migration), >>> applications automatically benefit from increased throughput when present >>> and automatic fallback on NetVSC otherwise without interruption thanks to >>> fail-safe's hot-plug handling. >>> >>> Once initialized, the sole job of the hyperv driver is to regularly scan >>> for PCI devices to associate with NetVSC interfaces and feed their >>> addresses to corresponding fail-safe instances. >>> >>> Signed-off-by: Adrien Mazarguil >> >> <...> >> >>> + RTE_ETH_FOREACH_DEV(port_id) { >> <..> >>> + ret = rte_eal_hotplug_remove(bus->name, dev->name); >> <..> >>> + ret = rte_eal_hotplug_add("vdev", ctx->devname, ctx->devargs); >> >> Overall why this logic implemented as network PMD? >> Yes technically you can implement *anything* as PMD :), but should we? >> >> This code does eal level work (scans bus, add/remove devices), and for control >> path, and not a generic solution either (specific to netvsc and failsafe). >> >> Only device argument part of a PMD seems used, rest is unrelated to being a PMD. >> Scans netvsc changes in background and reflects them into failsafe PMD... >> >> Why this is implemented as PMD, not another entity, like bus driver perhaps? >> >> Or indeed why this in DPDK instead of being in application? > > I'll address that last question first: the point of this driver is enabling > existing applications to run within a Hyper-V environment unmodified, > because they'd otherwise need to manage two driver instances correctly on > their own in addition to hot-plug events during VM migration. > > Some kind of driver generating a front end to what otherwise appears as two > distinct ethdev to applications is therefore necessary. > > Currently without it, users have to manually configure failsafe properly for > each NetVSC interface on their system. Besides the inconvenience, it's not > even a possibility with DPDK applications that don't rely on EAL > command-line arguments. > > As such it's more correctly defined as a "platform" driver rather than a > true PMD. It leaves VF device handling to their respective PMDs while > automatically managing the platform-specific part itself. There's no simpler > alternative when running in blacklist mode (i.e. not specifying any device > parameters on the command line). > > Regarding its presence in drivers/net rather than drivers/bus, the end > result from an application standpoint is that each instance exposes a single > ethdev, even if not its own (failsafe's). Busses don't do that. It also > allows passing arguments to individual devices through --vdev if needed. > > You're right about putting device detection at the bus level though, and I > think there's work in progress to do just that, this driver will be updated > to benefit from it once applied. In the meantime, the code as submitted > works fine with the current DPDK code base and addresses an existing use > case for which there is no solution at this point. This may be working but this looks like a hack to me. If we need a platform driver why not properly work on it. If we need to improve eal hotplug, this is a good motivation to improve it. And if this logic needs to be in application let it be, your argument is to not change the existing application but this logic may lead implementing many unrelated things as PMD to not change application, what is the line here. What is the work in progress, exact list, that will replace this solution? If this hackish solution will prevent that real work, I am against this solution. Is there a way to ensure this will be a temporary solution and that real work will happen?