netdev - Re: [PATCH net-next V1 4/4] net/mlx4_en: Add accelerated RFS support

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: <1342706957.2617.25.camel@bwh-desktop.uk.solarflarecom.com>
Date:	Thu, 19 Jul 2012 15:09:17 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Or Gerlitz <ogerlitz@...lanox.com>
CC:	<davem@...emloft.net>, <roland@...nel.org>,
	<netdev@...r.kernel.org>, <oren@...lanox.com>,
	<yevgenyp@...lanox.com>, Amir Vadai <amirv@...lanox.com>
Subject: Re: [PATCH net-next V1 4/4] net/mlx4_en: Add accelerated RFS support

On Thu, 2012-07-19 at 11:33 +0300, Or Gerlitz wrote:
> From: Amir Vadai <amirv@...lanox.com>
> 
> Use RFS infrastructure and flow steering in HW to keep CPU
> affinity of rx interrupts and application per TCP stream.
> 
> A flow steering filter is added to the HW whenever the RFS
> ndo callback is invoked by core networking code.
> 
> Because the invocation takes place in interrupt context, the
> actual setup of HW is done using workqueue. Whenever new filter
> is added, the driver checks for expiry of existing filters.
> 
> Since there's window in time between the point where the core
> RFS code invoked the ndo callback, to the point where the HW
> is configured from the workqueue context, the 2nd, 3rd etc
> packets from that stream will cause the net core to invoke
> the callback again and again.

Yes, and this is true even when the filter can be reprogrammed
synchronously as there may be more packets for the redirected flow
already in the RX queue.  This is a result of fixing the bug you pointed
out earlier (commit 09994d1b09bd9b0046a4708fa50d2106610a4058).  We
should try to find a way of noting pending redirects without
reintroducing that bug.

> To prevent inefficient/double configuration of the HW, the filters
> are kept in a database which is indexed using hash function to enable
> fast access.
> 
> Signed-off-by: Amir Vadai <amirv@...lanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@...lanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_cq.c     |    8 +-
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |  316 ++++++++++++++++++++++++
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c     |    3 +
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |   16 ++
>  4 files changed, 342 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> index 0ef6156..aa9c2f6 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> @@ -77,6 +77,12 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
>  	struct mlx4_en_dev *mdev = priv->mdev;
>  	int err = 0;
>  	char name[25];
> +	struct cpu_rmap *rmap =
> +#ifdef CONFIG_RFS_ACCEL
> +		priv->dev->rx_cpu_rmap;
> +#else
> +		NULL;
> +#endif

You can write this slightly more cleanly using IS_ENABLED().

[...]
> +static struct mlx4_en_filter *
> +mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
> +                    __be32 dst_ip, __be16 src_port, __be16 dst_port,
> +                    u32 flow_id)
> +{
[...]
> +       filter->id = priv->last_filter_id++;
[...]

You need to limit the filter IDs to be < RPS_NO_FILTER.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html