The command reads a flow string and an optional additional payload on
stdin and produces a hex-string representation of the corresponding
frame on stdout. It may receive more than a single input line.

The command may be useful in tests, where we may need to produce hex
frame payloads to compare observed frames against.

As an example of the tool use, a single test case is converted to it.
The test uses both normal and --bad-csum behavior of the tool,
confirming it works as advertised.

Signed-off-by: Ihar Hrachyshka <ihrac...@redhat.com>
---
v1: - initial version.
v2: - convert from appctl to ovstest.
    - add --bad-csum support.
    - drop separate test case and man for the tool.
---
 lib/flow.c                   | 11 +++-
 lib/flow.h                   |  2 +-
 lib/netdev-dummy.c           |  4 +-
 ofproto/ofproto-dpif-trace.c |  2 +-
 ofproto/ofproto-dpif.c       |  4 +-
 tests/dpif-netdev.at         | 45 ++++++++--------
 tests/test-odp.c             | 99 +++++++++++++++++++++++++++++++++++-
 utilities/ovs-ofctl.c        |  2 +-
 8 files changed, 135 insertions(+), 34 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index fe226cf0f..c3424ab43 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -3306,6 +3306,8 @@ packet_expand(struct dp_packet *p, const struct flow 
*flow, size_t size)
  * (This is useful only for testing, obviously, and the packet isn't really
  * valid.  Lots of fields are just zeroed.)
  *
+ * If 'bad_csum' is true, the final IP checksum is invalid.
+ *
  * For packets whose protocols can encapsulate arbitrary L7 payloads, 'l7' and
  * 'l7_len' determine that payload:
  *
@@ -3318,7 +3320,7 @@ packet_expand(struct dp_packet *p, const struct flow 
*flow, size_t size)
  *      from 'l7'. */
 void
 flow_compose(struct dp_packet *p, const struct flow *flow,
-             const void *l7, size_t l7_len)
+             const void *l7, size_t l7_len, bool bad_csum)
 {
     /* Add code to this function (or its callees) for emitting new fields or
      * protocols.  (This isn't essential, so it can be skipped for initial
@@ -3370,7 +3372,12 @@ flow_compose(struct dp_packet *p, const struct flow 
*flow,
         /* Checksum has already been zeroed by put_zeros call. */
         ip->ip_csum = csum(ip, sizeof *ip);

-        dp_packet_ol_set_ip_csum_good(p);
+        if (bad_csum) {
+            ip->ip_csum++;
+        } else {
+            dp_packet_ol_set_ip_csum_good(p);
+        }
+
         pseudo_hdr_csum = packet_csum_pseudoheader(ip);
         flow_compose_l4_csum(p, flow, pseudo_hdr_csum);
     } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
