[PATCH 0/3] net/mlx5e: Kconfig fixes

[PATCH 0/3] net/mlx5e: Kconfig fixes

[Arnd Bergmann]

I saw two new build errors with invalid configurations on mlx5e, and a
closer look revealed that one of them was caused by a slightly incorrect
fix for which I had sent a different patch when it first broke.

This series reverts the new fix for the VXLAN problem as it caused
another bug, and follows up with a new version of my patch (adapted
as originally requested), and another patch for the simiar RFS
bug that was just introduced.

        Arnd

[PATCH 1/3] Revert "net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue"

[Arnd Bergmann]

This reverts commit 69976fb1045850a742deb9790ea49cbc6f497531.

We cannot select VXLAN when IPv4 support is disabled, that just gives
us additional build errors, including:

warning: (MLX5_CORE_EN) selects VXLAN which has unmet direct dependencies (NETDEVICES && NET_CORE && INET)
In file included from ../drivers/net/vxlan.c:36:0:
include/net/udp_tunnel.h: In function 'udp_tunnel_handle_offloads':
include/net/udp_tunnel.h:112:9: error: implicit declaration of function 'iptunnel_handle_offloads' [-Werror=implicit-function-declaration]
  return iptunnel_handle_offloads(skb, type);
         ^~~~~~~~~~~~~~~~~~~~~~~~

I'm sending a proper fix for the original bug in a separate patch.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 559d11a443bc..1cf722eba607 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -14,7 +14,6 @@ config MLX5_CORE_EN
 	bool "Mellanox Technologies ConnectX-4 Ethernet support"
 	depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE
 	select PTP_1588_CLOCK
-	select VXLAN if MLX5_CORE=y
 	default n
 	---help---
 	  Ethernet support in Mellanox Technologies ConnectX-4 NIC.
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] net/mlx5e: make VXLAN support conditional

[Arnd Bergmann]

VXLAN can be disabled at compile-time or it can be a loadable
module while mlx5 is built-in, which leads to a link error:

drivers/net/built-in.o: In function `mlx5e_create_netdev':
ntb_netdev.c:(.text+0x106de4): undefined reference to `vxlan_get_rx_port'

This avoids the link error and makes the vxlan code optional,
like the other ethernet drivers do as well.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Link: https://patchwork.ozlabs.org/patch/589296/
Fixes: b3f63c3d5e2c ("net/mlx5e: Add netdev support for VXLAN tunneling")
---
I sent it originally on Feb 26 2016, but misread Saeed Mahameed's
reply as saying that he'd fix it up himself. The new version
should address the original comment.
---
 drivers/net/ethernet/mellanox/mlx5/core/Kconfig   |  7 +++++++
 drivers/net/ethernet/mellanox/mlx5/core/Makefile  |  4 +++-
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |  2 ++
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |  4 ++++
 drivers/net/ethernet/mellanox/mlx5/core/vxlan.h   | 11 +++++++++--
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 1cf722eba607..f5c3b9465d8d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -31,3 +31,10 @@ config MLX5_CORE_EN_DCB
 	  This flag is depended on the kernel's DCB support.
 
 	  If unsure, set to Y
+
+config MLX5_CORE_EN_VXLAN
+	bool "VXLAN offloads Support"
+	default y
+	depends on MLX5_CORE_EN && VXLAN && !(MLX5_CORE=y && VXLAN=m)
+	---help---
+	  Say Y here if you want to use VXLAN offloads in the driver.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 679e18ffb3a6..6b8da0b36acd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -6,7 +6,9 @@ mlx5_core-y :=	main.o cmd.o debugfs.o fw.o eq.o uar.o pagealloc.o \
 
 mlx5_core-$(CONFIG_MLX5_CORE_EN) += wq.o eswitch.o \
 		en_main.o en_fs.o en_ethtool.o en_tx.o en_rx.o \
