| Message ID | 20140625153020.18938.22186.stgit@phlsvslse11.ph.intel.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
Hi Alex, Still there is an issue here, please try the following: 1. pkey table contains the pkey 8001 2. echo 0x8001 > /sys/class/net/ib0/create_child ; ifconfig ib0.8001 1.1.1.1 up - till now all good. 3. change the sm partiotion file, take out the pkey value 8001 4. force the sm to send the new event (PKEY_CHANGE_EVENT) via pkill -HUP opensm 5. now return back the pkey 8001 to the partiotion file 6. again, force the sm to send the new event (PKEY_CHANGE_EVENT) via pkill -HUP opensm The new interface ib0.8001 remains "down" and its carrier is 0. please check that, Thanks, Erez > Currently, the parent interface keeps sending broadcast group join > requests even if p_key index 0 is invalid, which for itself is > possible/common in virtualized environment where a VF has been probed to > VM but the actual p_key configuration has not yet been assigned by the > management software. This creates unnecessary noise on the fabric and in > the kernel logs: > > ib0: multicast join failed for ff12:401b:8000:0000:0000:0000:ffff:ffff, > status -22 > > The original code run the multicast task regardless of the actual > p_key value, which can be avoided. The fix is to re-init resources and > bring interface up only if p_key index 0 is valid either when starting > up or on PKEY_CHANGE event. > > Fixes: c290414169 ('IPoIB: Fix pkey change flow for virtualization environments') > > Reviewed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Alex Estrin <alex.estrin@intel.com> > --- > Changes from v4: > - streamline child interface pkey event handling, > - shutdown of pkey polling thread depends on PKEY_STOP flag state only. > Original poib_ib_dev_down() could leave polling thread active > if PKEY_ASSIGNED flag was set. That could create a racing condition on followed > re-initialization of QP resources. > --- > drivers/infiniband/ulp/ipoib/ipoib_ib.c | 59 +++++++++++++++---------------- > 1 files changed, 29 insertions(+), 30 deletions(-) > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c > index 6a7003d..ac941e1 100644 > --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c > @@ -52,6 +52,7 @@ MODULE_PARM_DESC(data_debug_level, > #endif > > static DEFINE_MUTEX(pkey_mutex); > +static void ipoib_pkey_dev_check_presence(struct net_device *dev); > > struct ipoib_ah *ipoib_create_ah(struct net_device *dev, > struct ib_pd *pd, struct ib_ah_attr *attr) > @@ -669,12 +670,13 @@ int ipoib_ib_dev_open(struct net_device *dev) > struct ipoib_dev_priv *priv = netdev_priv(dev); > int ret; > > - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &priv->pkey_index)) { > - ipoib_warn(priv, "P_Key 0x%04x not found\n", priv->pkey); > - clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); > + ipoib_pkey_dev_check_presence(dev); > + > + if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) { > + ipoib_warn(priv, "P_Key 0x%04x is %s\n", priv->pkey, > + !(priv->pkey & 0x7fff) ? "Invalid" : "not found"); > return -1; > } > - set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); > > ret = ipoib_init_qp(dev); > if (ret) { > @@ -712,9 +714,10 @@ dev_stop: > static void ipoib_pkey_dev_check_presence(struct net_device *dev) > { > struct ipoib_dev_priv *priv = netdev_priv(dev); > - u16 pkey_index = 0; > > - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &pkey_index)) > + if (!(priv->pkey & 0x7fff) || > + ib_find_pkey(priv->ca, priv->port, priv->pkey, > + &priv->pkey_index)) > clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); > else > set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); > @@ -746,12 +749,10 @@ int ipoib_ib_dev_down(struct net_device *dev, int flush) > netif_carrier_off(dev); > > /* Shutdown the P_Key thread if still active */ > - if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) { > - mutex_lock(&pkey_mutex); > - set_bit(IPOIB_PKEY_STOP, &priv->flags); > + mutex_lock(&pkey_mutex); > + if (!test_and_set_bit(IPOIB_PKEY_STOP, &priv->flags)) > cancel_delayed_work_sync(&priv->pkey_poll_task); > - mutex_unlock(&pkey_mutex); > - } > + mutex_unlock(&pkey_mutex); > > ipoib_mcast_stop_thread(dev, flush); > ipoib_mcast_dev_flush(dev); > @@ -972,7 +973,7 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, > { > struct ipoib_dev_priv *cpriv; > struct net_device *dev = priv->dev; > - u16 new_index; > + u16 old_index; > int result; > > down_read(&priv->vlan_rwsem); > @@ -986,16 +987,17 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, > > up_read(&priv->vlan_rwsem); > > - if (!test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags)) { > - /* for non-child devices must check/update the pkey value here */ > - if (level == IPOIB_FLUSH_HEAVY && > - !test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) > - update_parent_pkey(priv); > + if (!test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags) && > + level != IPOIB_FLUSH_HEAVY) { > ipoib_dbg(priv, "Not flushing - IPOIB_FLAG_INITIALIZED not set.\n"); > return; > } > > if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) { > + /* interface is down. update pkey and leave. */ > + if (level == IPOIB_FLUSH_HEAVY && > + !test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) > + update_parent_pkey(priv); > ipoib_dbg(priv, "Not flushing - IPOIB_FLAG_ADMIN_UP not set.\n"); > return; > } > @@ -1005,20 +1007,15 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, > * (parent) devices should always takes what present in pkey index 0 > */ > if (test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) { > - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &new_index)) { > - clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); > - ipoib_ib_dev_down(dev, 0); > - ipoib_ib_dev_stop(dev, 0); > - if (ipoib_pkey_dev_delay_open(dev)) > - return; > - } > - /* restart QP only if P_Key index is changed */ > - if (test_and_set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags) && > - new_index == priv->pkey_index) { > + old_index = priv->pkey_index; > + ipoib_pkey_dev_check_presence(dev); > + > + if (test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags) && > + old_index == priv->pkey_index) { > + /* restart QP only if P_Key index is changed */ > ipoib_dbg(priv, "Not flushing - P_Key index not changed.\n"); > return; > } > - priv->pkey_index = new_index; > } else { > result = update_parent_pkey(priv); > /* restart QP only if P_Key value changed */ > @@ -1038,8 +1035,10 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, > ipoib_ib_dev_down(dev, 0); > > if (level == IPOIB_FLUSH_HEAVY) { > - ipoib_ib_dev_stop(dev, 0); > - ipoib_ib_dev_open(dev); > + if (test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags)) > + ipoib_ib_dev_stop(dev, 0); > + if (ipoib_ib_dev_open(dev) != 0) > + return; > } > > /* > > -- > 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 > -- 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, Jul 2, 2014 at 10:18 AM, Erez Shitrit <erezsh@dev.mellanox.co.il> wrote: > Still there is an issue here, please try the following: > 1. pkey table contains the pkey 8001 > 2. echo 0x8001 > /sys/class/net/ib0/create_child ; ifconfig ib0.8001 1.1.1.1 > up - till now all good. > 3. change the sm partiotion file, take out the pkey value 8001 > 4. force the sm to send the new event (PKEY_CHANGE_EVENT) via pkill -HUP opensm Just to make sure... the pkey change IB event isn't "sent" by the SM to applications. The HUP signal causes opensm to re-read configuration files and act accordingly, e.g to configure the modified partitioning scheme. The driver in turn delivers the event to the application. -- 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
SGkgRXJleiwNCg0KWWVwLCBJIGhhdmVuJ3QgY2xlYW51cCBwa2V5X2luZGV4IGJlZm9yZSBwX2tl eSBsb29rdXAgYW5kIGNvbXBhcmUsDQpzbyBjaGlsZCB3b24ndCBmbHVzaC9nZXQgYmFjayB1cCBv biBwX2tleSByZWFwcGVhcmFuY2UgZXZlbnQgDQppZiBpbmRleCBzdGF5cyB0aGUgc2FtZS4NCg0K VGhhbmtzLA0KQWxleC4NCg0KPiBTdGlsbCB0aGVyZSBpcyBhbiBpc3N1ZSBoZXJlLCBwbGVhc2Ug dHJ5IHRoZSBmb2xsb3dpbmc6DQo+IA0KPiAxLiBwa2V5IHRhYmxlIGNvbnRhaW5zIHRoZSBwa2V5 IDgwMDENCj4gMi4gZWNobyAweDgwMDEgPiAvc3lzL2NsYXNzL25ldC9pYjAvY3JlYXRlX2NoaWxk IDsgaWZjb25maWcgaWIwLjgwMDENCj4gMS4xLjEuMSB1cCAtIHRpbGwgbm93IGFsbCBnb29kLg0K PiAzLiBjaGFuZ2UgdGhlIHNtIHBhcnRpb3Rpb24gZmlsZSwgdGFrZSBvdXQgdGhlIHBrZXkgdmFs dWUgODAwMQ0KPiA0LiBmb3JjZSB0aGUgc20gdG8gc2VuZCB0aGUgbmV3IGV2ZW50IChQS0VZX0NI QU5HRV9FVkVOVCkgdmlhIHBraWxsIC1IVVANCj4gb3BlbnNtDQo+IDUuIG5vdyByZXR1cm4gYmFj ayB0aGUgcGtleSA4MDAxIHRvIHRoZSBwYXJ0aW90aW9uIGZpbGUNCj4gNi4gYWdhaW4sIGZvcmNl IHRoZSBzbSB0byBzZW5kIHRoZSBuZXcgZXZlbnQgKFBLRVlfQ0hBTkdFX0VWRU5UKSB2aWENCj4g cGtpbGwgLUhVUCBvcGVuc20NCj4gDQo+IFRoZSBuZXcgaW50ZXJmYWNlIGliMC44MDAxIHJlbWFp bnMgImRvd24iIGFuZCBpdHMgY2FycmllciBpcyAwLg0KPiBwbGVhc2UgY2hlY2sgdGhhdCwNCj4g DQo+IFRoYW5rcywgRXJleg0KPiANCj4gPiBDdXJyZW50bHksIHRoZSBwYXJlbnQgaW50ZXJmYWNl IGtlZXBzIHNlbmRpbmcgYnJvYWRjYXN0IGdyb3VwIGpvaW4NCj4gPiByZXF1ZXN0cyBldmVuIGlm IHBfa2V5IGluZGV4IDAgaXMgaW52YWxpZCwgd2hpY2ggZm9yIGl0c2VsZiBpcw0KPiA+IHBvc3Np YmxlL2NvbW1vbiBpbiB2aXJ0dWFsaXplZCBlbnZpcm9ubWVudCB3aGVyZSBhIFZGIGhhcyBiZWVu IHByb2JlZCB0bw0KPiA+IFZNIGJ1dCB0aGUgYWN0dWFsIHBfa2V5IGNvbmZpZ3VyYXRpb24gaGFz IG5vdCB5ZXQgYmVlbiBhc3NpZ25lZCBieSB0aGUNCj4gPiBtYW5hZ2VtZW50IHNvZnR3YXJlLiBU aGlzIGNyZWF0ZXMgdW5uZWNlc3Nhcnkgbm9pc2Ugb24gdGhlIGZhYnJpYyBhbmQgaW4NCj4gPiB0 aGUga2VybmVsIGxvZ3M6DQo+ID4NCj4gPiBpYjA6IG11bHRpY2FzdCBqb2luIGZhaWxlZCBmb3Ig ZmYxMjo0MDFiOjgwMDA6MDAwMDowMDAwOjAwMDA6ZmZmZjpmZmZmLA0KPiA+IHN0YXR1cyAtMjIN Cj4gPg0KPiA+IFRoZSBvcmlnaW5hbCBjb2RlIHJ1biB0aGUgbXVsdGljYXN0IHRhc2sgcmVnYXJk bGVzcyBvZiB0aGUgYWN0dWFsDQo+ID4gcF9rZXkgdmFsdWUsIHdoaWNoIGNhbiBiZSBhdm9pZGVk LiBUaGUgZml4IGlzIHRvIHJlLWluaXQgcmVzb3VyY2VzICBhbmQNCj4gPiBicmluZyBpbnRlcmZh Y2UgdXAgb25seSBpZiBwX2tleSBpbmRleCAwIGlzIHZhbGlkIGVpdGhlciB3aGVuIHN0YXJ0aW5n DQo+ID4gdXAgb3Igb24gUEtFWV9DSEFOR0UgZXZlbnQuDQo+ID4NCj4gPiBGaXhlczogYzI5MDQx NDE2OSAoJ0lQb0lCOiBGaXggcGtleSBjaGFuZ2UgZmxvdyBmb3IgdmlydHVhbGl6YXRpb24gZW52 aXJvbm1lbnRzJykNCj4gPg0KPiA+IFJldmlld2VkLWJ5OiBJcmEgV2VpbnkgPGlyYS53ZWlueUBp bnRlbC5jb20+DQo+ID4gU2lnbmVkLW9mZi1ieTogQWxleCBFc3RyaW4gPGFsZXguZXN0cmluQGlu dGVsLmNvbT4NCj4gPiAtLS0NCj4gPiBDaGFuZ2VzIGZyb20gdjQ6DQo+ID4gLSBzdHJlYW1saW5l IGNoaWxkIGludGVyZmFjZSBwa2V5IGV2ZW50IGhhbmRsaW5nLA0KPiA+IC0gc2h1dGRvd24gb2Yg cGtleSBwb2xsaW5nIHRocmVhZCBkZXBlbmRzIG9uIFBLRVlfU1RPUCBmbGFnIHN0YXRlIG9ubHku DQo+ID4gT3JpZ2luYWwgcG9pYl9pYl9kZXZfZG93bigpIGNvdWxkIGxlYXZlIHBvbGxpbmcgdGhy ZWFkIGFjdGl2ZQ0KPiA+IGlmIFBLRVlfQVNTSUdORUQgZmxhZyB3YXMgc2V0LiBUaGF0IGNvdWxk IGNyZWF0ZSBhIHJhY2luZyBjb25kaXRpb24gb24gZm9sbG93ZWQNCj4gPiByZS1pbml0aWFsaXph dGlvbiBvZiBRUCByZXNvdXJjZXMuDQo+ID4gLS0tDQo+ID4gICBkcml2ZXJzL2luZmluaWJhbmQv dWxwL2lwb2liL2lwb2liX2liLmMgfCAgIDU5ICsrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0t LS0NCj4gPiAgIDEgZmlsZXMgY2hhbmdlZCwgMjkgaW5zZXJ0aW9ucygrKSwgMzAgZGVsZXRpb25z KC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9pbmZpbmliYW5kL3VscC9pcG9pYi9p cG9pYl9pYi5jDQo+IGIvZHJpdmVycy9pbmZpbmliYW5kL3VscC9pcG9pYi9pcG9pYl9pYi5jDQo+ ID4gaW5kZXggNmE3MDAzZC4uYWM5NDFlMSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2luZmlu aWJhbmQvdWxwL2lwb2liL2lwb2liX2liLmMNCj4gPiArKysgYi9kcml2ZXJzL2luZmluaWJhbmQv dWxwL2lwb2liL2lwb2liX2liLmMNCj4gPiBAQCAtNTIsNiArNTIsNyBAQCBNT0RVTEVfUEFSTV9E RVNDKGRhdGFfZGVidWdfbGV2ZWwsDQo+ID4gICAjZW5kaWYNCj4gPg0KPiA+ICAgc3RhdGljIERF RklORV9NVVRFWChwa2V5X211dGV4KTsNCj4gPiArc3RhdGljIHZvaWQgaXBvaWJfcGtleV9kZXZf Y2hlY2tfcHJlc2VuY2Uoc3RydWN0IG5ldF9kZXZpY2UgKmRldik7DQo+ID4NCj4gPiAgIHN0cnVj dCBpcG9pYl9haCAqaXBvaWJfY3JlYXRlX2FoKHN0cnVjdCBuZXRfZGV2aWNlICpkZXYsDQo+ID4g ICAJCQkJIHN0cnVjdCBpYl9wZCAqcGQsIHN0cnVjdCBpYl9haF9hdHRyICphdHRyKQ0KPiA+IEBA IC02NjksMTIgKzY3MCwxMyBAQCBpbnQgaXBvaWJfaWJfZGV2X29wZW4oc3RydWN0IG5ldF9kZXZp Y2UgKmRldikNCj4gPiAgIAlzdHJ1Y3QgaXBvaWJfZGV2X3ByaXYgKnByaXYgPSBuZXRkZXZfcHJp dihkZXYpOw0KPiA+ICAgCWludCByZXQ7DQo+ID4NCj4gPiAtCWlmIChpYl9maW5kX3BrZXkocHJp di0+Y2EsIHByaXYtPnBvcnQsIHByaXYtPnBrZXksICZwcml2LT5wa2V5X2luZGV4KSkgew0KPiA+ IC0JCWlwb2liX3dhcm4ocHJpdiwgIlBfS2V5IDB4JTA0eCBub3QgZm91bmRcbiIsIHByaXYtPnBr ZXkpOw0KPiA+IC0JCWNsZWFyX2JpdChJUE9JQl9QS0VZX0FTU0lHTkVELCAmcHJpdi0+ZmxhZ3Mp Ow0KPiA+ICsJaXBvaWJfcGtleV9kZXZfY2hlY2tfcHJlc2VuY2UoZGV2KTsNCj4gPiArDQo+ID4g KwlpZiAoIXRlc3RfYml0KElQT0lCX1BLRVlfQVNTSUdORUQsICZwcml2LT5mbGFncykpIHsNCj4g PiArCQlpcG9pYl93YXJuKHByaXYsICJQX0tleSAweCUwNHggaXMgJXNcbiIsIHByaXYtPnBrZXks DQo+ID4gKwkJCSAgICEocHJpdi0+cGtleSAmIDB4N2ZmZikgPyAiSW52YWxpZCIgOiAibm90IGZv dW5kIik7DQo+ID4gICAJCXJldHVybiAtMTsNCj4gPiAgIAl9DQo+ID4gLQlzZXRfYml0KElQT0lC X1BLRVlfQVNTSUdORUQsICZwcml2LT5mbGFncyk7DQo+ID4NCj4gPiAgIAlyZXQgPSBpcG9pYl9p bml0X3FwKGRldik7DQo+ID4gICAJaWYgKHJldCkgew0KPiA+IEBAIC03MTIsOSArNzE0LDEwIEBA IGRldl9zdG9wOg0KPiA+ICAgc3RhdGljIHZvaWQgaXBvaWJfcGtleV9kZXZfY2hlY2tfcHJlc2Vu Y2Uoc3RydWN0IG5ldF9kZXZpY2UgKmRldikNCj4gPiAgIHsNCj4gPiAgIAlzdHJ1Y3QgaXBvaWJf ZGV2X3ByaXYgKnByaXYgPSBuZXRkZXZfcHJpdihkZXYpOw0KPiA+IC0JdTE2IHBrZXlfaW5kZXgg PSAwOw0KPiA+DQo+ID4gLQlpZiAoaWJfZmluZF9wa2V5KHByaXYtPmNhLCBwcml2LT5wb3J0LCBw cml2LT5wa2V5LCAmcGtleV9pbmRleCkpDQo+ID4gKwlpZiAoIShwcml2LT5wa2V5ICYgMHg3ZmZm KSB8fA0KPiA+ICsJICAgIGliX2ZpbmRfcGtleShwcml2LT5jYSwgcHJpdi0+cG9ydCwgcHJpdi0+ cGtleSwNCj4gPiArCQkJICZwcml2LT5wa2V5X2luZGV4KSkNCj4gPiAgIAkJY2xlYXJfYml0KElQ T0lCX1BLRVlfQVNTSUdORUQsICZwcml2LT5mbGFncyk7DQo+ID4gICAJZWxzZQ0KPiA+ICAgCQlz ZXRfYml0KElQT0lCX1BLRVlfQVNTSUdORUQsICZwcml2LT5mbGFncyk7DQo+ID4gQEAgLTc0Niwx MiArNzQ5LDEwIEBAIGludCBpcG9pYl9pYl9kZXZfZG93bihzdHJ1Y3QgbmV0X2RldmljZSAqZGV2 LCBpbnQgZmx1c2gpDQo+ID4gICAJbmV0aWZfY2Fycmllcl9vZmYoZGV2KTsNCj4gPg0KPiA+ICAg CS8qIFNodXRkb3duIHRoZSBQX0tleSB0aHJlYWQgaWYgc3RpbGwgYWN0aXZlICovDQo+ID4gLQlp ZiAoIXRlc3RfYml0KElQT0lCX1BLRVlfQVNTSUdORUQsICZwcml2LT5mbGFncykpIHsNCj4gPiAt CQltdXRleF9sb2NrKCZwa2V5X211dGV4KTsNCj4gPiAtCQlzZXRfYml0KElQT0lCX1BLRVlfU1RP UCwgJnByaXYtPmZsYWdzKTsNCj4gPiArCW11dGV4X2xvY2soJnBrZXlfbXV0ZXgpOw0KPiA+ICsJ aWYgKCF0ZXN0X2FuZF9zZXRfYml0KElQT0lCX1BLRVlfU1RPUCwgJnByaXYtPmZsYWdzKSkNCj4g PiAgIAkJY2FuY2VsX2RlbGF5ZWRfd29ya19zeW5jKCZwcml2LT5wa2V5X3BvbGxfdGFzayk7DQo+ ID4gLQkJbXV0ZXhfdW5sb2NrKCZwa2V5X211dGV4KTsNCj4gPiAtCX0NCj4gPiArCW11dGV4X3Vu bG9jaygmcGtleV9tdXRleCk7DQo+ID4NCj4gPiAgIAlpcG9pYl9tY2FzdF9zdG9wX3RocmVhZChk ZXYsIGZsdXNoKTsNCj4gPiAgIAlpcG9pYl9tY2FzdF9kZXZfZmx1c2goZGV2KTsNCj4gPiBAQCAt OTcyLDcgKzk3Myw3IEBAIHN0YXRpYyB2b2lkIF9faXBvaWJfaWJfZGV2X2ZsdXNoKHN0cnVjdCBp cG9pYl9kZXZfcHJpdiAqcHJpdiwNCj4gPiAgIHsNCj4gPiAgIAlzdHJ1Y3QgaXBvaWJfZGV2X3By aXYgKmNwcml2Ow0KPiA+ICAgCXN0cnVjdCBuZXRfZGV2aWNlICpkZXYgPSBwcml2LT5kZXY7DQo+ ID4gLQl1MTYgbmV3X2luZGV4Ow0KPiA+ICsJdTE2IG9sZF9pbmRleDsNCj4gPiAgIAlpbnQgcmVz dWx0Ow0KPiA+DQo+ID4gICAJZG93bl9yZWFkKCZwcml2LT52bGFuX3J3c2VtKTsNCj4gPiBAQCAt OTg2LDE2ICs5ODcsMTcgQEAgc3RhdGljIHZvaWQgX19pcG9pYl9pYl9kZXZfZmx1c2goc3RydWN0 IGlwb2liX2Rldl9wcml2DQo+ICpwcml2LA0KPiA+DQo+ID4gICAJdXBfcmVhZCgmcHJpdi0+dmxh bl9yd3NlbSk7DQo+ID4NCj4gPiAtCWlmICghdGVzdF9iaXQoSVBPSUJfRkxBR19JTklUSUFMSVpF RCwgJnByaXYtPmZsYWdzKSkgew0KPiA+IC0JCS8qIGZvciBub24tY2hpbGQgZGV2aWNlcyBtdXN0 IGNoZWNrL3VwZGF0ZSB0aGUgcGtleSB2YWx1ZSBoZXJlICovDQo+ID4gLQkJaWYgKGxldmVsID09 IElQT0lCX0ZMVVNIX0hFQVZZICYmDQo+ID4gLQkJICAgICF0ZXN0X2JpdChJUE9JQl9GTEFHX1NV QklOVEVSRkFDRSwgJnByaXYtPmZsYWdzKSkNCj4gPiAtCQkJdXBkYXRlX3BhcmVudF9wa2V5KHBy aXYpOw0KPiA+ICsJaWYgKCF0ZXN0X2JpdChJUE9JQl9GTEFHX0lOSVRJQUxJWkVELCAmcHJpdi0+ ZmxhZ3MpICYmDQo+ID4gKwkgICAgbGV2ZWwgIT0gSVBPSUJfRkxVU0hfSEVBVlkpIHsNCj4gPiAg IAkJaXBvaWJfZGJnKHByaXYsICJOb3QgZmx1c2hpbmcgLSBJUE9JQl9GTEFHX0lOSVRJQUxJWkVE IG5vdCBzZXQuXG4iKTsNCj4gPiAgIAkJcmV0dXJuOw0KPiA+ICAgCX0NCj4gPg0KPiA+ICAgCWlm ICghdGVzdF9iaXQoSVBPSUJfRkxBR19BRE1JTl9VUCwgJnByaXYtPmZsYWdzKSkgew0KPiA+ICsJ CS8qIGludGVyZmFjZSBpcyBkb3duLiB1cGRhdGUgcGtleSBhbmQgbGVhdmUuICovDQo+ID4gKwkJ aWYgKGxldmVsID09IElQT0lCX0ZMVVNIX0hFQVZZICYmDQo+ID4gKwkJICAgICF0ZXN0X2JpdChJ UE9JQl9GTEFHX1NVQklOVEVSRkFDRSwgJnByaXYtPmZsYWdzKSkNCj4gPiArCQkJdXBkYXRlX3Bh cmVudF9wa2V5KHByaXYpOw0KPiA+ICAgCQlpcG9pYl9kYmcocHJpdiwgIk5vdCBmbHVzaGluZyAt IElQT0lCX0ZMQUdfQURNSU5fVVAgbm90IHNldC5cbiIpOw0KPiA+ICAgCQlyZXR1cm47DQo+ID4g ICAJfQ0KPiA+IEBAIC0xMDA1LDIwICsxMDA3LDE1IEBAIHN0YXRpYyB2b2lkIF9faXBvaWJfaWJf ZGV2X2ZsdXNoKHN0cnVjdCBpcG9pYl9kZXZfcHJpdg0KPiAqcHJpdiwNCj4gPiAgIAkJICogKHBh cmVudCkgZGV2aWNlcyBzaG91bGQgYWx3YXlzIHRha2VzIHdoYXQgcHJlc2VudCBpbiBwa2V5IGlu ZGV4IDANCj4gPiAgIAkJICovDQo+ID4gICAJCWlmICh0ZXN0X2JpdChJUE9JQl9GTEFHX1NVQklO VEVSRkFDRSwgJnByaXYtPmZsYWdzKSkgew0KPiA+IC0JCQlpZiAoaWJfZmluZF9wa2V5KHByaXYt PmNhLCBwcml2LT5wb3J0LCBwcml2LT5wa2V5LCAmbmV3X2luZGV4KSkNCj4gew0KPiA+IC0JCQkJ Y2xlYXJfYml0KElQT0lCX1BLRVlfQVNTSUdORUQsICZwcml2LT5mbGFncyk7DQo+ID4gLQkJCQlp cG9pYl9pYl9kZXZfZG93bihkZXYsIDApOw0KPiA+IC0JCQkJaXBvaWJfaWJfZGV2X3N0b3AoZGV2 LCAwKTsNCj4gPiAtCQkJCWlmIChpcG9pYl9wa2V5X2Rldl9kZWxheV9vcGVuKGRldikpDQo+ID4g LQkJCQkJcmV0dXJuOw0KPiA+IC0JCQl9DQo+ID4gLQkJCS8qIHJlc3RhcnQgUVAgb25seSBpZiBQ X0tleSBpbmRleCBpcyBjaGFuZ2VkICovDQo+ID4gLQkJCWlmICh0ZXN0X2FuZF9zZXRfYml0KElQ T0lCX1BLRVlfQVNTSUdORUQsICZwcml2LT5mbGFncykgJiYNCj4gPiAtCQkJICAgIG5ld19pbmRl eCA9PSBwcml2LT5wa2V5X2luZGV4KSB7DQo+ID4gKwkJCW9sZF9pbmRleCA9IHByaXYtPnBrZXlf aW5kZXg7DQo+ID4gKwkJCWlwb2liX3BrZXlfZGV2X2NoZWNrX3ByZXNlbmNlKGRldik7DQo+ID4g Kw0KPiA+ICsJCQlpZiAodGVzdF9iaXQoSVBPSUJfUEtFWV9BU1NJR05FRCwgJnByaXYtPmZsYWdz KSAmJg0KPiA+ICsJCQkgICAgb2xkX2luZGV4ID09IHByaXYtPnBrZXlfaW5kZXgpIHsNCj4gPiAr CQkJCS8qIHJlc3RhcnQgUVAgb25seSBpZiBQX0tleSBpbmRleCBpcyBjaGFuZ2VkICovDQo+ID4g ICAJCQkJaXBvaWJfZGJnKHByaXYsICJOb3QgZmx1c2hpbmcgLSBQX0tleSBpbmRleCBub3QNCj4g Y2hhbmdlZC5cbiIpOw0KPiA+ICAgCQkJCXJldHVybjsNCj4gPiAgIAkJCX0NCj4gPiAtCQkJcHJp di0+cGtleV9pbmRleCA9IG5ld19pbmRleDsNCj4gPiAgIAkJfSBlbHNlIHsNCj4gPiAgIAkJCXJl c3VsdCA9IHVwZGF0ZV9wYXJlbnRfcGtleShwcml2KTsNCj4gPiAgIAkJCS8qIHJlc3RhcnQgUVAg b25seSBpZiBQX0tleSB2YWx1ZSBjaGFuZ2VkICovDQo+ID4gQEAgLTEwMzgsOCArMTAzNSwxMCBA QCBzdGF0aWMgdm9pZCBfX2lwb2liX2liX2Rldl9mbHVzaChzdHJ1Y3QgaXBvaWJfZGV2X3ByaXYN Cj4gKnByaXYsDQo+ID4gICAJCWlwb2liX2liX2Rldl9kb3duKGRldiwgMCk7DQo+ID4NCj4gPiAg IAlpZiAobGV2ZWwgPT0gSVBPSUJfRkxVU0hfSEVBVlkpIHsNCj4gPiAtCQlpcG9pYl9pYl9kZXZf c3RvcChkZXYsIDApOw0KPiA+IC0JCWlwb2liX2liX2Rldl9vcGVuKGRldik7DQo+ID4gKwkJaWYg KHRlc3RfYml0KElQT0lCX0ZMQUdfSU5JVElBTElaRUQsICZwcml2LT5mbGFncykpDQo+ID4gKwkJ CWlwb2liX2liX2Rldl9zdG9wKGRldiwgMCk7DQo+ID4gKwkJaWYgKGlwb2liX2liX2Rldl9vcGVu KGRldikgIT0gMCkNCj4gPiArCQkJcmV0dXJuOw0KPiA+ICAgCX0NCj4gPg0KPiA+ICAgCS8qDQo+ ID4NCj4gPiAtLQ0KPiA+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBs aW5lICJ1bnN1YnNjcmliZSBsaW51eC1yZG1hIiBpbg0KPiA+IHRoZSBib2R5IG9mIGEgbWVzc2Fn ZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnDQo+ID4gTW9yZSBtYWpvcmRvbW8gaW5mbyBh dCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQo+ID4NCg0K -- 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/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c index 6a7003d..ac941e1 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c @@ -52,6 +52,7 @@ MODULE_PARM_DESC(data_debug_level, #endif static DEFINE_MUTEX(pkey_mutex); +static void ipoib_pkey_dev_check_presence(struct net_device *dev); struct ipoib_ah *ipoib_create_ah(struct net_device *dev, struct ib_pd *pd, struct ib_ah_attr *attr) @@ -669,12 +670,13 @@ int ipoib_ib_dev_open(struct net_device *dev) struct ipoib_dev_priv *priv = netdev_priv(dev); int ret; - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &priv->pkey_index)) { - ipoib_warn(priv, "P_Key 0x%04x not found\n", priv->pkey); - clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); + ipoib_pkey_dev_check_presence(dev); + + if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) { + ipoib_warn(priv, "P_Key 0x%04x is %s\n", priv->pkey, + !(priv->pkey & 0x7fff) ? "Invalid" : "not found"); return -1; } - set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); ret = ipoib_init_qp(dev); if (ret) { @@ -712,9 +714,10 @@ dev_stop: static void ipoib_pkey_dev_check_presence(struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); - u16 pkey_index = 0; - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &pkey_index)) + if (!(priv->pkey & 0x7fff) || + ib_find_pkey(priv->ca, priv->port, priv->pkey, + &priv->pkey_index)) clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); else set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); @@ -746,12 +749,10 @@ int ipoib_ib_dev_down(struct net_device *dev, int flush) netif_carrier_off(dev); /* Shutdown the P_Key thread if still active */ - if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags)) { - mutex_lock(&pkey_mutex); - set_bit(IPOIB_PKEY_STOP, &priv->flags); + mutex_lock(&pkey_mutex); + if (!test_and_set_bit(IPOIB_PKEY_STOP, &priv->flags)) cancel_delayed_work_sync(&priv->pkey_poll_task); - mutex_unlock(&pkey_mutex); - } + mutex_unlock(&pkey_mutex); ipoib_mcast_stop_thread(dev, flush); ipoib_mcast_dev_flush(dev); @@ -972,7 +973,7 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, { struct ipoib_dev_priv *cpriv; struct net_device *dev = priv->dev; - u16 new_index; + u16 old_index; int result; down_read(&priv->vlan_rwsem); @@ -986,16 +987,17 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, up_read(&priv->vlan_rwsem); - if (!test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags)) { - /* for non-child devices must check/update the pkey value here */ - if (level == IPOIB_FLUSH_HEAVY && - !test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) - update_parent_pkey(priv); + if (!test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags) && + level != IPOIB_FLUSH_HEAVY) { ipoib_dbg(priv, "Not flushing - IPOIB_FLAG_INITIALIZED not set.\n"); return; } if (!test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) { + /* interface is down. update pkey and leave. */ + if (level == IPOIB_FLUSH_HEAVY && + !test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) + update_parent_pkey(priv); ipoib_dbg(priv, "Not flushing - IPOIB_FLAG_ADMIN_UP not set.\n"); return; } @@ -1005,20 +1007,15 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, * (parent) devices should always takes what present in pkey index 0 */ if (test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) { - if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &new_index)) { - clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags); - ipoib_ib_dev_down(dev, 0); - ipoib_ib_dev_stop(dev, 0); - if (ipoib_pkey_dev_delay_open(dev)) - return; - } - /* restart QP only if P_Key index is changed */ - if (test_and_set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags) && - new_index == priv->pkey_index) { + old_index = priv->pkey_index; + ipoib_pkey_dev_check_presence(dev); + + if (test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags) && + old_index == priv->pkey_index) { + /* restart QP only if P_Key index is changed */ ipoib_dbg(priv, "Not flushing - P_Key index not changed.\n"); return; } - priv->pkey_index = new_index; } else { result = update_parent_pkey(priv); /* restart QP only if P_Key value changed */ @@ -1038,8 +1035,10 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, ipoib_ib_dev_down(dev, 0); if (level == IPOIB_FLUSH_HEAVY) { - ipoib_ib_dev_stop(dev, 0); - ipoib_ib_dev_open(dev); + if (test_bit(IPOIB_FLAG_INITIALIZED, &priv->flags)) + ipoib_ib_dev_stop(dev, 0); + if (ipoib_ib_dev_open(dev) != 0) + return; } /*
Currently, the parent interface keeps sending broadcast group join requests even if p_key index 0 is invalid, which for itself is possible/common in virtualized environment where a VF has been probed to VM but the actual p_key configuration has not yet been assigned by the management software. This creates unnecessary noise on the fabric and in the kernel logs: ib0: multicast join failed for ff12:401b:8000:0000:0000:0000:ffff:ffff, status -22 The original code run the multicast task regardless of the actual p_key value, which can be avoided. The fix is to re-init resources and bring interface up only if p_key index 0 is valid either when starting up or on PKEY_CHANGE event. Fixes: c290414169 ('IPoIB: Fix pkey change flow for virtualization environments') Reviewed-by: Ira Weiny <ira.weiny@intel.com> Signed-off-by: Alex Estrin <alex.estrin@intel.com> --- Changes from v4: - streamline child interface pkey event handling, - shutdown of pkey polling thread depends on PKEY_STOP flag state only. Original poib_ib_dev_down() could leave polling thread active if PKEY_ASSIGNED flag was set. That could create a racing condition on followed re-initialization of QP resources. --- drivers/infiniband/ulp/ipoib/ipoib_ib.c | 59 +++++++++++++++---------------- 1 files changed, 29 insertions(+), 30 deletions(-) -- 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