[PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Michael Chan]

From: Dmitry Kravkov <dmitry@broadcom.com>

For an FCoE or iSCSI boot device, the networking side must stay "up" all
the time.  Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
will be reset and we'll lose the root file system.

If LRO is enabled, scripts that enable IP forwarding or bridging will
disable LRO and cause the device to be reset.  Disabling LRO on these
boot devices will prevent the reset.

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 6486ab8..4960048 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9794,6 +9794,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 	int func;
 	int timer_interval;
 	int rc;
+	u32 cnic_boot_device;
 
 	mutex_init(&bp->port.phy_mutex);
 	mutex_init(&bp->fw_mb_mutex);
@@ -9840,8 +9841,11 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 
 	bp->multi_mode = multi_mode;
 
+	cnic_boot_device =
+		!!SHMEM_RD(bp, func_mb[BP_FW_MB_IDX(bp)].iscsi_boot_signature);
+
 	/* Set TPA flags */
-	if (disable_tpa) {
+	if (disable_tpa || cnic_boot_device) {
 		bp->flags &= ~TPA_ENABLE_FLAG;
 		bp->dev->features &= ~NETIF_F_LRO;
 	} else {
-- 
1.7.1

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Rick Jones]

On 10/13/2011 08:38 PM, Michael Chan wrote:
> From: Dmitry Kravkov<dmitry@broadcom.com>
>
> For an FCoE or iSCSI boot device, the networking side must stay "up" all
> the time.  Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
> will be reset and we'll lose the root file system.
>
> If LRO is enabled, scripts that enable IP forwarding or bridging will
> disable LRO and cause the device to be reset.  Disabling LRO on these
> boot devices will prevent the reset.

Is this perhaps saying that a bnx2x-driven device being used for FCoE or 
iSCSI boot must not permit *any* run-time configuration change which 
leads to a NIC reset?

rick jones

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Michael Chan]

Rick Jones wrote:

> Is this perhaps saying that a bnx2x-driven device being used for FCoE
> or
> iSCSI boot must not permit *any* run-time configuration change which
> leads to a NIC reset?
> 

That is right.  Unless you have a multipath configuration with multiple
ports, then you can reset one port at a time.

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Rick Jones]

On 10/14/2011 08:53 AM, Michael Chan wrote:
> Rick Jones wrote:
>
>> Is this perhaps saying that a bnx2x-driven device being used for
>> FCoE or iSCSI boot must not permit *any* run-time configuration
>> change which leads to a NIC reset?
>>
>
> That is right.  Unless you have a multipath configuration with multiple
> ports, then you can reset one port at a time.

So, should there also be a "cnic_boot_device" check in many of the 
"capital letter" ethtool paths?

rick

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Michael Chan]

Rick Jones wrote:

> On 10/14/2011 08:53 AM, Michael Chan wrote:
> > Rick Jones wrote:
> >
> >> Is this perhaps saying that a bnx2x-driven device being used for
> >> FCoE or iSCSI boot must not permit *any* run-time configuration
> >> change which leads to a NIC reset?
> >>
> >
> > That is right.  Unless you have a multipath configuration with
> multiple
> > ports, then you can reset one port at a time.
> 
> So, should there also be a "cnic_boot_device" check in many of the
> "capital letter" ethtool paths?
> 

If the user is doing ethtool configuration changes or device shutdown,
it is more obvious what the consequence will be.  The user may also be
careful to do it on a multipath setup.

The reset caused by the auto turn-off of LRO when you enable
ip_forward or bridging will not be obvious to the user.  In addition,
all devices with LRO turned on will be reset at the same time so even
multipath will not survive.

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[John Fastabend]

On 10/14/2011 9:15 AM, Michael Chan wrote:
> Rick Jones wrote:
> 
>> On 10/14/2011 08:53 AM, Michael Chan wrote:
>>> Rick Jones wrote:
>>>
>>>> Is this perhaps saying that a bnx2x-driven device being used for
>>>> FCoE or iSCSI boot must not permit *any* run-time configuration
>>>> change which leads to a NIC reset?
>>>>
>>>
>>> That is right.  Unless you have a multipath configuration with
>> multiple
>>> ports, then you can reset one port at a time.
>>
>> So, should there also be a "cnic_boot_device" check in many of the
>> "capital letter" ethtool paths?
>>
> 
> If the user is doing ethtool configuration changes or device shutdown,
> it is more obvious what the consequence will be.  The user may also be
> careful to do it on a multipath setup.
> 
> The reset caused by the auto turn-off of LRO when you enable
> ip_forward or bridging will not be obvious to the user.  In addition,
> all devices with LRO turned on will be reset at the same time so even
> multipath will not survive.
>

