netdev - Re: [patch net-next 10/10] mlxsw: spectrum_router: Don't abort on l3mdev rules

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]

Message-ID: <767ac543-e089-56f7-a94e-e8627e57c14b@cumulusnetworks.com>
Date:   Mon, 13 Mar 2017 09:37:19 -0600
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org,
        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 9:22 AM, Ido Schimmel wrote:
> On Mon, Mar 13, 2017 at 08:59:11AM -0600, David Ahern wrote:
>> 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:
> 
> Are you aware of configurations employing the VRF device and leaving the
> rule for the local table at priority 0?

There is no explicit requirement to re-order, but as I mentioned in one
of the switchdev calls you can get false hits on the local table when
the lookup really should have gone to the VRF table. IMO, best practice
for VRF is to move the the local table rule after the l3mdev / per-VRF
FIB rules.