[PATCH net-next 0/3] mlxsw: Removing dependency of mlxsw on GRE

[PATCH net-next 0/3] mlxsw: Removing dependency of mlxsw on GRE

[Ido Schimmel]

Petr says:

mlxsw_spectrum supports offloading of a tc action mirred egress mirror
to a gretap or ip6gretap netdevice, which necessitates calls to
functions defined in ip_gre, ip6_gre and ip6_tunnel modules. Previously
this was enabled by introducing a hard dependency of MLXSW_SPECTRUM on
NET_IPGRE and IPV6_GRE. However the rest of mlxsw is careful about
picking which modules are absolutely required, and therefore the better
approach is to make mlxsw_spectrum tolerant of absence of one or both of
the GRE flavors.

One way this might be resolved is by keeping the code in mlxsw_spectrum
intact, and defining defaults for functions that mlxsw_spectrum depends
on. The downsides are that other modules end up littered with these
do-nothing defaults; that the driver ends up carrying quite a bit of
dead code; and that the driver ends up having to explicitly depend on
IPV6_TUNNEL to prevent configurations where mlxsw_spectrum is compiled
in and and ip6_tunnel is a module, something that it currently can treat
as an implementation detail of the IPV6_GRE dependency.

Alternatively, the driver should just bite the bullet and ifdef-out the
code that handles configurations that are not supported. Since that's
what we are doing for IPv6 dependency, let's do the same for the GRE
flavors.

Patch #1 introduces a wrapper function for determining the value of
ipv6.sysctl.multipath_hash_policy, which defaults to 0 on non-IPv6
builds. That function is then used from spectrum_router.c, instead of
the direct variable reference that was introduced there during the short
window when the Spectrum driver had a hard dependency on IPv6.

Patch #2 moves one function to keep together in one block all the
callbacks for handling (IPv4) gretap mirroring.

Patch #3 then introduces the ifdefs to hide the irrelevant code.

Petr Machata (3):
  net: ipv6: Introduce ip6_multipath_hash_policy()
  mlxsw: spectrum: Move mlxsw_sp_span_gretap4_route()
  mlxsw: spectrum: Don't depend on ip_gre and ip6_gre

 drivers/net/ethernet/mellanox/mlxsw/Kconfig        |  6 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  |  2 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_span.c    | 76 ++++++++++++----------
 include/net/ipv6.h                                 | 11 ++++
 4 files changed, 56 insertions(+), 39 deletions(-)

-- 
2.14.3

[PATCH net-next 1/3] net: ipv6: Introduce ip6_multipath_hash_policy()

[Ido Schimmel]

From: Petr Machata <petrm@mellanox.com>

In order to abstract away access to the
ipv6.sysctl.multipath_hash_policy variable, which is not available on
systems compiled without IPv6 support, introduce a wrapper function
ip6_multipath_hash_policy() that falls back to 0 on non-IPv6 systems.

Use this wrapper from mlxsw/spectrum_router instead of a direct
reference.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c |  2 +-
 include/net/ipv6.h                                    | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index a8a578610a7b..921bd1075edf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -7031,7 +7031,7 @@ static void mlxsw_sp_mp4_hash_init(char *recr2_pl)
 
 static void mlxsw_sp_mp6_hash_init(char *recr2_pl)
 {
-	bool only_l3 = !init_net.ipv6.sysctl.multipath_hash_policy;
+	bool only_l3 = !ip6_multipath_hash_policy(&init_net);
 
 	mlxsw_sp_mp_hash_header_set(recr2_pl,
 				    MLXSW_REG_RECR2_IPV6_EN_NOT_TCP_NOT_UDP);
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index cabd3cdd4015..50a6f0ddb878 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -888,6 +888,17 @@ static inline int ip6_default_np_autolabel(struct net *net)
 }
 #endif
 
