From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f41.google.com (mail-wg0-f41.google.com [74.125.82.41]) by dpdk.org (Postfix) with ESMTP id 0E9CC5A7A for ; Thu, 5 Mar 2015 15:18:49 +0100 (CET) Received: by wggx13 with SMTP id x13so13520876wgg.4 for ; Thu, 05 Mar 2015 06:18:48 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=aX5QIlnjwG4m121wnhx4axHq3geqNs/7ydD0Ic544pc=; b=b1ib8cEa0yLCz9Xbd3SUD8c8amkwFPK+7NHsPLejb0xzLfI/ZMVaKRC0r6aU4/w6jZ ZMuxTwQ2vamXrzETGY/Re6O1V93GjZ5jEJnkdRorX5Tt65T5G+BAde5FGl2Cu6nCbnp3 BPCSxa/KTRpk6mfK+m918TQBef/Xj7qoSxxBlCn8dXo4Pxy6Zs2vzwvl9jb3cwfPLMSo CJHWhzQkrDoktuXYKZiYQS4uE36fTM36oOx0jbknaSGGAokG4gYuHlR5dOWGHnWnMKqW BoGyI7UGnTqmnBaHHKvOVWo+/BOPYO15x22DsqHQubUwgTsQt0c4zxjGAqAXJWDXxe6z EFEg== X-Gm-Message-State: ALoCoQnAeF57Yhr24K/zVy2YH3znDygfrmT9wD8IBBNuOyngOgrOLmAu6nnZUQWe9e8iaguKGNbE X-Received: by 10.180.198.211 with SMTP id je19mr22115153wic.37.1425565128691; Thu, 05 Mar 2015 06:18:48 -0800 (PST) Received: from [10.0.0.166] ([212.143.139.214]) by mx.google.com with ESMTPSA id gf11sm11848866wic.18.2015.03.05.06.18.47 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 05 Mar 2015 06:18:48 -0800 (PST) Message-ID: <54F865C7.8030203@cloudius-systems.com> Date: Thu, 05 Mar 2015 16:18:47 +0200 From: Vlad Zolotarov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Thomas Monjalon References: <1425554885-16901-1-git-send-email-vladz@cloudius-systems.com> <3557886.R5Xnb4aBPR@xps13> <54F85C8F.3010501@cloudius-systems.com> <1835214.fEP04vn92S@xps13> In-Reply-To: <1835214.fEP04vn92S@xps13> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 5/6] common_linuxapp: Added CONFIG_RTE_ETHDEV_LRO_SUPPORT option 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: Thu, 05 Mar 2015 14:18:49 -0000 On 03/05/15 16:01, Thomas Monjalon wrote: > 2015-03-05 15:39, Vlad Zolotarov: >> On 03/05/15 15:19, Thomas Monjalon wrote: >>> 2015-03-05 13:28, Vlad Zolotarov: >>>> Enables LRO support in PMDs. >>>> >>>> Signed-off-by: Vlad Zolotarov >>>> --- >>>> config/common_linuxapp | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/config/common_linuxapp b/config/common_linuxapp >>>> index 97f1c9e..5b98595 100644 >>>> --- a/config/common_linuxapp >>>> +++ b/config/common_linuxapp >>>> @@ -137,6 +137,7 @@ CONFIG_RTE_MAX_ETHPORTS=32 >>>> CONFIG_RTE_LIBRTE_IEEE1588=n >>>> CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16 >>>> CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y >>>> +CONFIG_RTE_ETHDEV_LRO_SUPPORT=y >>> Sorry I don't really follow this ixgbe discussion but I wonder why you >>> would add a compile time option for this feature. >> The only reason is to be able to detect that the feature is present in >> the DPDK version u r compiling against because of the API change. >> Currently, this can't be done using the DPDK version thus we may either > Why you cannot use version? In development phase? > When released, you'll be able to test >= 2.1. Of course! When the version bumps, the #ifdef I've mentioned above may be replaced with the version check. > >> do a try-compilation and if it fails define some application-level macro >> disabling >> the feature usage or we may define a macro in the library level >> (together with tons of other such macros like those in the patch snippet >> above). > I'd prefer a request rte_eth_dev_info_get() to advertise that the feature > is available with the device and driver. > Please let's try to remove config options and #ifdefs. This is exactly what my patch does. But that's not ending there - there is a new feature bit added in rte_eth_rxmode (enable_lro) and in order to compile the application has to know somehow if this bit is present or not. How do u propose to do this now? Of course, I can put such macro in my own tree but then I'll have to rebase all the time and inform other developers that will have to work against my tree (and not upstream as it's supposed to be) - to update. This sounds like a hassle thus I added this macro to resolve this issue until the version is bumped. > >>> What is the benefit of disabling it? >> No benefit whatsoever. >> >>> And if really needed, this patch would make more sense merged with the >>> code under ifdef. >> I strongly disagree - the amount of #ifdefs in the DPDK source is >> absolutely enormous. It makes reading and understanding the code really >> hard. > I agree. You misunderstand me. > I was just saying that this patch should be merged. > >> Therefore, I tried to reduce the amount of time the already existing >> macros have to be queried (see PATCH4). And of course I don't see any >> sense of adding new ones more than really needed. And in LRO case - it's >> a single time, where the feature is manifested by the HW.