From c4617b3c28b9a96e09fdbbe2682b875dbfeaec5b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 18 Jan 2011 11:50:56 -0800 Subject: [PATCH] openflow: Use types and accessors for half-aligned 64-bit fields. Without this commit, many of the unit tests for ofp-print.c fail with bus errors on RISC architectures (tested on sparc) and presumably so would any other code that uses these same struct members. --- build-aux/check-structs | 1 + include/openflow/openflow.h | 55 ++++++++++++++++++------------------ lib/ofp-print.c | 56 ++++++++++++++++++++----------------- ofproto/ofproto.c | 47 ++++++++++++++++--------------- 4 files changed, 85 insertions(+), 74 deletions(-) diff --git a/build-aux/check-structs b/build-aux/check-structs index 536045fe..152c6a21 100755 --- a/build-aux/check-structs +++ b/build-aux/check-structs @@ -17,6 +17,7 @@ types['uint64_t'] = {"size": 8, "alignment": 8} types['ovs_be16'] = {"size": 2, "alignment": 2} types['ovs_be32'] = {"size": 4, "alignment": 4} types['ovs_be64'] = {"size": 8, "alignment": 8} +types['ovs_32aligned_be64'] = {"size": 8, "alignment": 4} token = None line = "" diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h index 5f1dd609..e92e70c9 100644 --- a/include/openflow/openflow.h +++ b/include/openflow/openflow.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010 Nicira Networks. + * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,7 +19,7 @@ #ifndef OPENFLOW_OPENFLOW_H #define OPENFLOW_OPENFLOW_H 1 -#include +#include "openvswitch/types.h" #ifdef SWIG #define OFP_ASSERT(EXPR) /* SWIG can't handle OFP_ASSERT. */ @@ -792,9 +792,9 @@ struct ofp_flow_stats { uint16_t idle_timeout; /* Number of seconds idle before expiration. */ uint16_t hard_timeout; /* Number of seconds before expiration. */ uint8_t pad2[6]; /* Align to 64 bits. */ - uint64_t cookie; /* Opaque controller-issued identifier. */ - uint64_t packet_count; /* Number of packets in flow. */ - uint64_t byte_count; /* Number of bytes in flow. */ + ovs_32aligned_be64 cookie; /* Opaque controller-issued identifier. */ + ovs_32aligned_be64 packet_count; /* Number of packets in flow. */ + ovs_32aligned_be64 byte_count; /* Number of bytes in flow. */ struct ofp_action_header actions[0]; /* Actions. */ }; OFP_ASSERT(sizeof(struct ofp_flow_stats) == 88); @@ -813,8 +813,8 @@ OFP_ASSERT(sizeof(struct ofp_aggregate_stats_request) == 44); /* Body of reply to OFPST_AGGREGATE request. */ struct ofp_aggregate_stats_reply { - uint64_t packet_count; /* Number of packets in flows. */ - uint64_t byte_count; /* Number of bytes in flows. */ + ovs_32aligned_be64 packet_count; /* Number of packets in flows. */ + ovs_32aligned_be64 byte_count; /* Number of bytes in flows. */ uint32_t flow_count; /* Number of flows. */ uint8_t pad[4]; /* Align to 64 bits. */ }; @@ -830,8 +830,8 @@ struct ofp_table_stats { supported by the table. */ uint32_t max_entries; /* Max number of entries supported. */ uint32_t active_count; /* Number of active entries. */ - uint64_t lookup_count; /* Number of packets looked up in table. */ - uint64_t matched_count; /* Number of packets that hit table. */ + ovs_32aligned_be64 lookup_count; /* # of packets looked up in table. */ + ovs_32aligned_be64 matched_count; /* Number of packets that hit table. */ }; OFP_ASSERT(sizeof(struct ofp_table_stats) == 64); @@ -849,21 +849,22 @@ OFP_ASSERT(sizeof(struct ofp_port_stats_request) == 8); struct ofp_port_stats { uint16_t port_no; uint8_t pad[6]; /* Align to 64-bits. */ - uint64_t rx_packets; /* Number of received packets. */ - uint64_t tx_packets; /* Number of transmitted packets. */ - uint64_t rx_bytes; /* Number of received bytes. */ - uint64_t tx_bytes; /* Number of transmitted bytes. */ - uint64_t rx_dropped; /* Number of packets dropped by RX. */ - uint64_t tx_dropped; /* Number of packets dropped by TX. */ - uint64_t rx_errors; /* Number of receive errors. This is a super-set - of receive errors and should be great than or - equal to the sum of all rx_*_err values. */ - uint64_t tx_errors; /* Number of transmit errors. This is a super-set - of transmit errors. */ - uint64_t rx_frame_err; /* Number of frame alignment errors. */ - uint64_t rx_over_err; /* Number of packets with RX overrun. */ - uint64_t rx_crc_err; /* Number of CRC errors. */ - uint64_t collisions; /* Number of collisions. */ + ovs_32aligned_be64 rx_packets; /* Number of received packets. */ + ovs_32aligned_be64 tx_packets; /* Number of transmitted packets. */ + ovs_32aligned_be64 rx_bytes; /* Number of received bytes. */ + ovs_32aligned_be64 tx_bytes; /* Number of transmitted bytes. */ + ovs_32aligned_be64 rx_dropped; /* Number of packets dropped by RX. */ + ovs_32aligned_be64 tx_dropped; /* Number of packets dropped by TX. */ + ovs_32aligned_be64 rx_errors; /* Number of receive errors. This is a + super-set of receive errors and should be + great than or equal to the sum of all + rx_*_err values. */ + ovs_32aligned_be64 tx_errors; /* Number of transmit errors. This is a + super-set of transmit errors. */ + ovs_32aligned_be64 rx_frame_err; /* Number of frame alignment errors. */ + ovs_32aligned_be64 rx_over_err; /* Number of packets with RX overrun. */ + ovs_32aligned_be64 rx_crc_err; /* Number of CRC errors. */ + ovs_32aligned_be64 collisions; /* Number of collisions. */ }; OFP_ASSERT(sizeof(struct ofp_port_stats) == 104); @@ -884,9 +885,9 @@ struct ofp_queue_stats { uint16_t port_no; uint8_t pad[2]; /* Align to 32-bits. */ uint32_t queue_id; /* Queue id. */ - uint64_t tx_bytes; /* Number of transmitted bytes. */ - uint64_t tx_packets; /* Number of transmitted packets. */ - uint64_t tx_errors; /* Number of packets dropped due to overrun. */ + ovs_32aligned_be64 tx_bytes; /* Number of transmitted bytes. */ + ovs_32aligned_be64 tx_packets; /* Number of transmitted packets. */ + ovs_32aligned_be64 tx_errors; /* # of packets dropped due to overrun. */ }; OFP_ASSERT(sizeof(struct ofp_queue_stats) == 32); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 73820e8b..0e2cb0f8 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010 Nicira Networks. + * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,6 +39,7 @@ #include "packets.h" #include "pcap.h" #include "type-props.h" +#include "unaligned.h" #include "util.h" static void ofp_print_port_name(struct ds *string, uint16_t port); @@ -1120,14 +1121,15 @@ ofp_print_ofpst_flow_reply(struct ds *string, const struct ofp_header *oh, } ds_put_format(string, " cookie=0x%"PRIx64", duration=", - ntohll(fs->cookie)); + ntohll(get_32aligned_be64(&fs->cookie))); ofp_print_duration(string, ntohl(fs->duration_sec), ntohl(fs->duration_nsec)); ds_put_format(string, ", table_id=%"PRIu8", ", fs->table_id); ds_put_format(string, "priority=%"PRIu16", ", ntohs(fs->priority)); ds_put_format(string, "n_packets=%"PRIu64", ", - ntohll(fs->packet_count)); - ds_put_format(string, "n_bytes=%"PRIu64", ", ntohll(fs->byte_count)); + ntohll(get_32aligned_be64(&fs->packet_count))); + ds_put_format(string, "n_bytes=%"PRIu64", ", + ntohll(get_32aligned_be64(&fs->byte_count))); if (fs->idle_timeout != htons(OFP_FLOW_PERMANENT)) { ds_put_format(string, "idle_timeout=%"PRIu16",", ntohs(fs->idle_timeout)); @@ -1223,8 +1225,10 @@ static void ofp_print_ofp_aggregate_stats_reply ( struct ds *string, const struct ofp_aggregate_stats_reply *asr) { - ds_put_format(string, " packet_count=%"PRIu64, ntohll(asr->packet_count)); - ds_put_format(string, " byte_count=%"PRIu64, ntohll(asr->byte_count)); + ds_put_format(string, " packet_count=%"PRIu64, + ntohll(get_32aligned_be64(&asr->packet_count))); + ds_put_format(string, " byte_count=%"PRIu64, + ntohll(get_32aligned_be64(&asr->byte_count))); ds_put_format(string, " flow_count=%"PRIu32, ntohl(asr->flow_count)); } @@ -1242,10 +1246,12 @@ ofp_print_nxst_aggregate_reply(struct ds *string, } static void print_port_stat(struct ds *string, const char *leader, - uint64_t stat, int more) + const ovs_32aligned_be64 *statp, int more) { + uint64_t stat = ntohll(get_32aligned_be64(statp)); + ds_put_cstr(string, leader); - if (stat != -1) { + if (stat != UINT64_MAX) { ds_put_format(string, "%"PRIu64, stat); } else { ds_put_char(string, '?'); @@ -1279,20 +1285,20 @@ ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh, ds_put_format(string, " port %2"PRIu16": ", ntohs(ps->port_no)); ds_put_cstr(string, "rx "); - print_port_stat(string, "pkts=", ntohll(ps->rx_packets), 1); - print_port_stat(string, "bytes=", ntohll(ps->rx_bytes), 1); - print_port_stat(string, "drop=", ntohll(ps->rx_dropped), 1); - print_port_stat(string, "errs=", ntohll(ps->rx_errors), 1); - print_port_stat(string, "frame=", ntohll(ps->rx_frame_err), 1); - print_port_stat(string, "over=", ntohll(ps->rx_over_err), 1); - print_port_stat(string, "crc=", ntohll(ps->rx_crc_err), 0); + print_port_stat(string, "pkts=", &ps->rx_packets, 1); + print_port_stat(string, "bytes=", &ps->rx_bytes, 1); + print_port_stat(string, "drop=", &ps->rx_dropped, 1); + print_port_stat(string, "errs=", &ps->rx_errors, 1); + print_port_stat(string, "frame=", &ps->rx_frame_err, 1); + print_port_stat(string, "over=", &ps->rx_over_err, 1); + print_port_stat(string, "crc=", &ps->rx_crc_err, 0); ds_put_cstr(string, " tx "); - print_port_stat(string, "pkts=", ntohll(ps->tx_packets), 1); - print_port_stat(string, "bytes=", ntohll(ps->tx_bytes), 1); - print_port_stat(string, "drop=", ntohll(ps->tx_dropped), 1); - print_port_stat(string, "errs=", ntohll(ps->tx_errors), 1); - print_port_stat(string, "coll=", ntohll(ps->collisions), 0); + print_port_stat(string, "pkts=", &ps->tx_packets, 1); + print_port_stat(string, "bytes=", &ps->tx_bytes, 1); + print_port_stat(string, "drop=", &ps->tx_dropped, 1); + print_port_stat(string, "errs=", &ps->tx_errors, 1); + print_port_stat(string, "coll=", &ps->collisions, 0); } } @@ -1318,9 +1324,9 @@ ofp_print_ofpst_table_reply(struct ds *string, const struct ofp_header *oh, ds_put_format(string, "active=%"PRIu32"\n", ntohl(ts->active_count)); ds_put_cstr(string, " "); ds_put_format(string, "lookup=%"PRIu64", ", - ntohll(ts->lookup_count)); + ntohll(get_32aligned_be64(&ts->lookup_count))); ds_put_format(string, "matched=%"PRIu64"\n", - ntohll(ts->matched_count)); + ntohll(get_32aligned_be64(&ts->matched_count))); } } @@ -1364,9 +1370,9 @@ ofp_print_ofpst_queue_reply(struct ds *string, const struct ofp_header *oh, ofp_print_queue_name(string, ntohl(qs->queue_id)); ds_put_cstr(string, ": "); - print_port_stat(string, "bytes=", ntohll(qs->tx_bytes), 1); - print_port_stat(string, "pkts=", ntohll(qs->tx_packets), 1); - print_port_stat(string, "errors=", ntohll(qs->tx_errors), 0); + print_port_stat(string, "bytes=", &qs->tx_bytes, 1); + print_port_stat(string, "pkts=", &qs->tx_packets, 1); + print_port_stat(string, "errors=", &qs->tx_errors, 0); } } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index c980f693..82008be1 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -59,6 +59,7 @@ #include "svec.h" #include "tag.h" #include "timeval.h" +#include "unaligned.h" #include "unixctl.h" #include "vconn.h" #include "vlog.h" @@ -3383,8 +3384,8 @@ handle_table_stats_request(struct ofconn *ofconn, ? htonl(OFPFW_ALL) : htonl(OVSFW_ALL)); ots->max_entries = htonl(1024 * 1024); /* An arbitrary big number. */ ots->active_count = htonl(classifier_count(&p->cls)); - ots->lookup_count = htonll(0); /* XXX */ - ots->matched_count = htonll(0); /* XXX */ + put_32aligned_be64(&ots->lookup_count, htonll(0)); /* XXX */ + put_32aligned_be64(&ots->matched_count, htonll(0)); /* XXX */ queue_tx(msg, ofconn, ofconn->reply_counter); return 0; @@ -3405,18 +3406,18 @@ append_port_stat(struct ofport *port, struct ofconn *ofconn, ops = append_ofp_stats_reply(sizeof *ops, ofconn, msgp); ops->port_no = htons(port->opp.port_no); memset(ops->pad, 0, sizeof ops->pad); - ops->rx_packets = htonll(stats.rx_packets); - ops->tx_packets = htonll(stats.tx_packets); - ops->rx_bytes = htonll(stats.rx_bytes); - ops->tx_bytes = htonll(stats.tx_bytes); - ops->rx_dropped = htonll(stats.rx_dropped); - ops->tx_dropped = htonll(stats.tx_dropped); - ops->rx_errors = htonll(stats.rx_errors); - ops->tx_errors = htonll(stats.tx_errors); - ops->rx_frame_err = htonll(stats.rx_frame_errors); - ops->rx_over_err = htonll(stats.rx_over_errors); - ops->rx_crc_err = htonll(stats.rx_crc_errors); - ops->collisions = htonll(stats.collisions); + put_32aligned_be64(&ops->rx_packets, htonll(stats.rx_packets)); + put_32aligned_be64(&ops->tx_packets, htonll(stats.tx_packets)); + put_32aligned_be64(&ops->rx_bytes, htonll(stats.rx_bytes)); + put_32aligned_be64(&ops->tx_bytes, htonll(stats.tx_bytes)); + put_32aligned_be64(&ops->rx_dropped, htonll(stats.rx_dropped)); + put_32aligned_be64(&ops->tx_dropped, htonll(stats.tx_dropped)); + put_32aligned_be64(&ops->rx_errors, htonll(stats.rx_errors)); + put_32aligned_be64(&ops->tx_errors, htonll(stats.tx_errors)); + put_32aligned_be64(&ops->rx_frame_err, htonll(stats.rx_frame_errors)); + put_32aligned_be64(&ops->rx_over_err, htonll(stats.rx_over_errors)); + put_32aligned_be64(&ops->rx_crc_err, htonll(stats.rx_crc_errors)); + put_32aligned_be64(&ops->collisions, htonll(stats.collisions)); } static int @@ -3498,6 +3499,7 @@ put_ofp_flow_stats(struct ofconn *ofconn, struct rule *rule, { struct ofp_flow_stats *ofs; uint64_t packet_count, byte_count; + ovs_be64 cookie; size_t act_len, len; if (rule_is_hidden(rule) || !rule_has_out_port(rule, out_port)) { @@ -3514,14 +3516,15 @@ put_ofp_flow_stats(struct ofconn *ofconn, struct rule *rule, ofs->table_id = 0; ofs->pad = 0; ofputil_cls_rule_to_match(&rule->cr, ofconn->flow_format, &ofs->match, - rule->flow_cookie, &ofs->cookie); + rule->flow_cookie, &cookie); + put_32aligned_be64(&ofs->cookie, cookie); calc_flow_duration(rule->created, &ofs->duration_sec, &ofs->duration_nsec); ofs->priority = htons(rule->cr.priority); ofs->idle_timeout = htons(rule->idle_timeout); ofs->hard_timeout = htons(rule->hard_timeout); memset(ofs->pad2, 0, sizeof ofs->pad2); - ofs->packet_count = htonll(packet_count); - ofs->byte_count = htonll(byte_count); + put_32aligned_be64(&ofs->packet_count, htonll(packet_count)); + put_32aligned_be64(&ofs->byte_count, htonll(byte_count)); if (rule->n_actions > 0) { memcpy(ofs->actions, rule->actions, act_len); } @@ -3701,8 +3704,8 @@ query_aggregate_stats(struct ofproto *ofproto, struct cls_rule *target, } oasr->flow_count = htonl(n_flows); - oasr->packet_count = htonll(total_packets); - oasr->byte_count = htonll(total_bytes); + put_32aligned_be64(&oasr->packet_count, htonll(total_packets)); + put_32aligned_be64(&oasr->byte_count, htonll(total_bytes)); memset(oasr->pad, 0, sizeof oasr->pad); } @@ -3775,9 +3778,9 @@ put_queue_stats(struct queue_stats_cbdata *cbdata, uint32_t queue_id, reply->port_no = htons(cbdata->ofport->opp.port_no); memset(reply->pad, 0, sizeof reply->pad); reply->queue_id = htonl(queue_id); - reply->tx_bytes = htonll(stats->tx_bytes); - reply->tx_packets = htonll(stats->tx_packets); - reply->tx_errors = htonll(stats->tx_errors); + put_32aligned_be64(&reply->tx_bytes, htonll(stats->tx_bytes)); + put_32aligned_be64(&reply->tx_packets, htonll(stats->tx_packets)); + put_32aligned_be64(&reply->tx_errors, htonll(stats->tx_errors)); } static void -- 2.30.2