+#if IS_ENABLED(CONFIG_IPV6)
+static inline int ip6_multipath_hash_policy(const struct net *net)
+{
+	return net->ipv6.sysctl.multipath_hash_policy;
+}
+#else
+static inline int ip6_multipath_hash_policy(const struct net *net)
+{
+	return 0;
+}
+#endif
 
 /*
  *	Header manipulation
-- 
2.14.3

[PATCH net-next 2/3] mlxsw: spectrum: Move mlxsw_sp_span_gretap4_route()

[Ido Schimmel]

From: Petr Machata <petrm@mellanox.com>

Move the function next to the rest of gretap4 functions. Thus the
generic functions shared between gretap4 and gretap6 are in one block at
the beginning, followed by a gretap4 block, followed by a gretap6 block.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_span.c    | 66 +++++++++++-----------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index f537e1de11d9..e82f5f4d66aa 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -133,39 +133,6 @@ struct mlxsw_sp_span_entry_ops mlxsw_sp_span_entry_ops_phys = {
 	.deconfigure = mlxsw_sp_span_entry_phys_deconfigure,
 };
 
-static struct net_device *
-mlxsw_sp_span_gretap4_route(const struct net_device *to_dev,
-			    __be32 *saddrp, __be32 *daddrp)
-{
-	struct ip_tunnel *tun = netdev_priv(to_dev);
-	struct net_device *dev = NULL;
-	struct ip_tunnel_parm parms;
-	struct rtable *rt = NULL;
-	struct flowi4 fl4;
-
-	/* We assume "dev" stays valid after rt is put. */
-	ASSERT_RTNL();
-
-	parms = mlxsw_sp_ipip_netdev_parms4(to_dev);
-	ip_tunnel_init_flow(&fl4, parms.iph.protocol, *daddrp, *saddrp,
-			    0, 0, parms.link, tun->fwmark);
-
-	rt = ip_route_output_key(tun->net, &fl4);
-	if (IS_ERR(rt))
-		return NULL;
-
-	if (rt->rt_type != RTN_UNICAST)
-		goto out;
-
-	dev = rt->dst.dev;
-	*saddrp = fl4.saddr;
-	*daddrp = rt->rt_gateway;
-
-out:
-	ip_rt_put(rt);
-	return dev;
-}
-
 static int mlxsw_sp_span_dmac(struct neigh_table *tbl,
 			      const void *pkey,
 			      struct net_device *l3edev,
@@ -227,6 +194,39 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
 	return 0;
 }
 
+static struct net_device *
+mlxsw_sp_span_gretap4_route(const struct net_device *to_dev,
+			    __be32 *saddrp, __be32 *daddrp)
+{
+	struct ip_tunnel *tun = netdev_priv(to_dev);
+	struct net_device *dev = NULL;
+	struct ip_tunnel_parm parms;
+	struct rtable *rt = NULL;
+	struct flowi4 fl4;
+
+	/* We assume "dev" stays valid after rt is put. */
+	ASSERT_RTNL();
+
+	parms = mlxsw_sp_ipip_netdev_parms4(to_dev);
+	ip_tunnel_init_flow(&fl4, parms.iph.protocol, *daddrp, *saddrp,
+			    0, 0, parms.link, tun->fwmark);
+
+	rt = ip_route_output_key(tun->net, &fl4);
+	if (IS_ERR(rt))
+		return NULL;
+
+	if (rt->rt_type != RTN_UNICAST)
+		goto out;
+
+	dev = rt->dst.dev;
+	*saddrp = fl4.saddr;
+	*daddrp = rt->rt_gateway;
+
+out:
+	ip_rt_put(rt);
+	return dev;
+}
+
 static int
 mlxsw_sp_span_entry_gretap4_parms(const struct net_device *to_dev,
 				  struct mlxsw_sp_span_parms *sparmsp)
-- 
2.14.3

[PATCH net-next 3/3] mlxsw: spectrum: Don't depend on ip_gre and ip6_gre

[Ido Schimmel]

From: Petr Machata <petrm@mellanox.com>

mlxsw_spectrum supports offloading of a tc action mirred egress mirror
to a gretap or an ip6gretap netdevice, which necessitates calls to
functions defined in ip_gre, ip6_gre and ip6_tunnel modules. Previously
this was enabled by introducing a hard dependency of MLXSW_SPECTRUM on
NET_IPGRE and IPV6_GRE. However the rest of mlxsw is careful about
picking which modules are absolutely required, and therefore the better
approach is to make mlxsw_spectrum tolerant of absence of one or both of
the GRE flavors.

