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

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: <20160922094723.GB1830@nanopsycho.orion>
Date:   Thu, 22 Sep 2016 11:47:23 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, idosch@...lanox.com,
        eladr@...lanox.com, yotamg@...lanox.com, nogahf@...lanox.com,
        ogerlitz@...lanox.com, roopa@...ulusnetworks.com,
        nikolay@...ulusnetworks.com, linville@...driver.com,
        andy@...yhouse.net, f.fainelli@...il.com, dsa@...ulusnetworks.com,
        jhs@...atatu.com, vivien.didelot@...oirfairelinux.com,
        andrew@...n.ch, ivecera@...hat.com, kaber@...sh.net,
        john@...ozen.org
Subject: 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@...sch.org wrote:
>On Wed, Sep 21, 2016 at 01:53:11PM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@...lanox.com>
>> 
>> 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.
>
>> 
>> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
>
>[...]
>
>> +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.



>
>> +		}
>> +	}
>> +	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.



>
>> +	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