netdev - RE: [net-next 6/9] ixgbe: fix X540 Completion timeout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <F6FB0E698C9B3143BDF729DF2228664685E27B19@ORSMSX110.amr.corp.intel.com>
Date:	Fri, 7 Nov 2014 17:06:55 +0000
From:	"Skidmore, Donald C" <donald.c.skidmore@...el.com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"davem@...emloft.net" <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"nhorman@...hat.com" <nhorman@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>,
	"jogreene@...hat.com" <jogreene@...hat.com>
Subject: RE: [net-next 6/9] ixgbe: fix X540 Completion timeout

> -----Original Message-----
> From: Sergei Shtylyov [mailto:sergei.shtylyov@...entembedded.com]
> Sent: Friday, November 07, 2014 6:35 AM
> To: Kirsher, Jeffrey T; davem@...emloft.net
> Cc: Skidmore, Donald C; netdev@...r.kernel.org; nhorman@...hat.com;
> sassmann@...hat.com; jogreene@...hat.com
> Subject: Re: [net-next 6/9] ixgbe: fix X540 Completion timeout
> 
> Hello.
> 
> On 11/7/2014 11:57 AM, Jeff Kirsher wrote:
> 
> > From: Don Skidmore <donald.c.skidmore@...el.com>
> 
> > On topologies including few levels of PCIe switching X540 can run into
> > an unexpected completion error.  We get around this by waiting after
> > enabling loopback a sufficient amount of time until Tx Data Fetch is
> > sent.  We then poll the pending transaction bit to ensure we received
> > the completion.  Only then do we go on to clear the buffers.
> 
> > Signed-of-by: Don Skidmore <donald.c.skidmore@...el.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> 
> > ---
> >   drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 21
> ++++++++++++++++++++-
> >   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > index b5f484b..e314b53 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> [...]
> > @@ -3600,6 +3601,24 @@ void ixgbe_clear_tx_pending(struct ixgbe_hw
> *hw)
> >   	hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> >   	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0 |
> IXGBE_HLREG0_LPBK);
> >
> > +	/* wait for a last completion before clearing buffers */
> > +	IXGBE_WRITE_FLUSH(hw);
> > +	usleep_range(3000, 6000);
> > +
> > +	/* Before proceeding, make sure that the PCIe block does not have
> > +	 * transactions pending.
> > +	 */
> > +	poll = ixgbe_pcie_timeout_poll(hw);
> > +	for (i = 0; i < poll; i++) {
> > +		usleep_range(100, 200);
> > +		value = ixgbe_read_pci_cfg_word(hw,
> IXGBE_PCI_DEVICE_STATUS);
> > +		if (ixgbe_removed(hw->hw_addr))
> > +			goto out;
> 
>     Why not just *break*?
> 
> > +		if (!(value &
> IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING))
> > +			goto out;
> 
>     Likewise.
> 
> > +	}
> > +
> > +out:
> >   	/* initiate cleaning flow for buffers in the PCIe transaction layer */
> >   	gcr_ext = IXGBE_READ_REG(hw, IXGBE_GCR_EXT);
> >   	IXGBE_WRITE_REG(hw, IXGBE_GCR_EXT,
> 
> WBR, Sergei

Your right the breaks would be much cleaner, thanks for pointing it out. :)  I can resubmit the patch threw Jeff or send a new one, whatever is preferred.

- Don Skidmore <donald.c.skidmore@...el.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html