| Message ID | 1455954465-15141-6-git-send-email-leon@leon.nu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 20/02/2016 09:47, Leon Romanovsky wrote: > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c > @@ -1762,7 +1762,7 @@ int ipoib_add_pkey_attr(struct net_device *dev) > > int ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca) > { > - priv->hca_caps = hca->attrs.device_cap_flags; > + priv->hca_caps = (int)hca->attrs.device_cap_flags; Won't it be better to increase the size of priv->hca_caps? Someone in the future will probably attempt to check one of the new device caps through this field and will be surprised to see it is only partial. > > if (priv->hca_caps & IB_DEVICE_UD_IP_CSUM) { > priv->dev->hw_features = NETIF_F_SG | > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 284b00c..2ff1fd1 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -273,7 +273,7 @@ struct ib_device_attr { > u32 hw_ver; > int max_qp; > int max_qp_wr; > - int device_cap_flags; > + u64 device_cap_flags; > int max_sge; > int max_sge_rd; > int max_cq; > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 21, 2016 at 09:39:33AM +0200, Haggai Eran wrote: > On 20/02/2016 09:47, Leon Romanovsky wrote: > > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c > > @@ -1762,7 +1762,7 @@ int ipoib_add_pkey_attr(struct net_device *dev) > > > > int ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca) > > { > > - priv->hca_caps = hca->attrs.device_cap_flags; > > + priv->hca_caps = (int)hca->attrs.device_cap_flags; > > Won't it be better to increase the size of priv->hca_caps? Someone in the > future will probably attempt to check one of the new device caps through > this field and will be surprised to see it is only partial. My changes were intended to minimize the impact on the existing code base. The future developer will be expected to check his code when he adds new device capability above 32 bit limit. In this change, he will update the ipoib properly expose it. Current implementation doesn't change ipoib behavior and easy for review without extra knowledge of ipoib internals, so let's stay this change as is. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 21, 2016 at 10:00:53AM +0200, Leon Romanovsky wrote: > Current implementation doesn't change ipoib behavior and easy for > review without extra knowledge of ipoib internals, so let's stay this > change as is. It is one line to fix ipoib's size, and avoids this subtle future land mine. I'm with Haggi on this. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c index c647d67..6887252 100644 --- a/drivers/infiniband/core/uverbs_cmd.c +++ b/drivers/infiniband/core/uverbs_cmd.c @@ -402,7 +402,7 @@ static void copy_query_dev_fields(struct ib_uverbs_file *file, resp->hw_ver = attr->hw_ver; resp->max_qp = attr->max_qp; resp->max_qp_wr = attr->max_qp_wr; - resp->device_cap_flags = attr->device_cap_flags; + resp->device_cap_flags = lower_32_bits(attr->device_cap_flags); resp->max_sge = attr->max_sge; resp->max_sge_rd = attr->max_sge_rd; resp->max_cq = attr->max_cq; diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 25509bb..6d53330 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -1762,7 +1762,7 @@ int ipoib_add_pkey_attr(struct net_device *dev) int ipoib_set_dev_features(struct ipoib_dev_priv *priv, struct ib_device *hca) { - priv->hca_caps = hca->attrs.device_cap_flags; + priv->hca_caps = (int)hca->attrs.device_cap_flags; if (priv->hca_caps & IB_DEVICE_UD_IP_CSUM) { priv->dev->hw_features = NETIF_F_SG | diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 284b00c..2ff1fd1 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -273,7 +273,7 @@ struct ib_device_attr { u32 hw_ver; int max_qp; int max_qp_wr; - int device_cap_flags; + u64 device_cap_flags; int max_sge; int max_sge_rd; int max_cq;