[PATCH rdma-next 1/2] IB/mlx5: Check supported flow table size

[PATCH rdma-next 1/2] IB/mlx5: Check supported flow table size

[Leon Romanovsky]

From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Check that the required flow table size is supported
by device. Return ENOMEM error if no space left.

In addition change the create flow table routine
to return ENOMEM instead of ENOSPC.

Fixes: 038d2ef87572 ('IB/mlx5: Add flow steering support')
Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 4dc0a8785fe0..1bef4f5da31c 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -2048,11 +2048,14 @@ static struct mlx5_ib_flow_prio *get_flow_table(struct mlx5_ib_dev *dev,
 	struct mlx5_flow_namespace *ns = NULL;
 	struct mlx5_ib_flow_prio *prio;
 	struct mlx5_flow_table *ft;
+	int max_table_size;
 	int num_entries;
 	int num_groups;
 	int priority;
 	int err = 0;
 
+	max_table_size = BIT(MLX5_CAP_FLOWTABLE_NIC_RX(dev->mdev,
+						       log_max_ft_size));
 	if (flow_attr->type == IB_FLOW_ATTR_NORMAL) {
 		if (flow_is_multicast_only(flow_attr) &&
 		    !dont_trap)
@@ -2091,6 +2094,9 @@ static struct mlx5_ib_flow_prio *get_flow_table(struct mlx5_ib_dev *dev,
 	if (!ns)
 		return ERR_PTR(-ENOTSUPP);
 
+	if (num_entries > max_table_size)
+		return ERR_PTR(-ENOMEM);
+
 	ft = prio->flow_table;
 	if (!ft) {
 		ft = mlx5_create_auto_grouped_flow_table(ns, priority,
@@ -2315,7 +2321,7 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
 	int err;
 
 	if (flow_attr->priority > MLX5_IB_FLOW_LAST_PRIO)
-		return ERR_PTR(-ENOSPC);
+		return ERR_PTR(-ENOMEM);
 
 	if (domain != IB_FLOW_DOMAIN_USER ||
 	    flow_attr->port > MLX5_CAP_GEN(dev->mdev, num_ports) ||
-- 
2.12.0

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

[PATCH rdma-next 2/2] IB/mlx5: Enlarge autogroup flow table

[Leon Romanovsky]

From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In order to enlarge the flow group size to 8k, we decrease
the number of flow group types to 6 and increase the flow
table size to 64k.

Flow group size is calculated as follow:
  group_size = table_size / (#group_types + 1)

Fixes: 038d2ef87572 ('IB/mlx5: Add flow steering support')
Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 1bef4f5da31c..ca2cb2861afc 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -2038,8 +2038,8 @@ enum flow_table_type {
 	MLX5_IB_FT_TX
 };
 
-#define MLX5_FS_MAX_TYPES	 10
-#define MLX5_FS_MAX_ENTRIES	 32000UL
+#define MLX5_FS_MAX_TYPES	 6
+#define MLX5_FS_MAX_ENTRIES	 BIT(16)
 static struct mlx5_ib_flow_prio *get_flow_table(struct mlx5_ib_dev *dev,
 						struct ib_flow_attr *flow_attr,
 						enum flow_table_type ft_type)
-- 
2.12.0

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

Re: [PATCH rdma-next 2/2] IB/mlx5: Enlarge autogroup flow table

[Bart Van Assche]

On Wed, 2017-03-29 at 06:09 +0300, Leon Romanovsky wrote:
> -#define MLX5_FS_MAX_TYPES	 10
> -#define MLX5_FS_MAX_ENTRIES	 32000UL
> +#define MLX5_FS_MAX_TYPES	 6
> +#define MLX5_FS_MAX_ENTRIES	 BIT(16)

Hello Leon and Maor,

The use of the BIT() macro here looks misleading to me. Elsewhere in the
kernel BIT() is used to represent a bitmask. My understanding is that
MLX5_FS_MAX_ENTRIES is not a bitmask but a value?

Thanks,

Bart.

Re: [PATCH rdma-next 2/2] IB/mlx5: Enlarge autogroup flow table

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On Wed, Mar 29, 2017 at 03:20:37AM +0000, Bart Van Assche wrote:
> On Wed, 2017-03-29 at 06:09 +0300, Leon Romanovsky wrote:
> > -#define MLX5_FS_MAX_TYPES	 10
> > -#define MLX5_FS_MAX_ENTRIES	 32000UL
> > +#define MLX5_FS_MAX_TYPES	 6
> > +#define MLX5_FS_MAX_ENTRIES	 BIT(16)
>
> Hello Leon and Maor,
>
> The use of the BIT() macro here looks misleading to me. Elsewhere in the
> kernel BIT() is used to represent a bitmask. My understanding is that
> MLX5_FS_MAX_ENTRIES is not a bitmask but a value?

Hello Bart,

I agree with you that the name "MAX_ENTRIES" is misleading. This define
MLX5_FS_MAX_ENTRIES is needed to compare num_entries with max_table_size
which is represented in BIT() format. The max_table was added in previous
patch and we thought that it will be much convenient for the reader to
compare the same BIT(..) constructions.

If you think that we abused the BIT() macro, let me know and I'll send
updated version (without BIT()).

>
> Thanks,

Thanks

>
> Bart.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 2/2] IB/mlx5: Enlarge autogroup flow table

[Bart Van Assche]

On Wed, 2017-03-29 at 07:23 +0300, Leon Romanovsky wrote:
> On Wed, Mar 29, 2017 at 03:20:37AM +0000, Bart Van Assche wrote:
> > On Wed, 2017-03-29 at 06:09 +0300, Leon Romanovsky wrote:
> > > -#define MLX5_FS_MAX_TYPES	 10
> > > -#define MLX5_FS_MAX_ENTRIES	 32000UL
> > > +#define MLX5_FS_MAX_TYPES	 6
> > > +#define MLX5_FS_MAX_ENTRIES	 BIT(16)
> > 
> > Hello Leon and Maor,
> > 
> > The use of the BIT() macro here looks misleading to me. Elsewhere in the
> > kernel BIT() is used to represent a bitmask. My understanding is that
> > MLX5_FS_MAX_ENTRIES is not a bitmask but a value?
> 
> Hello Bart,
> 
> I agree with you that the name "MAX_ENTRIES" is misleading. This define
> MLX5_FS_MAX_ENTRIES is needed to compare num_entries with max_table_size
> which is represented in BIT() format. The max_table was added in previous
> patch and we thought that it will be much convenient for the reader to
> compare the same BIT(..) constructions.
> 
> If you think that we abused the BIT() macro, let me know and I'll send
> updated version (without BIT()).

Hello Leon,

It's not that important to me, but does MLX5_FS_MAX_ENTRIES represent a number
or a bitmask? To me the name "MLX5_FS_MAX_ENTRIES" suggests that it is a number
and using BIT() suggests that it's a bitmask. This seems contradictory to me.

Thanks,

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/2] IB/mlx5: Enlarge autogroup flow table

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]

On Wed, Mar 29, 2017 at 04:15:10PM +0000, Bart Van Assche wrote:
> On Wed, 2017-03-29 at 07:23 +0300, Leon Romanovsky wrote:
> > On Wed, Mar 29, 2017 at 03:20:37AM +0000, Bart Van Assche wrote:
> > > On Wed, 2017-03-29 at 06:09 +0300, Leon Romanovsky wrote:
> > > > -#define MLX5_FS_MAX_TYPES	 10
> > > > -#define MLX5_FS_MAX_ENTRIES	 32000UL
> > > > +#define MLX5_FS_MAX_TYPES	 6
> > > > +#define MLX5_FS_MAX_ENTRIES	 BIT(16)
> > >
> > > Hello Leon and Maor,
> > >
> > > The use of the BIT() macro here looks misleading to me. Elsewhere in the
> > > kernel BIT() is used to represent a bitmask. My understanding is that
> > > MLX5_FS_MAX_ENTRIES is not a bitmask but a value?
> >
> > Hello Bart,
> >
> > I agree with you that the name "MAX_ENTRIES" is misleading. This define
> > MLX5_FS_MAX_ENTRIES is needed to compare num_entries with max_table_size
> > which is represented in BIT() format. The max_table was added in previous
> > patch and we thought that it will be much convenient for the reader to
> > compare the same BIT(..) constructions.
> >
> > If you think that we abused the BIT() macro, let me know and I'll send
> > updated version (without BIT()).
>
> Hello Leon,
>
> It's not that important to me, but does MLX5_FS_MAX_ENTRIES represent a number
> or a bitmask? To me the name "MLX5_FS_MAX_ENTRIES" suggests that it is a number
> and using BIT() suggests that it's a bitmask. This seems contradictory to me.

Down the road, that define is translated to bitmask, and it is hard for me
to categorize it. If we limit ourselves to the upper layer only, it will be a number,
otherwise it will be a bitmask.

>
> Thanks,
>
> Bart.--
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 1/2] IB/mlx5: Check supported flow table size

[Doug Ledford]

On Wed, 2017-03-29 at 06:09 +0300, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Check that the required flow table size is supported
> by device. Return ENOMEM error if no space left.
> 
> In addition change the create flow table routine
> to return ENOMEM instead of ENOSPC.
> 
> Fixes: 038d2ef87572 ('IB/mlx5: Add flow steering support')
> Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Thanks, series applied.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

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