From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vlad Zolotarov Subject: Re: [PATCH v2 5/6] common_linuxapp: Added CONFIG_RTE_ETHDEV_LRO_SUPPORT option Date: Thu, 05 Mar 2015 15:39:27 +0200 Message-ID: <54F85C8F.3010501@cloudius-systems.com> References: <1425554885-16901-1-git-send-email-vladz@cloudius-systems.com> <1425554885-16901-6-git-send-email-vladz@cloudius-systems.com> <3557886.R5Xnb4aBPR@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: dev-VfR2kkLFssw@public.gmane.org To: Thomas Monjalon Return-path: In-Reply-To: <3557886.R5Xnb4aBPR@xps13> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" 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 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). > 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. 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. >