Without CONFIG_SWITCHDEV, we run into a compile-time error:
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c: In function 'bnxt_vf_rep_netdev_init':
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c:305:7: error: 'struct net_device' has no member named 'switchdev_ops'; did you mean 'netdev_ops'?
This adds a Kconfig dependency to prevent running into this invalid
configuration.
Fixes: c124a62ff2dd ("bnxt_en: add support for port_attr_get and and get_phys_port_name")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ethernet/broadcom/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index 285f8bc25682..095bb816ab48 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -194,6 +194,7 @@ config BNXT
tristate "Broadcom NetXtreme-C/E support"
depends on PCI
depends on MAY_USE_DEVLINK
+ depends on NET_SWITCHDEV
select FW_LOADER
select LIBCRC32C
---help---
--
2.9.0
The sriov_lock is used to serialize the sriov code with the vfr code.
However, when SRIOV is disabled, the lock is not there at all, leading
to a build error:
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c: In function 'bnxt_dl_eswitch_mode_set':
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c:410:16: error: 'struct bnxt' has no member named 'sriov_lock'
We can either provide the mutex in this configuration, too, or
disable both SRIOV and VFR together. This implements the first
approach, since it seems like a reasonable configuration for
guest kernels to have, and the extra lock will be harmless when
there is no contention.
Fixes: 4ab0c6a8ffd7 ("bnxt_en: add support to enable VF-representors")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 82cbe1804821..9a9f5f394341 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7949,8 +7949,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
#ifdef CONFIG_BNXT_SRIOV
init_waitqueue_head(&bp->sriov_cfg_wait);
- mutex_init(&bp->sriov_lock);
#endif
+ mutex_init(&bp->sriov_lock);
bp->gro_func = bnxt_gro_func_5730x;
if (BNXT_CHIP_P4_PLUS(bp))
bp->gro_func = bnxt_gro_func_5731x;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 2d84d5719b70..a31ef843977a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1239,13 +1239,12 @@ struct bnxt {
wait_queue_head_t sriov_cfg_wait;
bool sriov_cfg;
#define BNXT_SRIOV_CFG_WAIT_TMO msecs_to_jiffies(10000)
-
+#endif
/* lock to protect VF-rep creation/cleanup via
* multiple paths such as ->sriov_configure() and
* devlink ->eswitch_mode_set()
*/
struct mutex sriov_lock;
-#endif
#define BNXT_NTP_FLTR_MAX_FLTR 4096
#define BNXT_NTP_FLTR_HASH_SIZE 512
--
2.9.0
On Tue, Jul 25, 2017 at 8:29 AM, Arnd Bergmann <[email protected]> wrote:
> The sriov_lock is used to serialize the sriov code with the vfr code.
> However, when SRIOV is disabled, the lock is not there at all, leading
> to a build error:
>
> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c: In function 'bnxt_dl_eswitch_mode_set':
> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c:410:16: error: 'struct bnxt' has no member named 'sriov_lock'
>
> We can either provide the mutex in this configuration, too, or
> disable both SRIOV and VFR together. This implements the first
> approach, since it seems like a reasonable configuration for
> guest kernels to have, and the extra lock will be harmless when
> there is no contention.
>
> Fixes: 4ab0c6a8ffd7 ("bnxt_en: add support to enable VF-representors")
> Signed-off-by: Arnd Bergmann <[email protected]>
Sathya already sent 3 patches to fix some of these issues. But I need
to rework one of his patch and resend.
Thanks.
On Tue, Jul 25, 2017 at 6:36 PM, Michael Chan <[email protected]> wrote:
> On Tue, Jul 25, 2017 at 8:29 AM, Arnd Bergmann <[email protected]> wrote:
>> The sriov_lock is used to serialize the sriov code with the vfr code.
>> However, when SRIOV is disabled, the lock is not there at all, leading
>> to a build error:
>>
>> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c: In function 'bnxt_dl_eswitch_mode_set':
>> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c:410:16: error: 'struct bnxt' has no member named 'sriov_lock'
>>
>> We can either provide the mutex in this configuration, too, or
>> disable both SRIOV and VFR together. This implements the first
>> approach, since it seems like a reasonable configuration for
>> guest kernels to have, and the extra lock will be harmless when
>> there is no contention.
>>
>> Fixes: 4ab0c6a8ffd7 ("bnxt_en: add support to enable VF-representors")
>> Signed-off-by: Arnd Bergmann <[email protected]>
>
> Sathya already sent 3 patches to fix some of these issues. But I need
> to rework one of his patch and resend.
Ok, thanks. I just ran into one more issue, and don't know if that's included
as well. If not, please also add the patch below (or fold it into the one
that adds the switchdev dependency to the ethernet driver):
8<----------
Subject: [PATCH] RDMA/bnxt_re: add NET_SWITCHDEV dependency
The rdma side of BNXT enables the ethernet driver and has a list
of its dependencies. However, the ethernet driver now also depends
on NET_SWITCHDEV, so we have to add that dependency for both:
warning: (INFINIBAND_BNXT_RE) selects BNXT which has unmet direct
dependencies (NETDEVICES && ETHERNET && NET_VENDOR_BROADCOM && PCI &&
MAY_USE_DEVLINK && NET_SWITCHDEV)
Signed-off-by: Arnd Bergmann <[email protected]>
diff --git a/drivers/infiniband/hw/bnxt_re/Kconfig
b/drivers/infiniband/hw/bnxt_re/Kconfig
index 19982a4a9bba..0c296f00e74f 100644
--- a/drivers/infiniband/hw/bnxt_re/Kconfig
+++ b/drivers/infiniband/hw/bnxt_re/Kconfig
@@ -1,6 +1,7 @@
config INFINIBAND_BNXT_RE
tristate "Broadcom Netxtreme HCA support"
depends on ETHERNET && NETDEVICES && PCI && INET && DCB
+ depends on NET_SWITCHDEV
select NET_VENDOR_BROADCOM
select BNXT
---help---
On Wed, Jul 26, 2017 at 2:35 PM, Arnd Bergmann <[email protected]> wrote:
[...]
>> Sathya already sent 3 patches to fix some of these issues. But I need
>> to rework one of his patch and resend.
>
> Ok, thanks. I just ran into one more issue, and don't know if that's included
> as well. If not, please also add the patch below (or fold it into the one
> that adds the switchdev dependency to the ethernet driver):
>
> 8<----------
> Subject: [PATCH] RDMA/bnxt_re: add NET_SWITCHDEV dependency
>
> The rdma side of BNXT enables the ethernet driver and has a list
> of its dependencies. However, the ethernet driver now also depends
> on NET_SWITCHDEV, so we have to add that dependency for both:
Arnd, after the patch "bnxt_en: use SWITCHDEV_SET_OPS() for setting
vf_rep_switchdev_ops" the bnxt_en driver doesn't need an explicit
NET_SWITCHDEV dependency. So, the bnxt_re driver shouldn't need one
either. Are you still seeing the bnxt_re issue even after pulling the
above patch??
On Wed, Jul 26, 2017 at 12:54 PM, Sathya Perla
<[email protected]> wrote:
> On Wed, Jul 26, 2017 at 2:35 PM, Arnd Bergmann <[email protected]> wrote:
> [...]
>>> Sathya already sent 3 patches to fix some of these issues. But I need
>>> to rework one of his patch and resend.
>>
>> Ok, thanks. I just ran into one more issue, and don't know if that's included
>> as well. If not, please also add the patch below (or fold it into the one
>> that adds the switchdev dependency to the ethernet driver):
>>
>> 8<----------
>> Subject: [PATCH] RDMA/bnxt_re: add NET_SWITCHDEV dependency
>>
>> The rdma side of BNXT enables the ethernet driver and has a list
>> of its dependencies. However, the ethernet driver now also depends
>> on NET_SWITCHDEV, so we have to add that dependency for both:
>
> Arnd, after the patch "bnxt_en: use SWITCHDEV_SET_OPS() for setting
> vf_rep_switchdev_ops" the bnxt_en driver doesn't need an explicit
> NET_SWITCHDEV dependency. So, the bnxt_re driver shouldn't need one
> either. Are you still seeing the bnxt_re issue even after pulling the
> above patch??
I think that's fine then. I missed that patch when it went in, so I only
needed the add-on since I still had my own earlier patch. I'll drop both
from my test tree now, and will let you know in case something else
remains.
Arnd
On Wed, Jul 26, 2017 at 3:18 PM, Arnd Bergmann <[email protected]> wrote:
> On Wed, Jul 26, 2017 at 12:54 PM, Sathya Perla
> <[email protected]> wrote:
>> On Wed, Jul 26, 2017 at 2:35 PM, Arnd Bergmann <[email protected]> wrote:
>> [...]
>>>> Sathya already sent 3 patches to fix some of these issues. But I need
>>>> to rework one of his patch and resend.
>>>
>>> Ok, thanks. I just ran into one more issue, and don't know if that's included
>>> as well. If not, please also add the patch below (or fold it into the one
>>> that adds the switchdev dependency to the ethernet driver):
>>>
>>> 8<----------
>>> Subject: [PATCH] RDMA/bnxt_re: add NET_SWITCHDEV dependency
>>>
>>> The rdma side of BNXT enables the ethernet driver and has a list
>>> of its dependencies. However, the ethernet driver now also depends
>>> on NET_SWITCHDEV, so we have to add that dependency for both:
>>
>> Arnd, after the patch "bnxt_en: use SWITCHDEV_SET_OPS() for setting
>> vf_rep_switchdev_ops" the bnxt_en driver doesn't need an explicit
>> NET_SWITCHDEV dependency. So, the bnxt_re driver shouldn't need one
>> either. Are you still seeing the bnxt_re issue even after pulling the
>> above patch??
>
> I think that's fine then. I missed that patch when it went in, so I only
> needed the add-on since I still had my own earlier patch. I'll drop both
> from my test tree now, and will let you know in case something else
> remains.
On today's linux-next:
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.o: In function `bnxt_dl_register':
bnxt_vfr.c:(.text+0x1440): undefined reference to `devlink_alloc'
bnxt_vfr.c:(.text+0x14c0): undefined reference to `devlink_register'
bnxt_vfr.c:(.text+0x14e0): undefined reference to `devlink_free'
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.o: In function `bnxt_dl_unregister':
bnxt_vfr.c:(.text+0x1534): undefined reference to `devlink_unregister'
bnxt_vfr.c:(.text+0x153c): undefined reference to `devlink_free'
I think you are just missing a "depends on MAY_USE_DEVLINK"
in INFINIBAND_BNXT_RE, which uses 'select BNXT'.
This is a tricky corner case for Kconfig, where the MAY_USE_DEVLINK
dependency is silently ignored for BNXT as long as MAY_USE_DEVLINK
is not "=n".
Arnd
On Thu, Jul 27, 2017 at 1:18 PM, Arnd Bergmann <[email protected]> wrote:
[...]
>
> On today's linux-next:
>
> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.o: In function `bnxt_dl_register':
> bnxt_vfr.c:(.text+0x1440): undefined reference to `devlink_alloc'
> bnxt_vfr.c:(.text+0x14c0): undefined reference to `devlink_register'
> bnxt_vfr.c:(.text+0x14e0): undefined reference to `devlink_free'
> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.o: In function `bnxt_dl_unregister':
> bnxt_vfr.c:(.text+0x1534): undefined reference to `devlink_unregister'
> bnxt_vfr.c:(.text+0x153c): undefined reference to `devlink_free'
>
> I think you are just missing a "depends on MAY_USE_DEVLINK"
> in INFINIBAND_BNXT_RE, which uses 'select BNXT'.
>
> This is a tricky corner case for Kconfig, where the MAY_USE_DEVLINK
> dependency is silently ignored for BNXT as long as MAY_USE_DEVLINK
> is not "=n".
Can you pls share your .config so that I can reproduce this issue and
ensure that my fix really works...
On Thu, Jul 27, 2017 at 11:00 AM, Sathya Perla
<[email protected]> wrote:
> On Thu, Jul 27, 2017 at 1:18 PM, Arnd Bergmann <[email protected]> wrote:
> [...]
>>
>> On today's linux-next:
>>
>> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.o: In function `bnxt_dl_register':
>> bnxt_vfr.c:(.text+0x1440): undefined reference to `devlink_alloc'
>> bnxt_vfr.c:(.text+0x14c0): undefined reference to `devlink_register'
>> bnxt_vfr.c:(.text+0x14e0): undefined reference to `devlink_free'
>> drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.o: In function `bnxt_dl_unregister':
>> bnxt_vfr.c:(.text+0x1534): undefined reference to `devlink_unregister'
>> bnxt_vfr.c:(.text+0x153c): undefined reference to `devlink_free'
>>
>> I think you are just missing a "depends on MAY_USE_DEVLINK"
>> in INFINIBAND_BNXT_RE, which uses 'select BNXT'.
>>
>> This is a tricky corner case for Kconfig, where the MAY_USE_DEVLINK
>> dependency is silently ignored for BNXT as long as MAY_USE_DEVLINK
>> is not "=n".
>
> Can you pls share your .config so that I can reproduce this issue and
> ensure that my fix really works...
The configuration I used happened to be for arm64, I've pasted it to
https://pastebin.com/dl/rSJ6jCQM now.
You should be able to reproduce it on x86 as well, with anything using
CONFIG_NET_DEVLINK=m and INFINIBAND_BNXT_RE=y.
Arnd