tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Improving the data supplied by BPF

On Thu, Dec 25, 2008 at 6:34 PM, Christos Zoulas 
<> wrote:
> In article 
> <>,
> Arnaud Lacombe <> wrote:
>>> The purpose of the sequence number is to provide the rolling counter
>>> of the packets captured for the one in question. Thus if in successive
>>> reads the count went from 2 to 5, you know 3 packets have been missed.
>>what if the count goes from 3 to... 3, ie. the seq number overflowed
>>(for whatever reason) ?
> Highly unlinkely.
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.

>>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.

>>>       uint32_t        ebr_nsecs;
>>why do you want nano second precision if you getting your information
>>from a micro second precision variable. There is no information gain
>>there, and your code reflect this (ie. you just "* 1000" to get the
>>nano second value from the micro second value).
> 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.

>>This field would have a meaning if you change the call the call to
>>microtime() to nanotime() in bpf_tap()/bpd_deliver() and build a
>>homegrown `struct timeval' in the non-extended capture format. You
>>don't have any precision loss in that case.
>>btw, why not just using a `struct timespec' ?
> fixed sizes. Most on wire structures use fixed sizes. this allows portability
> across different architectures.
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).

>>>       uint32_t        ebr_seqno;      /* sequence number in capture */
>>how to detect wrap in sequence number ?
> It does not happen in real situations.
I'm not no so sure about this, cf above.

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

 - Arnaud

Home | Main Index | Thread Index | Old Index