But after the reset the device should login and SCSI layer should
handle retries. So I don't see why this is a problem. Why do we
need to handle this any different from any other link events?

.John

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Michael Chan]

On Fri, 2011-10-14 at 13:17 -0700, John Fastabend wrote:
> On 10/14/2011 9:15 AM, Michael Chan wrote:
> > Rick Jones wrote:
> > 
> >> On 10/14/2011 08:53 AM, Michael Chan wrote:
> >>> Rick Jones wrote:
> >>>
> >>>> Is this perhaps saying that a bnx2x-driven device being used for
> >>>> FCoE or iSCSI boot must not permit *any* run-time configuration
> >>>> change which leads to a NIC reset?
> >>>>
> >>>
> >>> That is right.  Unless you have a multipath configuration with
> >> multiple
> >>> ports, then you can reset one port at a time.
> >>
> >> So, should there also be a "cnic_boot_device" check in many of the
> >> "capital letter" ethtool paths?
> >>
> > 
> > If the user is doing ethtool configuration changes or device shutdown,
> > it is more obvious what the consequence will be.  The user may also be
> > careful to do it on a multipath setup.
> > 
> > The reset caused by the auto turn-off of LRO when you enable
> > ip_forward or bridging will not be obvious to the user.  In addition,
> > all devices with LRO turned on will be reset at the same time so even
> > multipath will not survive.
> >
> 
> But after the reset the device should login and SCSI layer should
> handle retries. So I don't see why this is a problem. Why do we
> need to handle this any different from any other link events?
> 

During a link down event, the iSCSI state does not get reset.  When link
comes back up quickly enough, there should be just some retransmissions
and everything should recover.  The root file system won't tolerate a
chip reset that will reset the iSCSI state.

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 13 Oct 2011 20:38:01 -0700

> From: Dmitry Kravkov <dmitry@broadcom.com>
> 
> For an FCoE or iSCSI boot device, the networking side must stay "up" all
> the time.  Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
> will be reset and we'll lose the root file system.
> 
> If LRO is enabled, scripts that enable IP forwarding or bridging will
> disable LRO and cause the device to be reset.  Disabling LRO on these
> boot devices will prevent the reset.
> 
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>

You're still going to have bugs after this.

What if you get a FIFO overflow or other error condition which requires
a chip reset?  You'll lose the root filesystem.  Why bother resetting
the chip at all if it's going to be useless afterwards?

The bug is in the fact that iSCSI context state isn't preserved or
reloaded across a chip reset.

Please fix that instead, and that way you won't have to add special
hacks for the root filesystem.  Everything will "just work'.

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Michael Chan]

On Wed, 2011-10-19 at 13:06 -0700, David Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Thu, 13 Oct 2011 20:38:01 -0700
> 
> > From: Dmitry Kravkov <dmitry@broadcom.com>
> > 
> > For an FCoE or iSCSI boot device, the networking side must stay "up" all
> > the time.  Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
> > will be reset and we'll lose the root file system.
> > 
> > If LRO is enabled, scripts that enable IP forwarding or bridging will
> > disable LRO and cause the device to be reset.  Disabling LRO on these
> > boot devices will prevent the reset.
> > 
> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
> 
> You're still going to have bugs after this.
> 
> What if you get a FIFO overflow or other error condition which requires
> a chip reset?  You'll lose the root filesystem.

That would be no different than a scsi driver experiencing fatal errors,
wouldn't it?

>   Why bother resetting
> the chip at all if it's going to be useless afterwards?

If the user has configured multipath to the storage target, we can still
reset each port separately.

What we want to prevent is any hidden reset during normal operations.

> 
> The bug is in the fact that iSCSI context state isn't preserved or
> reloaded across a chip reset.
> 
> Please fix that instead, and that way you won't have to add special
> hacks for the root filesystem.  Everything will "just work'.
> 
> 

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 19 Oct 2011 13:12:52 -0700

