Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1181973 > unrolled thread

Re: [PATCH] ixgbe: Remove bimodal SR-IOV disabling

Started byAlex Williamson <alex.williamson@redhat.com>
First post2015-07-11 00:50 +0200
Last post2015-07-11 01:10 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] ixgbe: Remove bimodal SR-IOV disabling Alex Williamson <alex.williamson@redhat.com> - 2015-07-11 00:50 +0200
    RE: [PATCH] ixgbe: Remove bimodal SR-IOV disabling "Rose, Gregory V" <gregory.v.rose@intel.com> - 2015-07-11 01:10 +0200

#1181973 — Re: [PATCH] ixgbe: Remove bimodal SR-IOV disabling

FromAlex Williamson <alex.williamson@redhat.com>
Date2015-07-11 00:50 +0200
SubjectRe: [PATCH] ixgbe: Remove bimodal SR-IOV disabling
Message-ID<pKPhg-3mu-3@gated-at.bofh.it>
On Fri, 2015-07-10 at 21:36 +0000, Rose, Gregory V wrote:
> 
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Friday, July 10, 2015 2:32 PM
> > To: intel-wired-lan@lists.osuosl.org; Kirsher, Jeffrey T
> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Rose, Gregory V
> > Subject: [PATCH] ixgbe: Remove bimodal SR-IOV disabling
> > 
> > When unbinding an SR-IOV device with VFs configured from ixgbe, the driver
> > behaves in one of two ways.  If max_vfs was specified, the SR-IOV state is
> > disabled, removing the VFs.  The occurs regardless of whether the VF count
> > was later modified through sysfs.  If however max_vfs is zero, such as by
> > not specifying the module parameter, the VFs persist after the PF is
> > unbound from ixgbe.  If the PF is then bound to vfio-pci to be assigned to
> > a VM, the PF is non-functional.
> > 
> > >From the comment, commit da36b64736cf ("ixgbe: Implement PCI SR-IOV
> > sysfs callback operation") clearly intended this alternate behavior, but
> > probably didn't realize the PF doesn't work in this mode.
> > 
> > This bimodal behavior is confusing to users and results in a state where
> > the PF is broken for other uses unless the user sets sriov_numvfs to zero
> > prior to unbinding the device.  Remove this behavior so that VFs are
> > removed and the PF is functional for other uses after unbind, regardless
> > of the way VFs are enabled.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Greg Rose <gregory.v.rose@intel.com>
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > 
> > I can only think that not disabling SR-IOV was meant to enable some sort
> > of persistence for VFs, but that's probably better accomplished with
> > either udev rules and/or modprobe.d install scripts.
> > 
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index 5be12a0..de04e3e 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -8810,12 +8810,7 @@ static void ixgbe_remove(struct pci_dev *pdev)
> >  		unregister_netdev(netdev);
> > 
> >  #ifdef CONFIG_PCI_IOV
> > -	/*
> > -	 * Only disable SR-IOV on unload if the user specified the now
> > -	 * deprecated max_vfs module parameter.
> > -	 */
> > -	if (max_vfs)
> > -		ixgbe_disable_sriov(adapter);
> > +	ixgbe_disable_sriov(adapter);
> >  #endif
> >  	ixgbe_clear_interrupt_scheme(adapter);
> > 
> 
> Please remove max_vfs module parameter - it is deprecated and should be removed from upstream builds.  Dave let us get away with a kernel module a few years ago because the other necessary infrastructure to enable SR-IOV virtual functions via the PCIe interface was not available.  Now that it's there it should be removed and vendors/end users should be forced to move away from this.

I can't really say I'm in favor of removing that option.  It's probably
going to break a lot of people because doing the udev rules right is
hard.  The sysfs sriov interface has been tossed over the wall as the
right way to do things, but there's really no infrastructure to
facilitate even the simple peanut butter, everybody gets the same number
of VFs, interface that max_vfs provides.  I think the existence of this
bug is probably a good indication that the sysfs interface has not
really been adopted yet.  Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1181994

From"Rose, Gregory V" <gregory.v.rose@intel.com>
Date2015-07-11 01:10 +0200
Message-ID<pKPAC-3Im-23@gated-at.bofh.it>
In reply to#1181973
DQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEFsZXggV2lsbGlhbXNvbiBb
bWFpbHRvOmFsZXgud2lsbGlhbXNvbkByZWRoYXQuY29tXQ0KPiBTZW50OiBGcmlkYXksIEp1bHkg
MTAsIDIwMTUgMzo0NCBQTQ0KPiBUbzogUm9zZSwgR3JlZ29yeSBWDQo+IENjOiBpbnRlbC13aXJl
ZC1sYW5AbGlzdHMub3N1b3NsLm9yZzsgS2lyc2hlciwgSmVmZnJleSBUOw0KPiBuZXRkZXZAdmdl
ci5rZXJuZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJl
OiBbUEFUQ0hdIGl4Z2JlOiBSZW1vdmUgYmltb2RhbCBTUi1JT1YgZGlzYWJsaW5nDQo+IA0KPiBP
biBGcmksIDIwMTUtMDctMTAgYXQgMjE6MzYgKzAwMDAsIFJvc2UsIEdyZWdvcnkgViB3cm90ZToN
Cj4gPg0KPiA+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IEFsZXgg
V2lsbGlhbXNvbiBbbWFpbHRvOmFsZXgud2lsbGlhbXNvbkByZWRoYXQuY29tXQ0KPiA+ID4gU2Vu
dDogRnJpZGF5LCBKdWx5IDEwLCAyMDE1IDI6MzIgUE0NCj4gPiA+IFRvOiBpbnRlbC13aXJlZC1s
YW5AbGlzdHMub3N1b3NsLm9yZzsgS2lyc2hlciwgSmVmZnJleSBUDQo+ID4gPiBDYzogbmV0ZGV2
QHZnZXIua2VybmVsLm9yZzsgbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgUm9zZSwNCj4g
PiA+IEdyZWdvcnkgVg0KPiA+ID4gU3ViamVjdDogW1BBVENIXSBpeGdiZTogUmVtb3ZlIGJpbW9k
YWwgU1ItSU9WIGRpc2FibGluZw0KPiA+ID4NCj4gPiA+IFdoZW4gdW5iaW5kaW5nIGFuIFNSLUlP
ViBkZXZpY2Ugd2l0aCBWRnMgY29uZmlndXJlZCBmcm9tIGl4Z2JlLCB0aGUNCj4gPiA+IGRyaXZl
ciBiZWhhdmVzIGluIG9uZSBvZiB0d28gd2F5cy4gIElmIG1heF92ZnMgd2FzIHNwZWNpZmllZCwg
dGhlDQo+ID4gPiBTUi1JT1Ygc3RhdGUgaXMgZGlzYWJsZWQsIHJlbW92aW5nIHRoZSBWRnMuICBU
aGUgb2NjdXJzIHJlZ2FyZGxlc3MNCj4gPiA+IG9mIHdoZXRoZXIgdGhlIFZGIGNvdW50IHdhcyBs
YXRlciBtb2RpZmllZCB0aHJvdWdoIHN5c2ZzLiAgSWYNCj4gPiA+IGhvd2V2ZXIgbWF4X3ZmcyBp
cyB6ZXJvLCBzdWNoIGFzIGJ5IG5vdCBzcGVjaWZ5aW5nIHRoZSBtb2R1bGUNCj4gPiA+IHBhcmFt
ZXRlciwgdGhlIFZGcyBwZXJzaXN0IGFmdGVyIHRoZSBQRiBpcyB1bmJvdW5kIGZyb20gaXhnYmUu
ICBJZg0KPiA+ID4gdGhlIFBGIGlzIHRoZW4gYm91bmQgdG8gdmZpby1wY2kgdG8gYmUgYXNzaWdu
ZWQgdG8gYSBWTSwgdGhlIFBGIGlzDQo+IG5vbi1mdW5jdGlvbmFsLg0KPiA+ID4NCj4gPiA+ID5G
cm9tIHRoZSBjb21tZW50LCBjb21taXQgZGEzNmI2NDczNmNmICgiaXhnYmU6IEltcGxlbWVudCBQ
Q0kgU1ItSU9WDQo+ID4gPiBzeXNmcyBjYWxsYmFjayBvcGVyYXRpb24iKSBjbGVhcmx5IGludGVu
ZGVkIHRoaXMgYWx0ZXJuYXRlIGJlaGF2aW9yLA0KPiA+ID4gYnV0IHByb2JhYmx5IGRpZG4ndCBy
ZWFsaXplIHRoZSBQRiBkb2Vzbid0IHdvcmsgaW4gdGhpcyBtb2RlLg0KPiA+ID4NCj4gPiA+IFRo
aXMgYmltb2RhbCBiZWhhdmlvciBpcyBjb25mdXNpbmcgdG8gdXNlcnMgYW5kIHJlc3VsdHMgaW4g
YSBzdGF0ZQ0KPiA+ID4gd2hlcmUgdGhlIFBGIGlzIGJyb2tlbiBmb3Igb3RoZXIgdXNlcyB1bmxl
c3MgdGhlIHVzZXIgc2V0cw0KPiA+ID4gc3Jpb3ZfbnVtdmZzIHRvIHplcm8gcHJpb3IgdG8gdW5i
aW5kaW5nIHRoZSBkZXZpY2UuICBSZW1vdmUgdGhpcw0KPiA+ID4gYmVoYXZpb3Igc28gdGhhdCBW
RnMgYXJlIHJlbW92ZWQgYW5kIHRoZSBQRiBpcyBmdW5jdGlvbmFsIGZvciBvdGhlcg0KPiA+ID4g
dXNlcyBhZnRlciB1bmJpbmQsIHJlZ2FyZGxlc3Mgb2YgdGhlIHdheSBWRnMgYXJlIGVuYWJsZWQu
DQo+ID4gPg0KPiA+ID4gU2lnbmVkLW9mZi1ieTogQWxleCBXaWxsaWFtc29uIDxhbGV4LndpbGxp
YW1zb25AcmVkaGF0LmNvbT4NCj4gPiA+IENjOiBHcmVnIFJvc2UgPGdyZWdvcnkudi5yb3NlQGlu
dGVsLmNvbT4NCj4gPiA+IENjOiBKZWZmIEtpcnNoZXIgPGplZmZyZXkudC5raXJzaGVyQGludGVs
LmNvbT4NCj4gPiA+IC0tLQ0KPiA+ID4NCj4gPiA+IEkgY2FuIG9ubHkgdGhpbmsgdGhhdCBub3Qg
ZGlzYWJsaW5nIFNSLUlPViB3YXMgbWVhbnQgdG8gZW5hYmxlIHNvbWUNCj4gPiA+IHNvcnQgb2Yg
cGVyc2lzdGVuY2UgZm9yIFZGcywgYnV0IHRoYXQncyBwcm9iYWJseSBiZXR0ZXIgYWNjb21wbGlz
aGVkDQo+ID4gPiB3aXRoIGVpdGhlciB1ZGV2IHJ1bGVzIGFuZC9vciBtb2Rwcm9iZS5kIGluc3Rh
bGwgc2NyaXB0cy4NCj4gPiA+DQo+ID4gPiAgZHJpdmVycy9uZXQvZXRoZXJuZXQvaW50ZWwvaXhn
YmUvaXhnYmVfbWFpbi5jIHwgICAgNyArLS0tLS0tDQo+ID4gPiAgMSBmaWxlIGNoYW5nZWQsIDEg
aW5zZXJ0aW9uKCspLCA2IGRlbGV0aW9ucygtKQ0KPiA+ID4NCj4gPiA+IGRpZmYgLS1naXQgYS9k
cml2ZXJzL25ldC9ldGhlcm5ldC9pbnRlbC9peGdiZS9peGdiZV9tYWluLmMNCj4gPiA+IGIvZHJp
dmVycy9uZXQvZXRoZXJuZXQvaW50ZWwvaXhnYmUvaXhnYmVfbWFpbi5jDQo+ID4gPiBpbmRleCA1
YmUxMmEwLi5kZTA0ZTNlIDEwMDY0NA0KPiA+ID4gLS0tIGEvZHJpdmVycy9uZXQvZXRoZXJuZXQv
aW50ZWwvaXhnYmUvaXhnYmVfbWFpbi5jDQo+ID4gPiArKysgYi9kcml2ZXJzL25ldC9ldGhlcm5l
dC9pbnRlbC9peGdiZS9peGdiZV9tYWluLmMNCj4gPiA+IEBAIC04ODEwLDEyICs4ODEwLDcgQEAg
c3RhdGljIHZvaWQgaXhnYmVfcmVtb3ZlKHN0cnVjdCBwY2lfZGV2ICpwZGV2KQ0KPiA+ID4gIAkJ
dW5yZWdpc3Rlcl9uZXRkZXYobmV0ZGV2KTsNCj4gPiA+DQo+ID4gPiAgI2lmZGVmIENPTkZJR19Q
Q0lfSU9WDQo+ID4gPiAtCS8qDQo+ID4gPiAtCSAqIE9ubHkgZGlzYWJsZSBTUi1JT1Ygb24gdW5s
b2FkIGlmIHRoZSB1c2VyIHNwZWNpZmllZCB0aGUgbm93DQo+ID4gPiAtCSAqIGRlcHJlY2F0ZWQg
bWF4X3ZmcyBtb2R1bGUgcGFyYW1ldGVyLg0KPiA+ID4gLQkgKi8NCj4gPiA+IC0JaWYgKG1heF92
ZnMpDQo+ID4gPiAtCQlpeGdiZV9kaXNhYmxlX3NyaW92KGFkYXB0ZXIpOw0KPiA+ID4gKwlpeGdi
ZV9kaXNhYmxlX3NyaW92KGFkYXB0ZXIpOw0KPiA+ID4gICNlbmRpZg0KPiA+ID4gIAlpeGdiZV9j
bGVhcl9pbnRlcnJ1cHRfc2NoZW1lKGFkYXB0ZXIpOw0KPiA+ID4NCj4gPg0KPiA+IFBsZWFzZSBy
ZW1vdmUgbWF4X3ZmcyBtb2R1bGUgcGFyYW1ldGVyIC0gaXQgaXMgZGVwcmVjYXRlZCBhbmQgc2hv
dWxkIGJlDQo+IHJlbW92ZWQgZnJvbSB1cHN0cmVhbSBidWlsZHMuICBEYXZlIGxldCB1cyBnZXQg
YXdheSB3aXRoIGEga2VybmVsIG1vZHVsZSBhDQo+IGZldyB5ZWFycyBhZ28gYmVjYXVzZSB0aGUg
b3RoZXIgbmVjZXNzYXJ5IGluZnJhc3RydWN0dXJlIHRvIGVuYWJsZSBTUi1JT1YNCj4gdmlydHVh
bCBmdW5jdGlvbnMgdmlhIHRoZSBQQ0llIGludGVyZmFjZSB3YXMgbm90IGF2YWlsYWJsZS4gIE5v
dyB0aGF0IGl0J3MNCj4gdGhlcmUgaXQgc2hvdWxkIGJlIHJlbW92ZWQgYW5kIHZlbmRvcnMvZW5k
IHVzZXJzIHNob3VsZCBiZSBmb3JjZWQgdG8gbW92ZQ0KPiBhd2F5IGZyb20gdGhpcy4NCj4gDQo+
IEkgY2FuJ3QgcmVhbGx5IHNheSBJJ20gaW4gZmF2b3Igb2YgcmVtb3ZpbmcgdGhhdCBvcHRpb24u
ICBJdCdzIHByb2JhYmx5DQo+IGdvaW5nIHRvIGJyZWFrIGEgbG90IG9mIHBlb3BsZSBiZWNhdXNl
IGRvaW5nIHRoZSB1ZGV2IHJ1bGVzIHJpZ2h0IGlzIGhhcmQuDQo+IFRoZSBzeXNmcyBzcmlvdiBp
bnRlcmZhY2UgaGFzIGJlZW4gdG9zc2VkIG92ZXIgdGhlIHdhbGwgYXMgdGhlIHJpZ2h0IHdheQ0K
PiB0byBkbyB0aGluZ3MsIGJ1dCB0aGVyZSdzIHJlYWxseSBubyBpbmZyYXN0cnVjdHVyZSB0byBm
YWNpbGl0YXRlIGV2ZW4gdGhlDQo+IHNpbXBsZSBwZWFudXQgYnV0dGVyLCBldmVyeWJvZHkgZ2V0
cyB0aGUgc2FtZSBudW1iZXIgb2YgVkZzLCBpbnRlcmZhY2UNCj4gdGhhdCBtYXhfdmZzIHByb3Zp
ZGVzLiAgSSB0aGluayB0aGUgZXhpc3RlbmNlIG9mIHRoaXMgYnVnIGlzIHByb2JhYmx5IGENCj4g
Z29vZCBpbmRpY2F0aW9uIHRoYXQgdGhlIHN5c2ZzIGludGVyZmFjZSBoYXMgbm90IHJlYWxseSBi
ZWVuIGFkb3B0ZWQgeWV0Lg0KPiBUaGFua3MsDQoNCkFscmlnaHQsIEknbGwgZ28gd2l0aCB0aGF0
IHJlYXNvbmluZy4NCg0KQWNrZWQtYnk6IEdyZWcgUm9zZSA8Z3JlZ29yeS52LnJvc2VAaW50ZWwu
Y29tPg0KDQoNCj4gDQo+IEFsZXgNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web