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
