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

[PATCH net-next 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).

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

 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
2.14.3

[PATCH net-next 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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 8e4edb634b11..baea97560029 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5899,7 +5899,7 @@ static int mlxsw_sp_router_fib_rule_event(unsigned long event,
 	}
 
 	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 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 baea97560029..c9fce669cee4 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 1/2] mlxsw: spectrum_router: Return an error for non-default FIB rules

[David Ahern]

On 5/1/18 2:16 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.
> 
> 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 | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 8e4edb634b11..baea97560029 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -5899,7 +5899,7 @@ static int mlxsw_sp_router_fib_rule_event(unsigned long event,
>  	}
>  
>  	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;

shouldn't mlxsw_sp_router_fib_rule_event return -EOPNOTSUPP instead of
-1 (EPERM)?


>  }
> @@ -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);
> 

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

[Ido Schimmel]

On Tue, May 01, 2018 at 09:16:23AM -0600, David Ahern wrote:
> On 5/1/18 2:16 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.
> > 
> > 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 | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> > index 8e4edb634b11..baea97560029 100644
> > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> > @@ -5899,7 +5899,7 @@ static int mlxsw_sp_router_fib_rule_event(unsigned long event,
> >  	}
> >  
> >  	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;
> 
> shouldn't mlxsw_sp_router_fib_rule_event return -EOPNOTSUPP instead of
> -1 (EPERM)?

The -1 wasn't visible until now so it didn't matter. Will change to
-EOPNOTSUPP in v2. Thanks