From: Jiri Pirko <redacted>
Ido says:
Packets received from netdevs enslaved to different VRF devices are
forwarded using different FIB tables. In the Spectrum ASIC this is
achieved by binding different router interfaces (RIFs) to different
virtual routers (VRs). Each RIF represents an enslaved netdev and each
VR has its own FIB table according to which packets are forwarded.
The first three patches add helpers to check if the FIB rule's selector
matches all packets and extend the FIB notification chain to include the
rule's info as part of the RULE_{ADD,DEL} events. This allows offloading
drivers to sanitize the rules they don't support and flush their tables.
The fourth patch introduces a small change in the VRF driver to allow
capable drivers to more easily offload VRFs.
Finally, the last patches gradually add support for VRFs in the mlxsw
driver. First, on top of port netdevs, stacked LAG and VLAN devices and
then on top of bridges.
Some limitations I would like to point out:
1) The old model where 'oif' / 'iif' rules were programmed for each L3
master device isn't supported. Upon insertion of these rules the driver
will flush its tables and forwarding will be done by the kernel instead.
It's inferior in every way to the single 'l3mdev' rule, so this shouldn't
be an issue.
2) Inter-VRF routes pointing to a VRF device aren't offloaded. Packets
hitting these routes will be forwarded by the kernel. Inter-VRF routes
pointing to netdevs enslaved to a different VRF are offloaded.
3) There's a small discrepancy between the kernel's datapath and the
device's. By default, packets forwarded by the kernel first do a lookup
in the LOCAL table and then in the VRF's table (assuming no match). In
the device, lookup is done only in the VRF's table, which is probably
the intended behavior. Changes in v2 allow user to properly re-order the
default rules without triggering the abort mechanism.
Changes in v2:
* Drop default rule indication and allow re-ordering of default rules
(David Ahern).
* Remove ifdef around 'struct fib_rule_notifier_info' and drop redundant
dependency on IP_MULTIPLE_TABLES from rocker and mlxsw.
* Add David's Acked-by to the fourth patch.
* Remove netif_is_vrf_master() and use netif_is_l3_master() instead
(David Ahern).
Ido Schimmel (9):
net: fib_rules: Check if selector matches all packets
ipv4: fib_rules: Add notifier info to FIB rules notifications
ipv4: fib_rules: Dump FIB rules when registering FIB notifier
net: vrf: Set slave's private flag before linking
mlxsw: spectrum_router: Associate RIFs with correct VR
mlxsw: spectrum_router: Don't destroy RIF if L3 slave
mlxsw: spectrum_router: Add support for VRFs
mlxsw: spectrum_router: Add support for VRFs on top of bridges
mlxsw: spectrum_router: Don't abort on l3mdev rules
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 71 +++++++++++++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 8 ++
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 99 +++++++++++++++++++++-
drivers/net/ethernet/rocker/rocker_main.c | 26 +++++-
drivers/net/vrf.c | 8 +-
include/net/fib_rules.h | 1 +
include/net/ip_fib.h | 12 +++
net/core/fib_rules.c | 14 +++
net/ipv4/fib_rules.c | 42 +++++++--
9 files changed, 261 insertions(+), 20 deletions(-)
--
2.7.4
From: Ido Schimmel <redacted>
Whenever a FIB rule is added or removed, a notification is sent in the
FIB notification chain. However, listeners don't have a way to tell
which rule was added or removed.
This is problematic as we would like to give listeners the ability to
decide which action to execute based on the notified rule. Specifically,
offloading drivers should be able to determine if they support the
reflection of the notified FIB rule and flush their LPM tables in case
they don't.
Do that by adding a notifier info to these notifications and embed the
common FIB rule struct in it.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
include/net/ip_fib.h | 5 +++++
net/ipv4/fib_rules.c | 13 ++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)
@@ -213,6 +213,11 @@ struct fib_entry_notifier_info {u32tb_id;};+structfib_rule_notifier_info{+structfib_notifier_infoinfo;/* must be first */+structfib_rule*rule;+};+structfib_nh_notifier_info{structfib_notifier_infoinfo;/* must be first */structfib_nh*fib_nh;
From: David Ahern <hidden> Date: 2017-03-15 15:16:51
On 3/15/17 5:05 AM, Jiri Pirko wrote:
From: Ido Schimmel <redacted>
Whenever a FIB rule is added or removed, a notification is sent in the
FIB notification chain. However, listeners don't have a way to tell
which rule was added or removed.
This is problematic as we would like to give listeners the ability to
decide which action to execute based on the notified rule. Specifically,
offloading drivers should be able to determine if they support the
reflection of the notified FIB rule and flush their LPM tables in case
they don't.
Do that by adding a notifier info to these notifications and embed the
common FIB rule struct in it.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
include/net/ip_fib.h | 5 +++++
net/ipv4/fib_rules.c | 13 ++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)
From: Ido Schimmel <redacted>
Currently, when non-default (custom) FIB rules are used, devices capable
of layer 3 offloading flush their tables and let the kernel do the
forwarding instead.
When these devices' drivers are loaded they register to the FIB
notification chain, which lets them know about the existence of any
custom FIB rules. This is done by sending a RULE_ADD notification based
on the value of 'net->ipv4.fib_has_custom_rules'.
This approach is problematic when VRF offload is taken into account, as
upon the creation of the first VRF netdev, a l3mdev rule is programmed
to direct skbs to the VRF's table.
Instead of merely reading the above value and sending a single RULE_ADD
notification, we should iterate over all the FIB rules and send a
detailed notification for each, thereby allowing offloading drivers to
sanitize the rules they don't support and potentially flush their
tables.
While l3mdev rules are uniquely marked, the default rules are not.
Therefore, when they are being notified they might invoke offloading
drivers to unnecessarily flush their tables.
Solve this by adding helpers that check whether a FIB rule's selector
matches all packets and is thus essentially a NOP. Unlike the selector,
the rule's action is completely visible to potential listeners and can
therefore be sanitized by them.
As noted by David Ahern, uniquely marking the default rules is
insufficient. When using VRFs, it's common to avoid false hits by moving
the rule for the local table to just before the main table:
Default configuration:
$ ip rule show
0: from all lookup local
32766: from all lookup main
32767: from all lookup default
Common configuration with VRFs:
$ ip rule show
1000: from all lookup [l3mdev-table]
32765: from all lookup local
32766: from all lookup main
32767: from all lookup default
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
include/net/fib_rules.h | 1 +
include/net/ip_fib.h | 7 +++++++
net/core/fib_rules.c | 14 ++++++++++++++
net/ipv4/fib_rules.c | 10 ++++++++++
4 files changed, 32 insertions(+)
l3mdev should not be in that list. Setting l3mdev is functionally
equivalent to setting rule->table. The difference is that l3mdev means
go get the table from the device. It has no bearing on a 'matchall' intent.
l3mdev should not be in that list. Setting l3mdev is functionally
equivalent to setting rule->table. The difference is that l3mdev means
go get the table from the device. It has no bearing on a 'matchall' intent.
There are multiple conversions from 'fib_rule' to 'fib4_rule' in this
file, all use typecast.
understood. dst to rtable/rt6_info has the same problem. Rather than
continuing with the typecast, new code should use container_of and older
code can be converted in time.
There are multiple conversions from 'fib_rule' to 'fib4_rule' in this
file, all use typecast.
understood. dst to rtable/rt6_info has the same problem. Rather than
continuing with the typecast, new code should use container_of and older
code can be converted in time.
From: Ido Schimmel <redacted>
In commit c3852ef7f2f8 ("ipv4: fib: Replay events when registering FIB
notifier") we dumped the FIB tables and replayed the events to the
passed notification block.
However, we merely sent a RULE_ADD notification in case custom rules
were in use. As explained in previous patches, this approach won't work
anymore. Instead, we should notify the caller about all the FIB rules
and let it act accordingly.
Upon registration to the FIB notification chain, replay a RULE_ADD
notification for each programmed FIB rule, custom or not. The integrity
of the dump is ensured by the mechanism introduced in the above
mentioned commit.
Prevent regressions by making sure current listeners correctly sanitize
the notified rules.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 22 +++++++++++++++++-
drivers/net/ethernet/rocker/rocker_main.c | 26 ++++++++++++++++++++--
net/ipv4/fib_rules.c | 19 +++++++++++++---
3 files changed, 61 insertions(+), 6 deletions(-)
@@ -2548,7 +2560,10 @@ static void mlxsw_sp_router_fib_event_work(struct work_struct *work)break;caseFIB_EVENT_RULE_ADD:/* fall through */caseFIB_EVENT_RULE_DEL:-mlxsw_sp_router_fib4_abort(mlxsw_sp);+rule=fib_work->fr_info.rule;+if(!mlxsw_sp_fib4_rule_default(rule))+mlxsw_sp_router_fib4_abort(mlxsw_sp);+fib_rule_put(rule);break;caseFIB_EVENT_NH_ADD:/* fall through */caseFIB_EVENT_NH_DEL:
@@ -2591,6 +2606,11 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,*/fib_info_hold(fib_work->fen_info.fi);break;+caseFIB_EVENT_RULE_ADD:/* fall through */+caseFIB_EVENT_RULE_DEL:+memcpy(&fib_work->fr_info,ptr,sizeof(fib_work->fr_info));+fib_rule_get(fib_work->fr_info.rule);+break;caseFIB_EVENT_NH_ADD:/* fall through */caseFIB_EVENT_NH_DEL:memcpy(&fib_work->fnh_info,ptr,sizeof(fib_work->fnh_info));
@@ -2202,7 +2216,10 @@ static void rocker_router_fib_event_work(struct work_struct *work)break;caseFIB_EVENT_RULE_ADD:/* fall through */caseFIB_EVENT_RULE_DEL:-rocker_world_fib4_abort(rocker);+rule=fib_work->fr_info.rule;+if(!rocker_world_fib4_rule_default(rule))+rocker_world_fib4_abort(rocker);+fib_rule_put(rule);break;}rtnl_unlock();
@@ -2233,6 +2250,11 @@ static int rocker_router_fib_event(struct notifier_block *nb,*/fib_info_hold(fib_work->fen_info.fi);break;+caseFIB_EVENT_RULE_ADD:/* fall through */+caseFIB_EVENT_RULE_DEL:+memcpy(&fib_work->fr_info,ptr,sizeof(fib_work->fr_info));+fib_rule_get(fib_work->fr_info.rule);+break;}queue_work(rocker->rocker_owq,&fib_work->work);
@@ -174,6 +174,17 @@ static struct fib_table *fib_empty_table(struct net *net)returnNULL;}+staticintcall_fib_rule_notifier(structnotifier_block*nb,structnet*net,+enumfib_event_typeevent_type,+structfib_rule*rule)+{+structfib_rule_notifier_infoinfo={+.rule=rule,+};++returncall_fib_notifier(nb,net,event_type,&info.info);+}+staticintcall_fib_rule_notifiers(structnet*net,enumfib_event_typeevent_type,structfib_rule*rule)
@@ -185,12 +196,14 @@ static int call_fib_rule_notifiers(struct net *net,returncall_fib_notifiers(net,event_type,&info.info);}+/* Called with rcu_read_lock() */voidfib_rules_notify(structnet*net,structnotifier_block*nb){-structfib_notifier_infoinfo;+structfib_rules_ops*ops=net->ipv4.rules_ops;+structfib_rule*rule;-if(net->ipv4.fib_has_custom_rules)-call_fib_notifier(nb,net,FIB_EVENT_RULE_ADD,&info);+list_for_each_entry_rcu(rule,&ops->rules_list,list)+call_fib_rule_notifier(nb,net,FIB_EVENT_RULE_ADD,rule);}staticconststructnla_policyfib4_rule_policy[FRA_MAX+1]={
From: Ido Schimmel <redacted>
Allow listeners of the subsequent CHANGEUPPER notification to retrieve
the VRF's table ID by calling l3mdev_fib_table() with the slave netdev.
Without this change, the netdev won't be considered an L3 slave and the
function would return 0.
This is consistent with other master device such as bridge and bond that
set the slave's private flag before linking. It also makes
do_vrf_{add,del}_slave() symmetric.
Signed-off-by: Ido Schimmel <redacted>
Acked-by: David Ahern <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/vrf.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Ido Schimmel <redacted>
When a router interface (RIF) is created due to a netdev being enslaved
to a VRF master, then it should be associated with the appropriate
virtual router (VR) and not the default one.
If netdev is a VRF slave, lookup the VR based on the VRF's table ID.
Otherwise default to the MAIN table.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
From: Ido Schimmel <redacted>
Before we allow port netdevs to 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 <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Ido Schimmel <redacted>
We usually destroy the netdev's router interface (RIF) when the last IP
address is removed from it.
However, we shouldn't do that if it's enslaved to an L3 master device.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
@@ -2669,7 +2670,7 @@ static bool mlxsw_sp_rif_should_config(struct mlxsw_sp_rif *r,returntrue;returnfalse;caseNETDEV_DOWN:-if(r&&!in_dev->ifa_list)+if(r&&!in_dev->ifa_list&&!netif_is_l3_slave(r->dev))returntrue;/* It is possible we already removed the RIF ourselves*ifitwasassignedtoanetdevthatisnowabridge
From: Ido Schimmel <redacted>
In a similar fashion to the previous patch, allow bridges and VLAN
devices on top of bridges to be enslaved to a VRF master device.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 52 +++++++++++++++++++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 ++
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 26 +++++++++++
3 files changed, 81 insertions(+), 1 deletion(-)
@@ -4111,7 +4111,7 @@ static int mlxsw_sp_netdevice_bridge_event(struct net_device *br_dev,switch(event){caseNETDEV_PRECHANGEUPPER:upper_dev=info->upper_dev;-if(!is_vlan_dev(upper_dev))+if(!is_vlan_dev(upper_dev)&&!netif_is_l3_master(upper_dev))return-EINVAL;if(is_vlan_dev(upper_dev)&&br_dev!=mlxsw_sp->master_bridge.dev)
@@ -4126,6 +4126,12 @@ static int mlxsw_sp_netdevice_bridge_event(struct net_device *br_dev,elsemlxsw_sp_master_bridge_vlan_unlink(mlxsw_sp,upper_dev);+}elseif(netif_is_l3_master(upper_dev)){+if(info->linking)+err=mlxsw_sp_bridge_vrf_join(mlxsw_sp,+br_dev);+else+mlxsw_sp_bridge_vrf_leave(mlxsw_sp,br_dev);}else{err=-EINVAL;WARN_ON(1);
@@ -4415,6 +4421,47 @@ static int mlxsw_sp_netdevice_lag_vport_event(struct net_device *lag_dev,return0;}+staticintmlxsw_sp_netdevice_bridge_vlan_event(structnet_device*vlan_dev,+unsignedlongevent,void*ptr)+{+structnetdev_notifier_changeupper_info*info;+structmlxsw_sp*mlxsw_sp;+interr=0;++mlxsw_sp=mlxsw_sp_lower_get(vlan_dev);+if(!mlxsw_sp)+return0;++info=ptr;++switch(event){+caseNETDEV_PRECHANGEUPPER:+/* VLAN devices are only allowed on top of the+*VLAN-awarebridge.+*/+if(WARN_ON(vlan_dev_real_dev(vlan_dev)!=+mlxsw_sp->master_bridge.dev))+return-EINVAL;+if(!netif_is_l3_master(info->upper_dev))+return-EINVAL;+break;+caseNETDEV_CHANGEUPPER:+if(netif_is_l3_master(info->upper_dev)){+if(info->linking)+err=mlxsw_sp_bridge_vrf_join(mlxsw_sp,+vlan_dev);+else+mlxsw_sp_bridge_vrf_leave(mlxsw_sp,vlan_dev);+}else{+err=-EINVAL;+WARN_ON(1);+}+break;+}++returnerr;+}+staticintmlxsw_sp_netdevice_vlan_event(structnet_device*vlan_dev,unsignedlongevent,void*ptr){
@@ -4427,6 +4474,9 @@ static int mlxsw_sp_netdevice_vlan_event(struct net_device *vlan_dev,elseif(netif_is_lag_master(real_dev))returnmlxsw_sp_netdevice_lag_vport_event(real_dev,event,ptr,vid);+elseif(netif_is_bridge_master(real_dev))+returnmlxsw_sp_netdevice_bridge_vlan_event(vlan_dev,event,+ptr);return0;}
From: Ido Schimmel <redacted>
Allow port netdevs, LAG and VLAN devices stacked on top of these to be
enslaved to a VRF master device.
Upon enslavement, create a router interface (RIF) for the enslaved
netdev and associate it with a virtual router (VR) based on the VRF's
table ID.
If a RIF already exists for the netdev (f.e., due to the existence of an
IP address), then it's deleted and a new one is created with the
appropriate VR binding.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 19 ++++++++--
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 +++
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 41 ++++++++++++++++++++++
3 files changed, 61 insertions(+), 3 deletions(-)
@@ -3235,6 +3235,47 @@ int mlxsw_sp_netdevice_router_port_event(struct net_device *dev)returnerr;}+intmlxsw_sp_vport_vrf_join(structmlxsw_sp_port*mlxsw_sp_vport)+{+structmlxsw_sp_fid*f=mlxsw_sp_vport_fid_get(mlxsw_sp_vport);+structnet_device*dev=mlxsw_sp_vport->dev;++/* In case vPort already has a RIF, then we need to drop it.+*AnewonewillbecreatedusingtheVRF'sVR.+*/+if(f&&f->r)+mlxsw_sp_vport_rif_sp_leave(mlxsw_sp_vport);++returnmlxsw_sp_vport_rif_sp_join(mlxsw_sp_vport,dev);+}++voidmlxsw_sp_vport_vrf_leave(structmlxsw_sp_port*mlxsw_sp_vport)+{+mlxsw_sp_vport_rif_sp_leave(mlxsw_sp_vport);+}++intmlxsw_sp_port_vrf_join(structmlxsw_sp_port*mlxsw_sp_port)+{+structmlxsw_sp_port*mlxsw_sp_vport;++mlxsw_sp_vport=mlxsw_sp_port_vport_find(mlxsw_sp_port,1);+if(WARN_ON(!mlxsw_sp_vport))+return-EINVAL;++returnmlxsw_sp_vport_vrf_join(mlxsw_sp_vport);+}++voidmlxsw_sp_port_vrf_leave(structmlxsw_sp_port*mlxsw_sp_port)+{+structmlxsw_sp_port*mlxsw_sp_vport;++mlxsw_sp_vport=mlxsw_sp_port_vport_find(mlxsw_sp_port,1);+if(WARN_ON(!mlxsw_sp_vport))+return;++mlxsw_sp_vport_vrf_leave(mlxsw_sp_vport);+}+staticvoidmlxsw_sp_router_fib_dump_flush(structnotifier_block*nb){structmlxsw_sp*mlxsw_sp=container_of(nb,structmlxsw_sp,fib_nb);