Skip to contentSkip to conversation

libibnetdisc/ibnetdisc.c: fix insert of invalid lid 0xFFFF into lid_port hash_table

PATCH Applied Hal Rosenstock 2 participants 4 messages Dec 30, 2013 – Jan 25, 2014 3 messages not in the archive InfiniBand

lore
Share & tools
lore

Hal Rosenstock

author
#0 lore raw

PATCH 1/1 libibnetdisc/ibnetdisc.c: fix insert of invalid lid 0xFFFF into lid_port hash_table

From: Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 libibnetdisc/src/ibnetdisc.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
libibnetdisc/src/ibnetdisc.c +7 -5
index 9d120dd..121fe35 100644
@@ -647,11 +647,13 @@ void add_to_portlid_hash(ibnd_port_t * port, GHashTable *htable)
 	uint16_t base_lid = port->base_lid;
 	uint16_t lid_mask = ((1 << port->lmc) -1);
 	uint16_t lid = 0;
-
-	/* We add the port for all lids
-	 * so it is easier to find any "random" lid specified */
-	for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
-		g_hash_table_insert(htable, GINT_TO_POINTER(lid), port);
+	/* 0 < valid lid <= 0xbfff */
+	if (base_lid > 0 && base_lid <= 0xbfff) {
+		/* We add the port for all lids
+		 * so it is easier to find any "random" lid specified */
+		for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
+			g_hash_table_insert(htable, GINT_TO_POINTER(lid), port);
+		}
 	}
 }
Signature · 6 lines
1.7.8.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 52C18853.8080107-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

#2 lore raw

Good catch some comments below.

Quoted 21 lines

-----Original Message-----
From: Hal Rosenstock [mailto:[email protected]]
Sent: Monday, December 30, 2013 6:51 AM
To: Ira Weiny
Cc: linux-rdma ([email protected]); Dan Ben-Yosef
Subject: [PATCH infiniband-diags] libibnetdisc/ibnetdisc.c: fix insert of invalid lid 0xFFFF into lid_port hash_table

From: Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 libibnetdisc/src/ibnetdisc.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
libibnetdisc/src/ibnetdisc.c index renamed from libibnetdisc/src/ibnetdisc.c +0 -0

9d120dd..121fe35 100644

libibnetdisc/src/ibnetdisc.c +0 -0
@@ -647,11 +647,13 @@ void add_to_portlid_hash(ibnd_port_t * port,
 GHashTable *htable)

libibnetdisc/src/ibnetdisc.c void add_to_portlid_hash(ibnd_port_t * port, · line 652

Quoted 10 lines
libibnetdisc/src/ibnetdisc.c +2 -5
 	uint16_t base_lid = port->base_lid;
 	uint16_t lid_mask = ((1 << port->lmc) -1);
 	uint16_t lid = 0;
-
-	/* We add the port for all lids
-	 * so it is easier to find any "random" lid specified */
-	for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
-		g_hash_table_insert(htable, GINT_TO_POINTER(lid), port);
+	/* 0 < valid lid <= 0xbfff */
+	if (base_lid > 0 && base_lid <= 0xbfff) {

Shouldn't we check the "max LID" (based on LMC) here to make sure they are all vaild?

Ira

Quoted 11 lines
libibnetdisc/src/ibnetdisc.c +5 -0
+		/* We add the port for all lids
+		 * so it is easier to find any "random" lid specified */
+		for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
+			g_hash_table_insert(htable, GINT_TO_POINTER(lid),
 port);
+		}
 	}

}

--
1.7.8.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 2807E5FD2F6FDA4886F6618EAC48510E04A93D22-8k97q/[email protected]

author
#4 lore raw
Quoted 27 lines · Ira

Good catch some comments below.

Quoted 25 lines

-----Original Message-----
From: Hal Rosenstock [mailto:[email protected]]
Sent: Monday, December 30, 2013 6:51 AM
To: Ira Weiny
Cc: linux-rdma ([email protected]); Dan Ben-Yosef
Subject: [PATCH infiniband-diags] libibnetdisc/ibnetdisc.c: fix insert of invalid lid 0xFFFF into lid_port hash_table

From: Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 libibnetdisc/src/ibnetdisc.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
libibnetdisc/src/ibnetdisc.c index renamed from libibnetdisc/src/ibnetdisc.c +0 -0

9d120dd..121fe35 100644

libibnetdisc/src/ibnetdisc.c +0 -1
@@ -647,11 +647,13 @@ void add_to_portlid_hash(ibnd_port_t * port,
 GHashTable *htable)
 	uint16_t base_lid = port->base_lid;
 	uint16_t lid_mask = ((1 << port->lmc) -1);
 	uint16_t lid = 0;
