* [PATCH rdma-next 0/6] Set flow_label and RoCEv2 UDP source port for datagram QP @ 2020-03-18 9:52 Leon Romanovsky 2020-03-18 9:52 ` [PATCH mlx5-next 1/6] net/mlx5: Enable SW-defined RoCEv2 UDP source port Leon Romanovsky 0 siblings, 1 reply; 5+ messages in thread From: Leon Romanovsky @ 2020-03-18 9:52 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Leon Romanovsky, linux-rdma, Maor Gottlieb, Mark Zhang, netdev, Saeed Mahameed From: Leon Romanovsky <leonro@mellanox.com> From Mark: This series provide flow label and UDP source port definition in RoCE v2. Those fields are used to create entropy for network routes (ECMP), load balancers and 802.3ad link aggregation switching that are not aware of RoCE headers. Thanks. Mark Zhang (6): net/mlx5: Enable SW-defined RoCEv2 UDP source port RDMA/core: Add hash functions to calculate RoCEv2 flowlabel and UDP source port RDMA/mlx5: Define RoCEv2 udp source port when set path RDMA/cma: Initialize the flow label of CM's route path record RDMA/cm: Set flow label of recv_wc based on primary flow label RDMA/mlx5: Set UDP source port based on the grh.flow_label drivers/infiniband/core/cm.c | 7 +++ drivers/infiniband/core/cma.c | 23 ++++++++++ drivers/infiniband/hw/mlx5/ah.c | 21 ++++++++- drivers/infiniband/hw/mlx5/main.c | 4 +- drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +- drivers/infiniband/hw/mlx5/qp.c | 30 ++++++++++--- .../net/ethernet/mellanox/mlx5/core/main.c | 39 ++++++++++++++++ include/linux/mlx5/mlx5_ifc.h | 5 ++- include/rdma/ib_verbs.h | 44 +++++++++++++++++++ 9 files changed, 164 insertions(+), 13 deletions(-) -- 2.24.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH mlx5-next 1/6] net/mlx5: Enable SW-defined RoCEv2 UDP source port 2020-03-18 9:52 [PATCH rdma-next 0/6] Set flow_label and RoCEv2 UDP source port for datagram QP Leon Romanovsky @ 2020-03-18 9:52 ` Leon Romanovsky 2020-03-18 23:33 ` Saeed Mahameed 0 siblings, 1 reply; 5+ messages in thread From: Leon Romanovsky @ 2020-03-18 9:52 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Mark Zhang, linux-rdma, Maor Gottlieb, netdev, Saeed Mahameed From: Mark Zhang <markz@mellanox.com> When this is enabled, UDP source port for RoCEv2 packets are defined by software instead of firmware. Signed-off-by: Mark Zhang <markz@mellanox.com> Reviewed-by: Maor Gottlieb <maorg@mellanox.com> Signed-off-by: Leon Romanovsky <leonro@mellanox.com> --- .../net/ethernet/mellanox/mlx5/core/main.c | 39 +++++++++++++++++++ include/linux/mlx5/mlx5_ifc.h | 5 ++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c index 6b38ec72215a..bdc73370297b 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c @@ -585,6 +585,39 @@ static int handle_hca_cap(struct mlx5_core_dev *dev) return err; } +static int handle_hca_cap_roce(struct mlx5_core_dev *dev) +{ + int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in); + void *set_hca_cap; + void *set_ctx; + int err; + + if (!MLX5_CAP_GEN(dev, roce)) + return 0; + + err = mlx5_core_get_caps(dev, MLX5_CAP_ROCE); + if (err) + return err; + + if (MLX5_CAP_ROCE(dev, sw_r_roce_src_udp_port) || + !MLX5_CAP_ROCE_MAX(dev, sw_r_roce_src_udp_port)) + return 0; + + set_ctx = kzalloc(set_sz, GFP_KERNEL); + if (!set_ctx) + return -ENOMEM; + + set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx, capability); + memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ROCE], + MLX5_ST_SZ_BYTES(roce_cap)); + MLX5_SET(roce_cap, set_hca_cap, sw_r_roce_src_udp_port, 1); + + err = set_caps(dev, set_ctx, set_sz, MLX5_SET_HCA_CAP_OP_MOD_ROCE); + + kfree(set_ctx); + return err; +} + static int set_hca_cap(struct mlx5_core_dev *dev) { int err; @@ -607,6 +640,12 @@ static int set_hca_cap(struct mlx5_core_dev *dev) goto out; } + err = handle_hca_cap_roce(dev); + if (err) { + mlx5_core_err(dev, "handle_hca_cap_roce failed\n"); + goto out; + } + out: return err; } diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h index 208bf1127be7..bb217c3f30da 100644 --- a/include/linux/mlx5/mlx5_ifc.h +++ b/include/linux/mlx5/mlx5_ifc.h @@ -74,6 +74,7 @@ enum { MLX5_SET_HCA_CAP_OP_MOD_GENERAL_DEVICE = 0x0, MLX5_SET_HCA_CAP_OP_MOD_ODP = 0x2, MLX5_SET_HCA_CAP_OP_MOD_ATOMIC = 0x3, + MLX5_SET_HCA_CAP_OP_MOD_ROCE = 0x4, }; enum { @@ -902,7 +903,9 @@ struct mlx5_ifc_per_protocol_networking_offload_caps_bits { struct mlx5_ifc_roce_cap_bits { u8 roce_apm[0x1]; - u8 reserved_at_1[0x1f]; + u8 reserved_at_1[0x3]; + u8 sw_r_roce_src_udp_port[0x1]; + u8 reserved_at_5[0x1b]; u8 reserved_at_20[0x60]; -- 2.24.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH mlx5-next 1/6] net/mlx5: Enable SW-defined RoCEv2 UDP source port 2020-03-18 9:52 ` [PATCH mlx5-next 1/6] net/mlx5: Enable SW-defined RoCEv2 UDP source port Leon Romanovsky @ 2020-03-18 23:33 ` Saeed Mahameed 2020-03-19 6:05 ` Leon Romanovsky 0 siblings, 1 reply; 5+ messages in thread From: Saeed Mahameed @ 2020-03-18 23:33 UTC (permalink / raw) To: Jason Gunthorpe, leon, dledford Cc: Mark Zhang, Maor Gottlieb, netdev, linux-rdma On Wed, 2020-03-18 at 11:52 +0200, Leon Romanovsky wrote: > From: Mark Zhang <markz@mellanox.com> > > When this is enabled, UDP source port for RoCEv2 packets are defined > by software instead of firmware. > > Signed-off-by: Mark Zhang <markz@mellanox.com> > Reviewed-by: Maor Gottlieb <maorg@mellanox.com> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > --- > .../net/ethernet/mellanox/mlx5/core/main.c | 39 > +++++++++++++++++++ > include/linux/mlx5/mlx5_ifc.h | 5 ++- > 2 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c > b/drivers/net/ethernet/mellanox/mlx5/core/main.c > index 6b38ec72215a..bdc73370297b 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c > @@ -585,6 +585,39 @@ static int handle_hca_cap(struct mlx5_core_dev > *dev) > return err; > } > > +static int handle_hca_cap_roce(struct mlx5_core_dev *dev) > +{ > + int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in); > + void *set_hca_cap; > + void *set_ctx; > + int err; > + > + if (!MLX5_CAP_GEN(dev, roce)) > + return 0; > + > + err = mlx5_core_get_caps(dev, MLX5_CAP_ROCE); > + if (err) > + return err; > + > + if (MLX5_CAP_ROCE(dev, sw_r_roce_src_udp_port) || > + !MLX5_CAP_ROCE_MAX(dev, sw_r_roce_src_udp_port)) > + return 0; > + > + set_ctx = kzalloc(set_sz, GFP_KERNEL); > + if (!set_ctx) > + return -ENOMEM; > + all the sisters of this function allocate this and free it consecutively, why not allocate it from outside once, pass it to all handle_hca_cap_xyz functions, each one will memset it and reuse it. see below. > + set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx, > capability); > + memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ROCE], > + MLX5_ST_SZ_BYTES(roce_cap)); > + MLX5_SET(roce_cap, set_hca_cap, sw_r_roce_src_udp_port, 1); > + > + err = set_caps(dev, set_ctx, set_sz, > MLX5_SET_HCA_CAP_OP_MOD_ROCE); > + Do we really need to fail the whole driver if we just try to set a non mandatory cap ? > + kfree(set_ctx); > + return err; > +} > + > static int set_hca_cap(struct mlx5_core_dev *dev) > { > int err; let's allocate the set_ctx in this parent function and pass it to all hca cap handlers; set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in); set_ctx = kzalloc(set_sz, GFP_KERNEL); > @@ -607,6 +640,12 @@ static int set_hca_cap(struct mlx5_core_dev > *dev) > goto out; > } > > + err = handle_hca_cap_roce(dev); > + if (err) { > + mlx5_core_err(dev, "handle_hca_cap_roce failed\n"); > + goto out; > + } > + > out: > return err; > } > diff --git a/include/linux/mlx5/mlx5_ifc.h > b/include/linux/mlx5/mlx5_ifc.h > index 208bf1127be7..bb217c3f30da 100644 > --- a/include/linux/mlx5/mlx5_ifc.h > +++ b/include/linux/mlx5/mlx5_ifc.h > @@ -74,6 +74,7 @@ enum { > MLX5_SET_HCA_CAP_OP_MOD_GENERAL_DEVICE = 0x0, > MLX5_SET_HCA_CAP_OP_MOD_ODP = 0x2, > MLX5_SET_HCA_CAP_OP_MOD_ATOMIC = 0x3, > + MLX5_SET_HCA_CAP_OP_MOD_ROCE = 0x4, > }; > > enum { > @@ -902,7 +903,9 @@ struct > mlx5_ifc_per_protocol_networking_offload_caps_bits { > > struct mlx5_ifc_roce_cap_bits { > u8 roce_apm[0x1]; > - u8 reserved_at_1[0x1f]; > + u8 reserved_at_1[0x3]; > + u8 sw_r_roce_src_udp_port[0x1]; > + u8 reserved_at_5[0x1b]; > > u8 reserved_at_20[0x60]; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH mlx5-next 1/6] net/mlx5: Enable SW-defined RoCEv2 UDP source port 2020-03-18 23:33 ` Saeed Mahameed @ 2020-03-19 6:05 ` Leon Romanovsky 2020-03-20 1:16 ` Saeed Mahameed 0 siblings, 1 reply; 5+ messages in thread From: Leon Romanovsky @ 2020-03-19 6:05 UTC (permalink / raw) To: Saeed Mahameed Cc: Jason Gunthorpe, dledford, Mark Zhang, Maor Gottlieb, netdev, linux-rdma On Wed, Mar 18, 2020 at 11:33:46PM +0000, Saeed Mahameed wrote: > On Wed, 2020-03-18 at 11:52 +0200, Leon Romanovsky wrote: > > From: Mark Zhang <markz@mellanox.com> > > > > When this is enabled, UDP source port for RoCEv2 packets are defined > > by software instead of firmware. > > > > Signed-off-by: Mark Zhang <markz@mellanox.com> > > Reviewed-by: Maor Gottlieb <maorg@mellanox.com> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > > --- > > .../net/ethernet/mellanox/mlx5/core/main.c | 39 > > +++++++++++++++++++ > > include/linux/mlx5/mlx5_ifc.h | 5 ++- > > 2 files changed, 43 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c > > b/drivers/net/ethernet/mellanox/mlx5/core/main.c > > index 6b38ec72215a..bdc73370297b 100644 > > --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c > > @@ -585,6 +585,39 @@ static int handle_hca_cap(struct mlx5_core_dev > > *dev) > > return err; > > } > > > > +static int handle_hca_cap_roce(struct mlx5_core_dev *dev) > > +{ > > + int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in); > > + void *set_hca_cap; > > + void *set_ctx; > > + int err; > > + > > + if (!MLX5_CAP_GEN(dev, roce)) > > + return 0; > > + > > + err = mlx5_core_get_caps(dev, MLX5_CAP_ROCE); > > + if (err) > > + return err; > > + > > + if (MLX5_CAP_ROCE(dev, sw_r_roce_src_udp_port) || > > + !MLX5_CAP_ROCE_MAX(dev, sw_r_roce_src_udp_port)) > > + return 0; > > + > > + set_ctx = kzalloc(set_sz, GFP_KERNEL); > > + if (!set_ctx) > > + return -ENOMEM; > > + > > all the sisters of this function allocate this and free it > consecutively, why not allocate it from outside once, pass it to all > handle_hca_cap_xyz functions, each one will memset it and reuse it. > see below. Agree, I'll do it. > > > + set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx, > > capability); > > + memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ROCE], > > + MLX5_ST_SZ_BYTES(roce_cap)); > > + MLX5_SET(roce_cap, set_hca_cap, sw_r_roce_src_udp_port, 1); > > + > > + err = set_caps(dev, set_ctx, set_sz, > > MLX5_SET_HCA_CAP_OP_MOD_ROCE); > > + > > Do we really need to fail the whole driver if we just try to set a non > mandatory cap ? It is less important what caused to failure, but the fact that basic mlx5_cmd_exec() failed during initialization flow. I think that it is bad enough to stop the driver, because its operation is going to be unreliable. Please share your end-result decision on that and I'll align to it. > > > + kfree(set_ctx); > > + return err; > > +} > > + > > static int set_hca_cap(struct mlx5_core_dev *dev) > > { > > int err; > > let's allocate the set_ctx in this parent function and pass it to all > hca cap handlers; > > set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in); > set_ctx = kzalloc(set_sz, GFP_KERNEL); I'm doing it now. Thanks ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH mlx5-next 1/6] net/mlx5: Enable SW-defined RoCEv2 UDP source port 2020-03-19 6:05 ` Leon Romanovsky @ 2020-03-20 1:16 ` Saeed Mahameed 0 siblings, 0 replies; 5+ messages in thread From: Saeed Mahameed @ 2020-03-20 1:16 UTC (permalink / raw) To: leon Cc: Jason Gunthorpe, Mark Zhang, Maor Gottlieb, netdev, linux-rdma, dledford On Thu, 2020-03-19 at 08:05 +0200, Leon Romanovsky wrote: > On Wed, Mar 18, 2020 at 11:33:46PM +0000, Saeed Mahameed wrote: > > On Wed, 2020-03-18 at 11:52 +0200, Leon Romanovsky wrote: > > > From: Mark Zhang <markz@mellanox.com> > > > > > > When this is enabled, UDP source port for RoCEv2 packets are > > > defined > > > by software instead of firmware. > > > > > > Signed-off-by: Mark Zhang <markz@mellanox.com> > > > Reviewed-by: Maor Gottlieb <maorg@mellanox.com> > > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > > > --- > > > .../net/ethernet/mellanox/mlx5/core/main.c | 39 > > > +++++++++++++++++++ > > > include/linux/mlx5/mlx5_ifc.h | 5 ++- > > > 2 files changed, 43 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c > > > b/drivers/net/ethernet/mellanox/mlx5/core/main.c > > > index 6b38ec72215a..bdc73370297b 100644 > > > --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c > > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c > > > @@ -585,6 +585,39 @@ static int handle_hca_cap(struct > > > mlx5_core_dev > > > *dev) > > > return err; > > > } > > > > > > +static int handle_hca_cap_roce(struct mlx5_core_dev *dev) > > > +{ > > > + int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in); > > > + void *set_hca_cap; > > > + void *set_ctx; > > > + int err; > > > + > > > + if (!MLX5_CAP_GEN(dev, roce)) > > > + return 0; > > > + > > > + err = mlx5_core_get_caps(dev, MLX5_CAP_ROCE); > > > + if (err) > > > + return err; > > > + > > > + if (MLX5_CAP_ROCE(dev, sw_r_roce_src_udp_port) || > > > + !MLX5_CAP_ROCE_MAX(dev, sw_r_roce_src_udp_port)) > > > + return 0; > > > + > > > + set_ctx = kzalloc(set_sz, GFP_KERNEL); > > > + if (!set_ctx) > > > + return -ENOMEM; > > > + > > > > all the sisters of this function allocate this and free it > > consecutively, why not allocate it from outside once, pass it to > > all > > handle_hca_cap_xyz functions, each one will memset it and reuse it. > > see below. > > Agree, I'll do it. > > > > + set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx, > > > capability); > > > + memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ROCE], > > > + MLX5_ST_SZ_BYTES(roce_cap)); > > > + MLX5_SET(roce_cap, set_hca_cap, sw_r_roce_src_udp_port, 1); > > > + > > > + err = set_caps(dev, set_ctx, set_sz, > > > MLX5_SET_HCA_CAP_OP_MOD_ROCE); > > > + > > > > Do we really need to fail the whole driver if we just try to set a > > non > > mandatory cap ? > > It is less important what caused to failure, but the fact that basic > mlx5_cmd_exec() failed during initialization flow. I think that it > is bad enough to stop the driver, because its operation is going to > be unreliable. > > Please share your end-result decision on that and I'll align to it. > driver stability and reliability is not affected by this failing, since design-wise we don't count on setting the caps on this stage, we query them anyway in the next stages of the driver load. Many reason this could fail, old FW that doesn't handle this new CAP properly, new FW which has a bug only in the new feature flow. The driver should be resilient and provide basic functionality or in this case just drop this feature, since next cap query of this feature will return 0, and driver will not try to enable this feature anyway. if it is something really fundamental that caused the issue, then just let it be, if we fail in a more advanced mandatory stage then we will fail on that stage, if we didn't, then it is a win win. > > > + kfree(set_ctx); > > > + return err; > > > +} > > > + > > > static int set_hca_cap(struct mlx5_core_dev *dev) > > > { > > > int err; > > > > let's allocate the set_ctx in this parent function and pass it to > > all > > hca cap handlers; > > > > set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in); > > set_ctx = kzalloc(set_sz, GFP_KERNEL); > > I'm doing it now. > Awesome, Thanks ! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-20 1:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-18 9:52 [PATCH rdma-next 0/6] Set flow_label and RoCEv2 UDP source port for datagram QP Leon Romanovsky 2020-03-18 9:52 ` [PATCH mlx5-next 1/6] net/mlx5: Enable SW-defined RoCEv2 UDP source port Leon Romanovsky 2020-03-18 23:33 ` Saeed Mahameed 2020-03-19 6:05 ` Leon Romanovsky 2020-03-20 1:16 ` Saeed Mahameed
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).