Use struct module to parse binary data

[dermoth]

That was a low-hanging fruit... you can see the comparison (and validation on a sample keyframe) of speed using struct vs binascii here:

https://gist.github.com/dermoth/a40537d27f1230aaf663

Please test extensively; I merely tested it on the sample pcaps provided, and a one real keyframe I was trying to decode. I used the python code as an example as your keyframe spec is out of wack - according to https://eos.arista.com/timestamping-on-the-7150-series/, a keyframe is 62 bytes instead of the 46 bytes I see in real captures!

NB: I haven't even tested tsdump, but the line replaced were 1:1 compared to pcaptd, with the exception of some whitespace and a slightly different variable name (asicTime => asicTs)

Regards,

Thomas

[advornic]

Hi Thomas,

This looks great - thank you! The difference between the spec and the code are the Skew Numerator/Denominator. As the article states, starting 4.13.0F+ these can be configured to-be-added to the keyframe. The code only covers the case when those are not configured (at least for now) - I suspect this is also the reason why you don't see them.

I hope this makes sense - I will merge and test this and also try to add pcap-ng support soon.

Appreciate your contribution!

Thanks, Andrei

[dermoth]

Oh... I'm not too used to Python structs, mostly used perl pack/unpack so far... one change you can do is group the numbers; when I tried originally I had something else wrong, then I started symplifying the string intil it worked... never though of tring grouping again. ex: '>QQxxxxxxxxxxxxxxxxxxxxxxxxHxxxx' becomes '>2Q24xH4x'.

This also makes adding support for other formats very easy... something like:

skiplen = 24 # bytes to skip between UTC and deviceId
if len(eth.ip.data) == 62:
   skiplen += 16 # Add Skew Numerator/Denominator fields
elif len(eth.ip.data) == 42:
   pass
else:
   raise # raise something or skip packet, otherwise struct will raise anyway

(asicTime, utc, deviceId) = struct.unpack('>2Q%ixH4x' % skiplen, eth.ip.data)

I could submit a PR later for that but it would be great if had keyframe captures of both kinds (I can't submit patches from work without an extensive process, and can't use work pcaps from home...)

Regards,

Thomas

[advornic]

Or maybe:

skyplen = len(eth.ip.data) - 22

Will try to attach some captures later today.

[dermoth]

Oh btw, I think you can attach binary files in Gist... might be cleaner than a zip file attached as an image... ;)

[advornic]

I committed your proposed enhancement and added a "pcaps" folder where I placed two captures (one with short keyframes and one with long keyframes). Looks good so far to me - the code seems to be able to handle both. The config corresponding to them is:

vlanToDevId = {}
vlanToDevId[ '*' ] = 0 

and I ran:

# pcaptd -p -r -t -u pcaps/test-[1|2].pcap

for testing.

Let me know what you think.

[dermoth]

len( eth.ip.data ) - 22 -- brilliant (I looked at it too fast; had to see it in the code to understand)

I'm probably not going to use this unless it starts supporting pcap-ng. I'm implementing ts decoding on our in-house scripts.

My most likely use case for this, if possible, would be as a streaming filter to write pcap-ng data with Arista timestamps built into the pcap header.

Regards,

Thomas