-		en_txrx.o en_clock.o vxlan.o en_tc.o
+		en_txrx.o en_clock.o en_tc.o
+
+mlx5_core-$(CONFIG_MLX5_CORE_EN_VXLAN) += vxlan.o
 
 mlx5_core-$(CONFIG_MLX5_CORE_EN_DCB) +=  en_dcbnl.o
 mlx5_core-$(CONFIG_RFS_ACCEL) +=  en_arfs.o
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index bfa5daaaf5aa..19b1b021af2d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -522,7 +522,9 @@ struct mlx5e_priv {
 	struct mlx5e_direct_tir    direct_tir[MLX5E_MAX_NUM_CHANNELS];
 
 	struct mlx5e_flow_steering fs;
+#ifdef CONFIG_MLX5_CORE_EN_VXLAN
 	struct mlx5e_vxlan_db      vxlan;
+#endif
 
 	struct mlx5e_params        params;
 	struct workqueue_struct    *wq;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 7dfb73aa8e41..5e710315c5a9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2494,6 +2494,7 @@ static int mlx5e_get_vf_stats(struct net_device *dev,
 					    vf_stats);
 }
 
+#if IS_ENABLED(CONFIG_MLX5_CORE_EN_VXLAN)
 static void mlx5e_add_vxlan_port(struct net_device *netdev,
 				 sa_family_t sa_family, __be16 port)
 {
@@ -2565,6 +2566,7 @@ static netdev_features_t mlx5e_features_check(struct sk_buff *skb,
 
 	return features;
 }
+#endif
 
 static const struct net_device_ops mlx5e_netdev_ops_basic = {
 	.ndo_open                = mlx5e_open,
@@ -2599,9 +2601,11 @@ static const struct net_device_ops mlx5e_netdev_ops_sriov = {
 	.ndo_set_features        = mlx5e_set_features,
 	.ndo_change_mtu          = mlx5e_change_mtu,
 	.ndo_do_ioctl            = mlx5e_ioctl,
+#ifdef CONFIG_MLX5_CORE_EN_VXLAN
 	.ndo_add_vxlan_port      = mlx5e_add_vxlan_port,
 	.ndo_del_vxlan_port      = mlx5e_del_vxlan_port,
 	.ndo_features_check      = mlx5e_features_check,
+#endif
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	 = mlx5e_rx_flow_steer,
 #endif
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h b/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h
index 129f3527aa14..217ac530a514 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vxlan.h
@@ -48,14 +48,21 @@ struct mlx5e_vxlan_work {
 
 static inline bool mlx5e_vxlan_allowed(struct mlx5_core_dev *mdev)
 {
-	return (MLX5_CAP_ETH(mdev, tunnel_stateless_vxlan) &&
+	return IS_ENABLED(CONFIG_MLX5_CORE_EN_VXLAN) &&
+		(MLX5_CAP_ETH(mdev, tunnel_stateless_vxlan) &&
 		mlx5_core_is_pf(mdev));
 }
 
+#ifdef CONFIG_MLX5_CORE_EN_VXLAN
 void mlx5e_vxlan_init(struct mlx5e_priv *priv);
+void mlx5e_vxlan_cleanup(struct mlx5e_priv *priv);
+#else
+static inline void mlx5e_vxlan_init(struct mlx5e_priv *priv) {}
+static inline void mlx5e_vxlan_cleanup(struct mlx5e_priv *priv) {}
+#endif
+
 void mlx5e_vxlan_queue_work(struct mlx5e_priv *priv, sa_family_t sa_family,
 			    u16 port, int add);
 struct mlx5e_vxlan *mlx5e_vxlan_lookup_port(struct mlx5e_priv *priv, u16 port);
-void mlx5e_vxlan_cleanup(struct mlx5e_priv *priv);
 
 #endif /* __MLX5_VXLAN_H__ */
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional

[Saeed Mahameed]

On Wed, May 4, 2016 at 3:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> VXLAN can be disabled at compile-time or it can be a loadable
> module while mlx5 is built-in, which leads to a link error:
>
> drivers/net/built-in.o: In function `mlx5e_create_netdev':
> ntb_netdev.c:(.text+0x106de4): undefined reference to `vxlan_get_rx_port'
>
> This avoids the link error and makes the vxlan code optional,
> like the other ethernet drivers do as well.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Link: https://patchwork.ozlabs.org/patch/589296/
> Fixes: b3f63c3d5e2c ("net/mlx5e: Add netdev support for VXLAN tunneling")
> ---
> I sent it originally on Feb 26 2016, but misread Saeed Mahameed's
> reply as saying that he'd fix it up himself. The new version
> should address the original comment.
> ---

Hi Arnd,

I didn't post a fix up since it is not needed anymore, see
b7aade15485a ('vxlan: break dependency with netdev drivers') in
net-next.

The new issue is introduced due to : "net/mlx5: Kconfig: Fix
MLX5_EN/VXLAN build issue" which was merged from net tree.

Dave shouldn't have merged it into net-next, I explicitly asked him
that in the cover letter.  Maybe he missed it.

I just checked and It is sufficient to only take the revert patch:
[PATCH 1/3] Revert "net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue"
to net-next.

Can you please confirm that with only the revert patch, you don't see
the issue ?

Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional

[Arnd Bergmann]

On Thursday 05 May 2016 19:44:36 Saeed Mahameed wrote:
> On Wed, May 4, 2016 at 3:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > VXLAN can be disabled at compile-time or it can be a loadable
> > module while mlx5 is built-in, which leads to a link error:
> >
> > drivers/net/built-in.o: In function `mlx5e_create_netdev':
> > ntb_netdev.c:(.text+0x106de4): undefined reference to `vxlan_get_rx_port'
> >
> > This avoids the link error and makes the vxlan code optional,
> > like the other ethernet drivers do as well.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Link: https://patchwork.ozlabs.org/patch/589296/
> > Fixes: b3f63c3d5e2c ("net/mlx5e: Add netdev support for VXLAN tunneling")
> > ---
> > I sent it originally on Feb 26 2016, but misread Saeed Mahameed's
> > reply as saying that he'd fix it up himself. The new version
> > should address the original comment.
> > ---
> 
> Hi Arnd,
> 
> I didn't post a fix up since it is not needed anymore, see
> b7aade15485a ('vxlan: break dependency with netdev drivers') in
> net-next.
> 
> The new issue is introduced due to : "net/mlx5: Kconfig: Fix
> MLX5_EN/VXLAN build issue" which was merged from net tree.
> 
> Dave shouldn't have merged it into net-next, I explicitly asked him
> that in the cover letter.  Maybe he missed it.
> 
> I just checked and It is sufficient to only take the revert patch:
> [PATCH 1/3] Revert "net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue"
> to net-next.
> 
> Can you please confirm that with only the revert patch, you don't see
> the issue ?

Yes, it works, but not it is different from all the other drivers
(MLX4, BENET, IXGBE, I40E, FM10K, QLCNIC, and QEDE). If the 'select
VXLAN' is not the preferred way to handle this, we should change
the other ones the same way, right?

For reference, I've tried it out on the MLX4 driver, and it does
seem nicer that way, see below.

	Arnd
---
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
in case someone wants to pick up that patch and do the other
ones as well.

diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index 9ca3734ebb6b..88fff4484200 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -6,6 +6,7 @@ config MLX4_EN
 	tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
 	depends on MAY_USE_DEVLINK
 	depends on PCI
+	select VXLAN
 	select MLX4_CORE
 	select PTP_1588_CLOCK
 	---help---
@@ -24,13 +25,6 @@ config MLX4_EN_DCB
 
 	  If unsure, set to Y
 
-config MLX4_EN_VXLAN
-	bool "VXLAN offloads Support"
-	default y
-	depends on MLX4_EN && VXLAN && !(MLX4_EN=y && VXLAN=m)
-	---help---
-	  Say Y here if you want to use VXLAN offloads in the driver.
-
 config MLX4_CORE
 	tristate
 	depends on PCI
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 6f28ac58251c..ad887c425f2d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1691,10 +1691,8 @@ int mlx4_en_start_port(struct net_device *dev)
 	/* Schedule multicast task to populate multicast list */
 	queue_work(mdev->workqueue, &priv->rx_mode_task);
 
-#ifdef CONFIG_MLX4_EN_VXLAN
 	if (priv->mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
 		vxlan_get_rx_port(dev);
-#endif
 	priv->port_up = true;
 	netif_tx_start_all_queues(dev);
 	netif_device_attach(dev);
@@ -2337,7 +2335,6 @@ static int mlx4_en_get_phys_port_id(struct net_device *dev,
 	return 0;
 }
 
-#ifdef CONFIG_MLX4_EN_VXLAN
 static void mlx4_en_add_vxlan_offloads(struct work_struct *work)
 {
 	int ret;
@@ -2448,7 +2445,6 @@ static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
 
 	return features;
 }
-#endif
 
 static int mlx4_en_set_tx_maxrate(struct net_device *dev, int queue_index, u32 maxrate)
 {
@@ -2501,11 +2497,9 @@ static const struct net_device_ops mlx4_netdev_ops = {
 	.ndo_rx_flow_steer	= mlx4_en_filter_rfs,
 #endif
 	.ndo_get_phys_port_id	= mlx4_en_get_phys_port_id,
-#ifdef CONFIG_MLX4_EN_VXLAN
 	.ndo_add_vxlan_port	= mlx4_en_add_vxlan_port,
 	.ndo_del_vxlan_port	= mlx4_en_del_vxlan_port,
 	.ndo_features_check	= mlx4_en_features_check,
-#endif
 	.ndo_set_tx_maxrate	= mlx4_en_set_tx_maxrate,
 };
 
@@ -2539,11 +2533,9 @@ static const struct net_device_ops mlx4_netdev_ops_master = {
 	.ndo_rx_flow_steer	= mlx4_en_filter_rfs,
 #endif
 	.ndo_get_phys_port_id	= mlx4_en_get_phys_port_id,
-#ifdef CONFIG_MLX4_EN_VXLAN
 	.ndo_add_vxlan_port	= mlx4_en_add_vxlan_port,
 	.ndo_del_vxlan_port	= mlx4_en_del_vxlan_port,
 	.ndo_features_check	= mlx4_en_features_check,
-#endif
 	.ndo_set_tx_maxrate	= mlx4_en_set_tx_maxrate,
 };
 
@@ -2834,10 +2826,8 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	INIT_WORK(&priv->linkstate_task, mlx4_en_linkstate);
 	INIT_DELAYED_WORK(&priv->stats_task, mlx4_en_do_get_stats);
 	INIT_DELAYED_WORK(&priv->service_task, mlx4_en_service_task);
-#ifdef CONFIG_MLX4_EN_VXLAN
 	INIT_WORK(&priv->vxlan_add_task, mlx4_en_add_vxlan_offloads);
 	INIT_WORK(&priv->vxlan_del_task, mlx4_en_del_vxlan_offloads);
-#endif
 #ifdef CONFIG_RFS_ACCEL
 	INIT_LIST_HEAD(&priv->filters);
 	spin_lock_init(&priv->filters_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 63b1aeae2c03..cbab8741d60f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -546,10 +546,8 @@ struct mlx4_en_priv {
 	struct work_struct linkstate_task;
 	struct delayed_work stats_task;
 	struct delayed_work service_task;
-#ifdef CONFIG_MLX4_EN_VXLAN
 	struct work_struct vxlan_add_task;
 	struct work_struct vxlan_del_task;
-#endif
 	struct mlx4_en_perf_stats pstats;
 	struct mlx4_en_pkt_stats pkstats;
 	struct mlx4_en_counter_stats pf_stats;

Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional

[David Miller]

From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Date: Thu, 05 May 2016 20:09:19 +0200

> For reference, I've tried it out on the MLX4 driver, and it does
> seem nicer that way, see below.

Is it possible to wind down this conversation and have someone submit
whatever final patch everyone agrees to?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional

[Saeed Mahameed]

On Fri, May 6, 2016 at 10:35 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Date: Thu, 05 May 2016 20:09:19 +0200
>
>> For reference, I've tried it out on the MLX4 driver, and it does
>> seem nicer that way, see below.
>
> Is it possible to wind down this conversation and have someone submit
> whatever final patch everyone agrees to?
>

Yes, Just reposted to net what Arnd originally posted to net-next.
"[PATCH net 0/2] net/mlx5e: Kconfig fixes for VxLAN"
[...]
Arnd Bergmann (2):
  Revert "net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue"
  net/mlx5e: make VXLAN support conditional

I just rebased the patches on top of net.

Arnd please have a look.

Thanks,
Saeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional

[Arnd Bergmann]

On Sunday 08 May 2016 14:58:38 Saeed Mahameed wrote:
> On Fri, May 6, 2016 at 10:35 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> > From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> > Date: Thu, 05 May 2016 20:09:19 +0200
> >
> >> For reference, I've tried it out on the MLX4 driver, and it does
> >> seem nicer that way, see below.
> >
> > Is it possible to wind down this conversation and have someone submit
> > whatever final patch everyone agrees to?
> >
> 
> Yes, Just reposted to net what Arnd originally posted to net-next.
> "[PATCH net 0/2] net/mlx5e: Kconfig fixes for VxLAN"
> [...]
> Arnd Bergmann (2):
>   Revert "net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue"
>   net/mlx5e: make VXLAN support conditional
> 
> I just rebased the patches on top of net.
> 
> Arnd please have a look.

Sorry for the delay, it looks all fine now and I see no more warnings.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional

[Alexander Duyck]

On Thu, May 5, 2016 at 11:09 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 05 May 2016 19:44:36 Saeed Mahameed wrote:
>> On Wed, May 4, 2016 at 3:31 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> > VXLAN can be disabled at compile-time or it can be a loadable
>> > module while mlx5 is built-in, which leads to a link error:
>> >
>> > drivers/net/built-in.o: In function `mlx5e_create_netdev':
>> > ntb_netdev.c:(.text+0x106de4): undefined reference to `vxlan_get_rx_port'
>> >
>> > This avoids the link error and makes the vxlan code optional,
>> > like the other ethernet drivers do as well.
>> >
>> > Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> > Link: https://patchwork.ozlabs.org/patch/589296/
>> > Fixes: b3f63c3d5e2c ("net/mlx5e: Add netdev support for VXLAN tunneling")
>> > ---
>> > I sent it originally on Feb 26 2016, but misread Saeed Mahameed's
>> > reply as saying that he'd fix it up himself. The new version
>> > should address the original comment.
>> > ---
>>
>> Hi Arnd,
>>
>> I didn't post a fix up since it is not needed anymore, see
>> b7aade15485a ('vxlan: break dependency with netdev drivers') in
>> net-next.
>>
>> The new issue is introduced due to : "net/mlx5: Kconfig: Fix
>> MLX5_EN/VXLAN build issue" which was merged from net tree.
>>
>> Dave shouldn't have merged it into net-next, I explicitly asked him
>> that in the cover letter.  Maybe he missed it.
>>
>> I just checked and It is sufficient to only take the revert patch:
>> [PATCH 1/3] Revert "net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue"
>> to net-next.
>>
>> Can you please confirm that with only the revert patch, you don't see
>> the issue ?
>
> Yes, it works, but not it is different from all the other drivers
> (MLX4, BENET, IXGBE, I40E, FM10K, QLCNIC, and QEDE). If the 'select
> VXLAN' is not the preferred way to handle this, we should change
> the other ones the same way, right?
>
> For reference, I've tried it out on the MLX4 driver, and it does
> seem nicer that way, see below.
>
>         Arnd
> ---
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> in case someone wants to pick up that patch and do the other
> ones as well.
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
> index 9ca3734ebb6b..88fff4484200 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
> +++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
> @@ -6,6 +6,7 @@ config MLX4_EN
>         tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
>         depends on MAY_USE_DEVLINK
>         depends on PCI
> +       select VXLAN
>         select MLX4_CORE
>         select PTP_1588_CLOCK
>         ---help---

This piece is unnecessary and unwanted.  We just recently added the
ability to load the modules without the need for VXLAN.  Lets not take
that in the wrong direction by having the drivers select a module they
don't have to have.

The rest of this code is probably fine.  After the dependency was
broken via b7aade15485a ('vxlan: break dependency with netdev
drivers') you could probably go through and just pull all the VXLAN
ifdefs from all the drivers since I don't think there is anything that
explicitly relies on that module anymore as the only export still
hanging around is vxlan_dev_create and I don't think any Ethernet
drivers are directly spawning VXLAN interfaces.

> @@ -24,13 +25,6 @@ config MLX4_EN_DCB
>
>           If unsure, set to Y
>
> -config MLX4_EN_VXLAN
> -       bool "VXLAN offloads Support"
> -       default y
> -       depends on MLX4_EN && VXLAN && !(MLX4_EN=y && VXLAN=m)
> -       ---help---
> -         Say Y here if you want to use VXLAN offloads in the driver.
> -
>  config MLX4_CORE
>         tristate
>         depends on PCI
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 6f28ac58251c..ad887c425f2d 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -1691,10 +1691,8 @@ int mlx4_en_start_port(struct net_device *dev)
>         /* Schedule multicast task to populate multicast list */
>         queue_work(mdev->workqueue, &priv->rx_mode_task);
>
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         if (priv->mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
>                 vxlan_get_rx_port(dev);
> -#endif
>         priv->port_up = true;
>         netif_tx_start_all_queues(dev);
>         netif_device_attach(dev);
> @@ -2337,7 +2335,6 @@ static int mlx4_en_get_phys_port_id(struct net_device *dev,
>         return 0;
>  }
>
> -#ifdef CONFIG_MLX4_EN_VXLAN
>  static void mlx4_en_add_vxlan_offloads(struct work_struct *work)
>  {
>         int ret;
> @@ -2448,7 +2445,6 @@ static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
>
>         return features;
>  }
> -#endif
>
>  static int mlx4_en_set_tx_maxrate(struct net_device *dev, int queue_index, u32 maxrate)
>  {
> @@ -2501,11 +2497,9 @@ static const struct net_device_ops mlx4_netdev_ops = {
>         .ndo_rx_flow_steer      = mlx4_en_filter_rfs,
>  #endif
>         .ndo_get_phys_port_id   = mlx4_en_get_phys_port_id,
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         .ndo_add_vxlan_port     = mlx4_en_add_vxlan_port,
>         .ndo_del_vxlan_port     = mlx4_en_del_vxlan_port,
>         .ndo_features_check     = mlx4_en_features_check,
> -#endif
>         .ndo_set_tx_maxrate     = mlx4_en_set_tx_maxrate,
>  };
>
> @@ -2539,11 +2533,9 @@ static const struct net_device_ops mlx4_netdev_ops_master = {
>         .ndo_rx_flow_steer      = mlx4_en_filter_rfs,
>  #endif
>         .ndo_get_phys_port_id   = mlx4_en_get_phys_port_id,
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         .ndo_add_vxlan_port     = mlx4_en_add_vxlan_port,
>         .ndo_del_vxlan_port     = mlx4_en_del_vxlan_port,
>         .ndo_features_check     = mlx4_en_features_check,
> -#endif
>         .ndo_set_tx_maxrate     = mlx4_en_set_tx_maxrate,
>  };
>
> @@ -2834,10 +2826,8 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
>         INIT_WORK(&priv->linkstate_task, mlx4_en_linkstate);
>         INIT_DELAYED_WORK(&priv->stats_task, mlx4_en_do_get_stats);
>         INIT_DELAYED_WORK(&priv->service_task, mlx4_en_service_task);
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         INIT_WORK(&priv->vxlan_add_task, mlx4_en_add_vxlan_offloads);
>         INIT_WORK(&priv->vxlan_del_task, mlx4_en_del_vxlan_offloads);
> -#endif
>  #ifdef CONFIG_RFS_ACCEL
>         INIT_LIST_HEAD(&priv->filters);
>         spin_lock_init(&priv->filters_lock);
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> index 63b1aeae2c03..cbab8741d60f 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> @@ -546,10 +546,8 @@ struct mlx4_en_priv {
>         struct work_struct linkstate_task;
>         struct delayed_work stats_task;
>         struct delayed_work service_task;
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         struct work_struct vxlan_add_task;
>         struct work_struct vxlan_del_task;
> -#endif
>         struct mlx4_en_perf_stats pstats;
>         struct mlx4_en_pkt_stats pkstats;
>         struct mlx4_en_counter_stats pf_stats;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] net/mlx5e: don't build RFS without ethernet

[Arnd Bergmann]

The RFS acceleration support in mlx5 is enabled whenever CONFIG_RFS_ACCEL
is in use. However, this fails if a user turns off CONFIG_MLX5_CORE_EN,
because the rfs code now calls into the ethernet portion of the
driver:

ERROR: "mlx5e_destroy_flow_table" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!

This patch adds another Kconfig symbol to control the compilation of
the RFS code, to ensure it is only built if both the ethernet support
and the core RFS support are enabled.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/mellanox/mlx5/core/Kconfig  | 4 ++++
 drivers/net/ethernet/mellanox/mlx5/core/Makefile | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index f5c3b9465d8d..5efba14a1453 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -38,3 +38,7 @@ config MLX5_CORE_EN_VXLAN
 	depends on MLX5_CORE_EN && VXLAN && !(MLX5_CORE=y && VXLAN=m)
 	---help---
 	  Say Y here if you want to use VXLAN offloads in the driver.
+
+config MLX5_CORE_EN_RFS
+	def_bool y
+	depends on MLX5_CORE_EN && RFS_ACCEL
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 6b8da0b36acd..29118c1debb7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -11,4 +11,4 @@ mlx5_core-$(CONFIG_MLX5_CORE_EN) += wq.o eswitch.o \
 mlx5_core-$(CONFIG_MLX5_CORE_EN_VXLAN) += vxlan.o
 
 mlx5_core-$(CONFIG_MLX5_CORE_EN_DCB) +=  en_dcbnl.o
-mlx5_core-$(CONFIG_RFS_ACCEL) +=  en_arfs.o
+mlx5_core-$(CONFIG_MLX5_CORE_EN_RFS) +=  en_arfs.o
-- 
2.7.0

Re: [PATCH 3/3] net/mlx5e: don't build RFS without ethernet

[Saeed Mahameed]

On Wed, May 4, 2016 at 3:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> The RFS acceleration support in mlx5 is enabled whenever CONFIG_RFS_ACCEL
> is in use. However, this fails if a user turns off CONFIG_MLX5_CORE_EN,
> because the rfs code now calls into the ethernet portion of the
> driver:
>
> ERROR: "mlx5e_destroy_flow_table" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
>

Hi Arnd,

We already posted a fix for this, please see "[PATCH net-next V1
01/12] net/mlx5e: Fix aRFS compilation dependency", and give us your
feedback.

Saeed.

Re: [PATCH 3/3] net/mlx5e: don't build RFS without ethernet

[Arnd Bergmann]

On Wednesday 04 May 2016 16:09:16 Saeed Mahameed wrote:
> On Wed, May 4, 2016 at 3:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > The RFS acceleration support in mlx5 is enabled whenever CONFIG_RFS_ACCEL
> > is in use. However, this fails if a user turns off CONFIG_MLX5_CORE_EN,
> > because the rfs code now calls into the ethernet portion of the
> > driver:
> >
> > ERROR: "mlx5e_destroy_flow_table" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
> >
> 
> Hi Arnd,
> 
> We already posted a fix for this, please see "[PATCH net-next V1
> 01/12] net/mlx5e: Fix aRFS compilation dependency", and give us your
> feedback.

That version looks fine as well. I think it's better to be a little
more explicit as I was, but either way seems fine.

You can also achieve the same thing in yet another way using Makefile
syntax instead of cpp or Kconfig:

ifdef CONFIG_MLX5_CORE_EN
mlx5_core-y += wq.o eswitch.o \
               en_main.o en_fs.o en_ethtool.o en_tx.o en_rx.o \
               en_txrx.o en_clock.o vxlan.o en_tc.o
               en_txrx.o en_clock.o vxlan.o en_tc.o en_arfs.o

mlx5_core-$(CONFIG_RFS_ACCEL) += en_arfs.o
endif

If you prefer Maor's version, feel free to add

Acked-by: Arnd Bergmann <arnd@arndb.de>

	Arnd

Re: [PATCH 3/3] net/mlx5e: don't build RFS without ethernet

[Saeed Mahameed]

On Wed, May 4, 2016 at 5:23 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Wednesday 04 May 2016 16:09:16 Saeed Mahameed wrote:
>> On Wed, May 4, 2016 at 3:32 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> > The RFS acceleration support in mlx5 is enabled whenever CONFIG_RFS_ACCEL
>> > is in use. However, this fails if a user turns off CONFIG_MLX5_CORE_EN,
>> > because the rfs code now calls into the ethernet portion of the
>> > driver:
>> >
>> > ERROR: "mlx5e_destroy_flow_table" [drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko] undefined!
>> >
>>
>> Hi Arnd,
>>
>> We already posted a fix for this, please see "[PATCH net-next V1
>> 01/12] net/mlx5e: Fix aRFS compilation dependency", and give us your
>> feedback.
>
> That version looks fine as well. I think it's better to be a little
> more explicit as I was, but either way seems fine.
>

Simply we didn't want to add new Kconfig flag.

>
> You can also achieve the same thing in yet another way using Makefile
> syntax instead of cpp or Kconfig:
>
> ifdef CONFIG_MLX5_CORE_EN
> mlx5_core-y += wq.o eswitch.o \
>                en_main.o en_fs.o en_ethtool.o en_tx.o en_rx.o \
>                en_txrx.o en_clock.o vxlan.o en_tc.o
>                en_txrx.o en_clock.o vxlan.o en_tc.o en_arfs.o
>
> mlx5_core-$(CONFIG_RFS_ACCEL) += en_arfs.o
> endif
>
>
> If you prefer Maor's version, feel free to add
>
>
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>

I do prefer this version, but the difference is small, so will keep Maor's.
Thanks Arnd for the Ack and the suggestion.

Saeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html