diff --git a/lib/flow.c b/lib/flow.c index fe226cf0f..b8f99f66b 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,18 @@ 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) { + /* + * Internet checksum is a sum complement to zero, so any other + * value will result in an invalid checksum. Here, we flip one + * bit. + */ + ip->ip_csum ^= (OVS_FORCE ovs_be16) 0x1; + dp_packet_ip_checksum_bad(p); + } 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 fe82317d7..8c6e6d448 100644 --- a/lib/netdev-dummy.c +++ b/lib/netdev-dummy.c @@ -1769,7 +1769,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){ @@ -1777,7 +1777,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..d0359b5ea 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,eth_dst=0a:8f:39:4f:e0:73,dl_type=0x0800,\ + nw_src=192.168.123.2,nw_dst=192.168.123.1,nw_proto=6,nw_ttl=64,nw_frag=no,\ + tp_src=54392,tp_dst=5201,tcp_flags=ack" + +good_frame=$(ovs-ofctl compose-packet --bare "${flow_s}") + # 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=$(ovs-ofctl compose-packet --bare "${flow_expected}") 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=$(ovs-ofctl compose-packet --bare --bad-csum "${flow_s}") 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=$(ovs-ofctl compose-packet --bare --bad-csum "${flow_expected}") +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/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 24d0941cf..0a382f336 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -154,6 +154,12 @@ static int show_stats = 1; /* --pcap: Makes "compose-packet" print a pcap on stdout. */ static int print_pcap = 0; +/* --bare: Makes "compose-packet" print a bare hexified payload. */ +static int print_bare = 0; + +/* -bad-csum: Makes "compose-packet" generate an invalid checksum. */ +static int bad_csum = 0; + /* --raw: Makes "ofp-print" read binary data from stdin. */ static int raw = 0; @@ -243,6 +249,8 @@ parse_options(int argc, char *argv[]) {"color", optional_argument, NULL, OPT_COLOR}, {"may-create", no_argument, NULL, OPT_MAY_CREATE}, {"pcap", no_argument, &print_pcap, 1}, + {"bare", no_argument, &print_bare, 1}, + {"bad-csum", no_argument, &bad_csum, 1}, {"raw", no_argument, &raw, 1}, {"read-only", no_argument, NULL, OPT_READ_ONLY}, DAEMON_LONG_OPTIONS, @@ -4948,20 +4956,33 @@ ofctl_parse_key_value(struct ovs_cmdl_context *ctx) } } -/* "compose-packet [--pcap] FLOW [L7]": Converts the OpenFlow flow - * specification FLOW to a packet with flow_compose() and prints the hex bytes - * in the packet on stdout. Also verifies that the flow extracted from that - * packet matches the original FLOW. +/* "compose-packet [--pcap|--bare] [--bad-csum] FLOW [L7]": Converts the + * OpenFlow flow specification FLOW to a packet with flow_compose() and prints + * the hex bytes of the packet, with offsets, to stdout. * - * With --pcap, prints the packet to stdout instead as a pcap file, so that you - * can do something like "ovs-ofctl --pcap compose-packet udp | tcpdump -vvvv - * -r-" to use another tool to dump the packet contents. + * With --pcap, prints the packet in pcap format, so that you can do something + * like "ovs-ofctl --pcap compose-packet udp | tcpdump -vvvv -r-" to use + * another tool to dump the packet contents. + * + * With --bare, prints the packet as a single bare hex string with no + * spaces or offsets, so that you can pass the result directly to e.g. + * "ovs-appctl netdev-dummy/receive vif $(ovs-ofctl compose-packet --bare + * FLOW)" + * + * With --bad-csum, produces a packet with an invalid IP checksum. (For IPv4.) + * + * Regardless of the mode, the command also verifies that the flow extracted + * from that packet matches the original FLOW. * * If L7 is specified, draws the L7 payload data from it, otherwise defaults to * 64 bytes of payload. */ static void ofctl_compose_packet(struct ovs_cmdl_context *ctx) { + if (print_pcap && print_bare) { + ovs_fatal(1, "--bare and --pcap are mutually exclusive"); + } + if (print_pcap && isatty(STDOUT_FILENO)) { ovs_fatal(1, "not writing pcap data to stdout; redirect to a file " "or pipe to tcpdump instead"); @@ -4989,7 +5010,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, bad_csum); free(l7); if (print_pcap) { @@ -4997,6 +5018,16 @@ ofctl_compose_packet(struct ovs_cmdl_context *ctx) ovs_pcap_write_header(p_file); ovs_pcap_write(p_file, &p); ovs_pcap_close(p_file); + } else if (print_bare) { + /* Binary to a bare hex string. */ + for (int i = 0; i < dp_packet_size(&p); i++) { + uint8_t val = ((uint8_t *) dp_packet_data(&p))[i]; + /* Don't use ds_put_hex because it adds 0x prefix as well as + * it doesn't guarantee an even number of payload characters, which + * may be important elsewhere (e.g. in netdev-dummy/receive). */ + printf("%02" PRIx8, val); + } + } else { ovs_hex_dump(stdout, dp_packet_data(&p), dp_packet_size(&p), 0, false); }