Re: [patch net-next 3/6] mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls

Thu, Sep 22, 2016 at 08:51:02AM CEST, idosch@idosch.org wrote:

On Wed, Sep 21, 2016 at 01:53:11PM +0200, Jiri Pirko wrote:

quoted

From: Jiri Pirko <redacted>

Until now, in order to offload a FIB entry to HW we use switchdev op.
However that has limits. Mainly in case we need to make the HW aware of
all route prefixes configured in kernel. HW needs to know those in order
to properly trap appropriate packets and pass the to kernel to do
the forwarding. Abort mechanism is now handled within the mlxsw driver.

FWIW, I think it's smart to move abort into the driver instead of
flushing all the routes from the namespace as before.

quoted

Signed-off-by: Jiri Pirko <redacted>

[...]

quoted

+static void mlxsw_sp_router_fib4_abort(struct mlxsw_sp *mlxsw_sp)
+{
+	char ralue_pl[MLXSW_REG_RALUE_LEN];
+	struct mlxsw_resources *resources;
+	struct mlxsw_sp_fib_entry *fib_entry;
+	struct mlxsw_sp_fib_entry *tmp;
+	struct mlxsw_sp_vr *vr;
+	int i;
+	int err;
+
+	resources = mlxsw_core_resources_get(mlxsw_sp->core);
+	for (i = 0; i < resources->max_virtual_routers; i++) {
+		vr = &mlxsw_sp->router.vrs[i];
+		if (!vr->used)
+			continue;
+
+		list_for_each_entry_safe(fib_entry, tmp,
+					 &vr->fib->entry_list, list) {
+			fib_info_offload_dec(fib_entry->fi);
+			mlxsw_sp_fib_entry_del(mlxsw_sp, fib_entry);
+			mlxsw_sp_fib_entry_remove(fib_entry->vr->fib,
+						  fib_entry);
+			mlxsw_sp_fib_entry_put_all(mlxsw_sp, fib_entry);

If we now do the routing in slow path, then maybe it makes sense to also
flush all the neighbour entries and prevent new neighbours from being
programmed into the device?

Hmm, that makes sense. Since this patch does not change the existing
behaiour regarding this, I would like to address that in follow-up
patch.

quoted

+		}
+	}
+	mlxsw_sp->router.aborted = true;
+
+	mlxsw_reg_ralue_pack4(ralue_pl, MLXSW_SP_L3_PROTO_IPV4,
+			      MLXSW_REG_RALUE_OP_WRITE_WRITE, 0, 0, 0);

I'm not sure about that, but the loop above removed all the tables from
the device and now you are using table 0 again. Will this work w/o
binding some tree to it (0?)?

You are right. Will fix.

quoted

+	mlxsw_reg_ralue_act_ip2me_pack(ralue_pl);
+	err = mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(ralue), ralue_pl);
+	if (err)
+		dev_warn(mlxsw_sp->bus_info->dev, "Failed to set abort trap.\n");
+}

Thanks