| Message ID | 1432109615-19564-9-git-send-email-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
> +/** > + * rdma_max_mad_size - Return the max MAD size required by this RDMA > Port. > + * > + * @device: Device > + * @port_num: Port number > + * > + * Return the max MAD size required by the Port. Should return 0 if the > port > + * does not support MADs > + */ > +static inline size_t rdma_max_mad_size(struct ib_device *device, u8 > port_num) > +{ > + return device->port_immutable[port_num].max_mad_size; > +} Should this function check the mad_cap bit and return 0 if not set? If not, I would remove the 'should return 0...' statement from the comment above and state that the caller should check the mad_cap bit first. -- 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 Wed, May 20, 2015 at 11:37:47AM -0600, Hefty, Sean wrote: > > +/** > > + * rdma_max_mad_size - Return the max MAD size required by this RDMA > > Port. > > + * > > + * @device: Device > > + * @port_num: Port number > > + * > > + * Return the max MAD size required by the Port. Should return 0 if the > > port > > + * does not support MADs > > + */ > > +static inline size_t rdma_max_mad_size(struct ib_device *device, u8 > > port_num) > > +{ > > + return device->port_immutable[port_num].max_mad_size; > > +} > > Should this function check the mad_cap bit and return 0 if not set? If not, > I would remove the 'should return 0...' statement from the comment above and > state that the caller should check the mad_cap bit first. I'll change the comment. Ira -- 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 Wed, May 20, 2015 at 02:02:27PM -0400, ira.weiny wrote: > On Wed, May 20, 2015 at 11:37:47AM -0600, Hefty, Sean wrote: > > > +/** > > > + * rdma_max_mad_size - Return the max MAD size required by this RDMA > > > Port. > > > + * > > > + * @device: Device > > > + * @port_num: Port number > > > + * > > > + * Return the max MAD size required by the Port. Should return 0 if the > > > port > > > + * does not support MADs > > > + */ > > > +static inline size_t rdma_max_mad_size(struct ib_device *device, u8 > > > port_num) > > > +{ > > > + return device->port_immutable[port_num].max_mad_size; > > > +} > > > > Should this function check the mad_cap bit and return 0 if not set? If not, > > I would remove the 'should return 0...' statement from the comment above and > > state that the caller should check the mad_cap bit first. > > I'll change the comment. If there is no mad support the port_immutable.max_mad_size should be 0, force it during registration, then the comment is right. Force to 0 is better than 'some random value' 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
On Wed, May 20, 2015 at 12:19:28PM -0600, Jason Gunthorpe wrote: > On Wed, May 20, 2015 at 02:02:27PM -0400, ira.weiny wrote: > > On Wed, May 20, 2015 at 11:37:47AM -0600, Hefty, Sean wrote: > > > > +/** > > > > + * rdma_max_mad_size - Return the max MAD size required by this RDMA > > > > Port. > > > > + * > > > > + * @device: Device > > > > + * @port_num: Port number > > > > + * > > > > + * Return the max MAD size required by the Port. Should return 0 if the > > > > port > > > > + * does not support MADs > > > > + */ > > > > +static inline size_t rdma_max_mad_size(struct ib_device *device, u8 > > > > port_num) > > > > +{ > > > > + return device->port_immutable[port_num].max_mad_size; > > > > +} > > > > > > Should this function check the mad_cap bit and return 0 if not set? If not, > > > I would remove the 'should return 0...' statement from the comment above and > > > state that the caller should check the mad_cap bit first. > > > > I'll change the comment. > > If there is no mad support the port_immutable.max_mad_size should be > 0, force it during registration, then the comment is right. > > Force to 0 is better than 'some random value' When you say "force" do you mean have the drivers which don't support MAD explicitly set it to 0? The value is 0 by default (kzalloc) and is why I wrote the comment like I did. Ira -- 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
> The value is 0 by default (kzalloc) and is why I wrote the comment like I > did. My objection with the comment is that it was not describing what the function did, but rather the value that should be stored in the structure. Basically, it seemed misplaced. -- 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 Wed, May 20, 2015 at 03:03:52PM -0400, ira.weiny wrote: > On Wed, May 20, 2015 at 12:19:28PM -0600, Jason Gunthorpe wrote: > > On Wed, May 20, 2015 at 02:02:27PM -0400, ira.weiny wrote: > > > On Wed, May 20, 2015 at 11:37:47AM -0600, Hefty, Sean wrote: > > > > > +/** > > > > > + * rdma_max_mad_size - Return the max MAD size required by this RDMA > > > > > Port. > > > > > + * > > > > > + * @device: Device > > > > > + * @port_num: Port number > > > > > + * > > > > > + * Return the max MAD size required by the Port. Should return 0 if the > > > > > port > > > > > + * does not support MADs > > > > > + */ > > > > > +static inline size_t rdma_max_mad_size(struct ib_device *device, u8 > > > > > port_num) > > > > > +{ > > > > > + return device->port_immutable[port_num].max_mad_size; > > > > > +} > > > > > > > > Should this function check the mad_cap bit and return 0 if not set? If not, > > > > I would remove the 'should return 0...' statement from the comment above and > > > > state that the caller should check the mad_cap bit first. > > > > > > I'll change the comment. > > > > If there is no mad support the port_immutable.max_mad_size should be > > 0, force it during registration, then the comment is right. > > > > Force to 0 is better than 'some random value' > > When you say "force" do you mean have the drivers which don't support MAD > explicitly set it to 0? I'm not really sure it makes sense to have the drivers set this, since the value is fixed based on the existing caps. The core could fill it in, then we don't have to check the driver's work. If the driver fills it then the core should do BUG_ON(!mad_cap && max_mad_size != 0) After calling the driver to fill the values. The comment seems OK (change Should => Will), it is describing the function, and the function should be the only accessor for this value, so it also describes the value. Maybe clarify what mad size is .. Is it just the payload? 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
> > I'm not really sure it makes sense to have the drivers set this, since > the value is fixed based on the existing caps. The core could fill it > in, then we don't have to check the driver's work. This comment is making more sense to me now... We already discussed this at length and it was agreed that the MAD length was not to be hard coded to any particular MAD support "bit". Therefore I'm going to continue with this additional flexibility because I have added it. Ira -- 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/mad.c b/drivers/infiniband/core/mad.c index 8055195b5d60..ea25e9467cb5 100644 --- a/drivers/infiniband/core/mad.c +++ b/drivers/infiniband/core/mad.c @@ -2937,6 +2937,10 @@ static int ib_mad_port_open(struct ib_device *device, unsigned long flags; char name[sizeof "ib_mad123"]; int has_smi; + size_t max_mad_size = rdma_max_mad_size(device, port_num); + + if (WARN_ON(max_mad_size < IB_MGMT_MAD_SIZE)) + return -EFAULT; /* Create new device info */ port_priv = kzalloc(sizeof *port_priv, GFP_KERNEL); diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c index 5e30b72d3677..64c834b358d6 100644 --- a/drivers/infiniband/hw/ehca/ehca_main.c +++ b/drivers/infiniband/hw/ehca/ehca_main.c @@ -46,6 +46,7 @@ #include <linux/notifier.h> #include <linux/memory.h> +#include <rdma/ib_mad.h> #include "ehca_classes.h" #include "ehca_iverbs.h" #include "ehca_mrmw.h" @@ -444,6 +445,7 @@ static int ehca_port_immutable(struct ib_device *ibdev, u8 port_num, immutable->pkey_tbl_len = attr.pkey_tbl_len; immutable->gid_tbl_len = attr.gid_tbl_len; immutable->core_cap_flags = RDMA_CORE_PORT_IBA_IB; + immutable->max_mad_size = IB_MGMT_MAD_SIZE; return 0; } diff --git a/drivers/infiniband/hw/ipath/ipath_verbs.c b/drivers/infiniband/hw/ipath/ipath_verbs.c index 764081d305b6..c67e8c22dabc 100644 --- a/drivers/infiniband/hw/ipath/ipath_verbs.c +++ b/drivers/infiniband/hw/ipath/ipath_verbs.c @@ -1993,6 +1993,7 @@ static int ipath_port_immutable(struct ib_device *ibdev, u8 port_num, immutable->pkey_tbl_len = attr.pkey_tbl_len; immutable->gid_tbl_len = attr.gid_tbl_len; immutable->core_cap_flags = RDMA_CORE_PORT_IBA_IB; + immutable->max_mad_size = IB_MGMT_MAD_SIZE; return 0; } diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c index c49dd0bb251a..7c62daf58302 100644 --- a/drivers/infiniband/hw/mlx4/main.c +++ b/drivers/infiniband/hw/mlx4/main.c @@ -2132,6 +2132,8 @@ static int mlx4_port_immutable(struct ib_device *ibdev, u8 port_num, else immutable->core_cap_flags = RDMA_CORE_PORT_IBA_ROCE; + immutable->max_mad_size = IB_MGMT_MAD_SIZE; + return 0; } diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c index b2fdb9cfa645..bb7f718adc11 100644 --- a/drivers/infiniband/hw/mlx5/main.c +++ b/drivers/infiniband/hw/mlx5/main.c @@ -1195,6 +1195,7 @@ static int mlx5_port_immutable(struct ib_device *ibdev, u8 port_num, immutable->pkey_tbl_len = attr.pkey_tbl_len; immutable->gid_tbl_len = attr.gid_tbl_len; immutable->core_cap_flags = RDMA_CORE_PORT_IBA_IB; + immutable->max_mad_size = IB_MGMT_MAD_SIZE; return 0; } diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c index 509d59e7a15a..db11d0cc5d59 100644 --- a/drivers/infiniband/hw/mthca/mthca_provider.c +++ b/drivers/infiniband/hw/mthca/mthca_provider.c @@ -1257,6 +1257,7 @@ static int mthca_port_immutable(struct ib_device *ibdev, u8 port_num, immutable->pkey_tbl_len = attr.pkey_tbl_len; immutable->gid_tbl_len = attr.gid_tbl_len; immutable->core_cap_flags = RDMA_CORE_PORT_IBA_IB; + immutable->max_mad_size = IB_MGMT_MAD_SIZE; return 0; } diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_main.c b/drivers/infiniband/hw/ocrdma/ocrdma_main.c index f55289869357..8a1398b253a2 100644 --- a/drivers/infiniband/hw/ocrdma/ocrdma_main.c +++ b/drivers/infiniband/hw/ocrdma/ocrdma_main.c @@ -30,6 +30,7 @@ #include <rdma/ib_verbs.h> #include <rdma/ib_user_verbs.h> #include <rdma/ib_addr.h> +#include <rdma/ib_mad.h> #include <linux/netdevice.h> #include <net/addrconf.h> @@ -215,6 +216,7 @@ static int ocrdma_port_immutable(struct ib_device *ibdev, u8 port_num, immutable->pkey_tbl_len = attr.pkey_tbl_len; immutable->gid_tbl_len = attr.gid_tbl_len; immutable->core_cap_flags = RDMA_CORE_PORT_IBA_ROCE; + immutable->max_mad_size = IB_MGMT_MAD_SIZE; return 0; } diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c index dba1c92f1a54..e9e21f9c36e2 100644 --- a/drivers/infiniband/hw/qib/qib_verbs.c +++ b/drivers/infiniband/hw/qib/qib_verbs.c @@ -2053,6 +2053,7 @@ static int qib_port_immutable(struct ib_device *ibdev, u8 port_num, immutable->pkey_tbl_len = attr.pkey_tbl_len; immutable->gid_tbl_len = attr.gid_tbl_len; immutable->core_cap_flags = RDMA_CORE_PORT_IBA_IB; + immutable->max_mad_size = IB_MGMT_MAD_SIZE; return 0; } diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h index ddc82d65a9e5..20ed361a9224 100644 --- a/include/rdma/ib_mad.h +++ b/include/rdma/ib_mad.h @@ -135,6 +135,7 @@ enum { IB_MGMT_SA_DATA = 200, IB_MGMT_DEVICE_HDR = 64, IB_MGMT_DEVICE_DATA = 192, + IB_MGMT_MAD_SIZE = IB_MGMT_MAD_HDR + IB_MGMT_MAD_DATA, }; struct ib_mad_hdr { diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index ad499bda62a4..8859afea4e2f 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -1523,6 +1523,7 @@ struct ib_port_immutable { int pkey_tbl_len; int gid_tbl_len; u32 core_cap_flags; + u32 max_mad_size; }; struct ib_device { @@ -2040,6 +2041,20 @@ static inline bool rdma_cap_read_multi_sge(struct ib_device *device, return !(device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IWARP); } +/** + * rdma_max_mad_size - Return the max MAD size required by this RDMA Port. + * + * @device: Device + * @port_num: Port number + * + * Return the max MAD size required by the Port. Should return 0 if the port + * does not support MADs + */ +static inline size_t rdma_max_mad_size(struct ib_device *device, u8 port_num) +{ + return device->port_immutable[port_num].max_mad_size; +} + int ib_query_gid(struct ib_device *device, u8 port_num, int index, union ib_gid *gid);