> 
> On Wed, 2011-10-19 at 13:06 -0700, David Miller wrote:
>> From: "Michael Chan" <mchan@broadcom.com>
>> Date: Thu, 13 Oct 2011 20:38:01 -0700
>> 
>> > From: Dmitry Kravkov <dmitry@broadcom.com>
>> > 
>> > For an FCoE or iSCSI boot device, the networking side must stay "up" all
>> > the time.  Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
>> > will be reset and we'll lose the root file system.
>> > 
>> > If LRO is enabled, scripts that enable IP forwarding or bridging will
>> > disable LRO and cause the device to be reset.  Disabling LRO on these
>> > boot devices will prevent the reset.
>> > 
>> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> > Signed-off-by: Michael Chan <mchan@broadcom.com>
>> 
>> You're still going to have bugs after this.
>> 
>> What if you get a FIFO overflow or other error condition which requires
>> a chip reset?  You'll lose the root filesystem.
> 
> That would be no different than a scsi driver experiencing fatal errors,
> wouldn't it?

It's not fatal if you can bring the chip back up after the reset
because this is networking.

These things are protocols, built on top of networking technology,
with retransmits, handshakes, and all sorts of features designed
to provide reliability.

Things like a LRO change ought to be completely transparent.

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[John Fastabend]

On 10/19/2011 1:47 PM, David Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Wed, 19 Oct 2011 13:12:52 -0700
> 
>>
>> On Wed, 2011-10-19 at 13:06 -0700, David Miller wrote:
>>> From: "Michael Chan" <mchan@broadcom.com>
>>> Date: Thu, 13 Oct 2011 20:38:01 -0700
>>>
>>>> From: Dmitry Kravkov <dmitry@broadcom.com>
>>>>
>>>> For an FCoE or iSCSI boot device, the networking side must stay "up" all
>>>> the time.  Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
>>>> will be reset and we'll lose the root file system.
>>>>
>>>> If LRO is enabled, scripts that enable IP forwarding or bridging will
>>>> disable LRO and cause the device to be reset.  Disabling LRO on these
>>>> boot devices will prevent the reset.
>>>>
>>>> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>>>> Signed-off-by: Michael Chan <mchan@broadcom.com>
>>>
>>> You're still going to have bugs after this.
>>>
>>> What if you get a FIFO overflow or other error condition which requires
>>> a chip reset?  You'll lose the root filesystem.
>>
>> That would be no different than a scsi driver experiencing fatal errors,
>> wouldn't it?
> 
> It's not fatal if you can bring the chip back up after the reset
> because this is networking.
> 
> These things are protocols, built on top of networking technology,
> with retransmits, handshakes, and all sorts of features designed
> to provide reliability.
> 
> Things like a LRO change ought to be completely transparent.

As a reference point this works fine in both FCoE and iSCSI stacks
today. The device is reset or link is lost for whatever reason
when the link comes back up the stack logs back in, enumerates
the luns and the scsi stack recovers as expected.

Firmware should do the equivalent login, lun enumeration, etc as
needed.

.John

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[David Miller]

From: John Fastabend <john.r.fastabend@intel.com>
Date: Wed, 19 Oct 2011 13:53:42 -0700

> As a reference point this works fine in both FCoE and iSCSI stacks
> today. The device is reset or link is lost for whatever reason
> when the link comes back up the stack logs back in, enumerates
> the luns and the scsi stack recovers as expected.
> 
> Firmware should do the equivalent login, lun enumeration, etc as
> needed.

I rest my case :-)

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Michael Chan]

On Wed, 2011-10-19 at 13:53 -0700, John Fastabend wrote:
> As a reference point this works fine in both FCoE and iSCSI stacks
> today. The device is reset or link is lost for whatever reason
> when the link comes back up the stack logs back in, enumerates
> the luns and the scsi stack recovers as expected.
> 
> Firmware should do the equivalent login, lun enumeration, etc as
> needed.

Just a quick follow-up on this issue.  Our firmware actually performs
the same logout before the reset and login after the reset.  For iSCSI,
the problem on our device was actually caused by our userspace daemon
logging events to a log file in the root fs.  The file I/O was blocked
and the daemon could not proceed to do the important operations during
the reset, and this caused filesystem I/O errors.  We have now fixed the
problem in the userspace daemon.

