netdev - Re: [patch net-next v2 07/12] mlxsw: spectrum: Add the multicast routing offloading logic

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: <5ded28ba-0a0c-927c-5a5e-c0fc761ca49e@cumulusnetworks.com>
Date:   Mon, 25 Sep 2017 13:40:37 +0300
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, yotamg@...lanox.com, idosch@...lanox.com,
        mlxsw@...lanox.com, andrew@...n.ch
Subject: Re: [patch net-next v2 07/12] mlxsw: spectrum: Add the multicast
 routing offloading logic

On 24/09/17 20:22, Jiri Pirko wrote:
> From: Yotam Gigi <yotamg@...lanox.com>
> 
> Add the multicast router offloading logic, which is in charge of handling
> the VIF and MFC notifications and translating it to the hardware logic API.
> 
> The offloading logic has to overcome several obstacles in order to safely
> comply with the kernel multicast router user API:
>  - It must keep track of the mapping between VIFs to netdevices. The user
>    can add an MFC cache entry pointing to a VIF, delete the VIF and add
>    re-add it with a different netdevice. The offloading logic has to handle
>    this in order to be compatible with the kernel logic.
>  - It must keep track of the mapping between netdevices to spectrum RIFs,
>    as the current hardware implementation assume having a RIF for every
>    port in a multicast router.
>  - It must handle routes pointing to pimreg device to be trapped to the
>    kernel, as the packet should be delivered to userspace.
>  - It must handle routes pointing tunnel VIFs. The current implementation
>    does not support multicast forwarding to tunnels, thus routes that point
>    to a tunnel should be trapped to the kernel.
>  - It must be aware of proxy multicast routes, which include both (*,*)
>    routes and duplicate routes. Currently proxy routes are not offloaded
>    and trigger the abort mechanism: removal of all routes from hardware and
>    triggering the traffic to go through the kernel.
> 
> The multicast routing offloading logic also updates the counters of the
> offloaded MFC routes in a periodic work.
> 
> Signed-off-by: Yotam Gigi <yotamg@...lanox.com>
> Reviewed-by: Ido Schimmel <idosch@...lanox.com>
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> ---
> v1->v2:
>  - Update the lastuse MFC entry field too, in addition to packets an bytes.
> ---
>  drivers/net/ethernet/mellanox/mlxsw/Makefile      |    3 +-
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h    |    1 +
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c | 1014 +++++++++++++++++++++
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.h |  133 +++
>  4 files changed, 1150 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
>  create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.h
> 
[snip]
> +static void mlxsw_sp_mr_route_erase(struct mlxsw_sp_mr_table *mr_table,
> +				    struct mlxsw_sp_mr_route *mr_route)
> +{
> +	struct mlxsw_sp *mlxsw_sp = mr_table->mlxsw_sp;
> +	struct mlxsw_sp_mr *mr = mlxsw_sp->mr;
> +
> +	mr->mr_ops->route_destroy(mlxsw_sp, mr->priv, mr_route->route_priv);
> +	kfree(mr_route->route_priv);
> +}
> +
> +static struct mlxsw_sp_mr_route *
> +mlxsw_sp_mr_route4_create(struct mlxsw_sp_mr_table *mr_table,
> +			  struct mfc_cache *mfc)
> +{
> +	struct mlxsw_sp_mr_route_vif_entry *rve, *tmp;
> +	struct mlxsw_sp_mr_route *mr_route;
> +	int err;
> +	int i;
> +
> +	/* Allocate and init a new route and fill it with parameters */
> +	mr_route = kzalloc(sizeof(*mr_table), GFP_KERNEL);

sizeof(*mr_table) ? Shouldn't you allocate sizeof struct mlsw_sp_mr_route (*mr_route) here ?

> +	if (!mr_route)
> +		return ERR_PTR(-ENOMEM);
> +	INIT_LIST_HEAD(&mr_route->evif_list);
> +	mlxsw_sp_mr_route4_key(mr_table, &mr_route->key, mfc);
> +
> +	/* Find min_mtu and link iVIF and eVIFs */
> +	mr_route->min_mtu = ETH_MAX_MTU;
> +	ipmr_cache_hold(mfc);
> +	mr_route->mfc4 = mfc;
> +	mr_route->mr_table = mr_table;
> +	for (i = 0; i < MAXVIFS; i++) {
> +		if (mfc->mfc_un.res.ttls[i] != 255) {
> +			err = mlxsw_sp_mr_route_evif_link(mr_route,
> +							  &mr_table->vifs[i]);
> +			if (err)
> +				goto err;
> +			if (mr_table->vifs[i].dev &&
> +			    mr_table->vifs[i].dev->mtu < mr_route->min_mtu)
> +				mr_route->min_mtu = mr_table->vifs[i].dev->mtu;
> +		}
> +	}
> +	mlxsw_sp_mr_route_ivif_link(mr_route, &mr_table->vifs[mfc->mfc_parent]);
> +	if (err)
> +		goto err;
> +
> +	mr_route->route_action = mlxsw_sp_mr_route_action(mr_route);
> +	return mr_route;
> +err:
> +	ipmr_cache_put(mfc);
> +	list_for_each_entry_safe(rve, tmp, &mr_route->evif_list, route_node)
> +		mlxsw_sp_mr_route_evif_unlink(rve);
> +	kfree(mr_route);
> +	return ERR_PTR(err);
> +}
> +
> +static void mlxsw_sp_mr_route4_destroy(struct mlxsw_sp_mr_table *mr_table,
> +				       struct mlxsw_sp_mr_route *mr_route)
> +{
> +	struct mlxsw_sp_mr_route_vif_entry *rve, *tmp;
> +
> +	mlxsw_sp_mr_route_ivif_unlink(mr_route);
> +	ipmr_cache_put(mr_route->mfc4);
> +	list_for_each_entry_safe(rve, tmp, &mr_route->evif_list, route_node)
> +		mlxsw_sp_mr_route_evif_unlink(rve);
> +	kfree(mr_route);
> +}
[snip]