Hence rework the NET_IPGRE and IPV6_GRE dependencies to just guard
matching modularity, and hide the corresponding code in spectrum_span.c
in an #if IS_ENABLED. Mark mlxsw_sp_span_entry_tunnel_parms_common as
maybe unused, to muffle warnings if neither GRE flavor is selected,
which seems cleaner than introducing a composite #if.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/Kconfig         |  6 ++----
 drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c | 10 +++++++++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index 93d97b4676eb..f4d9c9975ac3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -76,12 +76,10 @@ config MLXSW_SPECTRUM
 	depends on PSAMPLE || PSAMPLE=n
 	depends on BRIDGE || BRIDGE=n
 	depends on IPV6 || IPV6=n
+	depends on NET_IPGRE || NET_IPGRE=n
+	depends on IPV6_GRE || IPV6_GRE=n
 	select PARMAN
 	select MLXFW
-	depends on NET_IPGRE
-	depends on !(MLXSW_CORE=y && NET_IPGRE=m)
-	depends on IPV6_GRE
-	depends on !(MLXSW_CORE=y && IPV6_GRE=m)
 	default m
 	---help---
 	  This driver supports Mellanox Technologies Spectrum Ethernet
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index e82f5f4d66aa..ae22a3daffbf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -167,7 +167,7 @@ mlxsw_sp_span_entry_unoffloadable(struct mlxsw_sp_span_parms *sparmsp)
 	return 0;
 }
 
-static int
+static __maybe_unused int
 mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
 					union mlxsw_sp_l3addr saddr,
 					union mlxsw_sp_l3addr daddr,
@@ -194,6 +194,7 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_NET_IPGRE)
 static struct net_device *
 mlxsw_sp_span_gretap4_route(const struct net_device *to_dev,
 			    __be32 *saddrp, __be32 *daddrp)
@@ -291,7 +292,9 @@ static const struct mlxsw_sp_span_entry_ops mlxsw_sp_span_entry_ops_gretap4 = {
 	.configure = mlxsw_sp_span_entry_gretap4_configure,
 	.deconfigure = mlxsw_sp_span_entry_gretap4_deconfigure,
 };
+#endif
 
+#if IS_ENABLED(CONFIG_IPV6_GRE)
 static struct net_device *
 mlxsw_sp_span_gretap6_route(const struct net_device *to_dev,
 			    struct in6_addr *saddrp,
@@ -389,12 +392,17 @@ struct mlxsw_sp_span_entry_ops mlxsw_sp_span_entry_ops_gretap6 = {
 	.configure = mlxsw_sp_span_entry_gretap6_configure,
 	.deconfigure = mlxsw_sp_span_entry_gretap6_deconfigure,
 };
+#endif
 
 static const
 struct mlxsw_sp_span_entry_ops *const mlxsw_sp_span_entry_types[] = {
 	&mlxsw_sp_span_entry_ops_phys,
+#if IS_ENABLED(CONFIG_NET_IPGRE)
 	&mlxsw_sp_span_entry_ops_gretap4,
+#endif
+#if IS_ENABLED(CONFIG_IPV6_GRE)
 	&mlxsw_sp_span_entry_ops_gretap6,
+#endif
 };
 
 static int
-- 
2.14.3

Re: [PATCH net-next 1/3] net: ipv6: Introduce ip6_multipath_hash_policy()

[David Ahern]

On 3/11/18 12:45 AM, Ido Schimmel wrote:
> From: Petr Machata <petrm@mellanox.com>
> 
> In order to abstract away access to the
> ipv6.sysctl.multipath_hash_policy variable, which is not available on
> systems compiled without IPv6 support, introduce a wrapper function
> ip6_multipath_hash_policy() that falls back to 0 on non-IPv6 systems.
> 
> Use this wrapper from mlxsw/spectrum_router instead of a direct
> reference.
> 
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c |  2 +-
>  include/net/ipv6.h                                    | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

For consistency, would be good to use that inline everywhere the sysctl
is referenced.