From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhe Tao Subject: Re: [PATCH v12 1/2] i40e: support floating VEB config Date: Mon, 27 Jun 2016 04:28:35 +0800 Message-ID: <20160626202835.GB99425@intel.com> References: <1465883834-9409-1-git-send-email-zhe.tao@intel.com> <1466756979-2175-1-git-send-email-zhe.tao@intel.com> <1466756979-2175-2-git-send-email-zhe.tao@intel.com> <576D1606.4090901@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, jingjing.wu@intel.com To: Ferruh Yigit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id D9871567A for ; Mon, 27 Jun 2016 06:45:24 +0200 (CEST) Content-Disposition: inline In-Reply-To: <576D1606.4090901@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Jun 24, 2016 at 12:14:14PM +0100, Ferruh Yigit wrote: > Hi Zhe, > > On 6/24/2016 9:29 AM, Zhe Tao wrote: > > Add the new floating VEB related arguments option in the devarg. > > Using this parameter, all the applications can decide whether to use legacy > > VEB/VEPA or floating VEB. > > To enable this feature, the user should pass a devargs parameter to the > > EAL like "-w 84:00.0,enable_floating_veb=1", and the application will > > tell PMD whether to use the floating VEB feature or not. > Is providing the enable_floating_veb=1 devarg enough, or should > application call an driver API to enable this feature, if so documenting > what application needs to do helps app developers. yes only provide the devarg is enough, the APP don't need to modify. > > > Once the floating VEB feature is enabled, all the VFs created by > > this PF device are connected to the floating VEB. > Technically there can be multiple floating VEBs, right? And with this > param only one floating VEB created and all VFs connected to it. Is > there any use case to create multiple VEBs with selective VFs connected > to them? discuss this feature internally with Walter and FAE team, have an agreement for this release we only implement one floating VEB. > > > > > Also user can specify which VF need to connect to this floating veb using > > "floating_veb_list". > > Like "-w 84:00.0,enable_floating_veb=1,floating_veb_list=1/3-4", means VF1, VF3, > > VF4 connect to the floating VEB, other VFs connect to the legacy VEB.The "/" > > is used for delimiter of the floating VEB list. > Is there a use case to change VF VEB connection on runtime? maybe there is a case, but from the customer requirement, we only support the static configuration for floating VEB. > > > > > All the VEB/VEPA concepts are not specific for FVL, they are defined in > > the 802.1Qbg spec. > > > > But for floating VEB, it has two major difference. > > 1. doesn't has a up link connection which means > > the traffic cannot go to outside world. > > 2. doesn't need to connect to the physical port which means > > when the physical link is down the floating VEB can still works > > fine. > > > > Signed-off-by: Zhe Tao > > --- > > ... > > > > > +static int vf_floating_veb_handler(__rte_unused const char *key, > What about floating_veb_list_handler, to be consistent with argument name? > > > + const char *floating_veb_value, > > + void *opaque) > > +{ > > + int idx = 0; > > + bool floating_veb; /* The flag to use the floating VEB */ > > + /* The floating enable flag for the specific VF */ > > + bool vf_floating_veb[I40E_MAX_VF]; > what about floating_veb_list, to be consistent with argument name? > > > }; > > > > enum pending_msg { > > All the comments in the code is great, I changed the code according to your comments, thanks again Zhe