From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 3B1EC1396 for ; Wed, 22 Mar 2017 11:33:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1490178797; x=1521714797; h=subject:to:references:cc:from:message-id:date: mime-version:in-reply-to:content-transfer-encoding; bh=gPkxHYmgmIz6ARtk0Yg99xh/yAyKW+A5tBGQDjGdR/Q=; b=sYnpk2j5eFCkXIaZA8r8xUx9U26qw385BcLBukHYAk1ljPWZArNC4phG Jt7U7VOG6v7LHwekmVRtvQjRi7WfJg==; Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Mar 2017 03:33:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,204,1486454400"; d="scan'208";a="79769275" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.122]) ([10.237.220.122]) by fmsmga006.fm.intel.com with ESMTP; 22 Mar 2017 03:33:15 -0700 To: Jerin Jacob References: <1490019040-6268-1-git-send-email-jerin.jacob@caviumnetworks.com> <20170321143832.swnf7xetxlxc3k5w@localhost.localdomain> <8059571c-9747-ca33-0d61-f453e43afa48@intel.com> <20170322093931.2u5z3n645cnobi63@localhost.localdomain> Cc: dev@dpdk.org, Christian Ehrhardt From: Ferruh Yigit Message-ID: <526d95ae-fe16-6c54-441c-32368df01c7d@intel.com> Date: Wed, 22 Mar 2017 10:33:14 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170322093931.2u5z3n645cnobi63@localhost.localdomain> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] net/thunderx: sync mailbox definitions with Linux PF driver 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: Wed, 22 Mar 2017 10:33:17 -0000 On 3/22/2017 9:39 AM, Jerin Jacob wrote: > On Tue, Mar 21, 2017 at 02:53:29PM +0000, Ferruh Yigit wrote: >> On 3/21/2017 2:38 PM, Jerin Jacob wrote: >>> On Tue, Mar 21, 2017 at 02:31:41PM +0000, Ferruh Yigit wrote: >>>> On 3/20/2017 2:10 PM, Jerin Jacob wrote: >>>>> - bgx_link_status mbox definition was changed in Linux >>>>> commit 1cc702591bae ("net: thunderx: Add ethtool support") >>>>> - NIC_MBOX_MSG_RES_BIT related changes were never part of Linux PF driver >>>>> >>>>> Signed-off-by: Jerin Jacob >>>> >>>> <...> >>>> >>>>> @@ -157,6 +151,7 @@ struct rss_cfg_msg { >>>>> /* Physical interface link status */ >>>>> struct bgx_link_status { >>>>> uint8_t msg; >>>>> + uint8_t mac_type; >>>> >>>> Hi Jerin, >>>> >>>> Is this modification related to this patch? >>> >>> Yes Ferruh. >>> >>> This was related to the following section in git log comment. >>> ---- >>> - bgx_link_status mbox definition was changed in Linux >>> commit 1cc702591bae ("net: thunderx: Add ethtool support") >>> --- >> >> I see now, thanks. Since this is to sync with Linux PF, shouldn't it be >> used in driver, perhaps something like in Linux driver: >> "nic->mac_type = mbx.link_status.mac_type" >> >> What is the point of just adding definition without using it? > > That is to keep "link_up"(next element in the struct bgx_link_status) points > to correct location after the kernel change. I see, thanks. It is pity that new field added into middle (or beginning if you exclude msg) of the struct. > I thought about, the backward > compatibility with older kernel, Is it OK to use linux/version.h in PMD drivers > to detect the kernel version? Technically possible, but kernel version check has its problems: 1) Adds maintenance cost, which gets worse by time. 2) If added a compile time check, it creates binary distribution problems, distro guys may not like PMD has dependency to kernel. 3) It will create dependency to Linux, assuming Thunderx PMD supported in FreeBSD > drivers/net/mlx5/mlx5_ethdev.c has similar > kernel detection mechanism to make it backward compatible. Yes, at least mlx added a dynamic check which prevents above issue 2. > If there are no issue with such approach, I will roll out a new revision. Is there any way to detect this dynamically, first things I can think of: - Any mbx versioning to rely on? - Any common NIC user header file provided by kernel, to use in DPDK, to prevent any this kind of issues in the future? - Any msg size value to detect if mac_type exists or not? - Heuristic approach to other values to detect mac_type existence? > > >> >>> >>> >>>> >>>>> uint8_t link_up; >>>>> uint8_t duplex; >>>>> uint32_t speed; >>>>> >>>> >>