[ovs-dev] [PATCH 27/41] ofp-print: Decode all async config messages the same way.
Jarno Rajahalme
jarno at ovn.org
Wed Jan 20 00:23:46 UTC 2016
Maybe the title should be “ofp-print: Print all async config messages the same way.” instead?
Acked-by: Jarno Rajahalme <jarno at ovn.org>
> On Jan 18, 2016, at 11:27 PM, Ben Pfaff <blp at ovn.org> wrote:
>
> We have a single function to decode all of these messages, so there's no
> reason to do it two different ways for printing.
>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
> lib/ofp-print.c | 117 +++++++++++---------------------------------------
> tests/ofp-print.at | 26 +++++++++--
> tests/ofproto-dpif.at | 6 +++
> tests/ofproto.at | 6 +++
> 4 files changed, 60 insertions(+), 95 deletions(-)
>
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index f36335b..bedfc94 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -2063,108 +2063,42 @@ ofp_async_config_reason_to_string(uint32_t reason,
>
> #define OFP_ASYNC_CONFIG_REASON_BUFSIZE (INT_STRLEN(int) + 1)
> static void
> -ofp_print_nxt_set_async_config(struct ds *string,
> - const struct ofp_header *oh)
> +ofp_print_set_async_config(struct ds *string, const struct ofp_header *oh,
> + enum ofptype type)
> {
> - int i, j;
> - enum ofpraw raw;
> -
> - ofpraw_decode(&raw, oh);
> -
> - if (raw == OFPRAW_OFPT13_SET_ASYNC ||
> - raw == OFPRAW_NXT_SET_ASYNC_CONFIG ||
> - raw == OFPRAW_OFPT13_GET_ASYNC_REPLY) {
> - const struct nx_async_config *nac = ofpmsg_body(oh);
> -
> - for (i = 0; i < 2; i++) {
> + struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT;
> + struct ofputil_async_cfg ac;
>
> - ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave");
> -
> - ds_put_cstr(string, " PACKET_IN:");
> - for (j = 0; j < 32; j++) {
> - if (nac->packet_in_mask[i] & htonl(1u << j)) {
> - char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
> - const char *reason;
> -
> - reason = ofputil_packet_in_reason_to_string(j, reasonbuf,
> - sizeof reasonbuf);
> - ds_put_format(string, " %s", reason);
> - }
> - }
> - if (!nac->packet_in_mask[i]) {
> - ds_put_cstr(string, " (off)");
> - }
> - ds_put_char(string, '\n');
> -
> - ds_put_cstr(string, " PORT_STATUS:");
> - for (j = 0; j < 32; j++) {
> - if (nac->port_status_mask[i] & htonl(1u << j)) {
> - char reasonbuf[OFP_PORT_REASON_BUFSIZE];
> - const char *reason;
> + bool is_reply = type == OFPTYPE_GET_ASYNC_REPLY;
> + enum ofperr error = ofputil_decode_set_async_config(oh, is_reply,
> + &basis, &ac);
> + if (error) {
> + ofp_print_error(string, error);
> + return;
> + }
>
> - reason = ofp_port_reason_to_string(j, reasonbuf,
> - sizeof reasonbuf);
> - ds_put_format(string, " %s", reason);
> - }
> - }
> - if (!nac->port_status_mask[i]) {
> - ds_put_cstr(string, " (off)");
> - }
> - ds_put_char(string, '\n');
> + for (int i = 0; i < 2; i++) {
> + ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave");
> + for (uint32_t type = 0; type < OAM_N_TYPES; type++) {
> + ds_put_format(string, "%16s:",
> + ofputil_async_msg_type_to_string(type));
>
> - ds_put_cstr(string, " FLOW_REMOVED:");
> - for (j = 0; j < 32; j++) {
> - if (nac->flow_removed_mask[i] & htonl(1u << j)) {
> - char reasonbuf[OFP_FLOW_REMOVED_REASON_BUFSIZE];
> + uint32_t role = i == 0 ? ac.master[type] : ac.slave[type];
> + for (int j = 0; j < 32; j++) {
> + if (role & (1u << j)) {
> + char reasonbuf[OFP_ASYNC_CONFIG_REASON_BUFSIZE];
> const char *reason;
>
> - reason = ofp_flow_removed_reason_to_string(j, reasonbuf,
> - sizeof reasonbuf);
> + reason = ofp_async_config_reason_to_string(
> + j, type, reasonbuf, sizeof reasonbuf);
> ds_put_format(string, " %s", reason);
> }
> }
> - if (!nac->flow_removed_mask[i]) {
> + if (!role) {
> ds_put_cstr(string, " (off)");
> }
> ds_put_char(string, '\n');
> }
> - } else if (raw == OFPRAW_OFPT14_SET_ASYNC ||
> - raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) {
> - struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT;
> - struct ofputil_async_cfg ac;
> -
> - bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY;
> - enum ofperr error = ofputil_decode_set_async_config(oh, is_reply,
> - &basis, &ac);
> - if (error) {
> - ofp_print_error(string, error);
> - return;
> - }
> -
> - for (i = 0; i < 2; i++) {
> - ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave");
> - for (uint32_t type = 0; type < OAM_N_TYPES; type++) {
> - ds_put_format(string, "%16s:",
> - ofputil_async_msg_type_to_string(type));
> -
> - uint32_t role = i == 0 ? ac.master[type] : ac.slave[type];
> - for (j = 0; j < 32; j++) {
> - if (role & (1u << j)) {
> - char reasonbuf[OFP_ASYNC_CONFIG_REASON_BUFSIZE];
> - const char *reason;
> -
> - reason = ofp_async_config_reason_to_string(j, type,
> - reasonbuf,
> - sizeof reasonbuf);
> - ds_put_format(string, " %s", reason);
> - }
> - }
> - if (!role) {
> - ds_put_cstr(string, " (off)");
> - }
> - ds_put_char(string, '\n');
> - }
> - }
> }
> }
>
> @@ -3137,8 +3071,9 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
> const void *msg = oh;
>
> ofp_header_to_string__(oh, raw, string);
> - switch (ofptype_from_ofpraw(raw)) {
>
> + enum ofptype type = ofptype_from_ofpraw(raw);
> + switch (type) {
> case OFPTYPE_GROUP_STATS_REQUEST:
> ofp_print_stats(string, oh);
> ofp_print_ofpst_group_request(string, oh);
> @@ -3373,7 +3308,7 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
>
> case OFPTYPE_GET_ASYNC_REPLY:
> case OFPTYPE_SET_ASYNC_CONFIG:
> - ofp_print_nxt_set_async_config(string, oh);
> + ofp_print_set_async_config(string, oh, type);
> break;
> case OFPTYPE_GET_ASYNC_REQUEST:
> break;
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index 6d508d3..c791cb2 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -2637,20 +2637,30 @@ AT_CLEANUP
>
> AT_SETUP([OFPT_SET_ASYNC - OF1.3])
> AT_KEYWORDS([ofp-print])
> +dnl This message has bit 12 set for the PACKET_IN messages (master and slave).
> +dnl Those aren't supported bits so they get silently ignored on decoding.
> +dnl That seems reasonable because OF1.3 doesn't define any error codes for
> +dnl OFPT_SET_ASYNC.
> AT_CHECK([ovs-ofctl ofp-print "\
> 04 1c 00 20 00 00 00 00 00 00 10 05 00 00 10 07 \
> 00 00 00 03 00 00 00 07 00 00 00 00 00 00 00 03 \
> "], [0], [dnl
> OFPT_SET_ASYNC (OF1.3) (xid=0x0):
> master:
> - PACKET_IN: no_match invalid_ttl 12
> + PACKET_IN: no_match invalid_ttl
> PORT_STATUS: add delete
> FLOW_REMOVED: (off)
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
>
> slave:
> - PACKET_IN: no_match action invalid_ttl 12
> + PACKET_IN: no_match action invalid_ttl
> PORT_STATUS: add delete modify
> FLOW_REMOVED: idle hard
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
> ])
> AT_CLEANUP
>
> @@ -2838,6 +2848,8 @@ AT_CLEANUP
>
> AT_SETUP([NXT_SET_ASYNC_CONFIG])
> AT_KEYWORDS([ofp-print])
> +dnl This message has bit 12 set for the PACKET_IN messages (master and slave).
> +dnl Those aren't supported bits so they get silently ignored on decoding.
> AT_CHECK([ovs-ofctl ofp-print "\
> 01 04 00 28 00 00 00 00 00 00 23 20 00 00 00 13 \
> 00 00 10 05 00 00 10 07 00 00 00 03 00 00 00 07 \
> @@ -2845,14 +2857,20 @@ AT_CHECK([ovs-ofctl ofp-print "\
> "], [0], [dnl
> NXT_SET_ASYNC_CONFIG (xid=0x0):
> master:
> - PACKET_IN: no_match invalid_ttl 12
> + PACKET_IN: no_match invalid_ttl
> PORT_STATUS: add delete
> FLOW_REMOVED: (off)
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
>
> slave:
> - PACKET_IN: no_match action invalid_ttl 12
> + PACKET_IN: no_match action invalid_ttl
> PORT_STATUS: add delete modify
> FLOW_REMOVED: idle hard
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
> ])
> AT_CLEANUP
>
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 4c2a995..bc1af8f 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -2810,11 +2810,17 @@ send: OFPT_SET_ASYNC (OF1.3) (xid=0x2):
> PACKET_IN: (off)
> PORT_STATUS: (off)
> FLOW_REMOVED: (off)
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
>
> slave:
> PACKET_IN: no_match
> PORT_STATUS: (off)
> FLOW_REMOVED: (off)
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
> dnl
> OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via no_match) data_len=60 (unbuffered)
> tcp,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index ab4d254..61a6be5 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -3845,11 +3845,17 @@ OFPT_GET_ASYNC_REPLY (OF1.3):
> PACKET_IN: no_match action
> PORT_STATUS: add delete modify
> FLOW_REMOVED: idle hard delete
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
>
> slave:
> PACKET_IN: (off)
> PORT_STATUS: add delete modify
> FLOW_REMOVED: (off)
> + ROLE_STATUS: (off)
> + TABLE_STATUS: (off)
> + REQUESTFORWARD: (off)
> OFPT_BARRIER_REPLY (OF1.3):
> ])
>
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list