use ifqueue packet statistics
ifqueues count packets, therefore if_enqueue doesnt have to. this adds reading of the ifq counters into the if_data values so userland still looks right. this is another step toward multiple tx queues. ok? Index: if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.475 diff -u -p -r1.475 if.c --- if.c22 Jan 2017 10:17:39 - 1.475 +++ if.c22 Jan 2017 10:35:30 - @@ -583,8 +583,7 @@ if_start_locked(struct ifnet *ifp) int if_enqueue(struct ifnet *ifp, struct mbuf *m) { - int length, error = 0; - unsigned short mflags; + int error = 0; #if NBRIDGE > 0 if (ifp->if_bridgeport && (m->m_flags & M_PROTO1) == 0) { @@ -595,9 +594,6 @@ if_enqueue(struct ifnet *ifp, struct mbu } #endif - length = m->m_pkthdr.len; - mflags = m->m_flags; - #if NPF > 0 pf_pkt_unlink_state_key(m); #endif /* NPF > 0 */ @@ -610,11 +606,6 @@ if_enqueue(struct ifnet *ifp, struct mbu if (error) return (error); - ifp->if_opackets++; - ifp->if_obytes += length; - if (mflags & M_MCAST) - ifp->if_omcasts++; - if_start(ifp); return (0); @@ -1799,10 +1790,13 @@ ifioctl(struct socket *so, u_long cmd, c ifr->ifr_hardmtu = ifp->if_hardmtu; break; - case SIOCGIFDATA: - error = copyout((caddr_t)&ifp->if_data, ifr->ifr_data, + case SIOCGIFDATA: { + struct if_data data; + if_data(&data, ifp); + error = copyout((caddr_t)&ifp->if_data, &data, sizeof(ifp->if_data)); break; + } case SIOCSIFFLAGS: if ((error = suser(p, 0)) != 0) @@ -2167,6 +2161,34 @@ ifconf(u_long cmd, caddr_t data) } ifc->ifc_len -= space; return (error); +} + +void +if_data(struct if_data *data, struct ifnet *ifp) +{ + struct ifqueue *ifq; + uint64_t opackets = 0; + uint64_t obytes = 0; + uint64_t omcasts = 0; + uint64_t oqdrops = 0; + + ifq = &ifp->if_snd; + + /* it's ugly reaching into an ifq like this */ + mtx_enter(&ifq->ifq_mtx); + opackets += ifq->ifq_packets; + obytes += ifq->ifq_bytes; + oqdrops += ifq->ifq_qdrops; + omcasts += ifq->ifq_mcasts; + /* ifq->ifq_errors */ + mtx_leave(&ifq->ifq_mtx); + + *data = ifp->if_data; + data->ifi_opackets += opackets; + data->ifi_obytes += obytes; + data->ifi_oqdrops += oqdrops; + data->ifi_omcasts += omcasts; + /* ifp->if_data.ifi_oerrors */ } /* Index: if.h === RCS file: /cvs/src/sys/net/if.h,v retrieving revision 1.181 diff -u -p -r1.181 if.h --- if.h12 Dec 2016 09:51:30 - 1.181 +++ if.h22 Jan 2017 10:35:30 - @@ -469,6 +469,7 @@ voidif_downall(void); void if_link_state_change(struct ifnet *); void if_up(struct ifnet *); intifconf(u_long, caddr_t); +void if_data(struct if_data *, struct ifnet *); void ifinit(void); intifioctl(struct socket *, u_long, caddr_t, struct proc *); intifpromisc(struct ifnet *, int); Index: rtsock.c === RCS file: /cvs/src/sys/net/rtsock.c,v retrieving revision 1.216 diff -u -p -r1.216 rtsock.c --- rtsock.c22 Jan 2017 04:31:02 - 1.216 +++ rtsock.c22 Jan 2017 10:35:30 - @@ -1268,7 +1268,7 @@ rt_ifmsg(struct ifnet *ifp) ifm->ifm_tableid = ifp->if_rdomain; ifm->ifm_flags = ifp->if_flags; ifm->ifm_xflags = ifp->if_xflags; - ifm->ifm_data = ifp->if_data; + if_data(&ifm->ifm_data, ifp); ifm->ifm_addrs = 0; route_input(m, AF_UNSPEC); } @@ -1473,7 +1473,7 @@ sysctl_iflist(int af, struct walkarg *w) ifm->ifm_index = ifp->if_index; ifm->ifm_tableid = ifp->if_rdomain; ifm->ifm_flags = ifp->if_flags; - ifm->ifm_data = ifp->if_data; + if_data(&ifm->ifm_data, ifp); ifm->ifm_addrs = info.rti_addrs; error = copyout(ifm, w->w_where, len); if (error)
Re: use ifqueue packet statistics
On 22/01/17(Sun) 20:38, David Gwynne wrote: > ifqueues count packets, therefore if_enqueue doesnt have to. > > this adds reading of the ifq counters into the if_data values so > userland still looks right. > > this is another step toward multiple tx queues. I like it, some comments below. > @@ -1799,10 +1790,13 @@ ifioctl(struct socket *so, u_long cmd, c > ifr->ifr_hardmtu = ifp->if_hardmtu; > break; > > - case SIOCGIFDATA: > - error = copyout((caddr_t)&ifp->if_data, ifr->ifr_data, > + case SIOCGIFDATA: { > + struct if_data data; I'd use ifdata that would avoid shadowing the 'data' pointer. > + if_data(&data, ifp); > + error = copyout((caddr_t)&ifp->if_data, &data, > sizeof(ifp->if_data)); Shouldn't this be: copyout(&data, ifr->ifr_data, sizeof(data)) > break; > + } > > case SIOCSIFFLAGS: > if ((error = suser(p, 0)) != 0) > @@ -2167,6 +2161,34 @@ ifconf(u_long cmd, caddr_t data) > } > ifc->ifc_len -= space; > return (error); > +} > + > +void > +if_data(struct if_data *data, struct ifnet *ifp) Could you pass ``ifp'' as first argument to keep coherency with the rest of if.c? > +{ > + struct ifqueue *ifq; > + uint64_t opackets = 0; > + uint64_t obytes = 0; > + uint64_t omcasts = 0; > + uint64_t oqdrops = 0; > + > + ifq = &ifp->if_snd; > + > + /* it's ugly reaching into an ifq like this */ What's ugly? I'm not sure this comment help. > + mtx_enter(&ifq->ifq_mtx); > + opackets += ifq->ifq_packets; > + obytes += ifq->ifq_bytes; > + oqdrops += ifq->ifq_qdrops; > + omcasts += ifq->ifq_mcasts; > + /* ifq->ifq_errors */ > + mtx_leave(&ifq->ifq_mtx); > + > + *data = ifp->if_data; > + data->ifi_opackets += opackets; > + data->ifi_obytes += obytes; > + data->ifi_oqdrops += oqdrops; > + data->ifi_omcasts += omcasts; > + /* ifp->if_data.ifi_oerrors */ > } > > /* > Index: if.h > === > RCS file: /cvs/src/sys/net/if.h,v > retrieving revision 1.181 > diff -u -p -r1.181 if.h > --- if.h 12 Dec 2016 09:51:30 - 1.181 > +++ if.h 22 Jan 2017 10:35:30 - > @@ -469,6 +469,7 @@ void if_downall(void); > void if_link_state_change(struct ifnet *); > void if_up(struct ifnet *); > int ifconf(u_long, caddr_t); > +void if_data(struct if_data *, struct ifnet *); > void ifinit(void); > int ifioctl(struct socket *, u_long, caddr_t, struct proc *); > int ifpromisc(struct ifnet *, int); > Index: rtsock.c > === > RCS file: /cvs/src/sys/net/rtsock.c,v > retrieving revision 1.216 > diff -u -p -r1.216 rtsock.c > --- rtsock.c 22 Jan 2017 04:31:02 - 1.216 > +++ rtsock.c 22 Jan 2017 10:35:30 - > @@ -1268,7 +1268,7 @@ rt_ifmsg(struct ifnet *ifp) > ifm->ifm_tableid = ifp->if_rdomain; > ifm->ifm_flags = ifp->if_flags; > ifm->ifm_xflags = ifp->if_xflags; > - ifm->ifm_data = ifp->if_data; > + if_data(&ifm->ifm_data, ifp); > ifm->ifm_addrs = 0; > route_input(m, AF_UNSPEC); > } > @@ -1473,7 +1473,7 @@ sysctl_iflist(int af, struct walkarg *w) > ifm->ifm_index = ifp->if_index; > ifm->ifm_tableid = ifp->if_rdomain; > ifm->ifm_flags = ifp->if_flags; > - ifm->ifm_data = ifp->if_data; > + if_data(&ifm->ifm_data, ifp); > ifm->ifm_addrs = info.rti_addrs; > error = copyout(ifm, w->w_where, len); > if (error) >
Re: use ifqueue packet statistics
On Mon, Jan 23, 2017 at 10:10:23AM +1000, Martin Pieuchot wrote: > On 22/01/17(Sun) 20:38, David Gwynne wrote: > > ifqueues count packets, therefore if_enqueue doesnt have to. > > > > this adds reading of the ifq counters into the if_data values so > > userland still looks right. > > > > this is another step toward multiple tx queues. > > I like it, some comments below. > > > > @@ -1799,10 +1790,13 @@ ifioctl(struct socket *so, u_long cmd, c > > ifr->ifr_hardmtu = ifp->if_hardmtu; > > break; > > > > - case SIOCGIFDATA: > > - error = copyout((caddr_t)&ifp->if_data, ifr->ifr_data, > > + case SIOCGIFDATA: { > > + struct if_data data; > > I'd use ifdata that would avoid shadowing the 'data' pointer. ok. > > + error = copyout((caddr_t)&ifp->if_data, &data, > > sizeof(ifp->if_data)); > > Shouldn't this be: > > copyout(&data, ifr->ifr_data, sizeof(data)) yes. > > +void > > +if_data(struct if_data *data, struct ifnet *ifp) > > Could you pass ``ifp'' as first argument to keep coherency with the rest > of if.c? i can. i thought of it as a memcpy like function, so dst src. fixed. > > + /* it's ugly reaching into an ifq like this */ > > What's ugly? I'm not sure this comment help. if.c doesnt know much about ifq internals, except for this bit. Index: if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.475 diff -u -p -r1.475 if.c --- if.c22 Jan 2017 10:17:39 - 1.475 +++ if.c23 Jan 2017 00:32:53 - @@ -583,8 +583,7 @@ if_start_locked(struct ifnet *ifp) int if_enqueue(struct ifnet *ifp, struct mbuf *m) { - int length, error = 0; - unsigned short mflags; + int error = 0; #if NBRIDGE > 0 if (ifp->if_bridgeport && (m->m_flags & M_PROTO1) == 0) { @@ -595,9 +594,6 @@ if_enqueue(struct ifnet *ifp, struct mbu } #endif - length = m->m_pkthdr.len; - mflags = m->m_flags; - #if NPF > 0 pf_pkt_unlink_state_key(m); #endif /* NPF > 0 */ @@ -610,11 +606,6 @@ if_enqueue(struct ifnet *ifp, struct mbu if (error) return (error); - ifp->if_opackets++; - ifp->if_obytes += length; - if (mflags & M_MCAST) - ifp->if_omcasts++; - if_start(ifp); return (0); @@ -1799,10 +1790,12 @@ ifioctl(struct socket *so, u_long cmd, c ifr->ifr_hardmtu = ifp->if_hardmtu; break; - case SIOCGIFDATA: - error = copyout((caddr_t)&ifp->if_data, ifr->ifr_data, - sizeof(ifp->if_data)); + case SIOCGIFDATA: { + struct if_data ifdata; + if_data(ifp, &ifdata); + error = copyout(&ifdata, &ifr->ifr_data, sizeof(ifdata)); break; + } case SIOCSIFFLAGS: if ((error = suser(p, 0)) != 0) @@ -2167,6 +2160,33 @@ ifconf(u_long cmd, caddr_t data) } ifc->ifc_len -= space; return (error); +} + +void +if_data(struct ifnet *ifp, struct if_data *data) +{ + struct ifqueue *ifq; + uint64_t opackets = 0; + uint64_t obytes = 0; + uint64_t omcasts = 0; + uint64_t oqdrops = 0; + + ifq = &ifp->if_snd; + + mtx_enter(&ifq->ifq_mtx); + opackets += ifq->ifq_packets; + obytes += ifq->ifq_bytes; + oqdrops += ifq->ifq_qdrops; + omcasts += ifq->ifq_mcasts; + /* ifq->ifq_errors */ + mtx_leave(&ifq->ifq_mtx); + + *data = ifp->if_data; + data->ifi_opackets += opackets; + data->ifi_obytes += obytes; + data->ifi_oqdrops += oqdrops; + data->ifi_omcasts += omcasts; + /* ifp->if_data.ifi_oerrors */ } /* Index: if.h === RCS file: /cvs/src/sys/net/if.h,v retrieving revision 1.181 diff -u -p -r1.181 if.h --- if.h12 Dec 2016 09:51:30 - 1.181 +++ if.h23 Jan 2017 00:32:53 - @@ -469,6 +469,7 @@ voidif_downall(void); void if_link_state_change(struct ifnet *); void if_up(struct ifnet *); intifconf(u_long, caddr_t); +void if_data(struct ifnet *, struct if_data *); void ifinit(void); intifioctl(struct socket *, u_long, caddr_t, struct proc *); intifpromisc(struct ifnet *, int); Index: rtsock.c === RCS file: /cvs/src/sys/net/rtsock.c,v retrieving revision 1.216 diff -u -p -r1.216 rtsock.c --- rtsock.c22 Jan 2017 04:31:02 - 1.216 +++ rtsock.c23 Jan 2017 00:32:53 - @@ -1268,7 +1268,7 @@ rt_ifmsg(struct ifnet *ifp) ifm->ifm_tableid = ifp->if_rdomain; ifm->ifm_flags = ifp->if_flags; ifm->ifm_xflags = ifp->if_xflags; - ifm->ifm_data = ifp->if_data; + if_data(ifp, &ifm->ifm_data); ifm->ifm_addrs = 0; route_input(m