* [PATCH rdma-next v1 0/3] Fix in-kernel active_speed type
@ 2020-09-02 7:45 Leon Romanovsky
2020-09-02 7:45 ` [PATCH mlx5-next v1 1/3] net/mlx5: Refactor query port speed functions Leon Romanovsky
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Leon Romanovsky @ 2020-09-02 7:45 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, Achiad Shochat, Adit Ranadive, Aharon Landau,
Ariel Elior, Dennis Dalessandro, Devesh Sharma, Jakub Kicinski,
linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn,
Naresh Kumar PBS, netdev, Saeed Mahameed, Selvin Xavier,
Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers
From: Leon Romanovsky <leonro@nvidia.com>
Changelog:
v1:
* Changed patch #1 to fix memory corruption to help with bisect. No
change in series, because the added code is changed anyway in patch
#3.
v0:
* https://lore.kernel.org/linux-rdma/20200824105826.1093613-1-leon@kernel.org
----------------------------------------------------------------------------------------
IBTA declares speed as 16 bits, but kernel stores it in u8. This series
fixes in-kernel declaration while keeping external interface intact.
Thanks
Aharon Landau (3):
net/mlx5: Refactor query port speed functions
RDMA/mlx5: Delete duplicated mlx5_ptys_width enum
RDMA: Fix link active_speed size
.../infiniband/core/uverbs_std_types_device.c | 3 +-
drivers/infiniband/core/verbs.c | 2 +-
drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +-
drivers/infiniband/hw/hfi1/verbs.c | 2 +-
drivers/infiniband/hw/mlx5/main.c | 41 +++++++------------
drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +-
drivers/infiniband/hw/qedr/verbs.c | 2 +-
drivers/infiniband/hw/qib/qib.h | 6 +--
.../infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +-
.../mellanox/mlx5/core/ipoib/ethtool.c | 31 ++------------
.../net/ethernet/mellanox/mlx5/core/port.c | 23 ++---------
include/linux/mlx5/port.h | 15 +++++--
include/rdma/ib_verbs.h | 4 +-
13 files changed, 47 insertions(+), 88 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH mlx5-next v1 1/3] net/mlx5: Refactor query port speed functions 2020-09-02 7:45 [PATCH rdma-next v1 0/3] Fix in-kernel active_speed type Leon Romanovsky @ 2020-09-02 7:45 ` Leon Romanovsky 2020-09-02 7:45 ` [PATCH mlx5-next v1 2/3] RDMA/mlx5: Delete duplicated mlx5_ptys_width enum Leon Romanovsky 2020-09-02 7:45 ` [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size Leon Romanovsky 2 siblings, 0 replies; 11+ messages in thread From: Leon Romanovsky @ 2020-09-02 7:45 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Aharon Landau, Achiad Shochat, Jakub Kicinski, linux-rdma, Michael Guralnik, netdev, Saeed Mahameed From: Aharon Landau <aharonl@mellanox.com> The functions mlx5_query_port_link_width_oper and mlx5_query_port_ib_proto_oper are always called together, so combine them to a new function called mlx5_query_port_oper to avoid duplication. And while the mlx5i_get_port_settings is the same as mlx5_query_port_oper therefore let's remove it. According to the IB spec link_width_oper and ib_proto_oper should be u16 and not as written u8, so perform casting as a preparation to cross-RDMA patch which will fix that type for all drivers in the RDMA subsystem. Fixes: ada68c31ba9c ("net/mlx5: Introduce a new header file for physical port functions") Signed-off-by: Aharon Landau <aharonl@mellanox.com> Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com> --- drivers/infiniband/hw/mlx5/main.c | 27 ++++++++++--------- .../mellanox/mlx5/core/ipoib/ethtool.c | 23 +++------------- .../net/ethernet/mellanox/mlx5/core/port.c | 23 +++------------- include/linux/mlx5/port.h | 7 +++-- 4 files changed, 25 insertions(+), 55 deletions(-) diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c index bfa8b6b3c681..ca33ff4b1d5e 100644 --- a/drivers/infiniband/hw/mlx5/main.c +++ b/drivers/infiniband/hw/mlx5/main.c @@ -326,8 +326,8 @@ void mlx5_ib_put_native_port_mdev(struct mlx5_ib_dev *ibdev, u8 port_num) spin_unlock(&port->mp.mpi_lock); } -static int translate_eth_legacy_proto_oper(u32 eth_proto_oper, u8 *active_speed, - u8 *active_width) +static int translate_eth_legacy_proto_oper(u32 eth_proto_oper, + u16 *active_speed, u8 *active_width) { switch (eth_proto_oper) { case MLX5E_PROT_MASK(MLX5E_1000BASE_CX_SGMII): @@ -384,7 +384,7 @@ static int translate_eth_legacy_proto_oper(u32 eth_proto_oper, u8 *active_speed, return 0; } -static int translate_eth_ext_proto_oper(u32 eth_proto_oper, u8 *active_speed, +static int translate_eth_ext_proto_oper(u32 eth_proto_oper, u16 *active_speed, u8 *active_width) { switch (eth_proto_oper) { @@ -436,7 +436,7 @@ static int translate_eth_ext_proto_oper(u32 eth_proto_oper, u8 *active_speed, return 0; } -static int translate_eth_proto_oper(u32 eth_proto_oper, u8 *active_speed, +static int translate_eth_proto_oper(u32 eth_proto_oper, u16 *active_speed, u8 *active_width, bool ext) { return ext ? @@ -457,6 +457,7 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num, bool put_mdev = true; u16 qkey_viol_cntr; u32 eth_prot_oper; + u16 active_speed; u8 mdev_port_num; bool ext; int err; @@ -490,9 +491,12 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num, props->active_width = IB_WIDTH_4X; props->active_speed = IB_SPEED_QDR; - translate_eth_proto_oper(eth_prot_oper, &props->active_speed, + translate_eth_proto_oper(eth_prot_oper, &active_speed, &props->active_width, ext); + WARN_ON_ONCE(active_speed & ~0xFF); + props->active_speed = (u8)active_speed; + props->port_cap_flags |= IB_PORT_CM_SUP; props->ip_gids = true; @@ -1183,8 +1187,8 @@ enum mlx5_ib_width { MLX5_IB_WIDTH_12X = 1 << 4 }; -static void translate_active_width(struct ib_device *ibdev, u8 active_width, - u8 *ib_width) +static void translate_active_width(struct ib_device *ibdev, u16 active_width, + u8 *ib_width) { struct mlx5_ib_dev *dev = to_mdev(ibdev); @@ -1277,7 +1281,7 @@ static int mlx5_query_hca_port(struct ib_device *ibdev, u8 port, u16 max_mtu; u16 oper_mtu; int err; - u8 ib_link_width_oper; + u16 ib_link_width_oper; u8 vl_hw_cap; rep = kzalloc(sizeof(*rep), GFP_KERNEL); @@ -1310,16 +1314,13 @@ static int mlx5_query_hca_port(struct ib_device *ibdev, u8 port, if (props->port_cap_flags & IB_PORT_CAP_MASK2_SUP) props->port_cap_flags2 = rep->cap_mask2; - err = mlx5_query_port_link_width_oper(mdev, &ib_link_width_oper, port); + err = mlx5_query_ib_port_oper(mdev, &ib_link_width_oper, + (u16 *)&props->active_speed, port); if (err) goto out; translate_active_width(ibdev, ib_link_width_oper, &props->active_width); - err = mlx5_query_port_ib_proto_oper(mdev, &props->active_speed, port); - if (err) - goto out; - mlx5_query_port_max_mtu(mdev, &max_mtu, port); props->max_mtu = mlx5_mtu_to_ib_mtu(max_mtu); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c index 1eef66ee849e..17f5be801d2f 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c @@ -174,24 +174,6 @@ static inline int mlx5_ptys_rate_enum_to_int(enum mlx5_ptys_rate rate) } } -static int mlx5i_get_port_settings(struct net_device *netdev, - u16 *ib_link_width_oper, u16 *ib_proto_oper) -{ - struct mlx5e_priv *priv = mlx5i_epriv(netdev); - struct mlx5_core_dev *mdev = priv->mdev; - u32 out[MLX5_ST_SZ_DW(ptys_reg)] = {0}; - int ret; - - ret = mlx5_query_port_ptys(mdev, out, sizeof(out), MLX5_PTYS_IB, 1); - if (ret) - return ret; - - *ib_link_width_oper = MLX5_GET(ptys_reg, out, ib_link_width_oper); - *ib_proto_oper = MLX5_GET(ptys_reg, out, ib_proto_oper); - - return 0; -} - static int mlx5i_get_speed_settings(u16 ib_link_width_oper, u16 ib_proto_oper) { int rate, width; @@ -209,11 +191,14 @@ static int mlx5i_get_speed_settings(u16 ib_link_width_oper, u16 ib_proto_oper) static int mlx5i_get_link_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *link_ksettings) { + struct mlx5e_priv *priv = mlx5i_epriv(netdev); + struct mlx5_core_dev *mdev = priv->mdev; u16 ib_link_width_oper; u16 ib_proto_oper; int speed, ret; - ret = mlx5i_get_port_settings(netdev, &ib_link_width_oper, &ib_proto_oper); + ret = mlx5_query_ib_port_oper(mdev, &ib_link_width_oper, &ib_proto_oper, + 1); if (ret) return ret; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/port.c b/drivers/net/ethernet/mellanox/mlx5/core/port.c index e4186e84b3ff..4bb219565c58 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/port.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/port.c @@ -154,24 +154,8 @@ int mlx5_set_port_beacon(struct mlx5_core_dev *dev, u16 beacon_duration) sizeof(out), MLX5_REG_MLCR, 0, 1); } -int mlx5_query_port_link_width_oper(struct mlx5_core_dev *dev, - u8 *link_width_oper, u8 local_port) -{ - u32 out[MLX5_ST_SZ_DW(ptys_reg)]; - int err; - - err = mlx5_query_port_ptys(dev, out, sizeof(out), MLX5_PTYS_IB, local_port); - if (err) - return err; - - *link_width_oper = MLX5_GET(ptys_reg, out, ib_link_width_oper); - - return 0; -} -EXPORT_SYMBOL_GPL(mlx5_query_port_link_width_oper); - -int mlx5_query_port_ib_proto_oper(struct mlx5_core_dev *dev, - u8 *proto_oper, u8 local_port) +int mlx5_query_ib_port_oper(struct mlx5_core_dev *dev, u16 *link_width_oper, + u16 *proto_oper, u8 local_port) { u32 out[MLX5_ST_SZ_DW(ptys_reg)]; int err; @@ -181,11 +165,12 @@ int mlx5_query_port_ib_proto_oper(struct mlx5_core_dev *dev, if (err) return err; + *link_width_oper = MLX5_GET(ptys_reg, out, ib_link_width_oper); *proto_oper = MLX5_GET(ptys_reg, out, ib_proto_oper); return 0; } -EXPORT_SYMBOL(mlx5_query_port_ib_proto_oper); +EXPORT_SYMBOL(mlx5_query_ib_port_oper); /* This function should be used after setting a port register only */ void mlx5_toggle_port_link(struct mlx5_core_dev *dev) diff --git a/include/linux/mlx5/port.h b/include/linux/mlx5/port.h index 2d45a6af52a4..4d33ae0c2d97 100644 --- a/include/linux/mlx5/port.h +++ b/include/linux/mlx5/port.h @@ -133,10 +133,9 @@ enum mlx5e_connector_type { int mlx5_set_port_caps(struct mlx5_core_dev *dev, u8 port_num, u32 caps); int mlx5_query_port_ptys(struct mlx5_core_dev *dev, u32 *ptys, int ptys_size, int proto_mask, u8 local_port); -int mlx5_query_port_link_width_oper(struct mlx5_core_dev *dev, - u8 *link_width_oper, u8 local_port); -int mlx5_query_port_ib_proto_oper(struct mlx5_core_dev *dev, - u8 *proto_oper, u8 local_port); + +int mlx5_query_ib_port_oper(struct mlx5_core_dev *dev, u16 *link_width_oper, + u16 *proto_oper, u8 local_port); void mlx5_toggle_port_link(struct mlx5_core_dev *dev); int mlx5_set_port_admin_status(struct mlx5_core_dev *dev, enum mlx5_port_status status); -- 2.26.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH mlx5-next v1 2/3] RDMA/mlx5: Delete duplicated mlx5_ptys_width enum 2020-09-02 7:45 [PATCH rdma-next v1 0/3] Fix in-kernel active_speed type Leon Romanovsky 2020-09-02 7:45 ` [PATCH mlx5-next v1 1/3] net/mlx5: Refactor query port speed functions Leon Romanovsky @ 2020-09-02 7:45 ` Leon Romanovsky 2020-09-02 7:45 ` [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size Leon Romanovsky 2 siblings, 0 replies; 11+ messages in thread From: Leon Romanovsky @ 2020-09-02 7:45 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Aharon Landau, Jakub Kicinski, linux-rdma, Michael Guralnik, netdev, Saeed Mahameed From: Aharon Landau <aharonl@mellanox.com> Combine two same enums to avoid duplication. Signed-off-by: Aharon Landau <aharonl@mellanox.com> Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com> --- drivers/infiniband/hw/mlx5/main.c | 20 ++++++------------- .../mellanox/mlx5/core/ipoib/ethtool.c | 8 -------- include/linux/mlx5/port.h | 8 ++++++++ 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c index ca33ff4b1d5e..545f23d27660 100644 --- a/drivers/infiniband/hw/mlx5/main.c +++ b/drivers/infiniband/hw/mlx5/main.c @@ -1179,32 +1179,24 @@ static int mlx5_ib_query_device(struct ib_device *ibdev, return 0; } -enum mlx5_ib_width { - MLX5_IB_WIDTH_1X = 1 << 0, - MLX5_IB_WIDTH_2X = 1 << 1, - MLX5_IB_WIDTH_4X = 1 << 2, - MLX5_IB_WIDTH_8X = 1 << 3, - MLX5_IB_WIDTH_12X = 1 << 4 -}; - static void translate_active_width(struct ib_device *ibdev, u16 active_width, u8 *ib_width) { struct mlx5_ib_dev *dev = to_mdev(ibdev); - if (active_width & MLX5_IB_WIDTH_1X) + if (active_width & MLX5_PTYS_WIDTH_1X) *ib_width = IB_WIDTH_1X; - else if (active_width & MLX5_IB_WIDTH_2X) + else if (active_width & MLX5_PTYS_WIDTH_2X) *ib_width = IB_WIDTH_2X; - else if (active_width & MLX5_IB_WIDTH_4X) + else if (active_width & MLX5_PTYS_WIDTH_4X) *ib_width = IB_WIDTH_4X; - else if (active_width & MLX5_IB_WIDTH_8X) + else if (active_width & MLX5_PTYS_WIDTH_8X) *ib_width = IB_WIDTH_8X; - else if (active_width & MLX5_IB_WIDTH_12X) + else if (active_width & MLX5_PTYS_WIDTH_12X) *ib_width = IB_WIDTH_12X; else { mlx5_ib_dbg(dev, "Invalid active_width %d, setting width to default value: 4x\n", - (int)active_width); + active_width); *ib_width = IB_WIDTH_4X; } diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c index 17f5be801d2f..cac8f085b16d 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c @@ -130,14 +130,6 @@ static int mlx5i_flash_device(struct net_device *netdev, return mlx5e_ethtool_flash_device(priv, flash); } -enum mlx5_ptys_width { - MLX5_PTYS_WIDTH_1X = 1 << 0, - MLX5_PTYS_WIDTH_2X = 1 << 1, - MLX5_PTYS_WIDTH_4X = 1 << 2, - MLX5_PTYS_WIDTH_8X = 1 << 3, - MLX5_PTYS_WIDTH_12X = 1 << 4, -}; - static inline int mlx5_ptys_width_enum_to_int(enum mlx5_ptys_width width) { switch (width) { diff --git a/include/linux/mlx5/port.h b/include/linux/mlx5/port.h index 4d33ae0c2d97..23edd2db4803 100644 --- a/include/linux/mlx5/port.h +++ b/include/linux/mlx5/port.h @@ -125,6 +125,14 @@ enum mlx5e_connector_type { MLX5E_CONNECTOR_TYPE_NUMBER, }; +enum mlx5_ptys_width { + MLX5_PTYS_WIDTH_1X = 1 << 0, + MLX5_PTYS_WIDTH_2X = 1 << 1, + MLX5_PTYS_WIDTH_4X = 1 << 2, + MLX5_PTYS_WIDTH_8X = 1 << 3, + MLX5_PTYS_WIDTH_12X = 1 << 4, +}; + #define MLX5E_PROT_MASK(link_mode) (1 << link_mode) #define MLX5_GET_ETH_PROTO(reg, out, ext, field) \ (ext ? MLX5_GET(reg, out, ext_##field) : \ -- 2.26.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-02 7:45 [PATCH rdma-next v1 0/3] Fix in-kernel active_speed type Leon Romanovsky 2020-09-02 7:45 ` [PATCH mlx5-next v1 1/3] net/mlx5: Refactor query port speed functions Leon Romanovsky 2020-09-02 7:45 ` [PATCH mlx5-next v1 2/3] RDMA/mlx5: Delete duplicated mlx5_ptys_width enum Leon Romanovsky @ 2020-09-02 7:45 ` Leon Romanovsky 2020-09-08 14:19 ` Jason Gunthorpe 2 siblings, 1 reply; 11+ messages in thread From: Leon Romanovsky @ 2020-09-02 7:45 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers From: Aharon Landau <aharonl@mellanox.com> According to the IB spec active_speed size should be u16 and not u8 as before. Changing it to allow further extensions in offered speeds. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Aharon Landau <aharonl@mellanox.com> Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com> --- drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- drivers/infiniband/core/verbs.c | 2 +- drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- drivers/infiniband/hw/hfi1/verbs.c | 2 +- drivers/infiniband/hw/mlx5/main.c | 8 ++------ drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- drivers/infiniband/hw/qedr/verbs.c | 2 +- drivers/infiniband/hw/qib/qib.h | 6 +++--- drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- include/rdma/ib_verbs.h | 4 ++-- 10 files changed, 15 insertions(+), 18 deletions(-) diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c index 75df2094a010..7b03446b6936 100644 --- a/drivers/infiniband/core/uverbs_std_types_device.c +++ b/drivers/infiniband/core/uverbs_std_types_device.c @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, resp->subnet_timeout = attr->subnet_timeout; resp->init_type_reply = attr->init_type_reply; resp->active_width = attr->active_width; - resp->active_speed = attr->active_speed; + WARN_ON(attr->active_speed & ~0xFF); + resp->active_speed = (u8)attr->active_speed; resp->phys_state = attr->phys_state; resp->link_layer = rdma_port_get_link_layer(ib_dev, port_num); } diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index c0b9bf0563bc..b9990297e4b0 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -1802,7 +1802,7 @@ int ib_modify_qp_with_udata(struct ib_qp *ib_qp, struct ib_qp_attr *attr, } EXPORT_SYMBOL(ib_modify_qp_with_udata); -int ib_get_eth_speed(struct ib_device *dev, u8 port_num, u8 *speed, u8 *width) +int ib_get_eth_speed(struct ib_device *dev, u8 port_num, u16 *speed, u8 *width) { int rc; u32 netdev_speed; diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h index a300588634c5..b930ea3dab7a 100644 --- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h @@ -150,7 +150,7 @@ struct bnxt_re_dev { struct delayed_work worker; u8 cur_prio_map; - u8 active_speed; + u16 active_speed; u8 active_width; /* FP Notification Queue (CQ & SRQ) */ diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c index 30865635b449..3591923abebb 100644 --- a/drivers/infiniband/hw/hfi1/verbs.c +++ b/drivers/infiniband/hw/hfi1/verbs.c @@ -1424,7 +1424,7 @@ static int query_port(struct rvt_dev_info *rdi, u8 port_num, props->gid_tbl_len = HFI1_GUIDS_PER_PORT; props->active_width = (u8)opa_width_to_ib(ppd->link_width_active); /* see rate_show() in ib core/sysfs.c */ - props->active_speed = (u8)opa_speed_to_ib(ppd->link_speed_active); + props->active_speed = opa_speed_to_ib(ppd->link_speed_active); props->max_vl_num = ppd->vls_supported; /* Once we are a "first class" citizen and have added the OPA MTUs to diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c index 545f23d27660..790b874fffe2 100644 --- a/drivers/infiniband/hw/mlx5/main.c +++ b/drivers/infiniband/hw/mlx5/main.c @@ -457,7 +457,6 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num, bool put_mdev = true; u16 qkey_viol_cntr; u32 eth_prot_oper; - u16 active_speed; u8 mdev_port_num; bool ext; int err; @@ -491,12 +490,9 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num, props->active_width = IB_WIDTH_4X; props->active_speed = IB_SPEED_QDR; - translate_eth_proto_oper(eth_prot_oper, &active_speed, + translate_eth_proto_oper(eth_prot_oper, &props->active_speed, &props->active_width, ext); - WARN_ON_ONCE(active_speed & ~0xFF); - props->active_speed = (u8)active_speed; - props->port_cap_flags |= IB_PORT_CM_SUP; props->ip_gids = true; @@ -1307,7 +1303,7 @@ static int mlx5_query_hca_port(struct ib_device *ibdev, u8 port, props->port_cap_flags2 = rep->cap_mask2; err = mlx5_query_ib_port_oper(mdev, &ib_link_width_oper, - (u16 *)&props->active_speed, port); + &props->active_speed, port); if (err) goto out; diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c index b24437619412..6ea3ceb1958d 100644 --- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c +++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c @@ -112,7 +112,7 @@ int ocrdma_query_device(struct ib_device *ibdev, struct ib_device_attr *attr, } static inline void get_link_speed_and_width(struct ocrdma_dev *dev, - u8 *ib_speed, u8 *ib_width) + u16 *ib_speed, u8 *ib_width) { int status; u8 speed; diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c index 2f28d4ef3c05..fd0bbee24ce0 100644 --- a/drivers/infiniband/hw/qedr/verbs.c +++ b/drivers/infiniband/hw/qedr/verbs.c @@ -163,7 +163,7 @@ int qedr_query_device(struct ib_device *ibdev, return 0; } -static inline void get_link_speed_and_width(int speed, u8 *ib_speed, +static inline void get_link_speed_and_width(int speed, u16 *ib_speed, u8 *ib_width) { switch (speed) { diff --git a/drivers/infiniband/hw/qib/qib.h b/drivers/infiniband/hw/qib/qib.h index 432d6d0fd7f4..ee211423058a 100644 --- a/drivers/infiniband/hw/qib/qib.h +++ b/drivers/infiniband/hw/qib/qib.h @@ -619,11 +619,11 @@ struct qib_pportdata { /* LID mask control */ u8 lmc; u8 link_width_supported; - u8 link_speed_supported; + u16 link_speed_supported; u8 link_width_enabled; - u8 link_speed_enabled; + u16 link_speed_enabled; u8 link_width_active; - u8 link_speed_active; + u16 link_speed_active; u8 vls_supported; u8 vls_operational; /* Rx Polarity inversion (compensate for ~tx on partner) */ diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h index 97ed8f952f6e..f0e5ffba2d51 100644 --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h @@ -176,7 +176,7 @@ struct pvrdma_port_attr { u8 subnet_timeout; u8 init_type_reply; u8 active_width; - u8 active_speed; + u16 active_speed; u8 phys_state; u8 reserved[2]; }; diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 73fb9f2455d9..a596f1657c44 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -670,7 +670,7 @@ struct ib_port_attr { u8 subnet_timeout; u8 init_type_reply; u8 active_width; - u8 active_speed; + u16 active_speed; u8 phys_state; u16 port_cap_flags2; }; @@ -4359,7 +4359,7 @@ void ib_drain_rq(struct ib_qp *qp); void ib_drain_sq(struct ib_qp *qp); void ib_drain_qp(struct ib_qp *qp); -int ib_get_eth_speed(struct ib_device *dev, u8 port_num, u8 *speed, u8 *width); +int ib_get_eth_speed(struct ib_device *dev, u8 port_num, u16 *speed, u8 *width); static inline u8 *rdma_ah_retrieve_dmac(struct rdma_ah_attr *attr) { -- 2.26.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-02 7:45 ` [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size Leon Romanovsky @ 2020-09-08 14:19 ` Jason Gunthorpe 2020-09-08 14:26 ` Leon Romanovsky 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-09-08 14:19 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers On Wed, Sep 02, 2020 at 10:45:03AM +0300, Leon Romanovsky wrote: > From: Aharon Landau <aharonl@mellanox.com> > > According to the IB spec active_speed size should be u16 and not u8 as > before. Changing it to allow further extensions in offered speeds. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Aharon Landau <aharonl@mellanox.com> > Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- > drivers/infiniband/core/verbs.c | 2 +- > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- > drivers/infiniband/hw/hfi1/verbs.c | 2 +- > drivers/infiniband/hw/mlx5/main.c | 8 ++------ > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > drivers/infiniband/hw/qedr/verbs.c | 2 +- > drivers/infiniband/hw/qib/qib.h | 6 +++--- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- > include/rdma/ib_verbs.h | 4 ++-- > 10 files changed, 15 insertions(+), 18 deletions(-) > > diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c > index 75df2094a010..7b03446b6936 100644 > +++ b/drivers/infiniband/core/uverbs_std_types_device.c > @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, > resp->subnet_timeout = attr->subnet_timeout; > resp->init_type_reply = attr->init_type_reply; > resp->active_width = attr->active_width; > - resp->active_speed = attr->active_speed; > + WARN_ON(attr->active_speed & ~0xFF); ?? This doesn't seem like a warn on situation.. > @@ -1307,7 +1303,7 @@ static int mlx5_query_hca_port(struct ib_device *ibdev, u8 port, > props->port_cap_flags2 = rep->cap_mask2; > > err = mlx5_query_ib_port_oper(mdev, &ib_link_width_oper, > - (u16 *)&props->active_speed, port); > + &props->active_speed, port); This hunk should be in the earlier patch Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-08 14:19 ` Jason Gunthorpe @ 2020-09-08 14:26 ` Leon Romanovsky 2020-09-08 14:27 ` Jason Gunthorpe 0 siblings, 1 reply; 11+ messages in thread From: Leon Romanovsky @ 2020-09-08 14:26 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers On Tue, Sep 08, 2020 at 11:19:24AM -0300, Jason Gunthorpe wrote: > On Wed, Sep 02, 2020 at 10:45:03AM +0300, Leon Romanovsky wrote: > > From: Aharon Landau <aharonl@mellanox.com> > > > > According to the IB spec active_speed size should be u16 and not u8 as > > before. Changing it to allow further extensions in offered speeds. > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Signed-off-by: Aharon Landau <aharonl@mellanox.com> > > Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- > > drivers/infiniband/core/verbs.c | 2 +- > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- > > drivers/infiniband/hw/hfi1/verbs.c | 2 +- > > drivers/infiniband/hw/mlx5/main.c | 8 ++------ > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > drivers/infiniband/hw/qib/qib.h | 6 +++--- > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- > > include/rdma/ib_verbs.h | 4 ++-- > > 10 files changed, 15 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c > > index 75df2094a010..7b03446b6936 100644 > > +++ b/drivers/infiniband/core/uverbs_std_types_device.c > > @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, > > resp->subnet_timeout = attr->subnet_timeout; > > resp->init_type_reply = attr->init_type_reply; > > resp->active_width = attr->active_width; > > - resp->active_speed = attr->active_speed; > > + WARN_ON(attr->active_speed & ~0xFF); > > ?? This doesn't seem like a warn on situation.. Why? We are returning u8 to the user, so need to catch overflow. > > > @@ -1307,7 +1303,7 @@ static int mlx5_query_hca_port(struct ib_device *ibdev, u8 port, > > props->port_cap_flags2 = rep->cap_mask2; > > > > err = mlx5_query_ib_port_oper(mdev, &ib_link_width_oper, > > - (u16 *)&props->active_speed, port); > > + &props->active_speed, port); > > This hunk should be in the earlier patch Sorry about that. > > Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-08 14:26 ` Leon Romanovsky @ 2020-09-08 14:27 ` Jason Gunthorpe 2020-09-08 14:42 ` Leon Romanovsky 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-09-08 14:27 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers On Tue, Sep 08, 2020 at 05:26:51PM +0300, Leon Romanovsky wrote: > On Tue, Sep 08, 2020 at 11:19:24AM -0300, Jason Gunthorpe wrote: > > On Wed, Sep 02, 2020 at 10:45:03AM +0300, Leon Romanovsky wrote: > > > From: Aharon Landau <aharonl@mellanox.com> > > > > > > According to the IB spec active_speed size should be u16 and not u8 as > > > before. Changing it to allow further extensions in offered speeds. > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > Signed-off-by: Aharon Landau <aharonl@mellanox.com> > > > Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > > drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- > > > drivers/infiniband/core/verbs.c | 2 +- > > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- > > > drivers/infiniband/hw/hfi1/verbs.c | 2 +- > > > drivers/infiniband/hw/mlx5/main.c | 8 ++------ > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > > drivers/infiniband/hw/qib/qib.h | 6 +++--- > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- > > > include/rdma/ib_verbs.h | 4 ++-- > > > 10 files changed, 15 insertions(+), 18 deletions(-) > > > > > > diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c > > > index 75df2094a010..7b03446b6936 100644 > > > +++ b/drivers/infiniband/core/uverbs_std_types_device.c > > > @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, > > > resp->subnet_timeout = attr->subnet_timeout; > > > resp->init_type_reply = attr->init_type_reply; > > > resp->active_width = attr->active_width; > > > - resp->active_speed = attr->active_speed; > > > + WARN_ON(attr->active_speed & ~0xFF); > > > > ?? This doesn't seem like a warn on situation.. > > Why? We are returning u8 to the user, so need to catch overflow. We need to have actual backwards compat here, not just throw a warning at the syscall boundary Why can't it just be truncated? Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-08 14:27 ` Jason Gunthorpe @ 2020-09-08 14:42 ` Leon Romanovsky 2020-09-08 14:46 ` Jason Gunthorpe 0 siblings, 1 reply; 11+ messages in thread From: Leon Romanovsky @ 2020-09-08 14:42 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers On Tue, Sep 08, 2020 at 11:27:56AM -0300, Jason Gunthorpe wrote: > On Tue, Sep 08, 2020 at 05:26:51PM +0300, Leon Romanovsky wrote: > > On Tue, Sep 08, 2020 at 11:19:24AM -0300, Jason Gunthorpe wrote: > > > On Wed, Sep 02, 2020 at 10:45:03AM +0300, Leon Romanovsky wrote: > > > > From: Aharon Landau <aharonl@mellanox.com> > > > > > > > > According to the IB spec active_speed size should be u16 and not u8 as > > > > before. Changing it to allow further extensions in offered speeds. > > > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > > Signed-off-by: Aharon Landau <aharonl@mellanox.com> > > > > Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> > > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > > > drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- > > > > drivers/infiniband/core/verbs.c | 2 +- > > > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- > > > > drivers/infiniband/hw/hfi1/verbs.c | 2 +- > > > > drivers/infiniband/hw/mlx5/main.c | 8 ++------ > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > > > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > > > drivers/infiniband/hw/qib/qib.h | 6 +++--- > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- > > > > include/rdma/ib_verbs.h | 4 ++-- > > > > 10 files changed, 15 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c > > > > index 75df2094a010..7b03446b6936 100644 > > > > +++ b/drivers/infiniband/core/uverbs_std_types_device.c > > > > @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, > > > > resp->subnet_timeout = attr->subnet_timeout; > > > > resp->init_type_reply = attr->init_type_reply; > > > > resp->active_width = attr->active_width; > > > > - resp->active_speed = attr->active_speed; > > > > + WARN_ON(attr->active_speed & ~0xFF); > > > > > > ?? This doesn't seem like a warn on situation.. > > > > Why? We are returning u8 to the user, so need to catch overflow. > > We need to have actual backwards compat here, not just throw a warning > at the syscall boundary We don't have fallback and don't have speed that crosses u8 limit yet. This WARN_ON() is needed to ensure that we properly extend ib_port_speed. > > Why can't it just be truncated? The coming IBTA will have extension in that area. > > Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-08 14:42 ` Leon Romanovsky @ 2020-09-08 14:46 ` Jason Gunthorpe 2020-09-08 15:04 ` Leon Romanovsky 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2020-09-08 14:46 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers On Tue, Sep 08, 2020 at 05:42:05PM +0300, Leon Romanovsky wrote: > On Tue, Sep 08, 2020 at 11:27:56AM -0300, Jason Gunthorpe wrote: > > On Tue, Sep 08, 2020 at 05:26:51PM +0300, Leon Romanovsky wrote: > > > On Tue, Sep 08, 2020 at 11:19:24AM -0300, Jason Gunthorpe wrote: > > > > On Wed, Sep 02, 2020 at 10:45:03AM +0300, Leon Romanovsky wrote: > > > > > From: Aharon Landau <aharonl@mellanox.com> > > > > > > > > > > According to the IB spec active_speed size should be u16 and not u8 as > > > > > before. Changing it to allow further extensions in offered speeds. > > > > > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > > > Signed-off-by: Aharon Landau <aharonl@mellanox.com> > > > > > Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> > > > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > > > > drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- > > > > > drivers/infiniband/core/verbs.c | 2 +- > > > > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- > > > > > drivers/infiniband/hw/hfi1/verbs.c | 2 +- > > > > > drivers/infiniband/hw/mlx5/main.c | 8 ++------ > > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > > > > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > > > > drivers/infiniband/hw/qib/qib.h | 6 +++--- > > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- > > > > > include/rdma/ib_verbs.h | 4 ++-- > > > > > 10 files changed, 15 insertions(+), 18 deletions(-) > > > > > > > > > > diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c > > > > > index 75df2094a010..7b03446b6936 100644 > > > > > +++ b/drivers/infiniband/core/uverbs_std_types_device.c > > > > > @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, > > > > > resp->subnet_timeout = attr->subnet_timeout; > > > > > resp->init_type_reply = attr->init_type_reply; > > > > > resp->active_width = attr->active_width; > > > > > - resp->active_speed = attr->active_speed; > > > > > + WARN_ON(attr->active_speed & ~0xFF); > > > > > > > > ?? This doesn't seem like a warn on situation.. > > > > > > Why? We are returning u8 to the user, so need to catch overflow. > > > > We need to have actual backwards compat here, not just throw a warning > > at the syscall boundary > > We don't have fallback and don't have speed that crosses u8 limit yet. > This WARN_ON() is needed to ensure that we properly extend > ib_port_speed. Until we have some compat story I don't want to just increase this value, it clearly renders the device unusable, so why do it? Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-08 14:46 ` Jason Gunthorpe @ 2020-09-08 15:04 ` Leon Romanovsky 2020-09-08 17:21 ` Jason Gunthorpe 0 siblings, 1 reply; 11+ messages in thread From: Leon Romanovsky @ 2020-09-08 15:04 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers On Tue, Sep 08, 2020 at 11:46:46AM -0300, Jason Gunthorpe wrote: > On Tue, Sep 08, 2020 at 05:42:05PM +0300, Leon Romanovsky wrote: > > On Tue, Sep 08, 2020 at 11:27:56AM -0300, Jason Gunthorpe wrote: > > > On Tue, Sep 08, 2020 at 05:26:51PM +0300, Leon Romanovsky wrote: > > > > On Tue, Sep 08, 2020 at 11:19:24AM -0300, Jason Gunthorpe wrote: > > > > > On Wed, Sep 02, 2020 at 10:45:03AM +0300, Leon Romanovsky wrote: > > > > > > From: Aharon Landau <aharonl@mellanox.com> > > > > > > > > > > > > According to the IB spec active_speed size should be u16 and not u8 as > > > > > > before. Changing it to allow further extensions in offered speeds. > > > > > > > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > > > > Signed-off-by: Aharon Landau <aharonl@mellanox.com> > > > > > > Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> > > > > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > > > > > drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- > > > > > > drivers/infiniband/core/verbs.c | 2 +- > > > > > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- > > > > > > drivers/infiniband/hw/hfi1/verbs.c | 2 +- > > > > > > drivers/infiniband/hw/mlx5/main.c | 8 ++------ > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > > > > > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > > > > > drivers/infiniband/hw/qib/qib.h | 6 +++--- > > > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- > > > > > > include/rdma/ib_verbs.h | 4 ++-- > > > > > > 10 files changed, 15 insertions(+), 18 deletions(-) > > > > > > > > > > > > diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c > > > > > > index 75df2094a010..7b03446b6936 100644 > > > > > > +++ b/drivers/infiniband/core/uverbs_std_types_device.c > > > > > > @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, > > > > > > resp->subnet_timeout = attr->subnet_timeout; > > > > > > resp->init_type_reply = attr->init_type_reply; > > > > > > resp->active_width = attr->active_width; > > > > > > - resp->active_speed = attr->active_speed; > > > > > > + WARN_ON(attr->active_speed & ~0xFF); > > > > > > > > > > ?? This doesn't seem like a warn on situation.. > > > > > > > > Why? We are returning u8 to the user, so need to catch overflow. > > > > > > We need to have actual backwards compat here, not just throw a warning > > > at the syscall boundary > > > > We don't have fallback and don't have speed that crosses u8 limit yet. > > This WARN_ON() is needed to ensure that we properly extend > > ib_port_speed. > > Until we have some compat story I don't want to just increase this > value, it clearly renders the device unusable, so why do it? Because IBTA port speed extension was already approved, it is just not published yet. I need to prepare code as much as possible. Thanks > > Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size 2020-09-08 15:04 ` Leon Romanovsky @ 2020-09-08 17:21 ` Jason Gunthorpe 0 siblings, 0 replies; 11+ messages in thread From: Jason Gunthorpe @ 2020-09-08 17:21 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Aharon Landau, Adit Ranadive, Ariel Elior, Dennis Dalessandro, Devesh Sharma, linux-rdma, Michael Guralnik, Michal Kalderon, Mike Marciniszyn, Naresh Kumar PBS, Selvin Xavier, Somnath Kotur, Sriharsha Basavapatna, VMware PV-Drivers On Tue, Sep 08, 2020 at 06:04:27PM +0300, Leon Romanovsky wrote: > On Tue, Sep 08, 2020 at 11:46:46AM -0300, Jason Gunthorpe wrote: > > On Tue, Sep 08, 2020 at 05:42:05PM +0300, Leon Romanovsky wrote: > > > On Tue, Sep 08, 2020 at 11:27:56AM -0300, Jason Gunthorpe wrote: > > > > On Tue, Sep 08, 2020 at 05:26:51PM +0300, Leon Romanovsky wrote: > > > > > On Tue, Sep 08, 2020 at 11:19:24AM -0300, Jason Gunthorpe wrote: > > > > > > On Wed, Sep 02, 2020 at 10:45:03AM +0300, Leon Romanovsky wrote: > > > > > > > From: Aharon Landau <aharonl@mellanox.com> > > > > > > > > > > > > > > According to the IB spec active_speed size should be u16 and not u8 as > > > > > > > before. Changing it to allow further extensions in offered speeds. > > > > > > > > > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > > > > > Signed-off-by: Aharon Landau <aharonl@mellanox.com> > > > > > > > Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> > > > > > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > > > > > > drivers/infiniband/core/uverbs_std_types_device.c | 3 ++- > > > > > > > drivers/infiniband/core/verbs.c | 2 +- > > > > > > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 +- > > > > > > > drivers/infiniband/hw/hfi1/verbs.c | 2 +- > > > > > > > drivers/infiniband/hw/mlx5/main.c | 8 ++------ > > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > > > > > > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > > > > > > drivers/infiniband/hw/qib/qib.h | 6 +++--- > > > > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- > > > > > > > include/rdma/ib_verbs.h | 4 ++-- > > > > > > > 10 files changed, 15 insertions(+), 18 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/infiniband/core/uverbs_std_types_device.c b/drivers/infiniband/core/uverbs_std_types_device.c > > > > > > > index 75df2094a010..7b03446b6936 100644 > > > > > > > +++ b/drivers/infiniband/core/uverbs_std_types_device.c > > > > > > > @@ -165,7 +165,8 @@ void copy_port_attr_to_resp(struct ib_port_attr *attr, > > > > > > > resp->subnet_timeout = attr->subnet_timeout; > > > > > > > resp->init_type_reply = attr->init_type_reply; > > > > > > > resp->active_width = attr->active_width; > > > > > > > - resp->active_speed = attr->active_speed; > > > > > > > + WARN_ON(attr->active_speed & ~0xFF); > > > > > > > > > > > > ?? This doesn't seem like a warn on situation.. > > > > > > > > > > Why? We are returning u8 to the user, so need to catch overflow. > > > > > > > > We need to have actual backwards compat here, not just throw a warning > > > > at the syscall boundary > > > > > > We don't have fallback and don't have speed that crosses u8 limit yet. > > > This WARN_ON() is needed to ensure that we properly extend > > > ib_port_speed. > > > > Until we have some compat story I don't want to just increase this > > value, it clearly renders the device unusable, so why do it? > > Because IBTA port speed extension was already approved, it is just > not published yet. So there should be a compat story. It can't just WARN_ON here. The u8 active_speed should saturate at the highest speed we can represent in the u8, and this will have to be updated somehow to pass the u16 version. Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-09-08 20:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-02 7:45 [PATCH rdma-next v1 0/3] Fix in-kernel active_speed type Leon Romanovsky 2020-09-02 7:45 ` [PATCH mlx5-next v1 1/3] net/mlx5: Refactor query port speed functions Leon Romanovsky 2020-09-02 7:45 ` [PATCH mlx5-next v1 2/3] RDMA/mlx5: Delete duplicated mlx5_ptys_width enum Leon Romanovsky 2020-09-02 7:45 ` [PATCH rdma-next v1 3/3] RDMA: Fix link active_speed size Leon Romanovsky 2020-09-08 14:19 ` Jason Gunthorpe 2020-09-08 14:26 ` Leon Romanovsky 2020-09-08 14:27 ` Jason Gunthorpe 2020-09-08 14:42 ` Leon Romanovsky 2020-09-08 14:46 ` Jason Gunthorpe 2020-09-08 15:04 ` Leon Romanovsky 2020-09-08 17:21 ` Jason Gunthorpe
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).