lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 13 Mar 2017 08:59:11 -0600
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, idosch@...lanox.com, mlxsw@...lanox.com,
        shm@...ulusnetworks.com, kuznet@....inr.ac.ru, jmorris@...ei.org,
        yoshfuji@...ux-ipv6.org, kaber@...sh.net, lorenzo@...gle.com,
        mateusz.bajorski@...ia.com
Subject: Re: [patch net-next 10/10] mlxsw: spectrum_router: Don't abort on
 l3mdev rules

On 3/13/17 1:38 AM, Jiri Pirko wrote:
> From: Ido Schimmel <idosch@...lanox.com>
> 
> Now that port netdevs can be enslaved to a VRF master we need to make
> sure the device's routing tables won't be flushed upon the insertion of
> a l3mdev rule.
> 
> Note that we assume the notified l3mdev rule is a simple rule as used by
> the VRF master. We don't check for the presence of other selectors such
> as 'iif' and 'oif'.
> 
> Signed-off-by: Ido Schimmel <idosch@...lanox.com>
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index de54382..fa73ee2 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -2552,7 +2552,8 @@ static void mlxsw_sp_router_fib_event_work(struct work_struct *work)
>  		break;
>  	case FIB_EVENT_RULE_ADD: /* fall through */
>  	case FIB_EVENT_RULE_DEL:
> -		if (!fib_work->fr_info.rule->def)
> +		if (!fib_work->fr_info.rule->def &&
> +		    !fib_work->fr_info.rule->l3mdev)
>  			mlxsw_sp_router_fib4_abort(mlxsw_sp);
>  		fib_rule_put(fib_work->fr_info.rule);
>  		break;
> 

You do not want to abort if the default rules are re-ordered. For
example, the rule for the local table is moved from priority 0 to just
before the main. ie., from this order:

$ ip ru ls
0:	from all lookup local
32766:	from all lookup main
32767:	from all lookup default

to this order
$ ip ru ls
1000:	from all lookup [l3mdev-table]
32765:	from all lookup local
32766:	from all lookup main
32767:	from all lookup default

should not abort offloads.

The default flag added to rules in patch 1 is not really needed; you can
key off basic references to hardcoded table ids: RT_TABLE_MAIN and
RT_TABLE_LOCAL are set in stone. Simple rules (from all lookup X) where
only the priority changes should be enough information to not abort.

In addition, if rules are added that reference oif or iif that is not a
port netdev for this h/w then why abort offloads? e.g, consider PBR
rules related to the management interface.

Powered by blists - more mailing lists