DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>,
	Qiming Yang <qiming.yang@intel.com>
Cc: dev@dpdk.org, Remy Horton <remy.horton@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 0/5] example/ethtool: add bus info and fw version get
Date: Thu, 22 Dec 2016 14:36:13 +0000	[thread overview]
Message-ID: <a191e031-c2fd-2a18-ea02-af5fff9668ec@intel.com> (raw)
In-Reply-To: <1578263.GeZ0IiYehl@xps13>

On 12/22/2016 11:07 AM, Thomas Monjalon wrote:
> 2016-12-08 16:34, Remy Horton:
>>
>> On 06/12/2016 15:16, Qiming Yang wrote:
>> [..]
>>> Qiming Yang (5):
>>>   ethdev: add firmware version get
>>>   net/e1000: add firmware version get
>>>   net/ixgbe: add firmware version get
>>>   net/i40e: add firmware version get
>>>   ethtool: dispaly bus info and firmware version
>>
>> s/dispaly/display
>>
>> doc/guides/rel_notes/release_17_02.rst ought to be updated as well. Code 
>> itself looks ok though..
>>
>> Acked-by: Remy Horton <remy.horton@intel.com>
> 
> It must be a feature in the table (doc/guides/nics/features/).
> The deprecation notice must be removed also.
> 
> I think it is OK to add a new dev_ops and a new API function for firmware
> query. Generally speaking, it is a good thing to avoid putting all
> informations in the same structure (e.g. rte_eth_dev_info). 

OK.

> However, there
> is a balance to find. Could we plan to add more info to this new query?
> Instead of
> 	rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int fw_length)

Here there is another problem, the content and the format of the string
is not defined. In this patchset it is not same for different PMDs.
This is OK for just printing the data, but not good for an API. How can
the application know what to expect.

> could it fill a struct?
> 	rte_eth_dev_fw_info_get(uint8_t port_id, struct rte_eth_dev_fw_info *fw_info)

I believe this is better. But the problem we are having with this usage
is: ABI breakage.

Since this struct will be a public structure, in the future if we want
to add a new field to the struct, it will break the ABI, and just this
change will cause a new version for whole ethdev library!

When all required fields received via arguments, one by one, instead of
struct, at least ABI versioning can be done on the API when new field
added, and can be possible to escape from ABI breakage. But this will be
ugly when number of arguments increased.

Or any other opinion on how to define API to reduce ABI breakage?

> 
> We already have
> 	rte_eth_dev_get_reg_info(uint8_t port_id, struct rte_dev_reg_info *info)
> with
> 	uint32_t version; /**< Device version */
> 
> There are also these functions (a bit related):
> 	rte_eth_dev_get_eeprom_length(uint8_t port_id)
> 	rte_eth_dev_get_eeprom(uint8_t port_id, struct rte_dev_eeprom_info *info)
> 

  reply	other threads:[~2016-12-22 14:36 UTC|newest]

Thread overview: 128+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-17  9:42 [dpdk-dev] [PATCH] e1000: add firmware " Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH 1/5] ethdev: " Qiming Yang
2016-11-17 13:36   ` Thomas Monjalon
2016-11-18  2:10     ` Yang, Qiming
2016-11-18  1:09   ` Remy Horton
2016-11-18  2:18     ` Yang, Qiming
2016-12-06  7:16   ` [dpdk-dev] [PATCH v2 0/5] example/ethtool: add bus info and fw " Qiming Yang
2016-12-06  7:16     ` [dpdk-dev] [PATCH v2 1/5] ethdev: add firmware " Qiming Yang
2016-12-08 11:07       ` Ferruh Yigit
2016-12-12  1:28         ` Yang, Qiming
2016-12-06  7:16     ` [dpdk-dev] [PATCH v2 2/5] net/e1000: " Qiming Yang
2016-12-07  1:16       ` Lu, Wenzhuo
2016-12-06  7:16     ` [dpdk-dev] [PATCH v2 3/5] net/ixgbe: " Qiming Yang
2016-12-06  7:16     ` [dpdk-dev] [PATCH v2 4/5] net/i40e: " Qiming Yang
2016-12-06  7:16     ` [dpdk-dev] [PATCH v2 5/5] ethtool: dispaly bus info and firmware version Qiming Yang
2016-12-23 12:50       ` Ferruh Yigit
2016-12-27 13:06       ` [dpdk-dev] [PATCH v3] " Qiming Yang
2017-01-04  7:51         ` Wu, Jingjing
2017-01-04 12:18         ` [dpdk-dev] [PATCH v4] ethtool: dispaly bus information Qiming Yang
2017-01-04 14:49           ` Mcnamara, John
2017-01-05  1:51             ` Yang, Qiming
2017-02-09 21:32           ` Thomas Monjalon
2016-12-08  8:34     ` [dpdk-dev] [PATCH v2 0/5] example/ethtool: add bus info and fw version get Remy Horton
2016-12-12  1:43       ` Yang, Qiming
2016-12-22 11:07       ` Thomas Monjalon
2016-12-22 14:36         ` Ferruh Yigit [this message]
2016-12-22 14:47           ` Thomas Monjalon
2016-12-22 15:05             ` Ferruh Yigit
2016-12-22 15:31               ` Thomas Monjalon
2016-12-23 12:48                 ` Ferruh Yigit
2017-01-05  3:04                 ` Zhang, Helin
2016-12-27 12:30     ` [dpdk-dev] [PATCH v3 0/4] new API 'rte_eth_dev_fw_info_get' Qiming Yang
2016-12-27 12:30       ` [dpdk-dev] [PATCH v3 1/4] ethdev: add firmware information get Qiming Yang
2017-01-02 15:38         ` Thomas Monjalon
     [not found]           ` <F5DF4F0E3AFEF648ADC1C3C33AD4DBF16EDC9515@SHSMSX101.ccr.corp.intel.com>
     [not found]             ` <10603884.vrshqR2O82@xps13>
