Hey Neil and Rob, I kind of assumed we did test this patch with SLES SP3, as issue was raised against it first.
I verified RHEL6.5 with this patch but I will once again verify upstream code and approve the patch. Thanks, Hiral On 10/7/13 10:30 AM, "Neil Horman" <[email protected]> wrote: >On Mon, Oct 07, 2013 at 05:20:00PM +0000, Love, Robert W wrote: >> > From: Neil Horman [mailto:[email protected]] >> > Sent: Monday, October 07, 2013 6:33 AM >> > To: Love, Robert W >> > Cc: [email protected]; [email protected]; [email protected] >> > Subject: Re: [Open-FCoE, RFC] libfcoe: Make fcoe_sysfs optional / fix >>fnic >> > NULL exception >> > >> > On Thu, Sep 05, 2013 at 07:47:27AM -0000, Robert Love wrote: >> > > fnic doesn't use any of the create/destroy/enable/disable interfaces >> > > either from the (legacy) module paramaters or the (new) fcoe_sysfs >> > > interfaces. When fcoe_sysfs was introduced fnic wasn't changed since >> > > it wasn't using the interfaces. libfcoe incorrectly assumed that >>that >> > > all of its users were using fcoe_sysfs and when adding and deleting >> > > FCFs would assume the existance of a fcoe_ctlr_device. fnic was not >> > > allocating this structure because it doesn't care about the standard >> > > user interfaces (fnic starts on link only). If/When libfcoe tried to >> > > use the fcoe_ctlr_device's lock for the first time a NULL pointer >> > > exception would be triggered. >> > > >> > > Since fnic doesn't care about sysfs or user interfaces, the solution >> > > is to drop libfcoe's assumption that all drivers are using >>fcoe_sysfs. >> > > >> > > This patch accomplishes this by changing some of the structure >> > > relationships. >> > > >> > > We need a way to determine when a LLD is using fcoe_sysfs or not and >> > > we can do that by checking for the existance of the >>fcoe_ctlr_device. >> > > Prior to this patch, it was assumed that the fcoe_ctlr structure was >> > > allocated with the fcoe_ctlr_device and immediately followed it in >> > > memory. To reach the fcoe_ctlr_device we would simply go back in >> > > memory from the fcoe_ctlr to get the fcoe_ctlr_device. >> > > >> > > Since fnic doesn't allocate the fcoe_ctlr_device, we cannot keep >>that >> > > assumption. This patch adds a pointer from the fcoe_ctlr to the >> > > fcoe_ctlr_device. For bnx2fc and fcoe we will continue to allocate >>the >> > > two structures together, but then we'll set the ctlr->cdev pointer >>to >> > > point at the fcoe_ctlr_device. fnic will not change and will >>continue >> > > to allocate the fcoe_ctlr itself, and ctlr->cdev will remain NULL. >> > > >> > > When libfcoe adds fcoe_fcf's to the fcoe_ctlr it will check if >> > > ctlr->cdev is set and only if so will it continue to interact with >>fcoe_sysfs. >> > > >> > > Cc: Hiral Patel <[email protected]> >> > > Signed-off-by: Robert Love <[email protected]> >> > > Acked-by: Neil Horman <[email protected]> >> > > >> > Rob- >> > Whats going on with this patch? I just noticed its been sitting >>in patchwork >> > for a month, and its still in the new state. I acked it, but still >>don't see it in the >> > fcoe tree. Are you still planning on merging it? >> > >> >> I was hoping the fnic maintainers would test it, and send a Tested-by >>line. >> I can't reproduce the problem and I don't know if this is all that is >>needed to >> get fnic working again. Do you think I should move it forward on the >>strength >> of your Ack alone? >> >> If people really want it to move forward without testing, I don't >>necessarily >> have a problem with that either. >> >Ah, sorry, I had presumed that cisco had tested upstream already. In >this bz: >https://bugzilla.redhat.com/show_bug.cgi?id=1014864 > >They tested with RHEL6.5 and claim that it works. I would like it if they >tested upstream as well though. Hiral, can you do that please? > >Neil > >> Thanks, //Rob _______________________________________________ fcoe-devel mailing list [email protected] http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel
