Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
On Thu, Mar 22, 2012 at 3:44 PM, Jason Gunthorpe wrote: > For hardware facing structures I'd combine this with a static assert > to verify structure size at compile time. > > So.. > > 1) Avoid using attributes unless the structure has unaligned members. > 2) Avoid creating structures with unaligned members (eg for userspace > communication) > 3) Frown at hardware/firmware developers who make communication > structures with unaligned members :) > 4) Be explicit about padding in your layout for 64/32 > compatibility. Excellent summary, thanks Jason. - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
On Thu, Mar 22, 2012 at 02:20:28PM -0700, parav.pan...@emulex.com wrote:
> I got a question here lately.
>
> aligned directive will ensure that it will fall on boundary. Say
> aligned(4) ensures that structure is aligned to 4 byte boundary.
> Compiler can (at least theoretically) still have 4 byte structure
> aligned to 8 byte boundary on 64-bit platform (which is 4 byte
> aligned too).
There are very specific rules defined in the platform's ABI for how C
structures are layed out in memory, each ABI (ie CPU) has its own
specific quirks, but broadly in Linux land you can boil it down to:
1) The alignment of a structure is the greatest alignment of all the
members
2) Each member is aligned to its alignment.
The alignment of the structure drives the total size of the structure,
and specifically the padding added at the end to reach that alignment.
So, no, a compiler that increased the alignment of a struct with one
u32 to 8 would violate the various ABIs and not be usuable for
Linux. It is important to bear in mind that Linux targets a particular
set of ABI conventions, and it is not 'anything goes'.
> struct {
> u32 field;
> };
So in this case: the u32 is aligned to 4, the structure is aligned to
4 and the total size of the structure is 4 on everything linux
supports.
> struct {
> u64 fielda
> u32 field;
> };
In this case: On 64 bit: the u64 is aligned to 8 and the u32 is aligned to 4. So
the structure is aligned to 8. A pad is inserted at the end of the
struct to bring it out. On 32 bit, the u64 is aligned to 4, so the
struct is aligned to 4, so no pad is added.
> struct {
> __float128 fielda
> u32 field;
> };
In this case the float128 is aligned to 16 and thus the structure is
aligned to 16 and 12 pad bytes are added.
> However requirement is to have this structure only 4 byte size(
> because adapter excepts it to be 4B sise) and therefor packed is
> used. I don't know the way to ensure size of 4 byte and alignment
> too. Or I am misunderstanding?
Yes, you are mis-understanding the rules for padding.. Structures are
only padded out to their alignment, which depends on their constituent
types. This is so arrays of structures have each array element
starting on its natural alignment.
The aligned attribute overrides the automatic determination of the
alignment based on the contents and just forces it.
So, as an example, if you have this hardware layout:
struct {
u32 fielda;
u64 fieldb;
} attribute ((aligned(4));
David is saying you will get a 12 byte struct and fieldb will be
unaligned. Since 12 is aligned to 4 no padding is added.
For hardware facing structures I'd combine this with a static assert
to verify structure size at compile time.
So..
1) Avoid using attributes unless the structure has unaligned members.
2) Avoid creating structures with unaligned members (eg for userspace
communication)
3) Frown at hardware/firmware developers who make communication
structures with unaligned members :)
4) Be explicit about padding in your layout for 64/32
compatibility.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
I got a question here lately.
aligned directive will ensure that it will fall on boundary.
Say aligned(4) ensures that structure is aligned to 4 byte boundary.
Compiler can (at least theoretically) still have 4 byte structure aligned to 8
byte boundary on 64-bit platform (which is 4 byte aligned too).
struct {
u32 field;
} attribute ((aligned(4));
However requirement is to have this structure only 4 byte size( because adapter
excepts it to be 4B sise) and therefor packed is used.
I don't know the way to ensure size of 4 byte and alignment too.
Or I am misunderstanding?
Parav
> -Original Message-
> From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> ow...@vger.kernel.org] On Behalf Of parav.pan...@emulex.com
> Sent: Friday, March 23, 2012 2:41 AM
> To: jguntho...@obsidianresearch.com
> Cc: david.lai...@aculab.com; rol...@purestorage.com; linux-
> r...@vger.kernel.org; net...@vger.kernel.org
> Subject: RE: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA
> adapter
>
>
>
> > -Original Message-
> > From: Jason Gunthorpe [mailto:jguntho...@obsidianresearch.com]
> > Sent: Friday, March 23, 2012 2:28 AM
> > To: Pandit, Parav
> > Cc: david.lai...@aculab.com; rol...@purestorage.com; linux-
> > r...@vger.kernel.org; net...@vger.kernel.org
> > Subject: Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA
> > adapter
> >
> > On Thu, Mar 22, 2012 at 01:52:30PM -0700, parav.pan...@emulex.com
> > wrote:
> >
> > > > This can be used to force 32bit alignment in amd64 code in order
> > > > to match definitions in 32bit userspace.
> > > > For new things it would make sense to force 64bit alignment of
> > > > 64bit fields for 32bit code.
> > >
> > > o.k. so I'll use aligned attribute to align user-kernel interface
> > > data structure to 8 byte boundary. That should work for 32-bit and
> > > 64-bit user and kernel space and does't hurt performance either?
> >
> > If the structure is only for user/kernel interfacing then it is much
> > better to add explicit padding fields to naturally place 64 bit
> > quantities on an 8 byte alignment than to mess with gcc specific
> > attributes (user space has a much wide choice of compilers).
> >
> > This was David's second suggestion. Better to do this now before the
> > driver is accepted :)
> >
> o.k. I'll align them to naturally 8 byte boundary.
>
> > Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the
> body of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
> -Original Message- > From: Jason Gunthorpe [mailto:jguntho...@obsidianresearch.com] > Sent: Friday, March 23, 2012 2:28 AM > To: Pandit, Parav > Cc: david.lai...@aculab.com; rol...@purestorage.com; linux- > r...@vger.kernel.org; net...@vger.kernel.org > Subject: Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA > adapter > > On Thu, Mar 22, 2012 at 01:52:30PM -0700, parav.pan...@emulex.com > wrote: > > > > This can be used to force 32bit alignment in amd64 code in order to > > > match definitions in 32bit userspace. > > > For new things it would make sense to force 64bit alignment of 64bit > > > fields for 32bit code. > > > > o.k. so I'll use aligned attribute to align user-kernel interface data > > structure to 8 byte boundary. That should work for 32-bit and 64-bit > > user and kernel space and does't hurt performance either? > > If the structure is only for user/kernel interfacing then it is much better to > add explicit padding fields to naturally place 64 bit quantities on an 8 byte > alignment than to mess with gcc specific attributes (user space has a much > wide choice of compilers). > > This was David's second suggestion. Better to do this now before the driver is > accepted :) > o.k. I'll align them to naturally 8 byte boundary. > Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
On Thu, Mar 22, 2012 at 01:52:30PM -0700, parav.pan...@emulex.com wrote: > > This can be used to force 32bit alignment in amd64 code in order to match > > definitions in 32bit userspace. > > For new things it would make sense to force 64bit alignment of 64bit fields > > for 32bit code. > > o.k. so I'll use aligned attribute to align user-kernel interface > data structure to 8 byte boundary. That should work for 32-bit and > 64-bit user and kernel space and does't hurt performance either? If the structure is only for user/kernel interfacing then it is much better to add explicit padding fields to naturally place 64 bit quantities on an 8 byte alignment than to mess with gcc specific attributes (user space has a much wide choice of compilers). This was David's second suggestion. Better to do this now before the driver is accepted :) Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
> -Original Message-
> From: David Laight [mailto:david.lai...@aculab.com]
> Sent: Wednesday, March 21, 2012 10:02 PM
> To: Roland Dreier; Pandit, Parav
> Cc: linux-rdma@vger.kernel.org; net...@vger.kernel.org
> Subject: RE: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA
> adapter
>
>
> > > - Header file for userspace library and kernel driver interface.
> >
> > > +struct ocrdma_alloc_ucontext_resp {
> > > + u32 dev_id;
> > > + u32 wqe_size;
> > > + u32 max_inline_data;
> > > + u32 dpp_wqe_size;
> > > + u64 ah_tbl_page;
> > > + u32 ah_tbl_len;
> > > + u32 rsvd;
> > > + u8 fw_ver[32];
> > > + u32 rqe_size;
> > > + u64 rsvd1;
> > > +} __packed;
> >
> > If I'm reading this correctly, you have the 8-byte rsvd1 member at an
> > offset only aligned to 4 bytes, because of the __packed directive. It
> > would be much better to have these structures laid out so they are
> > naturally the same on both 32-bit and 64-bit ABIs, and get rid of the
> > __packed directive, which wrecks gcc code generation in some cases.
> >
>
> gcc also supports defining types that have non-standard alignment
> constraints that can be used to force the same alignment for 64bit fields
> between i386 and amd64.
> Probably __attribute__((aligned,n)) or similar.
>
> This can be used to force 32bit alignment in amd64 code in order to match
> definitions in 32bit userspace.
> For new things it would make sense to force 64bit alignment of 64bit fields
> for 32bit code.
o.k. so I'll use aligned attribute to align user-kernel interface data
structure to 8 byte boundary.
That should work for 32-bit and 64-bit user and kernel space and does't hurt
performance either?
Driver-adapter structures will be aligned to 4 byte boundary using aligned
attribute instead of packed.
>
> Adding __packed (rather than 32bit alignment) forces the compiler to
> generate byte by byte accesses for all the fields on systems that can't do
> misaligned accesses in hardware (eg sparc).
>
> David
>
>
> David
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
> -Original Message- > From: Roland Dreier [mailto:rol...@purestorage.com] > Sent: Thursday, March 22, 2012 9:30 PM > To: Pandit, Parav > Cc: sean.he...@intel.com; linux-rdma@vger.kernel.org > Subject: Re: [PATCH 1/9] ocrdma: Driver for Emulex OneConnect RDMA > adapter > > On Thu, Mar 22, 2012 at 7:27 AM, wrote: > > I prefer to have it in this first patch and once the internal test cycle is > > finish > (with removed code) in few days, will submit the separate patch? > > It's fine to leave some of these items for future cleanups post-merge. > Just please try to get to the cleanups someday unlike some other drivers... Sure. Item is list and tracking the same. Mostly likely it's going to be 2nd patch after this first release before adding any new features. > (*cough* nes *cough*) > > - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] please pull infiniband.git
On Wed, 21 Mar 2012, Or Gerlitz wrote: > Christoph, I will not attend, but will love to get any related > feedback. Looking on the agenda > (https://www.openfabrics.org/images/docs/PR/schedule.pdf) I see that > on Wednesday noon there are two sessions dealing with exactly this - > "User Mode Ethernet Programming" and "Network Adapter Flow Steering" > by Tzahi Oved from Mellanox, so be there or be sqaure (e.g myself...) Ok. I can also mention this in my session Monday morning. ;-) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
On Thu, Mar 22, 2012 at 7:27 AM, wrote: > I prefer to have it in this first patch and once the internal test cycle is > finish (with removed code) in few days, will submit the separate patch? It's fine to leave some of these items for future cleanups post-merge. Just please try to get to the cleanups someday unlike some other drivers... (*cough* nes *cough*) - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
>> Does OneConnect not support UC? (I thought UC was required for >> compliance.) > As of now UC is unsupported. I need to check with other teams. I'll add the > support in follow on checkin after this initial version, if its supported. > Is it o.k. if UC is unsupported in this initial release? > I read the IB spec. > C17-11: HCAs shall be capable of supporting the Unreliable Datagram, > Reliable Connection, and Unreliable Connection transport service on any > QP supported by the HCA. It's fine to merge the driver without UC support and add it later. We're pretty pragmatic about things if your hardware doesn't do something (or doesn't do something yet). - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
> -Original Message-
> From: Hefty, Sean [mailto:sean.he...@intel.com]
> Sent: Wednesday, March 21, 2012 10:49 PM
> To: Pandit, Parav; linux-rdma@vger.kernel.org
> Subject: RE: [PATCH 1/9] ocrdma: Driver for Emulex OneConnect RDMA
> adapter
>
> > +struct ocrdma_cq {
> > + struct ib_cq ibcq;
> > + struct ocrdma_dev *dev;
>
> nit: There are several structures where you store ocrdma_dev *. You can
> remove these and use the struct ib_* to reach it as well.
Removed from all the ocrdma_xxx structures except ocrdma_eq and ocrdma_qp.
ocrdma_eq is anyway necessary.
Removing it from ocrdma_qp will require a lot bigger test cycle at my end.
I prefer to have it in this first patch and once the internal test cycle is
finish (with removed code) in few days, will submit the separate patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] opensm: Add support for partition enforcement types
Partition enforcement types are in, out, and both.
Prior to this support, both was being used so that is the default.
--
Changes since v2:
Cosmetic changes
Added comments in man, conf file and help
Signed-off-by: Hal Rosenstock
Signed-off-by: Alex Netes
---
include/opensm/osm_subnet.h | 15 +++
man/opensm.8.in | 11 +--
opensm/main.c | 31 ---
opensm/osm_pkey_mgr.c | 41 +
opensm/osm_subnet.c | 31 ++-
5 files changed, 107 insertions(+), 22 deletions(-)
diff --git a/include/opensm/osm_subnet.h b/include/opensm/osm_subnet.h
index 4101ab9..d88f9c7 100644
--- a/include/opensm/osm_subnet.h
+++ b/include/opensm/osm_subnet.h
@@ -68,6 +68,19 @@ BEGIN_C_DECLS
#define OSM_SUBNET_VECTOR_MIN_SIZE 0
#define OSM_SUBNET_VECTOR_GROW_SIZE1
#define OSM_SUBNET_VECTOR_CAPACITY 256
+
+#define OSM_PARTITION_ENFORCE_BOTH "both"
+#define OSM_PARTITION_ENFORCE_IN "in"
+#define OSM_PARTITION_ENFORCE_OUT "out"
+#define OSM_PARTITION_ENFORCE_OFF "off"
+
+typedef enum _osm_partition_enforce_type_enum {
+ OSM_PARTITION_ENFORCE_TYPE_BOTH,
+ OSM_PARTITION_ENFORCE_TYPE_IN,
+ OSM_PARTITION_ENFORCE_TYPE_OUT,
+ OSM_PARTITION_ENFORCE_TYPE_OFF
+} osm_partition_enforce_type_enum;
+
struct osm_opensm;
struct osm_qos_policy;
@@ -184,6 +197,8 @@ typedef struct osm_subn_opt {
unsigned long log_max_size;
char *partition_config_file;
boolean_t no_partition_enforcement;
+ char *part_enforce;
+ osm_partition_enforce_type_enum part_enforce_enum;
boolean_t allow_both_pkeys;
uint8_t sm_assigned_guid;
boolean_t qos;
diff --git a/man/opensm.8.in b/man/opensm.8.in
index ff19f82..1a41f48 100644
--- a/man/opensm.8.in
+++ b/man/opensm.8.in
@@ -44,7 +44,8 @@ opensm \- InfiniBand subnet manager and administration (SM/SA)
[\-f | \-\-log_file ]
[\-L | \-\-log_limit ] [\-e(rase_log_file)]
[\-P(config) ]
-[\-N | \-\-no_part_enforce]
+[\-N | \-\-no_part_enforce] (DEPRECATED)
+[\-Z | \-\-part_enforce [both | in | out | off]]
[\-W | \-\-allow_both_pkeys]
[\-Q | \-\-qos [\-Y | \-\-qos_policy_file ]]
[\-y | \-\-stay_on_fatal]
@@ -367,9 +368,15 @@ name is \fB\%@OPENSM_CONFIG_DIR@/@QOS_POLICY_FILE@\fP. See
QoS_management_in_OpenSM.txt in opensm doc for more information on
configuring QoS policy via this file.
.TP
-\fB\-N\fR, \fB\-\-no_part_enforce\fR
+\fB\-N\fR, \fB\-\-no_part_enforce\fR \fB(DEPRECATED)\fR
+This is a deprecated flag. Please use \fB\-\-part_enforce\fR instead.
This option disables partition enforcement on switch external ports.
.TP
+\fB\-Z\fR, \fB\-\-part_enforce\fR [both | in | out | off]
+This option indicates the partition enforcement type (for switches).
+Enforcement type can be inbound only (in), outbound only (out),
+both or disabled (off). Default is both.
+.TP
\fB\-W\fR, \fB\-\-allow_both_pkeys\fR
This option indicates whether both full and limited membership on the
same partition can be configured in the PKeyTable. Default is not
diff --git a/opensm/main.c b/opensm/main.c
index 3351681..8123c16 100644
--- a/opensm/main.c
+++ b/opensm/main.c
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
- * Copyright (c) 2002-2009 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2002-2011 Mellanox Technologies LTD. All rights reserved.
* Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
* Copyright (c) 2009 HNR Consulting. All rights reserved.
* Copyright (c) 2009 System Fabric Works, Inc. All rights reserved.
@@ -321,8 +321,13 @@ static void show_usage(void)
" This option defines the optional partition
configuration file.\n"
" The default name is \'"
OSM_DEFAULT_PARTITION_CONFIG_FILE "\'.\n\n");
- printf("--no_part_enforce, -N\n"
+ printf("--no_part_enforce, -N (DEPRECATED)\n"
+ " Use --part_enforce instead.\n"
" This option disables partition enforcement on switch
external ports.\n\n");
+ printf("--part_enforce, -Z [both, in, out, off]\n"
+ " This option indicates the partition enforcement type
(for switches)\n"
+ " Enforcement type can be outbound only (out), inbound
only (in), both or\n"
+ " disabled (off). Default is both.\n\n");
printf("--allow_both_pkeys, -W\n"
" This option indicates whether both full and limited
membership\n"
" on the same partition can be configured in the
PKeyTable.\n"
@@ -573,7 +578,7 @@ int main(int argc, char *argv[])
char *conf_template = NULL, *config_file = NULL;
[PATCH 4/4] opensm/osm_sa_guidinfo_record.c: Check block_num validity in set_guidinfo() and del_guidinfo() requests
block_num should be less than port's Alias GUID capability.
Signed-off-by: Alex Netes
Signed-off-by: Hal Rosenstock
---
opensm/osm_sa_guidinfo_record.c | 23 +++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/opensm/osm_sa_guidinfo_record.c b/opensm/osm_sa_guidinfo_record.c
index b100da1..37451b3 100644
--- a/opensm/osm_sa_guidinfo_record.c
+++ b/opensm/osm_sa_guidinfo_record.c
@@ -373,6 +373,7 @@ static void del_guidinfo(IN osm_sa_t *sa, IN osm_madw_t
*p_madw,
IN osm_port_t *p_port, IN uint8_t block_num)
{
int i;
+ uint32_t max_block;
ib_sa_mad_t *p_sa_mad;
ib_guidinfo_record_t *p_rcvd_rec;
ib_net64_t del_alias_guid;
@@ -386,6 +387,19 @@ static void del_guidinfo(IN osm_sa_t *sa, IN osm_madw_t
*p_madw,
if (!p_port->p_physp->p_guids)
goto Exit;
+ max_block = (p_port->p_physp->port_info.guid_cap +
GUID_TABLE_MAX_ENTRIES - 1) /
+GUID_TABLE_MAX_ENTRIES;
+
+ if (block_num > max_block) {
+ OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 5116: "
+ "block_num %d is higher than Max GUID Cap block %d "
+ "for port GUID 0x%" PRIx64 "\n",
+ block_num, max_block,
cl_ntoh64(p_port->p_physp->port_guid));
+ osm_sa_send_error(sa, p_madw,
+ IB_SA_MAD_STATUS_NO_RECORDS);
+ return;
+ }
+
p_sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
p_rcvd_rec =
(ib_guidinfo_record_t *) ib_sa_mad_get_payload_ptr(p_sa_mad);
@@ -479,6 +493,15 @@ static void set_guidinfo(IN osm_sa_t *sa, IN osm_madw_t
*p_madw,
max_block = (p_port->p_physp->port_info.guid_cap +
GUID_TABLE_MAX_ENTRIES - 1) /
GUID_TABLE_MAX_ENTRIES;
+ if (block_num > max_block) {
+ OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 5118: "
+ "block_num %d is higher than Max GUID Cap block %d "
+ "for port GUID 0x%" PRIx64 "\n",
+ block_num, max_block,
cl_ntoh64(p_port->p_physp->port_guid));
+ osm_sa_send_error(sa, p_madw,
+ IB_SA_MAD_STATUS_NO_RECORDS);
+ return;
+ }
if (!p_port->p_physp->p_guids) {
p_port->p_physp->p_guids = calloc(max_block *
GUID_TABLE_MAX_ENTRIES,
sizeof(ib_net64_t));
--
1.7.7.6
-- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] opensm: Fix continous looping when clearing accum_pkeys table
p_pkey_tbl->accum_pkeys maximal size is 0x1 (last used index is 0x),
so when we itterate over accum_pkeys we should make sure we don't go over the
bounds of uint16_t.
Signed-off-by: Alex Netes
Signed-off-by: Hal Rosenstock
---
opensm/osm_pkey.c |5 +++--
opensm/osm_pkey_mgr.c |5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/opensm/osm_pkey.c b/opensm/osm_pkey.c
index 632d44d..30a5767 100644
--- a/opensm/osm_pkey.c
+++ b/opensm/osm_pkey.c
@@ -205,7 +205,8 @@ void osm_pkey_tbl_clear_accum_pkeys(IN osm_pkey_tbl_t *
p_pkey_tbl,
{
void *ptr;
uintptr_t pkey_idx_ptr;
- uint16_t pkey_idx, last_pkey_idx, i;
+ uint16_t pkey_idx, last_pkey_idx;
+ uint32_t i;
ptr = cl_ptr_vector_get(&p_pkey_tbl->accum_pkeys, pkey);
if (ptr == NULL)
@@ -218,7 +219,7 @@ void osm_pkey_tbl_clear_accum_pkeys(IN osm_pkey_tbl_t *
p_pkey_tbl,
if (p_pkey_tbl->last_pkey_idx == pkey_idx) {
last_pkey_idx = 0;
- for (i = 0; i <
cl_ptr_vector_get_size(&p_pkey_tbl->accum_pkeys); i++) {
+ for (i = 1; i <
cl_ptr_vector_get_size(&p_pkey_tbl->accum_pkeys); i++) {
ptr = cl_ptr_vector_get(&p_pkey_tbl->accum_pkeys, i);
if (ptr != NULL) {
pkey_idx_ptr = (uintptr_t) ptr;
diff --git a/opensm/osm_pkey_mgr.c b/opensm/osm_pkey_mgr.c
index d051026..f7e6eae 100644
--- a/opensm/osm_pkey_mgr.c
+++ b/opensm/osm_pkey_mgr.c
@@ -255,12 +255,13 @@ pkey_mgr_enforce_partition(IN osm_log_t * p_log, osm_sm_t
* sm,
static void clear_accum_pkey_index(osm_pkey_tbl_t * p_pkey_tbl,
uint16_t pkey_index)
{
- uint16_t i, pkey_idx_bias, pkey_idx;
+ uint16_t pkey_idx_bias, pkey_idx;
+ uint32_t i;
void *ptr;
uintptr_t pkey_idx_ptr;
pkey_idx_bias = pkey_index + 1; // adjust for pkey index bias in
accum_pkeys
- for (i = 0; i < cl_ptr_vector_get_size(&p_pkey_tbl->accum_pkeys); i++) {
+ for (i = 1; i < cl_ptr_vector_get_size(&p_pkey_tbl->accum_pkeys); i++) {
ptr = cl_ptr_vector_get(&p_pkey_tbl->accum_pkeys, i);
if (ptr != NULL) {
pkey_idx_ptr = (uintptr_t) ptr;
--
1.7.7.6
-- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] opensm/osm_sa_guidinfo_record.c: end error resoponse to invalid LID in GUIDInfo request
Sending IB_SA_MAD_STATUS_NO_RECORDS for SA GUIDInfo requests with
invalid LID in record identifier.
Signed-off-by: Daniel Klein
Signed-off-by: Alex Netes
Signed-off-by: Hal Rosenstock
---
opensm/osm_sa_guidinfo_record.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/opensm/osm_sa_guidinfo_record.c b/opensm/osm_sa_guidinfo_record.c
index b23bc28..b100da1 100644
--- a/opensm/osm_sa_guidinfo_record.c
+++ b/opensm/osm_sa_guidinfo_record.c
@@ -748,9 +748,10 @@ void osm_gir_rcv_process(IN void *ctx, IN void *data)
p_rcvd_rec = (ib_guidinfo_record_t *)
ib_sa_mad_get_payload_ptr(p_rcvd_mad);
p_port = osm_get_port_by_lid(sa->p_subn, p_rcvd_rec->lid);
if (!p_port) {
- OSM_LOG(sa->p_log, OSM_LOG_DEBUG,
+ OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 5117: "
"Port with LID %u not found\n",
cl_ntoh16(p_rcvd_rec->lid));
+ osm_sa_send_error(sa, p_madw,
IB_SA_MAD_STATUS_NO_RECORDS);
goto Exit;
}
if (!osm_physp_share_pkey(sa->p_log, p_req_physp,
--
1.7.7.6
-- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] opensm/osm_pkey_mgr.c: fix segfault when trying to access not allocated block
If the program decides to add a pkey to block 1. It might not allocate memory
for block 0. As a result, when the program searches for a pkey, it might try
access a not allocated block.
Signed-off-by: Daniel Klein
Signed-off-by: Hal Rosenstock
Signed-off-by: Alex Netes
---
opensm/osm_pkey_mgr.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/opensm/osm_pkey_mgr.c b/opensm/osm_pkey_mgr.c
index 52c2280..d051026 100644
--- a/opensm/osm_pkey_mgr.c
+++ b/opensm/osm_pkey_mgr.c
@@ -556,6 +556,9 @@ static int new_pkey_exists(osm_pkey_tbs_t * p_pkey_tbl,
ib_net16_t pkey)
num_blocks = (uint16_t) cl_ptr_vector_get_size(&p_pkey_tbl->new_blocks);
for (block_index = 0; block_index < num_blocks; block_index++) {
block = osm_pkey_tbl_new_block_get(p_pkey_tbl, block_index);
+ if (!block)
+ continue;
+
for (pkey_idx = 0; pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
pkey_idx++) {
if (block->pkey_entry[pkey_idx] == pkey)
--
1.7.7.6
-- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Applied pending OpenSM SRIOV patches
After internal testing, applied the following list of the patches that were submitted to the list by Hal Rosenstock. 7fce500 opensm/Makefile.am: Add doc/opensm-sriov.txt to docs 264aeb1 opensm: Add documentation for SRIOV support c639832 opensm: Enhance osm_physp_share_this_pkey for allow_both_pkeys policy b17b63c opensm: When allowing both pkeys, on a switch external, (peer) port eliminate limited pkey when full pkey with same base is present a758da2 opensm: Add command line option for allow_both_pkeys f412de3 opensm: Update partition documentation and man page for (allowing) both (limited and full) memberships in the same partition 726ce6a Support allowing both full and limited members of same partition 4ccf32f opensm/PKeyMgr: Support pkey index reuse when there are no longer any previously unused indices available ef9cf49 opensm/osm_pkey.h: Commentary change eb375a6 opensm/osm_pkey_mgr.c: Detect pkey table overflow in pkey_mgr_update_port 411e742 opensm/PkeyMgr: Don't change end port pkey index when simultaneously adding and removing partitions 15e7223 opensm/osm_sa_guidinfo_record.c: In set_guidinfo, better SM reassigned guid handing 8d49c5d opensm/osm_sa_guidinfo_record.c: Fix locking e79b725 opensm: Handle SubnSet GUIDInfo asynchronously from GUIDInfoRecord handling 96c741d opensm: Some cosmetic formatting changes 1d5e370 opensm/osm_sa_guidinfo_record.c: Better status for SA response efd3ba2 opensm/osm_sa.c: Change log level of message db8b7da opensm/osm_sa_service_record.c: Alias GUID support 5330986 opensm/osm_sa_multipath_record.c: Add support for alias GUIDs 44168c9 opensm/osm_sa_guidinfo_record.c: In del_guidinfo, validate guid not in use 63eb65b opensm: Add multicast support for alias GUIDs 700d15f opensm/osm_sa_path_record.c: Add support for alias GUIDs f818387 opensm/osm_sa_guidinfo_record.c: Use OSM_VENDOR_ID_OPENIB define rather than IB_OPENIB_OUI 97e360e opensm: Dump/load SA GUIDInfoRecords fe74f1d opensm: Make SA assigned guids persistent across port down/up events eb8f1d9 opensm: Add support for alias GUIDs -- -- Alex -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
> -Original Message-
> From: Hefty, Sean [mailto:sean.he...@intel.com]
> Sent: Thursday, March 22, 2012 6:14 AM
> To: Pandit, Parav; linux-rdma@vger.kernel.org
> Subject: RE: [PATCH 5/9] ocrdma: Driver for Emulex OneConnect RDMA
> adapter
>
> > +static int ocrdma_inet6addr_event(struct notifier_block *,
> > + unsigned long, void *);
> > +
> > +static struct notifier_block ocrdma_inet6addr_notifier = {
> > + .notifier_call = ocrdma_inet6addr_event };
> > +
> > +int ocrdma_get_instance(void)
> > +{
> > + int instance = 0;
> > +
> > + /* Assign an unused number */
> > + if (!idr_pre_get(&ocrdma_dev_id, GFP_KERNEL))
> > + return -1;
> > + if (idr_get_new(&ocrdma_dev_id, NULL, &instance))
> > + return -1;
> > + return instance;
> > +}
> > +
> > +void ocrdma_get_guid(struct ocrdma_dev *dev, u8 *guid) {
> > + u8 mac_addr[6];
> > +
> > + memcpy(&mac_addr[0], &dev->nic_info.mac_addr[0], ETH_ALEN);
> > + guid[0] = mac_addr[0] ^ 2;
> > + guid[1] = mac_addr[1];
> > + guid[2] = mac_addr[2];
> > + guid[3] = 0xff;
> > + guid[4] = 0xfe;
> > + guid[5] = mac_addr[3];
> > + guid[6] = mac_addr[4];
> > + guid[7] = mac_addr[5];
> > +}
> > +
> > +static void ocrdma_build_sgid_mac(union ib_gid *sgid, unsigned char
> > *mac_addr,
> > + bool is_vlan, u16 vlan_id)
> > +{
> > + sgid->global.subnet_prefix = cpu_to_be64(0xfe80LL);
> > + sgid->raw[8] = mac_addr[0] ^ 2;
> > + sgid->raw[9] = mac_addr[1];
> > + sgid->raw[10] = mac_addr[2];
> > + if (is_vlan) {
> > + sgid->raw[11] = vlan_id >> 8;
> > + sgid->raw[12] = vlan_id & 0xff;
> > + } else {
> > + sgid->raw[11] = 0xff;
> > + sgid->raw[12] = 0xfe;
> > + }
> > + sgid->raw[13] = mac_addr[3];
> > + sgid->raw[14] = mac_addr[4];
> > + sgid->raw[15] = mac_addr[5];
> > +}
> > +
> > +static void ocrdma_add_sgid(struct ocrdma_dev *dev, unsigned char
> *mac_addr,
> > + bool is_vlan, u16 vlan_id)
> > +{
> > + int i;
> > + bool found = false;
> > + union ib_gid new_sgid;
> > + int free_idx = OCRDMA_MAX_SGID;
> > + unsigned long flags;
> > +
> > + memset(&ocrdma_zero_sgid, 0, sizeof(union ib_gid));
>
> ocrdma_zero_sgid should already be zeroed
>
Yes. Removed memset().
> Actually, can't we either use in6addr_any instead? I see that the mlx4 driver
> also has a zero gid. So, if we don't want to overload the use of in6addr_any,
> we can define gid_zero in ib_core.
o.k. once its defined in ib_core, will remove this and provide a subsequent
patch. It will be easier to handle it.
>
> > +
> > + ocrdma_build_sgid_mac(&new_sgid, mac_addr, is_vlan, vlan_id);
> > +
> > + spin_lock_irqsave(&dev->sgid_lock, flags);
> > + for (i = 0; i < OCRDMA_MAX_SGID; i++) {
> > + if (!memcmp(&dev->sgid_tbl[i], &ocrdma_zero_sgid,
> > + sizeof(union ib_gid))) {
> > + /* found free entry */
> > + if (!found) {
> > + free_idx = i;
> > + found = true;
> > + break;
> > + }
> > + } else if (!memcmp(&dev->sgid_tbl[i], &new_sgid,
> > + sizeof(union ib_gid))) {
> > + /* entry already present, no addition is required. */
> > + spin_unlock_irqrestore(&dev->sgid_lock, flags);
> > + return;
> > + }
> > + }
> > + /* if entry doesn't exist and if table has some space, add entry */
> > + if (found)
> > + memcpy(&dev->sgid_tbl[free_idx], &new_sgid,
> > + sizeof(union ib_gid));
>
> can move memcpy into for loop and remove found flag
>
Yes, removed.
> > + spin_unlock_irqrestore(&dev->sgid_lock, flags); }
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/9] ocrdma: Driver for Emulex OneConnect RDMA adapter
Inline.
> -Original Message-
> From: Hefty, Sean [mailto:sean.he...@intel.com]
> Sent: Thursday, March 22, 2012 5:50 AM
> To: Pandit, Parav; linux-rdma@vger.kernel.org
> Subject: RE: [PATCH 4/9] ocrdma: Driver for Emulex OneConnect RDMA
> adapter
>
> > +static inline void *ocrdma_get_eqe(struct ocrdma_eq *eq) {
> > + return (u8 *)eq->q.va + (eq->q.tail * sizeof(struct ocrdma_eqe)); }
>
> casts from (void *) to (u8 *) are not needed. This occurs in multiple places.
Removed.
>
> > +enum ib_qp_state get_ibqp_state(enum ocrdma_qp_state qps) {
> > + switch (qps) {
> > + case OCRDMA_QPS_RST:
> > + return IB_QPS_RESET;
> > + case OCRDMA_QPS_INIT:
> > + return IB_QPS_INIT;
> > + case OCRDMA_QPS_RTR:
> > + return IB_QPS_RTR;
> > + case OCRDMA_QPS_RTS:
> > + return IB_QPS_RTS;
> > + case OCRDMA_QPS_SQD:
> > + case OCRDMA_QPS_SQ_DRAINING:
> > + return IB_QPS_SQD;
> > + case OCRDMA_QPS_SQE:
> > + return IB_QPS_SQE;
> > + case OCRDMA_QPS_ERR:
> > + return IB_QPS_ERR;
> > + };
> > + return IB_QPS_ERR;
> > +}
> > +
> > +enum ocrdma_qp_state get_ocrdma_qp_state(enum ib_qp_state qps) {
> > + switch (qps) {
> > + case IB_QPS_RESET:
> > + return OCRDMA_QPS_RST;
> > + case IB_QPS_INIT:
> > + return OCRDMA_QPS_INIT;
> > + case IB_QPS_RTR:
> > + return OCRDMA_QPS_RTR;
> > + case IB_QPS_RTS:
> > + return OCRDMA_QPS_RTS;
> > + case IB_QPS_SQD:
> > + return OCRDMA_QPS_SQD;
> > + case IB_QPS_SQE:
> > + return OCRDMA_QPS_SQE;
> > + case IB_QPS_ERR:
> > + return OCRDMA_QPS_ERR;
> > + };
> > + return OCRDMA_QPS_ERR;
> > +}
> > +
> > +static int ocrdma_get_mbx_errno(u32 status) {
> > + int err_num = -EFAULT;
>
> no need to initialize, since it's set in all cases
>
Removed.
> > + u8 mbox_status = (status & OCRDMA_MBX_RSP_STATUS_MASK) >>
> > + OCRDMA_MBX_RSP_STATUS_SHIFT;
> > + u8 add_status = (status & OCRDMA_MBX_RSP_ASTATUS_MASK) >>
> > +
> OCRDMA_MBX_RSP_ASTATUS_SHIFT;
> > +
> > + switch (mbox_status) {
> > + case OCRDMA_MBX_STATUS_OOR:
> > + case OCRDMA_MBX_STATUS_MAX_QP_EXCEEDS:
> > + err_num = -EAGAIN;
> > + break;
> > +
> > + case OCRDMA_MBX_STATUS_INVALID_PD:
> > + case OCRDMA_MBX_STATUS_INVALID_CQ:
> > + case OCRDMA_MBX_STATUS_INVALID_SRQ_ID:
> > + case OCRDMA_MBX_STATUS_INVALID_QP:
> > + case OCRDMA_MBX_STATUS_INVALID_CHANGE:
> > + case OCRDMA_MBX_STATUS_MTU_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_INVALID_RNR_NAK_TIMER:
> > + case OCRDMA_MBX_STATUS_PKEY_INDEX_INVALID:
> > + case OCRDMA_MBX_STATUS_PKEY_INDEX_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_ILLEGAL_FIELD:
> > + case OCRDMA_MBX_STATUS_INVALID_PBL_ENTRY:
> > + case OCRDMA_MBX_STATUS_INVALID_LKEY:
> > + case OCRDMA_MBX_STATUS_INVALID_VA:
> > + case OCRDMA_MBX_STATUS_INVALID_LENGTH:
> > + case OCRDMA_MBX_STATUS_INVALID_FBO:
> > + case OCRDMA_MBX_STATUS_INVALID_ACC_RIGHTS:
> > + case OCRDMA_MBX_STATUS_INVALID_PBE_SIZE:
> > + case OCRDMA_MBX_STATUS_ATOMIC_OPS_UNSUP:
> > + case OCRDMA_MBX_STATUS_SRQ_ERROR:
> > + case OCRDMA_MBX_STATUS_SRQ_SIZE_UNDERUNS:
> > + err_num = -EINVAL;
> > + break;
> > +
> > + case OCRDMA_MBX_STATUS_PD_INUSE:
> > + case OCRDMA_MBX_STATUS_QP_BOUND:
> > + case OCRDMA_MBX_STATUS_MW_STILL_BOUND:
> > + case OCRDMA_MBX_STATUS_MW_BOUND:
> > + err_num = -EBUSY;
> > + break;
> > +
> > + case OCRDMA_MBX_STATUS_RECVQ_RQE_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_SGE_RECV_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_RQE_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_SRQ_LIMIT_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_ORD_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_IRD_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_SENDQ_WQE_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_SGE_SEND_EXCEEDS:
> > + case OCRDMA_MBX_STATUS_SGE_WRITE_EXCEEDS:
> > + err_num = -ENOBUFS;
> > + break;
> > +
> > + case OCRDMA_MBX_STATUS_FAILED:
> > + switch (add_status) {
> > + case
> OCRDMA_MBX_ADDI_STATUS_INSUFFICIENT_RESOURCES:
> > + err_num = -EAGAIN;
> > + break;
> > + }
> > + default:
> > + err_num = -EFAULT;
> > + }
> > + return err_num;
> > +}
> > +
> > +static int ocrdma_get_mbx_cqe_errno(u16 cqe_status) {
> > + int err_num = -EINVAL;
>
> no need to initialize
>
> > + switch (cqe_status) {
> > + case OCRDMA_MBX_CQE_STATUS_INSUFFICIENT_PRIVILEDGES:
> > + err_num = -EPERM;
> > + break;
> > + case OCRDMA_MBX_CQE_STATUS_INVALID_PARAMETER:
> > + err_num = -EINVAL;
> > + break;
> > + case OCRDMA_MBX_CQE_STATUS_INSUFFICIENT_RESOURCES:
> > + case OCRDMA_MBX_CQE_STATUS_QUEUE_FLUSHING:
> > + err_num = -EAGAIN;
> > + break;
> > + case OCRDMA_MBX_CQE_STATUS_DMA_FAILED:
> > + err_num = -EIO;
> > +
