[dpdk-dev] net/thunderx: sync mailbox definitions with Linux PF driver
Checks
Commit Message
- 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>
---
drivers/net/thunderx/base/nicvf_mbox.c | 7 ++-----
drivers/net/thunderx/base/nicvf_mbox.h | 11 +++--------
2 files changed, 5 insertions(+), 13 deletions(-)
Comments
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?
> uint8_t link_up;
> uint8_t duplex;
> uint32_t speed;
>
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")
---
>
> > uint8_t link_up;
> > uint8_t duplex;
> > uint32_t speed;
> >
>
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?
>
>
>>
>>> uint8_t link_up;
>>> uint8_t duplex;
>>> uint32_t speed;
>>>
>>
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 thought about, the backward
compatibility with older kernel, Is it OK to use linux/version.h in PMD drivers
to detect the kernel version? drivers/net/mlx5/mlx5_ethdev.c has similar
kernel detection mechanism to make it backward compatible.
If there are no issue with such approach, I will roll out a new revision.
>
> >
> >
> >>
> >>> uint8_t link_up;
> >>> uint8_t duplex;
> >>> uint32_t speed;
> >>>
> >>
>
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;
>>>>>
>>>>
>>
On Wed, Mar 22, 2017 at 10:33:14AM +0000, Ferruh Yigit wrote:
> 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?
Thanks for sharing the options.
First three options wont work. Heuristic option is pretty limited,
mac_type value can also be 0 or 1(same as link_up)
So I think, The best option now to use the patch as is(without kernel
check).
What do you think?
>
> >
> >
> >>
> >>>
> >>>
> >>>>
> >>>>> uint8_t link_up;
> >>>>> uint8_t duplex;
> >>>>> uint32_t speed;
> >>>>>
> >>>>
> >>
>
On 3/23/2017 1:20 PM, Jerin Jacob wrote:
> On Wed, Mar 22, 2017 at 10:33:14AM +0000, Ferruh Yigit wrote:
>> 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?
>
> Thanks for sharing the options.
> First three options wont work. Heuristic option is pretty limited,
> mac_type value can also be 0 or 1(same as link_up)
> So I think, The best option now to use the patch as is(without kernel
> check).
> What do you think?
If this won't cause a problem for you with older kernels, I would prefer
no kernel check option.
>
>>
>>>
>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>> uint8_t link_up;
>>>>>>> uint8_t duplex;
>>>>>>> uint32_t speed;
>>>>>>>
>>>>>>
>>>>
>>
On Thu, Mar 23, 2017 at 01:26:15PM +0000, Ferruh Yigit wrote:
> On 3/23/2017 1:20 PM, Jerin Jacob wrote:
> > On Wed, Mar 22, 2017 at 10:33:14AM +0000, Ferruh Yigit wrote:
> >> 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?
> >
> > Thanks for sharing the options.
> > First three options wont work. Heuristic option is pretty limited,
> > mac_type value can also be 0 or 1(same as link_up)
> > So I think, The best option now to use the patch as is(without kernel
> > check).
> > What do you think?
>
> If this won't cause a problem for you with older kernels, I would prefer
> no kernel check option.
OK. I will ask our kernel team to backport the offending the kernel change
to old versions based on the requirement. Please merge this patch if you don't
see any other issue.
I can post a separate patch add to the kernel version check if it
is really required.
>
> >
> >>
> >>>
> >>>
> >>>>
> >>>>>
> >>>>>
> >>>>>>
> >>>>>>> uint8_t link_up;
> >>>>>>> uint8_t duplex;
> >>>>>>> uint32_t speed;
> >>>>>>>
> >>>>>>
> >>>>
> >>
>
On 3/23/2017 2:00 PM, Jerin Jacob wrote:
> On Thu, Mar 23, 2017 at 01:26:15PM +0000, Ferruh Yigit wrote:
>> On 3/23/2017 1:20 PM, Jerin Jacob wrote:
>>> On Wed, Mar 22, 2017 at 10:33:14AM +0000, Ferruh Yigit wrote:
>>>> 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?
>>>
>>> Thanks for sharing the options.
>>> First three options wont work. Heuristic option is pretty limited,
>>> mac_type value can also be 0 or 1(same as link_up)
>>> So I think, The best option now to use the patch as is(without kernel
>>> check).
>>> What do you think?
>>
>> If this won't cause a problem for you with older kernels, I would prefer
>> no kernel check option.
>
> OK. I will ask our kernel team to backport the offending the kernel change
> to old versions based on the requirement. Please merge this patch if you don't
> see any other issue.
> I can post a separate patch add to the kernel version check if it
> is really required.
OK
>
>
>>
>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>> uint8_t link_up;
>>>>>>>>> uint8_t duplex;
>>>>>>>>> uint32_t speed;
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
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>
Applied to dpdk-next-net/master, thanks.
@@ -62,9 +62,6 @@ static const char *mbox_message[NIC_MBOX_MSG_MAX] = {
[NIC_MBOX_MSG_RESET_STAT_COUNTER] = "NIC_MBOX_MSG_RESET_STAT_COUNTER",
[NIC_MBOX_MSG_CFG_DONE] = "NIC_MBOX_MSG_CFG_DONE",
[NIC_MBOX_MSG_SHUTDOWN] = "NIC_MBOX_MSG_SHUTDOWN",
- [NIC_MBOX_MSG_RES_BIT] = "NIC_MBOX_MSG_RES_BIT",
- [NIC_MBOX_MSG_RSS_SIZE_RES_BIT] = "NIC_MBOX_MSG_RSS_SIZE",
- [NIC_MBOX_MSG_ALLOC_SQS_RES_BIT] = "NIC_MBOX_MSG_ALLOC_SQS",
};
static inline const char * __attribute__((unused))
@@ -176,7 +173,7 @@ nicvf_handle_mbx_intr(struct nicvf *nic)
case NIC_MBOX_MSG_NACK:
nic->pf_nacked = true;
break;
- case NIC_MBOX_MSG_RSS_SIZE_RES_BIT:
+ case NIC_MBOX_MSG_RSS_SIZE:
nic->rss_info.rss_size = mbx.rss_size.ind_tbl_size;
nic->pf_acked = true;
break;
@@ -186,7 +183,7 @@ nicvf_handle_mbx_intr(struct nicvf *nic)
nic->speed = mbx.link_status.speed;
nic->pf_acked = true;
break;
- case NIC_MBOX_MSG_ALLOC_SQS_RES_BIT:
+ case NIC_MBOX_MSG_ALLOC_SQS:
assert_primary(nic);
if (mbx.sqs_alloc.qs_count != nic->sqs_count) {
nicvf_log_error("Received %" PRIu8 "/%" PRIu8
@@ -68,16 +68,10 @@
#define NIC_MBOX_MSG_ALLOC_SQS 0x12 /* Allocate secondary Qset */
#define NIC_MBOX_MSG_LOOPBACK 0x16 /* Set interface in loopback */
#define NIC_MBOX_MSG_RESET_STAT_COUNTER 0x17 /* Reset statistics counters */
-#define NIC_MBOX_MSG_CFG_DONE 0x7E /* VF configuration done */
-#define NIC_MBOX_MSG_SHUTDOWN 0x7F /* VF is being shutdown */
-#define NIC_MBOX_MSG_RES_BIT 0x80 /* Reset bit from PF */
+#define NIC_MBOX_MSG_CFG_DONE 0xF0 /* VF configuration done */
+#define NIC_MBOX_MSG_SHUTDOWN 0xF1 /* VF is being shutdown */
#define NIC_MBOX_MSG_MAX 0x100 /* Maximum number of messages */
-#define NIC_MBOX_MSG_RSS_SIZE_RES_BIT \
- (NIC_MBOX_MSG_RSS_SIZE | NIC_MBOX_MSG_RES_BIT)
-#define NIC_MBOX_MSG_ALLOC_SQS_RES_BIT \
- (NIC_MBOX_MSG_ALLOC_SQS | NIC_MBOX_MSG_RES_BIT)
-
/* Get vNIC VF configuration */
struct nic_cfg_msg {
uint8_t msg;
@@ -157,6 +151,7 @@ struct rss_cfg_msg {
/* Physical interface link status */
struct bgx_link_status {
uint8_t msg;
+ uint8_t mac_type;
uint8_t link_up;
uint8_t duplex;
uint32_t speed;