For FCoE, there is no logging issue and the root fs failure seems to
happen only in a multipath configuration with all paths going down for a
short time (caused by reset in this case).  We believe this also affects
other devices and not just ours.  We are now working with the multipath
maintainer to understand this issue.

So this confirms that the original patch for bnx2x is not needed.

Thanks.

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Michael Chan]

On Mon, 2012-02-13 at 15:20 +0400, Vasily Averin wrote:
> Michael, Dmitry,
> 
> Could you please clarify how you have fixed this issue?
> I've noticed very similar problem in CentOS6.2 environment,
> could you please clarify how it's possible to fix or workaround it? 
> 

We made a number of fixes:

1. iscsiuio user daemon no longer logs to the log file in the root fs
during reset.  The longer term fix is to use a different thread for
logging that can block when the fs is not available

2. iscsiuio is locked into memory and won't be swapped out.

3. bnx2x now caches the firmware after initial open:

http://git.kernel.org/?p=linux/kernel/git/davem/net.git;a=commit;h=eb2afd4a622985eaccfa8c7fc83e890b8930e0ab

because request_firmware() may not be able to get the firmware when the
root fs is not available.

These fixes should address all issues involving bnx2x reset during
iSCSI/FCoE/network boot.  RHEL6.2 has #1 and #2 fixes, but not #3.

> On 10/27/2011 at 16:30 -0700, Michael Chan wrote:
> > On Wed, 2011-10-19 at 13:53 -0700, John Fastabend wrote:
> >> As a reference point this works fine in both FCoE and iSCSI stacks
> >> today. The device is reset or link is lost for whatever reason
> >> when the link comes back up the stack logs back in, enumerates
> >> the luns and the scsi stack recovers as expected.
> >>
> >> Firmware should do the equivalent login, lun enumeration, etc as
> >> needed.
> > 
> > Just a quick follow-up on this issue.  Our firmware actually performs
> > the same logout before the reset and login after the reset.  For iSCSI,
> > the problem on our device was actually caused by our userspace daemon
> > logging events to a log file in the root fs.  The file I/O was blocked
> > and the daemon could not proceed to do the important operations during
> > the reset, and this caused filesystem I/O errors.  We have now fixed the
> > problem in the userspace daemon.
> > 
> > For FCoE, there is no logging issue and the root fs failure seems to
> > happen only in a multipath configuration with all paths going down for a
> > short time (caused by reset in this case).  We believe this also affects
> > other devices and not just ours.  We are now working with the multipath
> > maintainer to understand this issue.
> > 
> > So this confirms that the original patch for bnx2x is not needed.
> 
> 

Re: [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device

[Vasily Averin]

Michael, Dmitry,

Could you please clarify how you have fixed this issue?
I've noticed very similar problem in CentOS6.2 environment,
could you please clarify how it's possible to fix or workaround it? 

thank you,
	Vasily Averin

On 10/27/2011 at 16:30 -0700, Michael Chan wrote:
> On Wed, 2011-10-19 at 13:53 -0700, John Fastabend wrote:
>> As a reference point this works fine in both FCoE and iSCSI stacks
>> today. The device is reset or link is lost for whatever reason
>> when the link comes back up the stack logs back in, enumerates
>> the luns and the scsi stack recovers as expected.
>>
>> Firmware should do the equivalent login, lun enumeration, etc as
>> needed.
> 
> Just a quick follow-up on this issue.  Our firmware actually performs
> the same logout before the reset and login after the reset.  For iSCSI,
> the problem on our device was actually caused by our userspace daemon
> logging events to a log file in the root fs.  The file I/O was blocked
> and the daemon could not proceed to do the important operations during
> the reset, and this caused filesystem I/O errors.  We have now fixed the
> problem in the userspace daemon.
> 
> For FCoE, there is no logging issue and the root fs failure seems to
> happen only in a multipath configuration with all paths going down for a
> short time (caused by reset in this case).  We believe this also affects
> other devices and not just ours.  We are now working with the multipath
> maintainer to understand this issue.
> 
> So this confirms that the original patch for bnx2x is not needed.