RDMA/mlx5: Init dest_type when create flow
Subsystems (2)
PATCH 1/1 RDMA/mlx5: Init dest_type when create flow
From: Daria Velikovsky <[email protected]>
When using action drop dest_type was never assigned to any value. Add initialization of dest_type to -1 since 0 is valid.
---
Based on
https://lore.kernel.org/lkml/[email protected]
--- drivers/infiniband/hw/mlx5/fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/fs.c +1 -1
@@ -1903,7 +1903,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(struct mlx5_flow_context flow_context = {.flow_tag =MLX5_FS_DEFAULT_FLOW_TAG };u32 *offset_attr, offset =0 , counter_id =0 ; -int dest_id, dest_type, inlen, len, ret, i; +int dest_id, dest_type =-1 , inlen, len, ret, i;struct mlx5_ib_flow_handler *flow_handler;struct mlx5_ib_flow_matcher *fs_matcher;struct ib_uobject **arr_flow_actions;
--
2.26.2
Jason Gunthorpe
maintainerQuoted 18 lines · Leon Romanovsky
From: Daria Velikovsky <[email protected]>
When using action drop
dest_typewas never assigned to any value. Add initialization ofdest_typeto -1 since 0 is valid.Fixes f29de9eee782 ("RDMA/mlx5: Add support for drop action in DV steering")Signed-off-by Daria Velikovsky <[email protected]>Reviewed-by Maor Gottlieb <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>Based on
https://lore.kernel.org/lkml/[email protected]drivers/infiniband/hw/mlx5/fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mlx5/fs.c +1
-0+++ b/drivers/infiniband/hw/mlx5/fs.c
drivers/infiniband/hw/mlx5/fs.c static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)( · line 1906
Quoted 5 lines
drivers/infiniband/hw/mlx5/fs.c +1
-1@@ -1903,7 +1903,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(struct mlx5_flow_context flow_context = {.flow_tag =MLX5_FS_DEFAULT_FLOW_TAG };u32 *offset_attr, offset =0 , counter_id =0 ; -int dest_id, dest_type, inlen, len, ret, i; +int dest_id, dest_type =-1 , inlen, len, ret, i;
I think this should be done inside get_dests() - it is pretty ugly to have an function with an output pointer that is only filled sometimes on success.
Jason
Quoted 15 lines · Jason Gunthorpe
Quoted 14 lines · Leon Romanovsky
From: Daria Velikovsky <[email protected]>
When using action drop
dest_typewas never assigned to any value. Add initialization ofdest_typeto -1 since 0 is valid.Fixes f29de9eee782 ("RDMA/mlx5: Add support for drop action in DV steering")Signed-off-by Daria Velikovsky <[email protected]>Reviewed-by Maor Gottlieb <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>Based on
https://lore.kernel.org/lkml/[email protected]drivers/infiniband/hw/mlx5/fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines
drivers/infiniband/hw/mlx5/fs.c static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)( · line 1906
Quoted 9 lines
drivers/infiniband/hw/mlx5/fs.c +2
-1+++ b/drivers/infiniband/hw/mlx5/fs.c@@ -1903,7 +1903,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(struct mlx5_flow_context flow_context = {.flow_tag =MLX5_FS_DEFAULT_FLOW_TAG };u32 *offset_attr, offset =0 , counter_id =0 ; -int dest_id, dest_type, inlen, len, ret, i; +int dest_id, dest_type =-1 , inlen, len, ret, i;I think this should be done inside
get_dests()- it is pretty ugly to have an function with an output pointer that is only filled sometimes on success.
This was original patch which I rewrote because don't like the approach when function changes fields when it doesn't need to change. I prefer the current approach where caller has explicitly decided which default value he wants.
Thanks
Quoted 2 lines
Jason
Jason Gunthorpe
maintainerQuoted 30 lines · Leon Romanovsky
Quoted 15 lines · Jason Gunthorpe
Quoted 14 lines · Leon Romanovsky
From: Daria Velikovsky <[email protected]>
When using action drop
dest_typewas never assigned to any value. Add initialization ofdest_typeto -1 since 0 is valid.Fixes f29de9eee782 ("RDMA/mlx5: Add support for drop action in DV steering")Signed-off-by Daria Velikovsky <[email protected]>Reviewed-by Maor Gottlieb <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>Based on
https://lore.kernel.org/lkml/[email protected]drivers/infiniband/hw/mlx5/fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 13 lines
drivers/infiniband/hw/mlx5/fs.c static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)( · line 1906
Quoted 9 lines
drivers/infiniband/hw/mlx5/fs.c +2
-1+++ b/drivers/infiniband/hw/mlx5/fs.c@@ -1903,7 +1903,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(struct mlx5_flow_context flow_context = {.flow_tag =MLX5_FS_DEFAULT_FLOW_TAG };u32 *offset_attr, offset =0 , counter_id =0 ; -int dest_id, dest_type, inlen, len, ret, i; +int dest_id, dest_type =-1 , inlen, len, ret, i;I think this should be done inside
get_dests()- it is pretty ugly to have an function with an output pointer that is only filled sometimes on success.
Quoted 4 lines
This was original patch which I rewrote because don't like the approach when function changes fields when it doesn't need to change. I prefer the current approach where caller has explicitly decided which default value he wants.
How is it a "default value" ? The function's job is to fill dest_type, it should fill it correctly or fail
Jason
Quoted 31 lines · Jason Gunthorpe
Quoted 30 lines · Leon Romanovsky
Quoted 28 lines · Jason Gunthorpe
Quoted 18 lines · Leon Romanovsky
From: Daria Velikovsky <[email protected]>
When using action drop
dest_typewas never assigned to any value. Add initialization ofdest_typeto -1 since 0 is valid.Fixes f29de9eee782 ("RDMA/mlx5: Add support for drop action in DV steering")Signed-off-by Daria Velikovsky <[email protected]>Reviewed-by Maor Gottlieb <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>Based on
https://lore.kernel.org/lkml/[email protected]drivers/infiniband/hw/mlx5/fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mlx5/fs.c +1
-0+++ b/drivers/infiniband/hw/mlx5/fs.cdrivers/infiniband/hw/mlx5/fs.c static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)( · line 1906
Quoted 5 lines
drivers/infiniband/hw/mlx5/fs.c +1
-1@@ -1903,7 +1903,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(struct mlx5_flow_context flow_context = {.flow_tag =MLX5_FS_DEFAULT_FLOW_TAG };u32 *offset_attr, offset =0 , counter_id =0 ; -int dest_id, dest_type, inlen, len, ret, i; +int dest_id, dest_type =-1 , inlen, len, ret, i;I think this should be done inside
get_dests()- it is pretty ugly to have an function with an output pointer that is only filled sometimes on success.
Quoted 7 lines
Quoted 4 lines
This was original patch which I rewrote because don't like the approach when function changes fields when it doesn't need to change. I prefer the current approach where caller has explicitly decided which default value he wants.
How is it a "default value" ? The function's job is to fill
dest_type, it should fill it correctly or fail
It is one of the possible implementations to fix the bug, I presented different way that is preferrable by me. Do you think that it is so important that we MUST do your variant?
Thanks
Quoted 2 lines
Jason
Quoted 9 lines · Leon Romanovsky
From: Daria Velikovsky <[email protected]>
When using action drop
dest_typewas never assigned to any value. Add initialization ofdest_typeto -1 since 0 is valid.Fixes f29de9eee782 ("RDMA/mlx5: Add support for drop action in DV steering")Signed-off-by Daria Velikovsky <[email protected]>Reviewed-by Maor Gottlieb <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>
Quoted 6 lines
---
Based on
https://lore.kernel.org/lkml/[email protected]--- drivers/infiniband/hw/mlx5/fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to for-next, thanks
Jason