diff --git a/lib/flow.h b/lib/flow.h
index a9d026e1c..75a9be3c1 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -127,7 +127,7 @@ void flow_set_mpls_bos(struct flow *, int idx, uint8_t 
stack);
 void flow_set_mpls_lse(struct flow *, int idx, ovs_be32 lse);

 void flow_compose(struct dp_packet *, const struct flow *,
-                  const void *l7, size_t l7_len);
+                  const void *l7, size_t l7_len, bool bad_csum);
 void packet_expand(struct dp_packet *, const struct flow *, size_t size);

 bool parse_ipv6_ext_hdrs(const void **datap, size_t *sizep, uint8_t *nw_proto,
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 1a54add87..9ead449e1 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1758,7 +1758,7 @@ eth_from_flow_str(const char *s, size_t packet_size,

     packet = dp_packet_new(0);
     if (packet_size) {
-        flow_compose(packet, flow, NULL, 0);
+        flow_compose(packet, flow, NULL, 0, false);
         if (dp_packet_size(packet) < packet_size) {
             packet_expand(packet, flow, packet_size);
         } else if (dp_packet_size(packet) > packet_size){
@@ -1766,7 +1766,7 @@ eth_from_flow_str(const char *s, size_t packet_size,
             packet = NULL;
         }
     } else {
-        flow_compose(packet, flow, NULL, 64);
+        flow_compose(packet, flow, NULL, 64, false);
     }

     ofpbuf_uninit(&odp_key);
diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 527e2f17e..b86e7fe07 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -440,7 +440,7 @@ parse_flow_and_packet(int argc, const char *argv[],
     if (generate_packet) {
         /* Generate a packet, as requested. */
         packet = dp_packet_new(0);
-        flow_compose(packet, flow, l7, l7_len);
+        flow_compose(packet, flow, l7, l7_len, false);
     } else if (packet) {
         /* Use the metadata from the flow and the packet argument to
          * reconstruct the flow. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ba5706f6a..9e8faf829 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1255,7 +1255,7 @@ check_ct_eventmask(struct dpif_backer *backer)

     /* Compose a dummy UDP packet. */
     dp_packet_init(&packet, 0);
-    flow_compose(&packet, &flow, NULL, 64);
+    flow_compose(&packet, &flow, NULL, 64, false);

     /* Execute the actions.  On older datapaths this fails with EINVAL, on
      * newer datapaths it succeeds. */
@@ -1348,7 +1348,7 @@ check_ct_timeout_policy(struct dpif_backer *backer)

     /* Compose a dummy UDP packet. */
     dp_packet_init(&packet, 0);
-    flow_compose(&packet, &flow, NULL, 64);
+    flow_compose(&packet, &flow, NULL, 64, false);

     /* Execute the actions.  On older datapaths this fails with EINVAL, on
      * newer datapaths it succeeds. */
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 85119fb81..4a3c2b53c 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -746,19 +746,25 @@ OVS_VSWITCHD_START(
 # Modify the ip_dst addr to force changing the IP csum.
 AT_CHECK([ovs-ofctl add-flow br1 
in_port=p1,actions=mod_nw_dst:192.168.1.1,output:p2])

+flow_s="\
+eth(src=8a:bf:7e:2f:05:84,dst=0a:8f:39:4f:e0:73),eth_type(0x0800),\
+ipv4(src=192.168.123.2,dst=192.168.123.1,proto=6,tos=0,ttl=64,frag=no),\
+tcp(src=54392,dst=5201),tcp_flags(ack)"
+
+good_frame=$(echo ${flow_s} | ovstest test-odp hexify-keys)
+
 # Check if no offload remains ok.
 AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap])
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum=false])
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum_set_good=false])
-AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
-0a8f394fe0738abf7e2f058408004500003433e0400040068f8fc0a87b02c0a87b01d4781451a962ad5417ed297b801000e547fd00000101080a2524d2345c7fe1c4
-])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])

 # Checksum should change to 0x990 with ip_dst changed to 192.168.1.1
 # by the datapath while processing the packet.
+flow_expected=$(echo ${flow_s} | sed 's/192.168.123.1/192.168.1.1/g')
+good_expected=$(echo ${flow_expected} | ovstest test-odp hexify-keys)
 AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
-AT_CHECK([tail -n 1 p2.pcap.txt], [0], [dnl
-0a8f394fe0738abf7e2f058408004500003433e0400040060990c0a87b02c0a80101d4781451a962ad5417ed297b801000e5c1fd00000101080a2524d2345c7fe1c4
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
 ])

 # Check if packets entering the datapath with csum offloading
@@ -766,12 +772,9 @@ AT_CHECK([tail -n 1 p2.pcap.txt], [0], [dnl
 # in the datapath and not by the netdev.
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum=false])
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum_set_good=true])
-AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
-0a8f394fe0738abf7e2f058408004500003433e0400040068f8fc0a87b02c0a87b01d4781451a962ad5417ed297b801000e547fd00000101080a2524d2345c7fe1c4
-])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}])
 AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