2017-01-03  9:05               ` Yang, Qiming
2017-01-03 14:49                 ` Ferruh Yigit
2017-01-04  3:33                   ` Yang, Qiming
2017-01-04  7:48                     ` Wu, Jingjing
2017-01-04  8:43                       ` Ferruh Yigit
2017-01-05  1:04                         ` Wu, Jingjing
2017-01-03 14:58         ` Ferruh Yigit
2016-12-27 12:30       ` [dpdk-dev] [PATCH v3 2/4] net/e1000: add firmware version get Qiming Yang
2017-01-03 15:02         ` Ferruh Yigit
2017-01-04  3:14           ` Yang, Qiming
2017-01-04  8:47             ` Ferruh Yigit
2016-12-27 12:30       ` [dpdk-dev] [PATCH v3 3/4] net/ixgbe: " Qiming Yang
2017-01-03 15:04         ` Ferruh Yigit
2017-01-04  2:44           ` Yang, Qiming
2017-01-04  9:06             ` Ferruh Yigit
2017-01-04  9:48               ` Yang, Qiming
2017-01-04 12:01                 ` Ferruh Yigit
2016-12-27 12:30       ` [dpdk-dev] [PATCH v3 4/4] net/i40e: " Qiming Yang
2017-01-04 12:03       ` [dpdk-dev] [PATCH v4 0/5] new API 'rte_eth_dev_fw_version_get' Qiming Yang
2017-01-04 12:03         ` [dpdk-dev] [PATCH v4 1/5] ethdev: add firmware version get Qiming Yang
2017-01-05 13:44           ` Thomas Monjalon
2017-01-08  3:09             ` Yang, Qiming
2017-01-04 12:03         ` [dpdk-dev] [PATCH v4 2/5] net/e1000: " Qiming Yang
2017-01-04 13:59           ` Ferruh Yigit
2017-01-05  1:50             ` Yang, Qiming
2017-01-04 12:03         ` [dpdk-dev] [PATCH v4 3/5] net/ixgbe: " Qiming Yang
2017-01-04 12:03         ` [dpdk-dev] [PATCH v4 4/5] net/i40e: " Qiming Yang
2017-01-04 14:00           ` Ferruh Yigit
2017-01-04 12:03         ` [dpdk-dev] [PATCH v4 5/5] ethtool: dispaly firmware version Qiming Yang
2017-01-04 14:00           ` Ferruh Yigit
2017-01-05  1:31             ` Yang, Qiming
2017-01-06 15:55               ` Remy Horton
2017-01-08  4:11         ` [dpdk-dev] [PATCH v5 0/5] new API 'rte_eth_dev_fw_version_get' Qiming Yang
2017-01-08  4:11           ` [dpdk-dev] [PATCH v5 1/5] ethdev: add firmware version get Qiming Yang
2017-01-08  6:38             ` Andrew Rybchenko
2017-01-08 23:05             ` Stephen Hemminger
2017-01-09  7:16               ` Yang, Qiming
2017-01-09 10:01                 ` Remy Horton
2017-01-09 17:23                   ` Stephen Hemminger
2017-01-08  4:11           ` [dpdk-dev] [PATCH v5 2/5] net/e1000: " Qiming Yang
2017-01-08 23:03             ` Stephen Hemminger
2017-01-09  1:48               ` Yang, Qiming
2017-01-08  4:11           ` [dpdk-dev] [PATCH v5 3/5] net/ixgbe: " Qiming Yang
2017-01-08  4:11           ` [dpdk-dev] [PATCH v5 4/5] net/i40e: " Qiming Yang
2017-01-08 23:08             ` Stephen Hemminger
2017-01-08  4:11           ` [dpdk-dev] [PATCH v5 5/5] ethtool: display firmware version Qiming Yang
2017-01-08 23:11             ` Stephen Hemminger
2017-01-10  9:00           ` [dpdk-dev] [DPDK 1/5] ethdev: add firmware version get Qiming Yang
2017-01-10  9:00             ` [dpdk-dev] [DPDK 2/5] net/e1000: " Qiming Yang
2017-01-10  9:00             ` [dpdk-dev] [DPDK 3/5] net/ixgbe: " Qiming Yang
2017-01-10  9:08           ` [dpdk-dev] [PATCH v6 0/5] new API 'rte_eth_dev_fw_version_get' Qiming Yang
2017-01-10  9:08             ` [dpdk-dev] [PATCH v6 1/5] ethdev: add firmware version get Qiming Yang
2017-01-11  6:41               ` [dpdk-dev] [PATCH v7 0/5] new API 'rte_eth_dev_fw_version_get' Qiming Yang
2017-01-11  6:41                 ` [dpdk-dev] [PATCH v7 1/5] ethdev: add firmware version get Qiming Yang
2017-01-11  6:41                 ` [dpdk-dev] [PATCH v7 2/5] net/e1000: " Qiming Yang
2017-01-11 15:45                   ` Remy Horton
2017-01-12  1:25                     ` Yang, Qiming
2017-01-11  6:41                 ` [dpdk-dev] [PATCH v7 3/5] net/ixgbe: " Qiming Yang
2017-01-11  6:41                 ` [dpdk-dev] [PATCH v7 4/5] net/i40e: " Qiming Yang
2017-01-11  6:41                 ` [dpdk-dev] [PATCH v7 5/5] ethtool: display firmware version Qiming Yang
2017-01-12  6:31                 ` [dpdk-dev] [PATCH v8 0/5] new API 'rte_eth_dev_fw_version_get' Qiming Yang
2017-01-12  6:31                   ` [dpdk-dev] [PATCH v8 1/5] ethdev: add firmware version get Qiming Yang
2017-01-15 20:56                     ` Thomas Monjalon
2017-01-16  5:44                     ` [dpdk-dev] [PATCH v9 0/5] new API 'rte_eth_dev_fw_version_get' Qiming Yang
2017-01-16  5:44                       ` [dpdk-dev] [PATCH v9 1/5] ethdev: add firmware version get Qiming Yang
2017-01-16  7:05                         ` Andrew Rybchenko
2017-01-16  8:51                           ` Yang, Qiming
2017-01-16  5:44                       ` [dpdk-dev] [PATCH v9 2/5] net/e1000: " Qiming Yang
2017-01-16  5:44                       ` [dpdk-dev] [PATCH v9 3/5] net/ixgbe: " Qiming Yang
2017-01-16  5:44                       ` [dpdk-dev] [PATCH v9 4/5] net/i40e: " Qiming Yang
2017-01-16  5:44                       ` [dpdk-dev] [PATCH v9 5/5] ethtool: display firmware version Qiming Yang
2017-01-16 10:48                       ` [dpdk-dev] [PATCH v10 0/5] new API 'rte_eth_dev_fw_version_get' Qiming Yang
2017-01-16 10:48                         ` [dpdk-dev] [PATCH v10 1/5] ethdev: add firmware version get Qiming Yang
2017-01-16 10:48                         ` [dpdk-dev] [PATCH v10 2/5] net/e1000: " Qiming Yang
2017-01-16 10:48                         ` [dpdk-dev] [PATCH v10 3/5] net/ixgbe: " Qiming Yang
2017-01-16 10:48                         ` [dpdk-dev] [PATCH v10 4/5] net/i40e: " Qiming Yang
2017-01-16 10:48                         ` [dpdk-dev] [PATCH v10 5/5] ethtool: display firmware version Qiming Yang
2017-01-17 21:35                         ` [dpdk-dev] [PATCH v10 0/5] new API 'rte_eth_dev_fw_version_get' Thomas Monjalon
2017-01-12  6:31                   ` [dpdk-dev] [PATCH v8 2/5] net/e1000: add firmware version get Qiming Yang
2017-01-12  6:31                   ` [dpdk-dev] [PATCH v8 3/5] net/ixgbe: " Qiming Yang
2017-01-12  6:31                   ` [dpdk-dev] [PATCH v8 4/5] net/i40e: " Qiming Yang
2017-01-12  6:31                   ` [dpdk-dev] [PATCH v8 5/5] ethtool: display firmware version Qiming Yang
2017-01-10  9:08             ` [dpdk-dev] [PATCH v6 2/5] net/e1000: add firmware version get Qiming Yang
2017-01-10  9:08             ` [dpdk-dev] [PATCH v6 3/5] net/ixgbe: " Qiming Yang
2017-01-10  9:08             ` [dpdk-dev] [PATCH v6 4/5] net/i40e: " Qiming Yang
2017-01-10  9:08             ` [dpdk-dev] [PATCH v6 5/5] ethtool: display firmware version Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH] ethtool: dispaly bus info and " Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH] i40e: add firmware version get Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH] ixgbe: " Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH 2/5] e1000: " Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH 3/5] ixgbe: " Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH 4/5] i40e: " Qiming Yang
2016-11-17  9:42 ` [dpdk-dev] [PATCH 5/5] ethtool: dispaly bus info and firmware version Qiming Yang
2016-11-18  1:10   ` Remy Horton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a191e031-c2fd-2a18-ea02-af5fff9668ec@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=qiming.yang@intel.com \
    --cc=remy.horton@intel.com \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).