Date: Fri, 19 Dec 2014 17:50:19 +0300 From: "Andrey V. Elsukov" <ae@FreeBSD.org> To: Adrian Chadd <adrian@freebsd.org> Cc: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: [RFC] add macros for ifnet statistic accounting Message-ID: <54943B2B.9000407@FreeBSD.org> In-Reply-To: <CAJ-Vmo=vP3TeV0G0vib4JuXx_%2B13x7y7CYMsv29_Ajv5U0c4XQ@mail.gmail.com> References: <546E25D1.7020900@FreeBSD.org> <CAJ-Vmo=vP3TeV0G0vib4JuXx_%2B13x7y7CYMsv29_Ajv5U0c4XQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--2q53MbB66jEuwLhXAgnSpjUvUk8hRlPPm
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable
On 20.11.2014 20:38, Adrian Chadd wrote:
> On 20 November 2014 09:33, Andrey V. Elsukov <ae@freebsd.org> wrote:
>> Hi All,
>>
>> we already did some changes in network stack in head/, that made abili=
ty
>> for merging changes into stable branches much harder.
>>
>> What you think about adding the following macro to head/:
>>
>> --- if_var.h (revision 274736)
>> +++ if_var.h (working copy)
>> @@ -111,6 +111,10 @@ typedef enum {
>> IFCOUNTERS /* Array size. */
>> } ift_counter;
>>
>> +#define IFSTAT_ADD(ifp, name, value) \
>> + if_inc_counter((ifp), IFCOUNTER_ ## name, (value))
>> +#define IFSTAT_INC(ifp, name) IFSTAT_ADD(ifp, name, 1)
>> +
>> typedef struct ifnet * if_t;
>>
>> typedef void (*if_start_fn_t)(if_t);
>>
>>
>> And then make a direct commit to stable/* branches like this:
>>
>> --- if_var.h (revision 274750)
>> +++ if_var.h (working copy)
>> @@ -104,6 +104,23 @@ VNET_DECLARE(struct pfil_head, link_pfil_hook); =
/*
>> #define V_link_pfil_hook VNET(link_pfil_hook)
>> #endif /* _KERNEL */
>>
>> +#define IFSTAT_ADD(ifp, name, value) \
>> + IFSTAT_ ## name ## _ADD(ifp, value)
>> +#define IFSTAT_INC(ifp, name) IFSTAT_ADD(ifp, name, 1)
>> +
>> +#define IFSTAT_IPACKETS_ADD(ifp, inc) (ifp)->if_ipackets +=3D=
(inc)
>> +#define IFSTAT_IERRORS_ADD(ifp, inc) (ifp)->if_ierrors +=3D=
(inc)
>> +#define IFSTAT_OPACKETS_ADD(ifp, inc) (ifp)->if_opackets +=3D=
(inc)
>> +#define IFSTAT_OERRORS_ADD(ifp, inc) (ifp)->if_oerrors +=3D=
(inc)
>> +#define IFSTAT_COLLISIONS_ADD(ifp, inc) (ifp)->if_collisions +=
=3D (inc)
>> +#define IFSTAT_IBYTES_ADD(ifp, inc) (ifp)->if_ibytes +=3D =
(inc)
>> +#define IFSTAT_OBYTES_ADD(ifp, inc) (ifp)->if_obytes +=3D =
(inc)
>> +#define IFSTAT_IMCASTS_ADD(ifp, inc) (ifp)->if_imcasts +=3D=
(inc)
>> +#define IFSTAT_OMCASTS_ADD(ifp, inc) (ifp)->if_omcasts +=3D=
(inc)
>> +#define IFSTAT_IQDROPS_ADD(ifp, inc) (ifp)->if_iqdrops +=3D=
(inc)
>> +#define IFSTAT_OQDROPS_ADD(ifp, inc) /* NOP */
>> +#define IFSTAT_NOPROTO_ADD(ifp, inc) (ifp)->if_noproto +=3D=
(inc)
>> +
>> /*
>> * Structure defining a queue for a network interface.
>> */
>>
>> This can make merging a little bit easier, at least for generic code i=
n
>> the network stack.
It looks there are not so much people, who wants this feature. We
discussed this with glebius@ and probably the better solution will be
merging ift_counter enum and adding if_inc_counter() function to
stable/10. I looked how other BSDs doing this accounting and only DfBSD
has macro IFNET_STAT_INC(). But since we are planning to make opaque
ifnet, it doesn't matter how we will do accounting, anyway it will be
incompatible with other BSDs.
If there is no objections I'll commit this after weekend.
Index: if.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- if.c (revision 275939)
+++ if.c (working copy)
@@ -1450,6 +1450,56 @@ if_rtdel(struct radix_node *rn, void *arg)
}
/*
+ * A compatibility function returns ifnet counter values.
+ */
+uint64_t
+if_get_counter_default(struct ifnet *ifp, ift_counter cnt)
+{
+
+ KASSERT(cnt < IFCOUNTERS, ("%s: invalid cnt %d", __func__, cnt));
+
+ switch (cnt) {
+ case IFCOUNTER_IPACKETS: return (ifp->if_ipackets);
+ case IFCOUNTER_IERRORS: return (ifp->if_ierrors);
+ case IFCOUNTER_OPACKETS: return (ifp->if_opackets);
+ case IFCOUNTER_OERRORS: return (ifp->if_oerrors);
+ case IFCOUNTER_COLLISIONS: return (ifp->if_collisions);
+ case IFCOUNTER_IBYTES: return (ifp->if_ibytes);
+ case IFCOUNTER_OBYTES: return (ifp->if_obytes);
+ case IFCOUNTER_IMCASTS: return (ifp->if_imcasts);
+ case IFCOUNTER_OMCASTS: return (ifp->if_omcasts);
+ case IFCOUNTER_IQDROPS: return (ifp->if_iqdrops);
+ case IFCOUNTER_NOPROTO: return (ifp->if_noproto);
+ };
+ return (0);
+}
+
+/*
+ * Increase an ifnet counter. Usually used for counters shared
+ * between the stack and a driver, but function supports them all.
+ */
+void
+if_inc_counter(struct ifnet *ifp, ift_counter cnt, int64_t inc)
+{
+
+ KASSERT(cnt < IFCOUNTERS, ("%s: invalid cnt %d", __func__, cnt));
+
+ switch (cnt) {
+ case IFCOUNTER_IPACKETS: ifp->if_ipackets +=3D inc; break;
+ case IFCOUNTER_IERRORS: ifp->if_ierrors +=3D inc; break;
+ case IFCOUNTER_OPACKETS: ifp->if_opackets +=3D inc; break;
+ case IFCOUNTER_OERRORS: ifp->if_oerrors +=3D inc; break;
+ case IFCOUNTER_COLLISIONS: ifp->if_collisions +=3D inc; break;
+ case IFCOUNTER_IBYTES: ifp->if_ibytes +=3D inc; break;
+ case IFCOUNTER_OBYTES: ifp->if_obytes +=3D inc; break;
+ case IFCOUNTER_IMCASTS: ifp->if_imcasts +=3D inc; break;
+ case IFCOUNTER_OMCASTS: ifp->if_omcasts +=3D inc; break;
+ case IFCOUNTER_IQDROPS: ifp->if_iqdrops +=3D inc; break;
+ case IFCOUNTER_NOPROTO: ifp->if_noproto +=3D inc; break;
+ };
+}
+
+/*
* Wrapper functions for struct ifnet address list locking macros.
These are
* used by kernel modules to avoid encoding programming interface or bin=
ary
* interface assumptions that may be violated when kernel-internal locki=
ng
Index: if_var.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- if_var.h (revision 275939)
+++ if_var.h (working copy)
@@ -104,6 +104,22 @@ VNET_DECLARE(struct pfil_head, link_pfil_hook); /*
#define V_link_pfil_hook VNET(link_pfil_hook)
#endif /* _KERNEL */
+typedef enum {
+ IFCOUNTER_IPACKETS =3D 0,
+ IFCOUNTER_IERRORS,
+ IFCOUNTER_OPACKETS,
+ IFCOUNTER_OERRORS,
+ IFCOUNTER_COLLISIONS,
+ IFCOUNTER_IBYTES,
+ IFCOUNTER_OBYTES,
+ IFCOUNTER_IMCASTS,
+ IFCOUNTER_OMCASTS,
+ IFCOUNTER_IQDROPS,
+ IFCOUNTER_OQDROPS,
+ IFCOUNTER_NOPROTO,
+ IFCOUNTERS /* Array size. */
+} ift_counter;
+
/*
* Structure defining a queue for a network interface.
*/
@@ -981,6 +997,8 @@ typedef void *if_com_alloc_t(u_char type, struct i
typedef void if_com_free_t(void *com, u_char type);
void if_register_com_alloc(u_char type, if_com_alloc_t *a,
if_com_free_t *f);
void if_deregister_com_alloc(u_char type);
+uint64_t if_get_counter_default(struct ifnet *, ift_counter);
+void if_inc_counter(struct ifnet *, ift_counter, int64_t);
#define IF_LLADDR(ifp) \
LLADDR((struct sockaddr_dl *)((ifp)->if_addr->ifa_addr))
--=20
WBR, Andrey V. Elsukov
--2q53MbB66jEuwLhXAgnSpjUvUk8hRlPPm
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJUlDsyAAoJEAHF6gQQyKF64gYH/3sfTX6mrhxU+4nUsT70GaOe
Es7sl+RKJgaf7Orz72IngTbFSP5Glq0huTriznGPC49PoZKIFeq0G/v7GuZLd+hC
WUDcJLKHHCPQ4LG5KC5vTi7krnnoh+IufDRTF4c4eytpQT76TVaeUTDCv/nWYzFY
sgGFBsUV8UeaWBwcPsOY25BfwMWi8Ige2dXB+4JIEZUTmRJ3ljOVe0coUKBsqffm
5nk7xpk0n9Sk8IS4ROQsElqRkfDXuHcbhm/06zH1TupXtFjzx6ypmw0o3PzsLjMe
ozB9HLy3cwh9fKC7eT0jsQbuJTzLarstGko+VeyPadvTnWgA37Rcxz4Vu7y2w/w=
=nUTn
-----END PGP SIGNATURE-----
--2q53MbB66jEuwLhXAgnSpjUvUk8hRlPPm--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54943B2B.9000407>