From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 26 Jun 2016 21:45:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,535,1459839600"; d="scan'208";a="725463911" Received: from adambynes.sh.intel.com (HELO adambynes) ([10.239.129.240]) by FMSMGA003.fm.intel.com with ESMTP; 26 Jun 2016 21:45:23 -0700 Date: Mon, 27 Jun 2016 04:28:35 +0800 From: Zhe Tao To: Ferruh Yigit Cc: dev@dpdk.org, jingjing.wu@intel.com 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 Content-Disposition: inline In-Reply-To: <576D1606.4090901@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v12 1/2] i40e: support floating VEB config 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: Mon, 27 Jun 2016 04:45:25 -0000 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