Re: Improving the data supplied by BPF

[christos%zoulas.com@localhost (Christos Zoulas)]

On Dec 25, 11:45pm, lacombar%gmail.com@localhost ("Arnaud Lacombe") wrote:
-- Subject: Re: Improving the data supplied by BPF

| 2^32 1500-bytes packets is about 6TB of data, on 100Mbit link, the
| sequence number will wrap after 5.6 days (if you consider
| uni-directional traffic), on a 1Gb link, half a day and a bit more
| than 1 hour on a 10Gb link. This is the worst case scenario. Two
| records taken at the <wrap_time> interval will likely collide on
| high-load link.

we could make it 64 bit, but still what's the scenario here? That
we look at the log file and we can't tell if wrapped?

| >>why unsigned ? currently `tv_sec' is signed. Why not using time_t ?
| >>There is an obvious ABI breakage when we will switch to 64bits time_t
| >>but this is be a better type than raw integer. The breakage is a
| >>different trouble and should be dealt with separately.
| >
| > You answered your own question. All types in the struct should be fixed size
| > and time_t will change soon to be 64 bits.
| >
| This is not the problem here.
| 
| The ABI breakage will anyway cause a problem with all system call
| taking a `struct timespec' as a parameter or including a `struct
| timespec' in one of their parameter.

This has already been taken care of in the branch, along with timeval.
All the calls that use them have been versioned.

| > In the time_t branch it already uses nanoseconds. Plus why you would want to
| > stick with micros in the new interface?
| >
| The patch I commented used a micro value to get a nano value. I don't
| see your point. I don't get the link between Darren's patch and the
| time_t branch either.

In the time_t branch time is 64 bits and most networking kernel sampling
code has been converted to timespecs.

| the current BPF don't... It use a `struct timeval' whose tv_sec and
| tv_usec are `long' which is not portable across architecture (i386's
| long == 4, amd64's long == 8).

And has caused me a lot of trouble in the past. This fixes it.

| btw, why is the current `struct bpf_hdr' not packed ? this would avoid
| the SIZEOF_BPF_HDR hack...

packing a structure should be avoided when we can force optimal packing
by re-ordering members because it makes the code compiler neutral.

christos