opensm/osm_lid_mgr.c: Allow switch lids to be non LMC aligned

PATCH Applied Eli Dorfman (Voltaire) 3 participants 5 messages Jun 29, 2010 – Jul 5, 2010 3 messages not in the archive InfiniBand

lore

Share & tools

lore

Eli Dorfman (Voltaire)

author

2010-06-29 13:49 UTC #0 lore raw

PATCH 1/1 opensm/osm_lid_mgr.c: Allow switch lids to be non LMC aligned

Fixes a bug of failover between SM's that are running on a switch. Both have lids below LMC start lid (equals to 2^LMC - 1)
In case of failover the LID range between 1 and start lid was considered not free and their (switch's) lids were changed.

Signed-off-by Eli Dorfman <elid-smomgflXvOZWk0Htik3J/[email protected]>

---
 opensm/opensm/osm_lid_mgr.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

opensm/opensm/osm_lid_mgr.c +0 -4

index 97b1de6..5d0247a 100644
@@ -563,10 +563,6 @@ static boolean_t lid_mgr_is_range_not_persistent(IN osm_lid_mgr_t * p_mgr,
 						 IN uint16_t num_lids)
 {
 	uint16_t i;
-	uint8_t start_lid = (uint8_t) (1 << p_mgr->p_subn->opt.lmc);
-
-	if (lid < start_lid)
-		return FALSE;
 
 	for (i = lid; i < lid + num_lids; i++)
 		if (p_mgr->used_lids[i])

Signature · 6 lines

1.5.3.6

--
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

Hal Rosenstock

2010-06-30 13:01 UTC #2 lore raw

In reply to a message not in the archive

Quoted 5 lines · Eli Dorfman (Voltaire)

Fixes a bug of failover between SM's that are running on a switch. Both have lids below LMC start lid (equals to 2^LMC - 1)
In case of failover the LID range between 1 and start lid was considered not free and their (switch's) lids were changed.

Was the problem case for this when LMC > 0 but num_lids is 1 ?

Quoted 10 lines

Signed-off-by Eli Dorfman <elid-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_lid_mgr.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
opensm/opensm/osm_lid_mgr.c +0 -0
index 97b1de6..5d0247a 100644

opensm/opensm/osm_lid_mgr.c static boolean_t lid_mgr_is_range_not_persistent(IN osm_lid_mgr_t * p_mgr, · line 569

Quoted 7 lines

opensm/opensm/osm_lid_mgr.c +0 -4

@@ -563,10 +563,6 @@ static boolean_t lid_mgr_is_range_not_persistent(IN osm_lid_mgr_t * p_mgr,
                                                IN uint16_t num_lids)
 {
       uint16_t i;
-       uint8_t start_lid = (uint8_t) (1 << p_mgr->p_subn->opt.lmc);
-
-       if (lid < start_lid)
-               return FALSE;

I think eliminating this test can cause other breakage.

Shouldn't it be the following:

if (num_lids > 1 && lid < start_lid)

    return FALSE;

-- Hal

Quoted 10 lines

opensm/opensm/osm_lid_mgr.c +0 -1
       for (i = lid; i < lid + num_lids; i++)
               if (p_mgr->used_lids[i])
--
1.5.3.6
--
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

--
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

Eli Dorfman (Voltaire)

author

2010-06-30 14:44 UTC #4 lore raw

In reply to a message not in the archive

Quoted 8 lines · Hal Rosenstock

Quoted 4 lines · Eli Dorfman (Voltaire)
Fixes a bug of failover between SM's that are running on a switch. Both have lids below LMC start lid (equals to 2^LMC - 1)
In case of failover the LID range between 1 and start lid was considered not free and their (switch's) lids were changed.
Was the problem case for this when LMC > 0 but num_lids is 1 ?

yes, it was for SM running on a switch.

Quoted 21 lines

Quoted 11 lines
Signed-off-by Eli Dorfman <elid-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_lid_mgr.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
opensm/opensm/osm_lid_mgr.c +0 -0
index 97b1de6..5d0247a 100644
opensm/opensm/osm_lid_mgr.c static boolean_t lid_mgr_is_range_not_persistent(IN osm_lid_mgr_t * p_mgr, · line 569
Quoted 7 lines
opensm/opensm/osm_lid_mgr.c +0 -4
@@ -563,10 +563,6 @@ static boolean_t lid_mgr_is_range_not_persistent(IN osm_lid_mgr_t * p_mgr,
                                                IN uint16_t num_lids)
 {
       uint16_t i;
-       uint8_t start_lid = (uint8_t) (1 << p_mgr->p_subn->opt.lmc);
-
-       if (lid < start_lid)
-               return FALSE;
I think eliminating this test can cause other breakage.

Quoted 4 lines

Shouldn't it be the following:
if (num_lids > 1 && lid < start_lid)
    return FALSE;

This could be ok, but even without this test it should work since this function is called only if ((min_lid & lmc_mask) == min_lid)

So this would be false for any lid below start_lid.

right?

Eli

Quoted 13 lines

-- Hal
Quoted 10 lines
opensm/opensm/osm_lid_mgr.c +0 -1
       for (i = lid; i < lid + num_lids; i++)
               if (p_mgr->used_lids[i])
--
1.5.3.6
--
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

--
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

Hal Rosenstock

2010-06-30 15:23 UTC #6 lore raw

In reply to a message not in the archive

Quoted 9 lines · Eli Dorfman (Voltaire)

Quoted 8 lines · Hal Rosenstock
Quoted 5 lines · Eli Dorfman (Voltaire)
Fixes a bug of failover between SM's that are running on a switch. Both have lids below LMC start lid (equals to 2^LMC - 1)
In case of failover the LID range between 1 and start lid was considered not free
and their (switch's) lids were changed.

Quoted 3 lines

Quoted 1 line
Was the problem case for this when LMC > 0 but num_lids is 1 ?
yes, it was for SM running on a switch.

There are two cases for SM running on a switch. There is also the case where num_lids > 1 which should work without this change.

Quoted 28 lines

Quoted 23 lines
Quoted 12 lines
Signed-off-by Eli Dorfman <elid-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_lid_mgr.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
opensm/opensm/osm_lid_mgr.c +0 -0
index 97b1de6..5d0247a 100644
opensm/opensm/osm_lid_mgr.c static boolean_t lid_mgr_is_range_not_persistent(IN · line 570
Quoted 8 lines
opensm/opensm/osm_lid_mgr.c +0 -4
@@ -563,10 +563,6 @@ static boolean_t lid_mgr_is_range_not_persistent(IN
 osm_lid_mgr_t * p_mgr,
                                                IN uint16_t num_lids)
 {
       uint16_t i;
-       uint8_t start_lid = (uint8_t) (1 << p_mgr->p_subn->opt.lmc);
-
-       if (lid < start_lid)
-               return FALSE;
I think eliminating this test can cause other breakage.
Quoted 4 lines
Shouldn't it be the following:
if (num_lids > 1 && lid < start_lid)
    return FALSE;

Quoted 7 lines

This could be ok, but even without this test it should work since this function is called only
if ((min_lid & lmc_mask) == min_lid)
So this would be false for any lid below start_lid.
right?

Right (so lid_mgr_is_range_not_persistent wouldn't be called for the case that I was concerned about).

-- Hal

Quoted 18 lines

Eli
Quoted 13 lines
-- Hal
Quoted 10 lines
opensm/opensm/osm_lid_mgr.c +0 -1
       for (i = lid; i < lid + num_lids; i++)
               if (p_mgr->used_lids[i])
--
1.5.3.6
--
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

--
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

Sasha Khapyorsky

Applied

2010-07-05 18:04 UTC #7 lore raw

In reply to a message not in the archive

Quoted 7 lines · Eli Dorfman (Voltaire)

Fixes a bug of failover between SM's that are running on a switch. Both have lids below LMC start lid (equals to 2^LMC - 1)
In case of failover the LID range between 1 and start lid was considered not free and their (switch's) lids were changed.
Signed-off-by Eli Dorfman <elid-smomgflXvOZWk0Htik3J/[email protected]>

Applied. Thanks.

Sasha
--
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