Skip to contentSkip to conversation

IB/mlx5: Check supported flow table size

PATCH In review Leon Romanovsky 3 participants 7 messages Mar 29, 2017 – Apr 24, 2017 5 messages not in the archive

Subsystems (2)
InfiniBand Mellanox MLX5 IB driver
lore
Share & tools
lore

Leon Romanovsky

author
#0 lore raw

PATCH 1/2 IB/mlx5: Check supported flow table size

From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/hw/mlx5/main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/main.c +7 -1
index 4dc0a8785fe0..1bef4f5da31c 100644
@@ -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) ||
Signature · 6 lines
2.12.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170329030901.5772-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

author
#2 lore raw

PATCH 2/2 IB/mlx5: Enlarge autogroup flow table

From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/hw/mlx5/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/hw/mlx5/main.c +2 -2
index 1bef4f5da31c..ca2cb2861afc 100644
@@ -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)
Signature · 6 lines
2.12.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170329030901.5772-2-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#4 lore raw

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

T24gV2VkLCAyMDE3LTAzLTI5IGF0IDA2OjA5ICswMzAwLCBMZW9uIFJvbWFub3Zza3kgd3JvdGU6 DQo+IC0jZGVmaW5lIE1MWDVfRlNfTUFYX1RZUEVTCSAxMA0KPiAtI2RlZmluZSBNTFg1X0ZTX01B WF9FTlRSSUVTCSAzMjAwMFVMDQo+ICsjZGVmaW5lIE1MWDVfRlNfTUFYX1RZUEVTCSA2DQo+ICsj ZGVmaW5lIE1MWDVfRlNfTUFYX0VOVFJJRVMJIEJJVCgxNikNCg0KSGVsbG8gTGVvbiBhbmQgTWFv ciwNCg0KVGhlIHVzZSBvZiB0aGUgQklUKCkgbWFjcm8gaGVyZSBsb29rcyBtaXNsZWFkaW5nIHRv IG1lLiBFbHNld2hlcmUgaW4gdGhlDQprZXJuZWwgQklUKCkgaXMgdXNlZCB0byByZXByZXNlbnQg YSBiaXRtYXNrLiBNeSB1bmRlcnN0YW5kaW5nIGlzIHRoYXQNCk1MWDVfRlNfTUFYX0VOVFJJRVMg aXMgbm90IGEgYml0bWFzayBidXQgYSB2YWx1ZT8NCg0KVGhhbmtzLA0KDQpCYXJ0Lg== --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#6 lore raw

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

Quoted 11 lines · Bart Van Assche

drivers/infiniband/hw/mlx5/main.c enum flow_table_type { · line 2042

Quoted 4 lines · Leon Romanovsky
drivers/infiniband/hw/mlx5/main.c +2 -2
-#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()).

Quoted 2 lines

Thanks,

Thanks

Quoted 2 lines

Bart.

A message that is not in the archive (sent off-list or not yet received): 20170329042320.GJ20443-U/[email protected]

#8 lore raw

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

Quoted 15 lines · Leon Romanovsky
Quoted 11 lines · Bart Van Assche

drivers/infiniband/hw/mlx5/main.c enum flow_table_type { · line 2042

Quoted 4 lines · Leon Romanovsky
drivers/infiniband/hw/mlx5/main.c +2 -2
-#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,

Quoted 8 lines

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#10 lore raw

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

Quoted 22 lines · Bart Van Assche
Quoted 21 lines · Leon Romanovsky
Quoted 11 lines · Bart Van Assche

Review

Quoted 4 lines · Leon Romanovsky
(quoted diff) +2 -2
-#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.

Quoted 8 lines
Quoted 2 lines

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.

Quoted 7 lines

Thanks,

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#11 lore raw
In reply to a message not in the archive
Quoted 11 lines · Leon Romanovsky

From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/[email protected]>

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/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>

Thanks, series applied.

Signature · 9 lines
Doug Ledford <[email protected]>
    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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html