Re: [RFC PATCH net-next 13/15] mlxsw: spectrum_router: Mark routes as "in hardware"

[Ido Schimmel <idosch@idosch.org>]

On Wed, Oct 02, 2019 at 08:27:30PM +0200, Jiri Pirko wrote:
> Wed, Oct 02, 2019 at 10:41:01AM CEST, idosch@idosch.org wrote:
> >From: Ido Schimmel <idosch@mellanox.com>
> >
> >Make use of the recently introduced APIs and mark notified routes as "in
> >hardware" after they were programmed to the device's LPM tree.
> >
> >Similarly, when a route is replaced by an higher priority one, clear the
> >"in hardware" indication from it.
> >
> >Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> >---
> > .../ethernet/mellanox/mlxsw/spectrum_router.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> >diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> >index 5a4e61f1feec..26ab8ae482ec 100644
> >--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> >+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> >@@ -4769,7 +4769,10 @@ static void mlxsw_sp_fib4_entry_replace(struct mlxsw_sp *mlxsw_sp,
> > 					struct mlxsw_sp_fib4_entry *fib4_entry)
> > {
> > 	struct mlxsw_sp_fib_node *fib_node = fib4_entry->common.fib_node;
> >+	struct net *net = mlxsw_sp_net(mlxsw_sp);
> >+	u32 *addr = (u32 *) fib_node->key.addr;
> > 	struct mlxsw_sp_fib4_entry *replaced;
> >+	struct fib_info *fi;
> > 
> > 	if (list_is_singular(&fib_node->entry_list))
> > 		return;
> >@@ -4777,6 +4780,10 @@ static void mlxsw_sp_fib4_entry_replace(struct mlxsw_sp *mlxsw_sp,
> > 	/* We inserted the new entry before replaced one */
> > 	replaced = list_next_entry(fib4_entry, common.list);
> > 
> >+	fi = mlxsw_sp_nexthop4_group_fi(replaced->common.nh_group);
> >+	fib_alias_in_hw_clear(net, *addr, fib_node->key.prefix_len, fi,
> >+			      replaced->tos, replaced->type, replaced->tb_id);
> >+
> > 	mlxsw_sp_fib4_node_entry_unlink(mlxsw_sp, replaced);
> > 	mlxsw_sp_fib4_entry_destroy(mlxsw_sp, replaced);
> > 	mlxsw_sp_fib_node_put(mlxsw_sp, fib_node);
> >@@ -4786,6 +4793,7 @@ static int
> > mlxsw_sp_router_fib4_replace(struct mlxsw_sp *mlxsw_sp,
> > 			     const struct fib_entry_notifier_info *fen_info)
> > {
> >+	struct net *net = mlxsw_sp_net(mlxsw_sp);
> > 	struct mlxsw_sp_fib4_entry *fib4_entry;
> > 	struct mlxsw_sp_fib_node *fib_node;
> > 	int err;
> >@@ -4815,6 +4823,10 @@ mlxsw_sp_router_fib4_replace(struct mlxsw_sp *mlxsw_sp,
> > 		goto err_fib4_node_entry_link;
> > 	}
> > 
> >+	fib_alias_in_hw_set(net, fen_info->dst, fen_info->dst_len,
> >+			    fen_info->fi, fen_info->tos, fen_info->type,
> >+			    fen_info->tb_id);
> 
> Can't you pass "fa" through fen_info and down to fib_alias_in_hw_set and
> avoid lookup?

No, because we don't have a reference count on 'fa' and we can't
guarantee that it will not disappear by the time we want to mark it.

> 
> 
> >+
> > 	mlxsw_sp_fib4_entry_replace(mlxsw_sp, fib4_entry);
> > 
> > 	return 0;
> >@@ -5731,11 +5743,18 @@ static void mlxsw_sp_fib4_node_flush(struct mlxsw_sp *mlxsw_sp,
> > 				     struct mlxsw_sp_fib_node *fib_node)
> > {
> > 	struct mlxsw_sp_fib4_entry *fib4_entry, *tmp;
> >+	struct net *net = mlxsw_sp_net(mlxsw_sp);
> >+	u32 *addr = (u32 *) fib_node->key.addr;
> > 
> > 	list_for_each_entry_safe(fib4_entry, tmp, &fib_node->entry_list,
> > 				 common.list) {
> > 		bool do_break = &tmp->common.list == &fib_node->entry_list;
> >+		struct fib_info *fi;
> > 
> >+		fi = mlxsw_sp_nexthop4_group_fi(fib4_entry->common.nh_group);
> >+		fib_alias_in_hw_clear(net, *addr, fib_node->key.prefix_len, fi,
> >+				      fib4_entry->tos, fib4_entry->type,
> >+				      fib4_entry->tb_id);
> > 		mlxsw_sp_fib4_node_entry_unlink(mlxsw_sp, fib4_entry);
> > 		mlxsw_sp_fib4_entry_destroy(mlxsw_sp, fib4_entry);
> > 		mlxsw_sp_fib_node_put(mlxsw_sp, fib_node);
> >-- 
> >2.21.0
> >