add flag -p for timestamp precision for micros or nanos

[Nequo]

Currently joincap writes all output pcaps with microsecond precision. I have a use case with nanosecond precision pcaps being used as input. I would like to preserve the nanosecond timestamps in the merged output. In mergecap, I would use -Fnsecpcap.

I have made a first attempt at implementing this for joincap by using the pcapgo.NewWriterNanos method added in https://github.com/google/gopacket/commit/7cc6592eca24b42f05b6d13a5521d9d2558cf53b

The current implementation adds a -p flag which sets the writer to NewWriterNanos if -p=nanos. Happy to change this to be implemented in whatever other preferred way. I will aim to add some tests and a test_pcap to try this with in the coming days.

[assafmo]

Thanks for contributing! This looks good to me, I'd just like to see some tests for the new case before I merge anything.

[Nequo]

I have added an initial test which checks the magic number of the output pcap when using -p=nanos. I think this can still be refined as it will fail in one of the endianness cases where the magic number for nanosecond timestamp precision could be read as 0xa1b23c4d. https://www.tcpdump.org/manpages/pcap-savefile.5.html We could just check for both values.

I wanted to get an initial test out and get your opinion on if it would be sufficient to verify the new -p feature. Happy to change/add more tests as required. Currently all tests pass:

git log | head -1; go test
commit 7b5ebf966730021caefb44bcb7c697ab3e9fed9a
PASS
ok  	github.com/assafmo/joincap	0.583s

Thanks!

[assafmo]

Yeah, you should check both values, or alternatively check the endiness somehow. Ideally I'd want to replicate the current test suite for nanoseconds as well, do you think you can do this?

[Nequo]

Added the second check for the new test!

For running the existing test suite for nanoseconds as well, I was thinking about options which would not require rewriting the same tests in new functions. I considered table driven tests but I think this approach might work instead:

diff --git a/main_test.go b/main_test.go
index 58fe094..6cce517 100644
--- a/main_test.go
+++ b/main_test.go
@@ -119,6 +119,7 @@ func testIsOrdered(t *testing.T, pcapPath string) {
 // should be the sum of the packet counts of the
 // input pcaps
 func TestCount(t *testing.T) {
+       for _, extra_args := range []string{"", "-p=nanos"} {
                outputFile, err := ioutil.TempFile("", "joincap_output_")
                if err != nil {
                        t.Fatal(err)
@@ -128,6 +129,7 @@ func TestCount(t *testing.T) {
 
                err = joincap([]string{"joincap",
                        "-v", "-w", outputFile.Name(),
+                       extra_args,
                        okPcap, okPcap})
                if err != nil {
                        t.Fatal(err)
@@ -137,6 +139,7 @@ func TestCount(t *testing.T) {
                        t.Fatal("error counting")
                }
        }
+}

What do you think?

[assafmo]

This looks good, but please do it in sub-tests. :slightly_smiling_face:

[Nequo]

How does this look:

diff --git a/main_test.go b/main_test.go
index e17ed7d..598ffc3 100644
--- a/main_test.go
+++ b/main_test.go
@@ -119,6 +119,8 @@ func testIsOrdered(t *testing.T, pcapPath string) {
 // should be the sum of the packet counts of the
 // input pcaps
 func TestCount(t *testing.T) {
+       for name, extra_args := range map[string]string{"DefaultMicros": "", "Nanos": "-p=nanos"} {
+               t.Run(name, func(t *testing.T) {
                        outputFile, err := ioutil.TempFile("", "joincap_output_")
                        if err != nil {
                                t.Fatal(err)
@@ -128,6 +130,7 @@ func TestCount(t *testing.T) {
 
                        err = joincap([]string{"joincap",
                                "-v", "-w", outputFile.Name(),
+                               extra_args,
                                okPcap, okPcap})
                        if err != nil {
                                t.Fatal(err)
@@ -136,11 +139,14 @@ func TestCount(t *testing.T) {
                        if packetCount(t, outputFile.Name()) != packetCount(t, okPcap)*2 {
                                t.Fatal("error counting")
                        }
+               })
+       }
 }

go test -v -run TestCount
=== RUN   TestCount
=== RUN   TestCount/DefaultMicros
=== RUN   TestCount/Nanos
--- PASS: TestCount (0.04s)
    --- PASS: TestCount/DefaultMicros (0.02s)
    --- PASS: TestCount/Nanos (0.02s)
PASS
ok  	github.com/assafmo/joincap	0.066s

[assafmo]

Awesome! I'll try to play with it myself soon and then push another version.

[Nequo]

Great, I'll update all the tests to match the above and then you should be good to go!

[Nequo]

Just realised that I was running git diff -w and github's GUI will default to showing whitespace diffs. TIL there is a toggle for this and it might make looking at the changes easier:

I ran go fmt on the file before committing which would have inserted tabs and caused the additional whitespace to appear in the diffs.

[assafmo]

Cool, I guess gofmt has changed in the past couple of years

[Nequo]

Hey @assafmo it's been a while! I was coming back to using joincap and would love to have this merged and stop using my branch. I've rebased and resolved conflicts with the new added counter feature. Let me know if you need anything else.

[assafmo]

I'm afk for the next few days, will test locally and merge next week