-
Quoted 8 lines

libibnetdisc/src/ibnetdisc.c void add_to_portlid_hash(ibnd_port_t * port, GHashTable *htable) · line 651

Quoted 6 lines
libibnetdisc/src/ibnetdisc.c +2 -4
-	/* We add the port for all lids
-	 * so it is easier to find any "random" lid specified */
-	for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
-		g_hash_table_insert(htable, GINT_TO_POINTER(lid), port);
+	/* 0 < valid lid <= 0xbfff */
+	if (base_lid > 0 && base_lid <= 0xbfff) {

Shouldn't we check the "max LID" (based on LMC) here to make sure they are all vaild?

I guess it depends how strict we want to be for this check and whether it's consistent with other such checks in the library.

I think that the check of the base LID based on the LMC should be a separate additional check. Is this currently done anywhere else in the library ?

-- Hal

Quoted 12 lines

Ira

Quoted 9 lines
libibnetdisc/src/ibnetdisc.c +5 -0
+		/* We add the port for all lids
+		 * so it is easier to find any "random" lid specified */
+		for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
+			g_hash_table_insert(htable, GINT_TO_POINTER(lid),
 port);
+		}
 	}

}

Quoted 4 lines
Quoted 2 lines

--
1.7.8.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 52D3E42C.4030201-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

Applied
#6 lore raw
Quoted 45 lines

-----Original Message-----
From: Hal Rosenstock [mailto:[email protected]]
Sent: Monday, January 13, 2014 5:04 AM
To: Weiny, Ira
Cc: Ira Weiny; linux-rdma ([email protected]); Dan Ben-Yosef
Subject: Re: [PATCH infiniband-diags] libibnetdisc/ibnetdisc.c: fix insert of invalid lid 0xFFFF into lid_port hash_table

Quoted 28 lines · Ira

Good catch some comments below.

Quoted 26 lines

-----Original Message-----
From: Hal Rosenstock [mailto:[email protected]]
Sent: Monday, December 30, 2013 6:51 AM
To: Ira Weiny
Cc: linux-rdma ([email protected]); Dan Ben-Yosef
Subject: [PATCH infiniband-diags] libibnetdisc/ibnetdisc.c: fix
insert of invalid lid 0xFFFF into lid_port hash_table

From: Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>

Signed-off-by Dan Ben Yosef <danby-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 libibnetdisc/src/ibnetdisc.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
(quoted diff) +0 -0

b/libibnetdisc/src/ibnetdisc.c index
9d120dd..121fe35 100644

libibnetdisc/src/ibnetdisc.c +0 -1
@@ -647,11 +647,13 @@ void add_to_portlid_hash(ibnd_port_t * port,
 GHashTable *htable)
 	uint16_t base_lid = port->base_lid;
 	uint16_t lid_mask = ((1 << port->lmc) -1);
 	uint16_t lid = 0;
-
Quoted 8 lines

libibnetdisc/src/ibnetdisc.c void add_to_portlid_hash(ibnd_port_t * port, GHashTable *htable) · line 651

Quoted 6 lines
libibnetdisc/src/ibnetdisc.c +2 -4
-	/* We add the port for all lids
-	 * so it is easier to find any "random" lid specified */
-	for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
-		g_hash_table_insert(htable, GINT_TO_POINTER(lid), port);
+	/* 0 < valid lid <= 0xbfff */
+	if (base_lid > 0 && base_lid <= 0xbfff) {

Shouldn't we check the "max LID" (based on LMC) here to make sure they

Quoted 4 lines

are all vaild?

I guess it depends how strict we want to be for this check and whether it's consistent with other such checks in the library.

As this fixes a bug I'll apply it.

Thanks applied.

Quoted 3 lines

I think that the check of the base LID based on the LMC should be a separate additional check. Is this currently done anywhere else in the library ?

The library does not perform any checks as it does not rely on the lids for any processing. It is simply reporting what is out there. So things like duplicate LID's could be checked for.

Ira

Quoted 19 lines

-- Hal

Quoted 16 lines

Ira

Quoted 11 lines
libibnetdisc/src/ibnetdisc.c +5 -0
+		/* We add the port for all lids
+		 * so it is easier to find any "random" lid specified */
+		for (lid = base_lid; lid <= (base_lid + lid_mask); lid++) {
+			g_hash_table_insert(htable, GINT_TO_POINTER(lid),
 port);
+		}
 	}

}

--
1.7.8.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html