-AT_CHECK([tail -n 1 p2.pcap.txt], [0], [dnl
-0a8f394fe0738abf7e2f058408004500003433e0400040060990c0a87b02c0a80101d4781451a962ad5417ed297b801000e5c1fd00000101080a2524d2345c7fe1c4
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
 ])

 # Check if packets entering the datapath with csum offloading
@@ -779,36 +782,30 @@ AT_CHECK([tail -n 1 p2.pcap.txt], [0], [dnl
 # by the datapath.
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum=true])
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum_set_good=true])
-AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
-0a8f394fe0738abf7e2f058408004500003433e0400040068f8fc0a87b02c0a87b01d4781451a962ad5417ed297b801000e547fd00000101080a2524d2345c7fe1c4
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${good_frame}
 ])
 AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
-AT_CHECK([tail -n 1 p2.pcap.txt], [0], [dnl
-0a8f394fe0738abf7e2f058408004500003433e0400040060990c0a87b02c0a80101d4781451a962ad5417ed297b801000e5c1fd00000101080a2524d2345c7fe1c4
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
 ])

 # Push a packet with bad csum and offloading disabled to check
 # if the datapath updates the csum, but does not fix the issue.
+bad_frame=$(echo ${flow_s} | ovstest test-odp hexify-keys --bad-csum)
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum=false])
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum_set_good=false])
-AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
-0a8f394fe0738abf7e2f058408004500003433e0400040068f03c0a87b02c0a87b01d4781451a962ad5417ed297b801000e547fd00000101080a2524d2345c7fe1c4
-])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
 AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
-AT_CHECK([tail -n 1 p2.pcap.txt], [0], [dnl
-0a8f394fe0738abf7e2f058408004500003433e0400040060904c0a87b02c0a80101d4781451a962ad5417ed297b801000e5c1fd00000101080a2524d2345c7fe1c4
+bad_expected=$(echo ${flow_expected} | ovstest test-odp hexify-keys --bad-csum)
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${bad_expected}
 ])

 # Push a packet with bad csum and offloading enabled to check
 # if the driver updates and fixes the csum.
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum=true])
 AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_csum_set_good=true])
-AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
-0a8f394fe0738abf7e2f058408004500003433e0400040068f03c0a87b02c0a87b01d4781451a962ad5417ed297b801000e547fd00000101080a2524d2345c7fe1c4
-])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 ${bad_frame}])
 AT_CHECK([ovs-pcap p2.pcap > p2.pcap.txt 2>&1])
