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 -0000 1.181
> +++ if.h 22 Jan 2017 10:35:30 -0000
> @@ -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 -0000 1.216
> +++ rtsock.c 22 Jan 2017 10:35:30 -0000
> @@ -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)
>
