Well, it unifies both. I changed it to "Decode and print".
On Tue, Jan 19, 2016 at 04:23:46PM -0800, Jarno Rajahalme wrote:
> Maybe the title should be “ofp-print: Print all async config messages the
> same way.” instead?
>
> Acked-by: Jarno Rajahalme <ja...@ovn.org>
>
> > On Jan 18, 2016, at 11:27 PM, Ben Pfaff <b...@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 <b...@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@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
