Date: Wed, 9 Apr 2008 13:16:20 -0700 From: "David Christensen" <davidch@broadcom.com> To: "Peter Wemm" <peter@wemm.org> Cc: "cvs-src@freebsd.org" <cvs-src@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "cvs-all@freebsd.org" <cvs-all@freebsd.org> Subject: RE: cvs commit: src/sys/dev/bce if_bce.c if_bcefw.h if_bcereg.h Message-ID: <5D267A3F22FD854F8F48B3D2B523819324EF6DEE3B@IRVEXCHCCR01.corp.ad.broadcom.com> In-Reply-To: <e7db6d980804030521h7af1ecd4pc492caa0bce14138@mail.gmail.com> References: <200802220046.m1M0kMPM008814@repoman.freebsd.org> <e7db6d980803310032p7b3eec0et355919afdb147218@mail.gmail.com> <5D267A3F22FD854F8F48B3D2B523819324EF633FCC@IRVEXCHCCR01.corp.ad.broadcom.com> <e7db6d980803311234q1602d7a4ncf9e3fadaa5f679b@mail.gmail.com> <e7db6d980804030521h7af1ecd4pc492caa0bce14138@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
> On 8.0/i386, with PAE enabled, I get messages on the console and the
> system hangs when trying to do a nfs mount. Backing out the driver
> fixes it. The same driver doesn't cause quite as spectacular a
> failure on 8.0/amd64, but it isn't exactly happy..
>
> Additional IP options:.^M
> Mounting NFS file systebcms:e1: link state changed to UP^M
> bce1: discard frame w/o leading ethernet header (len 0 pkt len 0)^M
> bce1: discard frame w/o leading ethernet header (len 0 pkt len 0)^M
> bce1: discard frame w/o leading ethernet header (len 0 pkt len 0)^M
> bce1: discard frame w/o leading ethernet header (len 0 pkt len 0)^M
> [..forever..]
>
> NFS over UDP, fwiw. Server is a netapp.
I may have found the performance problem. It looks like an issue
with handling the producer/consumer indices for the send ring that
would cause outgoing frames to just sit there until a received
frame came in and kickstarted the interrupt handler. This is
consistent with your observation that enabling promiscuous mode
seemed to make things better, there's more received traffic. I
found that pinging the SUT from another system has the same
effect. Can you try the attached patch to head and let me know?
I tested on RELENG_7 and I haven't seen the discard errors yet.
Dave
[-- Attachment #2 --]
--- if_bcereg.h.head 2008-04-03 13:58:19.000000000 -0700
+++ if_bcereg.h 2008-04-09 13:09:04.000000000 -0700
@@ -132,6 +132,9 @@
"\02b1" \
"\01b0"
+/* DRC - Start */
+/* #define BCE_DEBUG 1 */
+/* DRC - End */
/****************************************************************************/
/* Debugging macros and definitions. */
/****************************************************************************/
@@ -5134,6 +5137,8 @@ struct bce_softc
/* TX DMA mapping failure counter. */
u32 tx_dma_map_failures;
+ u64 rx_intr_time;
+
#ifdef BCE_DEBUG
/* Track the number of enqueued mbufs. */
int debug_tx_mbuf_alloc;
@@ -5147,7 +5152,6 @@ struct bce_softc
u32 tx_interrupts;
/* Track interrupt time (25MHz clock). */
- u64 rx_intr_time;
u64 tx_intr_time;
u32 rx_low_watermark; /* Lowest number of rx_bd's free. */
[-- Attachment #3 --]
--- if_bce.c.head 2008-04-03 13:58:19.000000000 -0700
+++ if_bce.c 2008-04-09 10:39:47.000000000 -0700
@@ -54,7 +54,7 @@ __FBSDID("$FreeBSD: src/sys/dev/bce/if_b
/* BCE Debug Options */
/****************************************************************************/
#ifdef BCE_DEBUG
- u32 bce_debug = BCE_WARN;
+ u32 bce_debug = BCE_WARN_SEND | BCE_WARN_RECV;
/* 0 = Never */
/* 1 = 1 in 2,147,483,648 */
@@ -4392,7 +4392,7 @@ bce_free_pg_chain(struct bce_softc *sc)
{
int i;
- DBPRINT(sc, BCE_VERBOSE_RESET, "Entering %s()\n", __FUNCTION__);
+ DBPRINT(sc, BCE_EXCESSIVE_RESET, "Entering %s()\n", __FUNCTION__);
/* Free any mbufs still in the mbuf page chain. */
for (i = 0; i < TOTAL_PG_BD; i++) {
@@ -4417,7 +4417,7 @@ bce_free_pg_chain(struct bce_softc *sc)
BCE_PRINTF("%s(): Memory leak! Lost %d mbufs from page chain!\n",
__FUNCTION__, sc->debug_pg_mbuf_alloc));
- DBPRINT(sc, BCE_VERBOSE_RESET, "Exiting %s()\n", __FUNCTION__);
+ DBPRINT(sc, BCE_EXCESSIVE_RESET, "Exiting %s()\n", __FUNCTION__);
}
@@ -4561,6 +4561,7 @@ bce_get_hw_rx_cons(struct bce_softc *sc)
return hw_cons;
}
+
/****************************************************************************/
/* Handles received frame interrupt events. */
/* */
@@ -4576,9 +4577,10 @@ bce_rx_intr(struct bce_softc *sc)
u16 sw_rx_cons, sw_rx_cons_idx, sw_pg_cons, sw_pg_cons_idx, hw_rx_cons;
u32 status;
+ u32 timer_start, timer_end;
+ timer_start = REG_RD(sc, BCE_TIMER_25MHZ_FREE_RUN);
+
#ifdef BCE_DEBUG
- u32 rx_intr_start, rx_intr_end;
- rx_intr_start = REG_RD(sc, BCE_TIMER_25MHZ_FREE_RUN);
sc->rx_interrupts++;
#endif
@@ -4599,6 +4601,10 @@ bce_rx_intr(struct bce_softc *sc)
sw_rx_cons = sc->rx_cons;
sw_pg_cons = sc->pg_cons;
+ DBPRINT(sc, BCE_INFO_RECV, "%s(enter): rx_prod = 0x%04X, "
+ "rx_cons = 0x%04X, rx_prod_bseq = 0x%08X\n",
+ __FUNCTION__, sc->rx_prod, sc->rx_cons, sc->rx_prod_bseq);
+
/* Update some debug statistics counters */
DBRUNIF((sc->free_rx_bd < sc->rx_low_watermark),
sc->rx_low_watermark = sc->free_rx_bd);
@@ -4608,7 +4614,7 @@ bce_rx_intr(struct bce_softc *sc)
/* ToDo: Consider setting a limit on the number of packets processed. */
while (sw_rx_cons != hw_rx_cons) {
struct mbuf *m0;
-
+
/* Convert the producer/consumer indices to an actual rx_bd index. */
sw_rx_cons_idx = RX_CHAIN_IDX(sw_rx_cons);
@@ -4681,6 +4687,10 @@ bce_rx_intr(struct bce_softc *sc)
* is filled and the remaining bytes are placed
* in the page chain.
*/
+
+ DBPRINT(sc, BCE_INFO_RECV, "%s(): Found a large packet.\n",
+ __FUNCTION__);
+
if (status & L2_FHDR_STATUS_SPLIT)
m0->m_len = l2fhdr->l2_fhdr_ip_xsum;
@@ -4736,6 +4746,9 @@ bce_rx_intr(struct bce_softc *sc)
* 154 bytes or less in size.
*/
+ DBPRINT(sc, BCE_INFO_RECV, "%s(): Found a small packet.\n",
+ __FUNCTION__);
+
/* Set the total packet length. */
m0->m_pkthdr.len = m0->m_len = pkt_len;
}
@@ -4746,6 +4759,12 @@ bce_rx_intr(struct bce_softc *sc)
/* Check that the resulting mbuf chain is valid. */
DBRUN(m_sanity(m0, FALSE));
+ DBRUNIF((m0->m_len < ETHER_HDR_LEN),
+ BCE_PRINTF("%s(): Unexpected length = %d!.\n",
+ __FUNCTION__, m0->m_len);
+ bce_breakpoint(sc));
+
+
DBRUNIF(DB_RANDOMTRUE(bce_debug_l2fhdr_status_check),
BCE_PRINTF("Simulating l2_fhdr status error.\n");
status = status | L2_FHDR_ERRORS_PHY_DECODE);
@@ -4856,8 +4875,28 @@ bce_rx_int_next_rx:
"rx_cons = 0x%04X, rx_prod_bseq = 0x%08X\n",
__FUNCTION__, sc->rx_prod, sc->rx_cons, sc->rx_prod_bseq);
- DBRUN(rx_intr_end = REG_RD(sc, BCE_TIMER_25MHZ_FREE_RUN);
- sc->rx_intr_time += (u64) BCE_TIME_DELTA(rx_intr_start, rx_intr_end));
+ timer_end = REG_RD(sc, BCE_TIMER_25MHZ_FREE_RUN);
+ sc->rx_intr_time += (u64) (timer_start > timer_end ?
+ (timer_start - timer_end) : (~timer_start + timer_end + 1));
+}
+
+
+/****************************************************************************/
+/* Reads the receive consumer value from the status block (skipping over */
+/* chain page pointer if necessary). */
+/* */
+/* Returns: */
+/* hw_cons */
+/****************************************************************************/
+static inline u16
+bce_get_hw_tx_cons(struct bce_softc *sc)
+{
+ u16 hw_cons = sc->status_block->status_tx_quick_consumer_index0;
+
+ if ((hw_cons & USABLE_TX_BD_PER_PAGE) == USABLE_TX_BD_PER_PAGE)
+ hw_cons++;
+
+ return hw_cons;
}
@@ -4870,7 +4909,6 @@ bce_rx_int_next_rx:
static void
bce_tx_intr(struct bce_softc *sc)
{
- struct status_block *sblk = sc->status_block;
struct ifnet *ifp = sc->bce_ifp;
u16 hw_tx_cons, sw_tx_cons, sw_tx_chain_cons;
@@ -4883,12 +4921,7 @@ bce_tx_intr(struct bce_softc *sc)
BCE_LOCK_ASSERT(sc);
/* Get the hardware's view of the TX consumer index. */
- hw_tx_cons = sc->hw_tx_cons = sblk->status_tx_quick_consumer_index0;
-
- /* Skip to the next entry if this is a chain page pointer. */
- if ((hw_tx_cons & USABLE_TX_BD_PER_PAGE) == USABLE_TX_BD_PER_PAGE)
- hw_tx_cons++;
-
+ hw_tx_cons = sc->hw_tx_cons = bce_get_hw_tx_cons(sc);
sw_tx_cons = sc->tx_cons;
/* Prevent speculative reads from getting ahead of the status block. */
@@ -4957,9 +4990,7 @@ bce_tx_intr(struct bce_softc *sc)
sw_tx_cons = NEXT_TX_BD(sw_tx_cons);
/* Refresh hw_cons to see if there's new work. */
- hw_tx_cons = sc->hw_tx_cons = sblk->status_tx_quick_consumer_index0;
- if ((hw_tx_cons & USABLE_TX_BD_PER_PAGE) == USABLE_TX_BD_PER_PAGE)
- hw_tx_cons++;
+ hw_tx_cons = sc->hw_tx_cons = bce_get_hw_tx_cons(sc);
/* Prevent speculative reads from getting ahead of the status block. */
bus_space_barrier(sc->bce_btag, sc->bce_bhandle, 0, 0,
@@ -5366,7 +5397,7 @@ bce_tx_encap_skip_tso:
#endif
DBPRINT(sc, BCE_INFO_SEND,
- "%s(): Start: prod = 0x%04X, chain_prod = %04X, "
+ "%s(start): prod = 0x%04X, chain_prod = 0x%04X, "
"prod_bseq = 0x%08X\n",
__FUNCTION__, prod, chain_prod, prod_bseq);
@@ -5398,7 +5429,7 @@ bce_tx_encap_skip_tso:
DBRUNMSG(BCE_EXCESSIVE_SEND, bce_dump_tx_chain(sc, debug_prod, nsegs));
DBPRINT(sc, BCE_INFO_SEND,
- "%s(): End: prod = 0x%04X, chain_prod = %04X, "
+ "%s( end ): prod = 0x%04X, chain_prod = 0x%04X, "
"prod_bseq = 0x%08X\n",
__FUNCTION__, prod, chain_prod, prod_bseq);
@@ -5444,22 +5475,28 @@ bce_start_locked(struct ifnet *ifp)
int count = 0;
u16 tx_prod, tx_chain_prod;
- /* If there's no link or the transmit queue is empty then just exit. */
- if (!sc->bce_link || IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
- DBPRINT(sc, BCE_INFO_SEND, "%s(): No link or transmit queue empty.\n",
- __FUNCTION__);
- goto bce_start_locked_exit;
- }
-
/* prod points to the next free tx_bd. */
tx_prod = sc->tx_prod;
tx_chain_prod = TX_CHAIN_IDX(tx_prod);
DBPRINT(sc, BCE_INFO_SEND,
- "%s(): Start: tx_prod = 0x%04X, tx_chain_prod = %04X, "
+ "%s(enter): tx_prod = 0x%04X, tx_chain_prod = 0x%04X, "
"tx_prod_bseq = 0x%08X\n",
__FUNCTION__, tx_prod, tx_chain_prod, sc->tx_prod_bseq);
+ /* If there's no link or the transmit queue is empty then just exit. */
+ if (!sc->bce_link) {
+ DBPRINT(sc, BCE_INFO_SEND, "%s(): No link.\n",
+ __FUNCTION__);
+ goto bce_start_locked_exit;
+ }
+
+ if (IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
+ DBPRINT(sc, BCE_INFO_SEND, "%s(): Transmit queue empty.\n",
+ __FUNCTION__);
+ goto bce_start_locked_exit;
+ }
+
/*
* Keep adding entries while there is space in the ring.
*/
@@ -5502,11 +5539,6 @@ bce_start_locked(struct ifnet *ifp)
/* Update the driver's counters. */
tx_chain_prod = TX_CHAIN_IDX(sc->tx_prod);
- DBPRINT(sc, BCE_INFO_SEND,
- "%s(): End: tx_prod = 0x%04X, tx_chain_prod = 0x%04X, "
- "tx_prod_bseq = 0x%08X\n",
- __FUNCTION__, tx_prod, tx_chain_prod, sc->tx_prod_bseq);
-
/* Start the transmit. */
REG_WR16(sc, MB_TX_CID_ADDR + BCE_L2CTX_TX_HOST_BIDX, sc->tx_prod);
REG_WR(sc, MB_TX_CID_ADDR + BCE_L2CTX_TX_HOST_BSEQ, sc->tx_prod_bseq);
@@ -5515,6 +5547,11 @@ bce_start_locked(struct ifnet *ifp)
sc->watchdog_timer = BCE_TX_TIMEOUT;
bce_start_locked_exit:
+ DBPRINT(sc, BCE_INFO_SEND,
+ "%s(exit ): tx_prod = 0x%04X, tx_chain_prod = 0x%04X, "
+ "tx_prod_bseq = 0x%08X\n",
+ __FUNCTION__, tx_prod, tx_chain_prod, sc->tx_prod_bseq);
+
return;
}
@@ -5776,68 +5813,6 @@ bce_watchdog(struct bce_softc *sc)
}
-#ifdef DEVICE_POLLING
-static void
-bce_poll_locked(struct ifnet *ifp, enum poll_cmd cmd, int count)
-{
- struct bce_softc *sc = ifp->if_softc;
-
- BCE_LOCK_ASSERT(sc);
-
- sc->bce_rxcycles = count;
-
- bus_dmamap_sync(sc->status_tag, sc->status_map,
- BUS_DMASYNC_POSTWRITE);
-
- /* Check for any completed RX frames. */
- if (sc->status_block->status_rx_quick_consumer_index0 !=
- sc->hw_rx_cons)
- bce_rx_intr(sc);
-
- /* Check for any completed TX frames. */
- if (sc->status_block->status_tx_quick_consumer_index0 !=
- sc->hw_tx_cons)
- bce_tx_intr(sc);
-
- /* Check for new frames to transmit. */
- if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
- bce_start_locked(ifp);
-
-}
-
-
-static void
-bce_poll(struct ifnet *ifp, enum poll_cmd cmd, int count)
-{
- struct bce_softc *sc = ifp->if_softc;
-
- BCE_LOCK(sc);
- if (ifp->if_drv_flags & IFF_DRV_RUNNING)
- bce_poll_locked(ifp, cmd, count);
- BCE_UNLOCK(sc);
-}
-#endif /* DEVICE_POLLING */
-
-
-#if 0
-static inline int
-bce_has_work(struct bce_softc *sc)
-{
- struct status_block *stat = sc->status_block;
-
- if ((stat->status_rx_quick_consumer_index0 != sc->hw_rx_cons) ||
- (stat->status_tx_quick_consumer_index0 != sc->hw_tx_cons))
- return 1;
-
- if (((stat->status_attn_bits & STATUS_ATTN_BITS_LINK_STATE) != 0) !=
- bp->link_up)
- return 1;
-
- return 0;
-}
-#endif
-
-
/*
* Interrupt handler.
*/
@@ -5855,6 +5830,7 @@ bce_intr(void *xsc)
struct bce_softc *sc;
struct ifnet *ifp;
u32 status_attn_bits;
+ u16 hw_rx_cons, hw_tx_cons;
sc = xsc;
ifp = sc->bce_ifp;
@@ -5864,13 +5840,6 @@ bce_intr(void *xsc)
DBRUN(sc->interrupts_generated++);
-#ifdef DEVICE_POLLING
- if (ifp->if_capenable & IFCAP_POLLING) {
- DBPRINT(sc, BCE_INFO_MISC, "Polling enabled!\n");
- goto bce_intr_exit;
- }
-#endif
-
bus_dmamap_sync(sc->status_tag, sc->status_map,
BUS_DMASYNC_POSTWRITE);
@@ -5889,6 +5858,9 @@ bce_intr(void *xsc)
BCE_PCICFG_INT_ACK_CMD_USE_INT_HC_PARAM |
BCE_PCICFG_INT_ACK_CMD_MASK_INT);
+ hw_rx_cons = bce_get_hw_rx_cons(sc);
+ hw_tx_cons = bce_get_hw_tx_cons(sc);
+
/* Keep processing data as long as there is work to do. */
for (;;) {
@@ -5922,12 +5894,22 @@ bce_intr(void *xsc)
}
/* Check for any completed RX frames. */
- if (sc->status_block->status_rx_quick_consumer_index0 != sc->hw_rx_cons)
+ if (hw_rx_cons != sc->hw_rx_cons) {
+ DBPRINT(sc, BCE_INFO_RECV, "%s(): hw_rx_cons = 0x%04X, "
+ "sc->hw_rx_cons = 0x%04X\n", __FUNCTION__,
+ hw_rx_cons, sc->hw_rx_cons);
+
bce_rx_intr(sc);
+ }
/* Check for any completed TX frames. */
- if (sc->status_block->status_tx_quick_consumer_index0 != sc->hw_tx_cons)
+ if (hw_tx_cons != sc->hw_tx_cons) {
+ DBPRINT(sc, BCE_INFO_SEND, "%s(): hw_tx_cons = 0x%04X, "
+ "sc->hw_tx_cons = 0x%04X\n", __FUNCTION__,
+ hw_tx_cons, sc->hw_tx_cons);
+
bce_tx_intr(sc);
+ }
/* Save the status block index value for use during the next interrupt. */
sc->last_status_idx = sc->status_block->status_idx;
@@ -5937,8 +5919,10 @@ bce_intr(void *xsc)
BUS_SPACE_BARRIER_READ);
/* If there's no work left then exit the interrupt service routine. */
- if ((sc->status_block->status_rx_quick_consumer_index0 == sc->hw_rx_cons) &&
- (sc->status_block->status_tx_quick_consumer_index0 == sc->hw_tx_cons))
+ hw_rx_cons = bce_get_hw_rx_cons(sc);
+ hw_tx_cons = bce_get_hw_tx_cons(sc);
+
+ if ((hw_rx_cons == sc->hw_rx_cons) && (hw_tx_cons == sc->hw_tx_cons))
break;
}
@@ -6314,6 +6298,7 @@ bce_tick(void *xsc)
struct bce_softc *sc = xsc;
struct mii_data *mii;
struct ifnet *ifp;
+ u16 hw_rx_cons, hw_tx_cons;
ifp = sc->bce_ifp;
@@ -6332,6 +6317,20 @@ bce_tick(void *xsc)
/* Check that chip hasn't hung. */
bce_watchdog(sc);
+ hw_rx_cons = bce_get_hw_rx_cons(sc);
+ if (hw_rx_cons != sc->hw_rx_cons) {
+ DBPRINT(sc, BCE_INFO_RECV, "%s(): status_block->rx_cons0 = 0x%04X, "
+ "sc->hw_rx_cons = 0x%04X\n", __FUNCTION__,
+ hw_rx_cons, sc->hw_rx_cons);
+ }
+
+ hw_tx_cons = bce_get_hw_tx_cons(sc);
+ if (hw_tx_cons != sc->hw_tx_cons) {
+ DBPRINT(sc, BCE_INFO_SEND, "%s(): status_block->tx_cons0 = 0x%04X, "
+ "sc->hw_tx_cons = 0x%04X\n", __FUNCTION__,
+ hw_tx_cons, sc->hw_tx_cons);
+ }
+
/* If link is up already up then we're done. */
if (sc->bce_link)
goto bce_tick_locked_exit;
@@ -6347,6 +6346,7 @@ bce_tick(void *xsc)
IFM_SUBTYPE(mii->mii_media_active) == IFM_1000_SX) &&
bootverbose)
BCE_PRINTF("Gigabit link up\n");
+
/* Now that link is up, handle any outstanding TX traffic. */
if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
bce_start_locked(ifp);
@@ -6693,17 +6693,17 @@ bce_add_sysctls(struct bce_softc *sc)
0, "Number of TX interrupts");
SYSCTL_ADD_ULONG(ctx, children, OID_AUTO,
- "rx_intr_time",
- CTLFLAG_RD, &sc->rx_intr_time,
- "RX interrupt time");
-
- SYSCTL_ADD_ULONG(ctx, children, OID_AUTO,
"tx_intr_time",
CTLFLAG_RD, &sc->tx_intr_time,
"TX interrupt time");
#endif
+ SYSCTL_ADD_ULONG(ctx, children, OID_AUTO,
+ "rx_intr_time",
+ CTLFLAG_RD, &sc->rx_intr_time,
+ "RX interrupt time");
+
SYSCTL_ADD_INT(ctx, children, OID_AUTO,
"mbuf_alloc_failed",
CTLFLAG_RD, &sc->mbuf_alloc_failed,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5D267A3F22FD854F8F48B3D2B523819324EF6DEE3B>