aregm / nff-go

NFF-Go -Network Function Framework for GO (former YANFF)
BSD 3-Clause "New" or "Revised" License
1.38k stars 154 forks source link

Aditya/generator pcap #650

Open adigadi opened 5 years ago

adigadi commented 5 years ago

Added read/write to/from pcap files functionality

gshimansky commented 5 years ago

I see several problems with this approach. You're adding SetReceiverFile instead of SetReceiver and SetSenderFile instead of SetSender. This looks like this:

Original design:
 Generator           Test system
+----------+        +-----------+
| Receiver |<-------|           |
|          |        |           |
|  Sender  |------->|           |
+----------+        +-----------+

Design with file reader
    Generator              Test system
+----------------+        +-----------+
| Read from file |   ?<---|           |
|                |        |           |
|     Sender     |------->|           |
+----------------+        +-----------+

Design with file writer. BTW I don't think we need it in this form
    Generator              Test system
+----------------+        +-----------+
|    Receiver    |<-------|           |
|                |        |           |
| Write to file  |   ?--->|           |
+----------------+        +-----------+

You replaced Receive and Send in wrong places. What I meant by reading PCAP files is that generating function should read packets from PCAP instead of creating their contents randomly. Ideal place for PCAP file specification would be inside of JSON config file, e.g. instead of writing a generator config like this

{
    "ether": {
        "saddr": "00:25:96:FF:FE:12",
        "daddr": "00:FF:96:FF:FE:12",
        "ipv4": {
            "saddr": {
                "range": {
                    "min": "192.16.0.0",
                    "max": "192.16.0.255"
                }
            },
            "daddr": {
                "range": {
                    "min": "172.16.0.0",
                    "max": "172.16.0.10"
                }
            },
            "udp": {
                "sport": {
                    "range": {
                        "min": 1120,
                        "max": 1152
                    }
                },
                "dport": 1020,
                "raw": {
                    "data": "123456789012345678"
                }
            }
        }
    }
}

we could write something like

{
    "ether": {
        "pcap-file": "sample-network-sequence.pcap"
    }
}

In this case generating function would read packets from pcap file infinitely in a loop (no need for repeat count, packets number should be limited by packet rate and duration) and send them at the specified rate.

We could add file writer functionality. It would be useful for debugging requests like #645 where we need to answer a question, whether forwarded packets come reassembled or not. We need to not only receive packets, but also dump them into an output trace because tcpdump functionality is not available on DPDK ports. But writing packets into a file should be done only in addition to receiving them from the test application, not instead of it.

Several additional notes on your pull request:

  1. Please avoid whitespace changes. It is a good idea to run gofmt automatically on every file save which would always bring file to the coding convention that all Go code should be using. If you use VIM you can take a look at this link, for Emacs this link.
  2. There is no need to specify useReader and useWriter. These switches are redundant if you also specify file names. I think only file output switch should remain in this minPktLoss and perfTest applications while file reading directives should reside in JSON configuration for generator.