-AT_CHECK([tail -n 1 p2.pcap.txt], [0], [dnl
-0a8f394fe0738abf7e2f058408004500003433e0400040060990c0a87b02c0a80101d4781451a962ad5417ed297b801000e5c1fd00000101080a2524d2345c7fe1c4
+AT_CHECK_UNQUOTED([tail -n 1 p2.pcap.txt], [0], [${good_expected}
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
diff --git a/tests/test-odp.c b/tests/test-odp.c
index 0ddfd4070..073e1031a 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -19,6 +19,7 @@
 #include "odp-util.h"
 #include <stdio.h>
 #include "openvswitch/dynamic-string.h"
+#include "dp-packet.h"
 #include "flow.h"
 #include "openvswitch/match.h"
 #include "openvswitch/ofpbuf.h"
@@ -155,6 +156,96 @@ parse_actions(void)
     return 0;
 }

+static int
+hexify_keys(bool bad_csum)
+{
+    int exit_code = 0;
+    struct ds in;
+
+    uint8_t *l7 = NULL;
+    size_t l7_len = 0;
+
+    char *error = NULL;
+    struct dp_packet *packet = NULL;
+
+    ds_init(&in);
+    vlog_set_levels_from_string_assert("odp_util:console:dbg");
+    while (!ds_get_test_line(&in, stdin)) {
+        const char *line = ds_cstr(&in);
+
+        struct flow flow;
+        struct ofpbuf odp_key;
+        struct ofpbuf odp_mask;
+        ofpbuf_init(&odp_key, 0);
+        ofpbuf_init(&odp_mask, 0);
+
+        /* Handle trailing hex payload after a space char, if present. */
+        const char *first_space = strchr(line, ' ');
+        if (first_space) {
+            const char *hex_payload = first_space + 1;
+            struct dp_packet payload;
+            memset(&payload, 0, sizeof payload);
+            dp_packet_init(&payload, 0);
+            if (dp_packet_put_hex(&payload, hex_payload, NULL)[0] != '\0') {
+                dp_packet_uninit(&payload);
+                error = xstrdup("Trailing garbage in payload data");
+                goto next;
+            }
+            l7_len = dp_packet_size(&payload);
+            l7 = dp_packet_steal_data(&payload);
+            ds_truncate(&in, first_space - line);
+        }
+
+        /* Parse flow string. */
+        if (odp_flow_from_string(
+                ds_cstr(&in), NULL, &odp_key, &odp_mask, &error)) {
+            goto next;
+        }
+        if (odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow, &error)
+            == ODP_FIT_ERROR) {
+            goto next;
+        }
+
+        /* Flow to binary. */
+        packet = dp_packet_new(0);
+        flow_compose(packet, &flow, l7, l7_len, bad_csum);
+
+        /* Binary to hex string. */
+        struct ds result = DS_EMPTY_INITIALIZER;
+        for (int i = 0; i < dp_packet_size(packet); i++) {
+            uint8_t val = ((uint8_t *) dp_packet_data(packet))[i];
+            /* Don't use ds_put_hex because it adds 0x prefix as well as
+             * it doesn't guarantee even number of payload characters, which is
+             * important elsewhere (e.g. in `receive` command. */
+            ds_put_format(&result, "%02" PRIx8, val);
+        }
+
+        puts(ds_cstr(&result));
+next:
+        ds_destroy(&result);
+        if (error) {
+            printf("%s", error);
+            free(error);
+            exit_code = 1;
+        }
+
+        dp_packet_delete(packet);
+        packet = NULL;
+        free(l7);
+        l7 = NULL;
+
+        ofpbuf_uninit(&odp_mask);
+        ofpbuf_uninit(&odp_key);
+        ds_destroy(&in);
+
+        if (exit_code) {
+            break;
+        }
+    }
+
+    return exit_code;
+}
+
 static int
 parse_filter(char *filter_parse)
 {
@@ -247,8 +338,14 @@ test_odp_main(int argc, char *argv[])
         exit_code = parse_actions();
     } else if (argc == 3 && !strcmp(argv[1], "parse-filter")) {
         exit_code =parse_filter(argv[2]);
+    } else if (argc == 2 && !strcmp(argv[1], "hexify-keys")) {
+        exit_code = hexify_keys(false);
+    } else if (argc == 3 && !strcmp(argv[1], "hexify-keys") &&
+            !strcmp(argv[2], "--bad-csum")) {
+        exit_code = hexify_keys(true);
     } else {
-        ovs_fatal(0, "usage: %s parse-keys | parse-wc-keys | parse-actions", 
argv[0]);
+        ovs_fatal(0, "usage: %s parse-keys | parse-wc-keys | parse-actions | "
+                     "hexify-keys [--bad-csum]", argv[0]);
     }

     exit(exit_code);
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 24d0941cf..31da8c2d4 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -4989,7 +4989,7 @@ ofctl_compose_packet(struct ovs_cmdl_context *ctx)
         l7_len = dp_packet_size(&payload);
         l7 = dp_packet_steal_data(&payload);
     }
-    flow_compose(&p, &flow1, l7, l7_len);
+    flow_compose(&p, &flow1, l7, l7_len, false);
     free(l7);

     if (print_pcap) {
--
2.41.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to