ofp-print: Remove tcpdump from ofp_packet_to_string().

Instead this patch uses flow_format() which gives very similar
output.  This patch will improve the reliability of unit tests in
future patches which rely on the results of ofp_packet_to_string().

Signed-off-by: Ethan Jackson <ethan@nicira.com>

diff --git a/INSTALL.Linux b/INSTALL.Linux
index 7a55ccd169141db40d17cacd9447fdaa72aacd1f..8ef2315777f7850ef29613378cd81ec3e06d48d2 100644 (file)
--- a/INSTALL.Linux
+++ b/INSTALL.Linux
@@ -117,11 +117,6 @@ following software:
       iproute2 (part of all major distributions and available at
       http://www.linux-foundation.org/en/Net:Iproute2).
 
-    - For debugging purposes, Open vSwitch expects that "tcpdump" is
-      installed as /usr/sbin/tcpdump.  If tcpdump is not installed, or
-      if it is installed in a different location, then some Open
-      vSwitch log messages will not be as detailed.
-
 You should ensure that /dev/urandom exists.  To support TAP devices,
 you must also ensure that /dev/net/tun exists.
 

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 7ed3ce12674dc3ec8805060cee1b22ab16b87969..fe4af4c800edd3619c39bc129cdbae5dce171272 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -49,58 +49,19 @@ static void ofp_print_error(struct ds *, int error);
 
 
 /* Returns a string that represents the contents of the Ethernet frame in the
- * 'len' bytes starting at 'data' to 'stream' as output by tcpdump.
- *
- * The caller must free the returned string.
- *
- * This starts and kills a tcpdump subprocess so it's quite expensive. */
+ * 'len' bytes starting at 'data'.  The caller must free the returned string.*/
 char *
 ofp_packet_to_string(const void *data, size_t len)
 {
     struct ds ds = DS_EMPTY_INITIALIZER;
     struct ofpbuf buf;
-
-    char command[128];
-    FILE *pcap;
-    FILE *tcpdump;
-    int status;
-    int c;
+    struct flow flow;
 
     ofpbuf_use_const(&buf, data, len);
+    flow_extract(&buf, 0, 0, 0, &flow);
+    flow_format(&ds, &flow);
+    ds_put_char(&ds, '\n');
 
-    pcap = tmpfile();
-    if (!pcap) {
-        ovs_error(errno, "tmpfile");
-        return xstrdup("<error>");
-    }
-    pcap_write_header(pcap);
-    pcap_write(pcap, &buf);
-    fflush(pcap);
-    if (ferror(pcap)) {
-        ovs_error(errno, "error writing temporary file");
-    }
-    rewind(pcap);
-
-    snprintf(command, sizeof command, "/usr/sbin/tcpdump -t -e -n -r /dev/fd/%d 2>/dev/null",
-             fileno(pcap));
-    tcpdump = popen(command, "r");
-    fclose(pcap);
-    if (!tcpdump) {
-        ovs_error(errno, "exec(\"%s\")", command);
-        return xstrdup("<error>");
-    }
-
-    while ((c = getc(tcpdump)) != EOF) {
-        ds_put_char(&ds, c);
-    }
-
-    status = pclose(tcpdump);
-    if (WIFEXITED(status)) {
-        if (WEXITSTATUS(status))
-            ovs_error(0, "tcpdump exited with status %d", WEXITSTATUS(status));
-    } else if (WIFSIGNALED(status)) {
-        ovs_error(0, "tcpdump exited with signal %d", WTERMSIG(status));
-    }
     return ds_cstr(&ds);
 }
 
@@ -134,15 +95,6 @@ ofp_print_packet_in(struct ds *string, const struct ofp_packet_in *op,
     ds_put_char(string, '\n');
 
     if (verbosity > 0) {
-        struct flow flow;
-        struct ofpbuf packet;
-
-        ofpbuf_use_const(&packet, op->data, data_len);
-        flow_extract(&packet, 0, 0, ntohs(op->in_port), &flow);
-        flow_format(string, &flow);
-        ds_put_char(string, '\n');
-    }
-    if (verbosity > 1) {
         char *packet = ofp_packet_to_string(op->data, data_len);
         ds_put_cstr(string, packet);
         free(packet);
@@ -1589,10 +1541,7 @@ ofp_print(FILE *stream, const void *oh, size_t len, int verbosity)
 }
 
 /* Dumps the contents of the Ethernet frame in the 'len' bytes starting at
- * 'data' to 'stream' using tcpdump.  'total_len' specifies the full length of
- * the Ethernet frame (of which 'len' bytes were captured).
- *
- * This starts and kills a tcpdump subprocess so it's quite expensive. */
+ * 'data' to 'stream'. */
 void
 ofp_print_packet(FILE *stream, const void *data, size_t len)
 {

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 09b785bcf3e8dfb771b3b029e235a6755fdad5ff..5734174d50521c2ade25f0a36d1789c6667e52b3 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -225,20 +225,15 @@ AT_CLEANUP
 
 AT_SETUP([OFPT_PACKET_IN])
 AT_KEYWORDS([ofp-print])
-AT_SKIP_IF([test ! -x /usr/sbin/tcpdump])
 AT_CHECK([ovs-ofctl ofp-print "\
 01 0a 00 4e 00 00 00 00 00 00 01 11 00 3c 00 03 \
 00 00 50 54 00 00 00 06 50 54 00 00 00 05 08 00 \
 45 00 00 28 bd 12 00 00 40 06 3c 6a c0 a8 00 01 \
 c0 a8 00 02 27 2f 00 00 78 50 cc 5b 57 af 42 1e \
 50 00 02 00 26 e8 00 00 00 00 00 00 00 00 \
-"], [0], [stdout])
-dnl The tcpdump output format differs slightly from one version to another,
-dnl so trim off the end of the line where differences appear.
-AT_CHECK([sed 's/\(length 60:\).*/\1 .../' stdout], [0], [dnl
+"], [0], [dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=3 data_len=60 buffer=0x00000111
-priority:0,tunnel:0,in_port:0003,tci(0) mac(50:54:00:00:00:05->50:54:00:00:00:06) type:0800 proto:6 tos:0 ttl:64 ip(192.168.0.1->192.168.0.2) port(10031->0)
-50:54:00:00:00:05 > 50:54:00:00:00:06, ethertype IPv4 (0x0800), length 60: ...
+priority:0,tunnel:0,in_port:0000,tci(0) mac(50:54:00:00:00:05->50:54:00:00:00:06) type:0800 proto:6 tos:0 ttl:64 ip(192.168.0.1->192.168.0.2) port(10031->0)
 ])
 AT_CLEANUP