From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) by dpdk.org (Postfix) with ESMTP id D323C7E8E for ; Wed, 15 Oct 2014 10:03:25 +0200 (CEST) Received: by mail-wi0-f175.google.com with SMTP id d1so12176601wiv.14 for ; Wed, 15 Oct 2014 01:11:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=EGNn6/koWup0H8sMATLwzLy1YQu/xEq68CeKIVbPJN4=; b=aOX/GW/6TCdRMVNL1YouCOa2XBt7KbnQP8RgF/l7DSMsUZrxphmiuwn+eotedIRK+8 WvdxezyGCmCX/9nz38hWX5YdtqpMOuot99A0LCJZgJUHv0X9EeORYLscmmcRd50inXUa eRam61lJlJIidxp2IySMP9akMvZ+TkP4O0ZSuQZl15nhQaVjrYnbSBEGlp9Ca1pz8YF+ ukzD44YZ34UGA5tKeL7eEYu/tukZgeiDQ4zabTOh8bORwE+C58Os7Ajg31x0uYHx2o9G 9ALsqdcSvz4cUK9fJfocaRUXOnwz/9uZW/G+x+vlOSs0lHyRYBWobDaOvTFKBDGR3GN2 OYXw== X-Gm-Message-State: ALoCoQkSTJg3p3CHmceXBwb4Oax1mpzTCWylu+oezNgHBUlnxK7q7cOVWBdhYTEqlfJdkNOhFraR X-Received: by 10.194.61.164 with SMTP id q4mr10376421wjr.60.1413360673667; Wed, 15 Oct 2014 01:11:13 -0700 (PDT) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id p1sm22896781wjy.22.2014.10.15.01.11.12 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Oct 2014 01:11:12 -0700 (PDT) From: Thomas Monjalon To: "Chen, Jing D" Date: Wed, 15 Oct 2014 10:10:58 +0200 Message-ID: <10317086.OmtuOpvoDk@xps13> Organization: 6WIND User-Agent: KMail/4.14.1 (Linux/3.16.4-1-ARCH; KDE/4.14.1; x86_64; ; ) In-Reply-To: <4341B239C0EFF9468EE453F9E9F4604D015F35FE@shsmsx102.ccr.corp.intel.com> References: <1411478047-1251-1-git-send-email-jing.d.chen@intel.com> <20784375.UWMeVtoV91@xps13> <4341B239C0EFF9468EE453F9E9F4604D015F35FE@shsmsx102.ccr.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 1/6] ether: enhancement for VMDQ support 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: Wed, 15 Oct 2014 08:03:26 -0000 2014-10-15 06:59, Chen, Jing D: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > enum rte_eth_rx_mq_mode { > > > - ETH_MQ_RX_NONE = 0, /**< None of DCB,RSS or VMDQ mode */ > > > - > > > - ETH_MQ_RX_RSS, /**< For RX side, only RSS is on */ > > > - ETH_MQ_RX_DCB, /**< For RX side,only DCB is on. */ > > > - ETH_MQ_RX_DCB_RSS, /**< Both DCB and RSS enable */ > > > - > > > - ETH_MQ_RX_VMDQ_ONLY, /**< Only VMDQ, no RSS nor DCB */ > > > - ETH_MQ_RX_VMDQ_RSS, /**< RSS mode with VMDQ */ > > > - ETH_MQ_RX_VMDQ_DCB, /**< Use VMDQ+DCB to route traffic to > > queues */ > > > - ETH_MQ_RX_VMDQ_DCB_RSS, /**< Enable both VMDQ and DCB in > > VMDq */ > > > + /**< None of DCB,RSS or VMDQ mode */ > > > + ETH_MQ_RX_NONE = 0, > > > + > > > + /**< For RX side, only RSS is on */ > > > + ETH_MQ_RX_RSS = ETH_MQ_RX_RSS_FLAG, > > > + /**< For RX side,only DCB is on. */ > > > + ETH_MQ_RX_DCB = ETH_MQ_RX_DCB_FLAG, > > > + /**< Both DCB and RSS enable */ > > > + ETH_MQ_RX_DCB_RSS = ETH_MQ_RX_RSS_FLAG | > > ETH_MQ_RX_DCB_FLAG, > > > + > > > + /**< Only VMDQ, no RSS nor DCB */ > > > + ETH_MQ_RX_VMDQ_ONLY = ETH_MQ_RX_VMDQ_FLAG, > > > + /**< RSS mode with VMDQ */ > > > + ETH_MQ_RX_VMDQ_RSS = ETH_MQ_RX_RSS_FLAG | > > ETH_MQ_RX_VMDQ_FLAG, > > > + /**< Use VMDQ+DCB to route traffic to queues */ > > > + ETH_MQ_RX_VMDQ_DCB = ETH_MQ_RX_VMDQ_FLAG | > > ETH_MQ_RX_DCB_FLAG, > > > + /**< Enable both VMDQ and DCB in VMDq */ > > > + ETH_MQ_RX_VMDQ_DCB_RSS = ETH_MQ_RX_RSS_FLAG | > > ETH_MQ_RX_DCB_FLAG | > > > + ETH_MQ_RX_VMDQ_FLAG, > > > }; > > > > Why not simply remove all these combinations and keep only flags? > > Please keep it simple. > > One reason is back-compatibility. I understand but I think we should prefer cleanup. As there is no way to advertise deprecation of flags, it should be simply removed. > Another reason is not all NIC driver support all the combined modes, only limited sets > driver supported. Under this condition, it's better to use the combination definition > (VMDQ_DCB, DCB_RSS, etc) to let driver check whether it supports. Driver can do the same checks with simple flags and it's probably simpler (e.g. a driver which doesn't support VMDQ had no need to check all VMDQ combinations). > > There are only few typos and minor things but it would help to have more > > careful reviews. Having a list of people at the beginning of the patch > > didn't help in this case. > > I listed all the code reviewers out to reduce their workload to reply the email, > not mean to make it easier to be applied. I have no problem with listing of reviewers when submitting patches. To say more, I prefer you list them by yourself and you add new reviewers when sending new versions of the patchset. But I would like reviewers to be more careful. They are especially useful to discuss design choices and check typos. Having reviewer give credits to the patch only if we are confident that the review task is generally seriously achieved. -- Thomas