From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Cc: dev@dpdk.org, Christian Ehrhardt <christian.ehrhardt@canonical.com>
Subject: Re: [dpdk-dev] [PATCH] net/thunderx: sync mailbox definitions with Linux PF driver
Date: Wed, 22 Mar 2017 10:33:14 +0000 [thread overview]
Message-ID: <526d95ae-fe16-6c54-441c-32368df01c7d@intel.com> (raw)
In-Reply-To: <20170322093931.2u5z3n645cnobi63@localhost.localdomain>
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 <jerin.jacob@caviumnetworks.com>
>>>>
>>>> <...>
>>>>
>>>>> @@ -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;
>>>>>
>>>>
>>
next prev parent reply other threads:[~2017-03-22 10:33 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-20 14:10 Jerin Jacob
2017-03-21 14:31 ` Ferruh Yigit
2017-03-21 14:38 ` Jerin Jacob
2017-03-21 14:53 ` Ferruh Yigit
2017-03-22 9:39 ` Jerin Jacob
2017-03-22 10:33 ` Ferruh Yigit [this message]
2017-03-23 13:20 ` Jerin Jacob
2017-03-23 13:26 ` Ferruh Yigit
2017-03-23 14:00 ` Jerin Jacob
2017-03-23 14:02 ` Ferruh Yigit
2017-03-23 14:09 ` Ferruh Yigit
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=526d95ae-fe16-6c54-441c-32368df01c7d@intel.com \
--to=ferruh.yigit@intel.com \
--cc=christian.ehrhardt@canonical.com \
--cc=dev@dpdk.org \
--cc=jerin.jacob@caviumnetworks.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).