Date: Fri, 23 Nov 2012 11:19:44 +0000 (UTC) From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r243440 - in stable/9/sys: dev/bxe dev/e1000 dev/ixgbe dev/mxge dev/oce dev/vxge net ofed/drivers/net/mlx4 sys Message-ID: <201211231119.qANBJi3O001767@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: glebius Date: Fri Nov 23 11:19:43 2012 New Revision: 243440 URL: http://svnweb.freebsd.org/changeset/base/243440 Log: Merge r241037 from head: The drbr(9) API appeared to be so unclear, that most drivers in tree used it incorrectly, which lead to inaccurate overrated if_obytes accounting. The drbr(9) used to update ifnet stats on drbr_enqueue(), which is not accurate since enqueuing doesn't imply successful processing by driver. Dequeuing neither mean that. Most drivers also called drbr_stats_update() which did accounting again, leading to doubled if_obytes statistics. And in case of severe transmitting, when a packet could be several times enqueued and dequeued it could have been accounted several times. o Thus, make drbr(9) API thinner. Now drbr(9) merely chooses between ALTQ queueing or buf_ring(9) queueing. - It doesn't touch the buf_ring stats any more. - It doesn't touch ifnet stats anymore. - drbr_stats_update() no longer exists. o buf_ring(9) handles its stats itself: - It handles br_drops itself. - br_prod_bytes stats are dropped. Rationale: no one ever reads them but update of a common counter on every packet negatively affects performance due to excessive cache invalidation. - buf_ring_enqueue_bytes() reduced to buf_ring_enqueue(), since we no longer account bytes. o Drivers handle their stats theirselves: if_obytes, if_omcasts. o mlx4(4), igb(4), em(4), vxge(4), oce(4) and ixv(4) no longer use drbr_stats_update(), and update ifnet stats theirselves. o bxe(4) was the most correct driver, it didn't call drbr_stats_update(), thus it was the only driver accurate under moderate load. Now it also maintains stats itself. o ixgbe(4) had already taken stats from hardware, so just - drop software stats updating. - take multicast packet count from hardware as well. o mxge(4) just no longer needs NO_SLOW_STATS define. o cxgb(4), cxgbe(4) need no change, since they obtain stats from hardware. Reviewed by: jfv, gnn Modified: stable/9/sys/dev/bxe/if_bxe.c stable/9/sys/dev/e1000/if_em.c stable/9/sys/dev/e1000/if_igb.c stable/9/sys/dev/ixgbe/ixgbe.c stable/9/sys/dev/ixgbe/ixv.c stable/9/sys/dev/mxge/if_mxge.c stable/9/sys/dev/oce/oce_if.c stable/9/sys/dev/vxge/vxge.c stable/9/sys/net/if_var.h stable/9/sys/ofed/drivers/net/mlx4/en_tx.c stable/9/sys/sys/buf_ring.h Directory Properties: stable/9/sys/ (props changed) stable/9/sys/dev/ (props changed) stable/9/sys/dev/e1000/ (props changed) stable/9/sys/dev/ixgbe/ (props changed) Modified: stable/9/sys/dev/bxe/if_bxe.c ============================================================================== --- stable/9/sys/dev/bxe/if_bxe.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/bxe/if_bxe.c Fri Nov 23 11:19:43 2012 (r243440) @@ -9552,6 +9552,11 @@ bxe_tx_mq_start_locked(struct ifnet *ifp /* The transmit frame was enqueued successfully. */ tx_count++; + /* Update stats */ + ifp->if_obytes += next->m_pkthdr.len; + if (next->m_flags & M_MCAST) + ifp->if_omcasts++; + /* Send a copy of the frame to any BPF listeners. */ BPF_MTAP(ifp, next); Modified: stable/9/sys/dev/e1000/if_em.c ============================================================================== --- stable/9/sys/dev/e1000/if_em.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/e1000/if_em.c Fri Nov 23 11:19:43 2012 (r243440) @@ -922,7 +922,9 @@ em_mq_start_locked(struct ifnet *ifp, st break; } enq++; - drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags); + ifp->if_obytes += next->m_pkthdr.len; + if (next->m_flags & M_MCAST) + ifp->if_omcasts++; ETHER_BPF_MTAP(ifp, next); if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) break; Modified: stable/9/sys/dev/e1000/if_igb.c ============================================================================== --- stable/9/sys/dev/e1000/if_igb.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/e1000/if_igb.c Fri Nov 23 11:19:43 2012 (r243440) @@ -1006,7 +1006,9 @@ igb_mq_start_locked(struct ifnet *ifp, s break; } enq++; - drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags); + ifp->if_obytes += next->m_pkthdr.len; + if (next->m_flags & M_MCAST) + ifp->if_omcasts++; ETHER_BPF_MTAP(ifp, next); if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) break; Modified: stable/9/sys/dev/ixgbe/ixgbe.c ============================================================================== --- stable/9/sys/dev/ixgbe/ixgbe.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/ixgbe/ixgbe.c Fri Nov 23 11:19:43 2012 (r243440) @@ -841,7 +841,6 @@ ixgbe_mq_start_locked(struct ifnet *ifp, break; } enqueued++; - drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags); /* Send a copy of the frame to the BPF listener */ ETHER_BPF_MTAP(ifp, next); if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) @@ -5258,6 +5257,7 @@ ixgbe_update_stats_counters(struct adapt ifp->if_ibytes = adapter->stats.gorc; ifp->if_obytes = adapter->stats.gotc; ifp->if_imcasts = adapter->stats.mprc; + ifp->if_omcasts = adapter->stats.mptc; ifp->if_collisions = 0; /* Rx Errors */ Modified: stable/9/sys/dev/ixgbe/ixv.c ============================================================================== --- stable/9/sys/dev/ixgbe/ixv.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/ixgbe/ixv.c Fri Nov 23 11:19:43 2012 (r243440) @@ -636,7 +636,9 @@ ixv_mq_start_locked(struct ifnet *ifp, s break; } enqueued++; - drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags); + ifp->if_obytes += next->m_pkthdr.len; + if (next->m_flags & M_MCAST) + ifp->if_omcasts++; /* Send a copy of the frame to the BPF listener */ ETHER_BPF_MTAP(ifp, next); if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) Modified: stable/9/sys/dev/mxge/if_mxge.c ============================================================================== --- stable/9/sys/dev/mxge/if_mxge.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/mxge/if_mxge.c Fri Nov 23 11:19:43 2012 (r243440) @@ -47,8 +47,6 @@ __FBSDID("$FreeBSD$"); #include <sys/sx.h> #include <sys/taskqueue.h> -/* count xmits ourselves, rather than via drbr */ -#define NO_SLOW_STATS #include <net/if.h> #include <net/if_arp.h> #include <net/ethernet.h> Modified: stable/9/sys/dev/oce/oce_if.c ============================================================================== --- stable/9/sys/dev/oce/oce_if.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/oce/oce_if.c Fri Nov 23 11:19:43 2012 (r243440) @@ -1222,7 +1222,9 @@ oce_multiq_transmit(struct ifnet *ifp, s } break; } - drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags); + ifp->if_obytes += next->m_pkthdr.len; + if (next->m_flags & M_MCAST) + ifp->if_omcasts++; ETHER_BPF_MTAP(ifp, next); next = drbr_dequeue(ifp, br); } Modified: stable/9/sys/dev/vxge/vxge.c ============================================================================== --- stable/9/sys/dev/vxge/vxge.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/dev/vxge/vxge.c Fri Nov 23 11:19:43 2012 (r243440) @@ -709,7 +709,9 @@ vxge_mq_send_locked(ifnet_t ifp, vxge_vp VXGE_DRV_STATS(vpath, tx_again); break; } - drbr_stats_update(ifp, next->m_pkthdr.len, next->m_flags); + ifp->if_obytes += next->m_pkthdr.len; + if (next->m_flags & M_MCAST) + ifp->if_omcasts++; /* Send a copy of the frame to the BPF listener */ ETHER_BPF_MTAP(ifp, next); Modified: stable/9/sys/net/if_var.h ============================================================================== --- stable/9/sys/net/if_var.h Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/net/if_var.h Fri Nov 23 11:19:43 2012 (r243440) @@ -589,22 +589,10 @@ do { \ } while (0) #ifdef _KERNEL -static __inline void -drbr_stats_update(struct ifnet *ifp, int len, int mflags) -{ -#ifndef NO_SLOW_STATS - ifp->if_obytes += len; - if (mflags & M_MCAST) - ifp->if_omcasts++; -#endif -} - static __inline int drbr_enqueue(struct ifnet *ifp, struct buf_ring *br, struct mbuf *m) { int error = 0; - int len = m->m_pkthdr.len; - int mflags = m->m_flags; #ifdef ALTQ if (ALTQ_IS_ENABLED(&ifp->if_snd)) { @@ -612,12 +600,10 @@ drbr_enqueue(struct ifnet *ifp, struct b return (error); } #endif - if ((error = buf_ring_enqueue_bytes(br, m, len)) == ENOBUFS) { - br->br_drops++; + error = buf_ring_enqueue(br, m); + if (error) m_freem(m); - } else - drbr_stats_update(ifp, len, mflags); - + return (error); } Modified: stable/9/sys/ofed/drivers/net/mlx4/en_tx.c ============================================================================== --- stable/9/sys/ofed/drivers/net/mlx4/en_tx.c Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/ofed/drivers/net/mlx4/en_tx.c Fri Nov 23 11:19:43 2012 (r243440) @@ -948,7 +948,9 @@ mlx4_en_transmit_locked(struct ifnet *de break; } enqueued++; - drbr_stats_update(dev, next->m_pkthdr.len, next->m_flags); + dev->if_obytes += next->m_pkthdr.len; + if (next->m_flags & M_MCAST) + dev->if_omcasts++; /* Send a copy of the frame to the BPF listener */ ETHER_BPF_MTAP(dev, next); if ((dev->if_drv_flags & IFF_DRV_RUNNING) == 0) Modified: stable/9/sys/sys/buf_ring.h ============================================================================== --- stable/9/sys/sys/buf_ring.h Fri Nov 23 10:14:54 2012 (r243439) +++ stable/9/sys/sys/buf_ring.h Fri Nov 23 11:19:43 2012 (r243440) @@ -48,7 +48,6 @@ struct buf_ring { int br_prod_mask; uint64_t br_drops; uint64_t br_prod_bufs; - uint64_t br_prod_bytes; /* * Pad out to next L2 cache line */ @@ -74,7 +73,7 @@ struct buf_ring { * */ static __inline int -buf_ring_enqueue_bytes(struct buf_ring *br, void *buf, int nbytes) +buf_ring_enqueue(struct buf_ring *br, void *buf) { uint32_t prod_head, prod_next; uint32_t cons_tail; @@ -95,6 +94,7 @@ buf_ring_enqueue_bytes(struct buf_ring * prod_next = (prod_head + 1) & br->br_prod_mask; if (prod_next == cons_tail) { + br->br_drops++; critical_exit(); return (ENOBUFS); } @@ -117,19 +117,11 @@ buf_ring_enqueue_bytes(struct buf_ring * while (br->br_prod_tail != prod_head) cpu_spinwait(); br->br_prod_bufs++; - br->br_prod_bytes += nbytes; br->br_prod_tail = prod_next; critical_exit(); return (0); } -static __inline int -buf_ring_enqueue(struct buf_ring *br, void *buf) -{ - - return (buf_ring_enqueue_bytes(br, buf, 0)); -} - /* * multi-consumer safe dequeue *
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201211231119.qANBJi3O001767>