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 <ido...@mellanox.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 <ido...@mellanox.com> > > Signed-off-by: Jiri Pirko <j...@mellanox.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? > > $ 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. OK. Will look into that. I didn't like having the local rule first, so this might the solution. > 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. I'm aware, but not sure this belongs in this patchset. I prefer to deal with the l3mdev rule and re-ordering of the default rules first, then move to more advanced use cases. Thanks David.
