From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f65.google.com (mail-vk0-f65.google.com [209.85.213.65]) by dpdk.org (Postfix) with ESMTP id E78A91C00 for ; Mon, 4 Dec 2017 20:37:49 +0100 (CET) Received: by mail-vk0-f65.google.com with SMTP id l187so10366517vke.5 for ; Mon, 04 Dec 2017 11:37:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=scylladb-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=J2lfXBRbICgDKM28X6jnobG/qrfeK37Xut05/EnNtKc=; b=eBRNDjcMuTCg1VyTI0e291Mwf7dLz0J86zrzb9qVoRVGkxD6I6UkCszvJZbVmndmkR 9N+4k54QwO0bzeFhxfscCGob9sYm9LmsIWqFq+tF1L/WNQtw6Pr5EOXV4kIe2X8SSDbC kwEk9MwVD0TTmnl6vSDeSgGDX1R/EG81b09fZ+KtPUt0f65CtBNkSVoEOdljOBaztK3l XOD8MlB7hoPi24VBG/pIbTajpLAFhZ5j4/arfSk14OovFoumi6ZX7RtQPoBId5NH7+DE 6OOqif1iG/YXtw6kt9eAHPUQjwfDzOO4GZc5Vb1WBPBdnjgmQmtWGN9UDYmUYOBeq1Fe LQOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=J2lfXBRbICgDKM28X6jnobG/qrfeK37Xut05/EnNtKc=; b=JhHsRH+/pmUDYRrkpN3sM1oKCyvO71zy7j2SvsFVnxayJgzdk23rHXIz+z2m5SQG2B vciCC47vgUL0TtgTXsaiF7mIulZg7wbvDaP91C+C44sOjiaimN6mrz25PE42VI3gfBxj ZDJR3g/hOaNJRNh6Gtkj8Es0IT1ONBRNj/lJd1OeH72hsHEhArEw5ksCuf8EwFNqfqOC KDQqbj7BlneL9M8q6nbtFajaOHsOdGkAG+woHOMFtL2nyDX6DmHV4/iMzewRhf7dCMmN 4AhFymfCtbhxEYPnFyD0uV6cE8YRAeLEEBhYC6wc+M1vxObA1T+8hcJpYCO1J4MHz+4K Q9FA== X-Gm-Message-State: AKGB3mJOZNT80LUrtwRyoykL/xpzLVlFBpcakbgosoAw3/liM0mtUJcx TgA4AdB8oTT0bRK5Kn5MWs1jpg== X-Google-Smtp-Source: AGs4zMZbAjgpuG4zj0Lffiqw2SalzQV5lDDZg3fYZWkM0hOUWOp+YKiChPw95pp3yNnk37//S/5GWQ== X-Received: by 10.31.61.207 with SMTP id k198mr227362vka.43.1512416269077; Mon, 04 Dec 2017 11:37:49 -0800 (PST) Received: from [192.168.1.138] (ool-68f614aa.dyn.optonline.net. [104.246.20.170]) by smtp.googlemail.com with ESMTPSA id h8sm3486573vkc.51.2017.12.04.11.37.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Dec 2017 11:37:47 -0800 (PST) To: Ferruh Yigit , Thomas Monjalon Cc: dev@dpdk.org, vladz@cloudius-systems.com References: <20171201022957.64329-1-ferruh.yigit@intel.com> <49eb13f1-e12f-071a-b58b-1cd4852b7c8b@scylladb.com> <2588d596-19be-483a-507f-565b01ff5615@intel.com> <3ccbeca0-79d5-d94d-ad77-e5a11e387049@intel.com> From: Vlad Zolotarov Message-ID: <6cec5132-7c8e-42a1-4c69-5c4c4bf1ef2a@scylladb.com> Date: Mon, 4 Dec 2017 14:37:46 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <3ccbeca0-79d5-d94d-ad77-e5a11e387049@intel.com> Content-Language: en-US Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 1/7] ethdev: remove unused flag from header X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Dec 2017 19:37:50 -0000 On 12/04/2017 12:54 PM, Ferruh Yigit wrote: > On 12/1/2017 6:22 PM, Vlad Zolotarov wrote: >> >> On 12/01/2017 06:51 PM, Ferruh Yigit wrote: >>> On 12/1/2017 2:17 PM, Vlad Zolotarov wrote: >>>> On 11/30/2017 09:29 PM, Ferruh Yigit wrote: >>>>> remove RTE_ETHDEV_HAS_LRO_SUPPORT flag from header. >>>>> >>>>> Flag seems added with the patch that adds LRO support, and intention >>>>> looks like giving a pointer to application that library supports LRO. >>>> Exactly. Removing this flag may make the existing application "think" that LRO >>>> is not supported. >>>> Why do you want to remove it to begin with? >>> I agree that this is a little controversial, and I don't have a strong opinion >>> about it, I thought twice before sending or not sending the patch :) >>> >>> This flag can be useful for the applications that use different versions of the >>> DPDK library (with the ones does and doesn't support LRO), so that application >>> can be more portable. >>> I would understand a dynamic approach, which would be useful for distributed >>> applications that you don't know and can't control what version of the DPDK >>> library used. >>> But here to benefit from this flag you need to compile your application against >>> DPDK library, and if you are compiling it you already know if your DPDK supports >>> LRO or not. And this is not something keeps changing platform to platform etc, >>> after a specific release LRO is always supported. >> True but DPDK is usually not a part of your source tree - it (should) come as a >> library and currently there isn't any proper way to query feature set. >> This flag was also added after a lot of consideration as a rather ugly >> compromise since there wasn't (and there isn't) a proper way to do that at that >> (this) time. See more on that below. >> >>> And this is the only capability support flag in the ethdev, there are many new >>> features introduced in each release but they are not advertised by a capability >>> flag. Only having LRO flag can be confusing. >>> >>> _Perhaps_ DPDK should have a way to expose the supported features, and a dynamic >>> runtime way can be better option than compile time macros, but it should be >>> generic nothing specific to LRO support. >> IMO it's not "perhaps" it's a "must" _however_ > This is even wider than ethdev, more like in DPDK scope, we need someone to > attack this issue. absolutely! > >> you can't remove this flag without giving some other tool to do the same. > Fair enough, I will drop the patch. > >> Querying a DPDK version would be a wrong direction because some Linux >> distributions (yes, I'm talking about you, Ubuntu) tend to have it's own trees >> with their own backports and these trees may be light years ahead in their patch >> level compared to vanilla trees with the same version. > +1 to have better way than version check. > >>>>> Fixes: 8eecb3295aed ("ixgbe: add LRO support") >>>>> Cc: vladz@cloudius-systems.com >>>>> >>>>> Signed-off-by: Ferruh Yigit >>>>> --- >>>>> lib/librte_ether/rte_ethdev.h | 3 --- >>>>> 1 file changed, 3 deletions(-) >>>>> >>>>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h >>>>> index 341c2d624..e620c3706 100644 >>>>> --- a/lib/librte_ether/rte_ethdev.h >>>>> +++ b/lib/librte_ether/rte_ethdev.h >>>>> @@ -172,9 +172,6 @@ extern "C" { >>>>> >>>>> #include >>>>> >>>>> -/* Use this macro to check if LRO API is supported */ >>>>> -#define RTE_ETHDEV_HAS_LRO_SUPPORT >>>>> - >>>>> #include >>>>> #include >>>>> #include