[PATCH net-next v2 0/2] mlxsw: Reject unsupported FIB configurations

[PATCH net-next v2 0/2] mlxsw: Reject unsupported FIB configurations

[Ido Schimmel]

Recently it became possible for listeners of the FIB notification chain
to veto operations such as addition of routes and rules.

Adjust the mlxsw driver to take advantage of it and return an error for
unsupported FIB rules and for routes configured after the abort
mechanism was triggered (due to exceeded resources for example).

v2:
* Change error code in first patch to -EOPNOTSUPP (David Ahern).

Ido Schimmel (2):
  mlxsw: spectrum_router: Return an error for non-default FIB rules
  mlxsw: spectrum_router: Return an error for routes added after abort

 .../net/ethernet/mellanox/mlxsw/spectrum_router.c   | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

-- 
2.14.3

[PATCH net-next v2 1/2] mlxsw: spectrum_router: Return an error for non-default FIB rules

[Ido Schimmel]

Since commit 9776d32537d2 ("net: Move call_fib_rule_notifiers up in
fib_nl_newrule") it is possible to forbid the installation of
unsupported FIB rules.

Have mlxsw return an error for non-default FIB rules in addition to the
existing extack message.

Example:
# ip rule add from 198.51.100.1 table 10
Error: mlxsw_spectrum: FIB rules not supported.

Note that offload is only aborted when non-default FIB rules are already
installed and merely replayed during module initialization.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 8e4edb634b11..added380e344 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5882,24 +5882,24 @@ static int mlxsw_sp_router_fib_rule_event(unsigned long event,
 	switch (info->family) {
 	case AF_INET:
 		if (!fib4_rule_default(rule) && !rule->l3mdev)
-			err = -1;
+			err = -EOPNOTSUPP;
 		break;
 	case AF_INET6:
 		if (!fib6_rule_default(rule) && !rule->l3mdev)
-			err = -1;
+			err = -EOPNOTSUPP;
 		break;
 	case RTNL_FAMILY_IPMR:
 		if (!ipmr_rule_default(rule) && !rule->l3mdev)
-			err = -1;
+			err = -EOPNOTSUPP;
 		break;
 	case RTNL_FAMILY_IP6MR:
 		if (!ip6mr_rule_default(rule) && !rule->l3mdev)
-			err = -1;
+			err = -EOPNOTSUPP;
 		break;
 	}
 
 	if (err < 0)
-		NL_SET_ERR_MSG_MOD(extack, "FIB rules not supported. Aborting offload");
+		NL_SET_ERR_MSG_MOD(extack, "FIB rules not supported");
 
 	return err;
 }
@@ -5926,8 +5926,8 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
 	case FIB_EVENT_RULE_DEL:
 		err = mlxsw_sp_router_fib_rule_event(event, info,
 						     router->mlxsw_sp);
-		if (!err)
-			return NOTIFY_DONE;
+		if (!err || info->extack)
+			return notifier_from_errno(err);
 	}
 
 	fib_work = kzalloc(sizeof(*fib_work), GFP_ATOMIC);
-- 
2.14.3

[PATCH net-next v2 2/2] mlxsw: spectrum_router: Return an error for routes added after abort

[Ido Schimmel]

We currently do not perform accounting in the driver and thus can't
reject routes before resources are exceeded.

However, in order to make users aware of the fact that routes are no
longer offloaded we can return an error for routes configured after the
abort mechanism was triggered.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index added380e344..8028d221aece 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5928,6 +5928,13 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
 						     router->mlxsw_sp);
 		if (!err || info->extack)
 			return notifier_from_errno(err);
+		break;
+	case FIB_EVENT_ENTRY_ADD:
+		if (router->aborted) {
+			NL_SET_ERR_MSG_MOD(info->extack, "FIB offload was aborted. Not configuring route");
+			return notifier_from_errno(-EINVAL);
+		}
+		break;
 	}
 
 	fib_work = kzalloc(sizeof(*fib_work), GFP_ATOMIC);
-- 
2.14.3

Re: [PATCH net-next v2 1/2] mlxsw: spectrum_router: Return an error for non-default FIB rules

[David Ahern]

On 5/2/18 1:17 AM, Ido Schimmel wrote:
> Since commit 9776d32537d2 ("net: Move call_fib_rule_notifiers up in
> fib_nl_newrule") it is possible to forbid the installation of
> unsupported FIB rules.
> 
> Have mlxsw return an error for non-default FIB rules in addition to the
> existing extack message.
> 
> Example:
> # ip rule add from 198.51.100.1 table 10
> Error: mlxsw_spectrum: FIB rules not supported.

Next, I think we need a user override flag for rules to prevent hardware
from limiting the rules that can be added and aborting offload. For
example, if the user knows that a rule is meant to direct traffic out of
a management VRF and is unrelated to front panel ports the offload
should ignore it.

> 
> Note that offload is only aborted when non-default FIB rules are already
> installed and merely replayed during module initialization.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

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

Re: [PATCH net-next v2 2/2] mlxsw: spectrum_router: Return an error for routes added after abort

[David Ahern]

On 5/2/18 1:17 AM, Ido Schimmel wrote:
> We currently do not perform accounting in the driver and thus can't
> reject routes before resources are exceeded.
> 
> However, in order to make users aware of the fact that routes are no
> longer offloaded we can return an error for routes configured after the
> abort mechanism was triggered.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index added380e344..8028d221aece 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -5928,6 +5928,13 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
>  						     router->mlxsw_sp);
>  		if (!err || info->extack)
>  			return notifier_from_errno(err);
> +		break;
> +	case FIB_EVENT_ENTRY_ADD:
> +		if (router->aborted) {
> +			NL_SET_ERR_MSG_MOD(info->extack, "FIB offload was aborted. Not configuring route");
> +			return notifier_from_errno(-EINVAL);
> +		}
> +		break;
>  	}
>  
>  	fib_work = kzalloc(sizeof(*fib_work), GFP_ATOMIC);
> 

Reasonable next step.

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

Re: [PATCH net-next v2 0/2] mlxsw: Reject unsupported FIB configurations

[David Miller]

From: Ido Schimmel <idosch@mellanox.com>
Date: Wed,  2 May 2018 10:17:33 +0300

> Recently it became possible for listeners of the FIB notification chain
> to veto operations such as addition of routes and rules.
> 
> Adjust the mlxsw driver to take advantage of it and return an error for
> unsupported FIB rules and for routes configured after the abort
> mechanism was triggered (due to exceeded resources for example).
> 
> v2:
> * Change error code in first patch to -EOPNOTSUPP (David Ahern).

Series applied, thanks.