Here's an incremental. Ethan
--- include/openflow/nicira-ext.h | 5 ++--- lib/ofp-util.c | 37 +++++++++++++++++++------------------ tests/ofp-print.at | 30 ++++++++++++++++++++++++++++++ tests/ofproto-dpif.at | 8 ++++---- utilities/ovs-ofctl.8.in | 10 ++++++---- utilities/ovs-ofctl.c | 14 ++++++++++++++ 6 files changed, 75 insertions(+), 29 deletions(-) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 8933dcb..bb0fb3a 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -259,13 +259,12 @@ enum nx_packet_in_format { struct nxt_set_packet_in_format { struct nicira_header nxh; ovs_be32 format; /* One of NXPIF_*. */ - uint8_t pad[4]; }; -OFP_ASSERT(sizeof(struct nxt_set_packet_in_format) == 24); +OFP_ASSERT(sizeof(struct nxt_set_packet_in_format) == 20); /* NXT_PACKET_IN (analogous to OFPT_PACKET_IN). * - * The NXT_PACKET_IN format is intended to model the OpenFLow-1.2 PACKET_IN + * The NXT_PACKET_IN format is intended to model the OpenFlow-1.2 PACKET_IN * with some minor tweaks. Most notably NXT_PACKET_IN includes the cookie of * the rule which triggered the NXT_PACKET_IN message, and the match fields are * in NXM format. diff --git a/lib/ofp-util.c b/lib/ofp-util.c index e9c88f5..1c9ceaf 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -1631,7 +1631,10 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin, if (error) { return error; } - ofpbuf_pull(&b, 2); + + if (!ofpbuf_try_pull(&b, 2)) { + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); + } pin->packet = b.data; pin->packet_len = b.size; @@ -1664,24 +1667,23 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin, enum nx_packet_in_format packet_in_format) { size_t send_len = MIN(pin->send_len, pin->packet_len); - struct ofpbuf *rw_packet = ofpbuf_new(0); + struct ofpbuf *packet; /* Add OFPT_PACKET_IN. */ if (packet_in_format == NXPIF_OPENFLOW10) { size_t header_len = offsetof(struct ofp_packet_in, data); struct ofp_packet_in *opi; - ofpbuf_prealloc_tailroom(rw_packet, send_len + header_len); - - opi = ofpbuf_put_zeros(rw_packet, header_len); + packet = ofpbuf_new(send_len + header_len); + opi = ofpbuf_put_zeros(packet, header_len); opi->header.version = OFP_VERSION; opi->header.type = OFPT_PACKET_IN; - opi->total_len = htons(pin->packet_len); + opi->total_len = htons(pin->total_len); opi->in_port = htons(pin->fmd.in_port); opi->reason = pin->reason; opi->buffer_id = htonl(pin->buffer_id); - ofpbuf_put(rw_packet, pin->packet, send_len); + ofpbuf_put(packet, pin->packet, send_len); } else if (packet_in_format == NXPIF_NXM) { struct nxt_packet_in *npi; struct cls_rule rule; @@ -1691,9 +1693,8 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin, /* Estimate of required PACKET_IN length includes the NPI header, space * for the match (2 times sizeof the metadata seems like enough), 2 * bytes for padding, and the packet length. */ - ofpbuf_prealloc_tailroom(rw_packet, - sizeof *npi + sizeof(struct flow_metadata) * 2 - + 2 + send_len); + packet = ofpbuf_new(sizeof *npi + sizeof(struct flow_metadata) * 2 + + 2 + send_len); cls_rule_init_catchall(&rule, 0); cls_rule_set_tun_id_masked(&rule, pin->fmd.tun_id, @@ -1706,19 +1707,19 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin, cls_rule_set_in_port(&rule, pin->fmd.in_port); - ofpbuf_put_zeros(rw_packet, sizeof *npi); - match_len = nx_put_match(rw_packet, &rule, 0, 0); - ofpbuf_put_zeros(rw_packet, 2); - ofpbuf_put(rw_packet, pin->packet, send_len); + ofpbuf_put_zeros(packet, sizeof *npi); + match_len = nx_put_match(packet, &rule, 0, 0); + ofpbuf_put_zeros(packet, 2); + ofpbuf_put(packet, pin->packet, send_len); - npi = rw_packet->data; + npi = packet->data; npi->nxh.header.version = OFP_VERSION; npi->nxh.header.type = OFPT_VENDOR; npi->nxh.vendor = htonl(NX_VENDOR_ID); npi->nxh.subtype = htonl(NXT_PACKET_IN); npi->buffer_id = htonl(pin->buffer_id); - npi->total_len = htons(pin->packet_len); + npi->total_len = htons(pin->total_len); npi->reason = pin->reason; npi->table_id = pin->table_id; npi->cookie = pin->cookie; @@ -1726,9 +1727,9 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin, } else { NOT_REACHED(); } - update_openflow_length(rw_packet); + update_openflow_length(packet); - return rw_packet; + return packet; } /* Returns a string representing the message type of 'type'. The string is the diff --git a/tests/ofp-print.at b/tests/ofp-print.at index aa3a218..2ca07c4 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -641,6 +641,36 @@ NXT_ROLE_REPLY (xid=0x2): role=slave ]) AT_CLEANUP +AT_SETUP([NXT_SET_PACKET_IN]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +01 04 00 14 00 00 00 02 00 00 23 20 00 00 00 10 \ +00 00 00 01 \ +"], [0], [dnl +NXT_SET_PACKET_IN_FORMAT (xid=0x2): format=nxm +]) +AT_CLEANUP + +AT_SETUP([NXT_PACKET_IN]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +01 04 00 aa 00 00 00 00 00 00 23 20 00 00 00 11 \ +ff ff ff ff 00 40 01 07 00 00 00 00 00 00 00 09 \ +00 3a 00 00 00 00 00 00 00 00 00 02 00 01 00 01 \ +20 08 00 00 00 00 00 00 00 06 00 01 00 04 00 00 \ +00 01 00 01 02 04 00 00 00 02 00 01 04 04 00 00 \ +00 03 00 01 06 04 00 00 00 04 00 01 08 04 00 00 \ +00 05 00 00 00 00 00 00 00 00 82 82 82 82 82 82 \ +80 81 81 81 81 81 81 00 00 50 08 00 45 00 00 28 \ +00 00 00 00 00 06 32 05 53 53 53 53 54 54 54 54 \ +00 55 00 56 00 00 00 00 00 00 00 00 50 00 00 00 \ +31 6d 00 00 00 00 00 00 00 00 \ +"], [0], [dnl +NXT_PACKET_IN (xid=0x0): table_id=7 cookie=0x9 total_len=64 in_port=1 tun_id=0x6 reg0=0x1 reg1=0x2 reg2=0x3 reg3=0x4 reg4=0x5 (via action) data_len=64 (unbuffered) +priority:0,tunnel:0,in_port:0000,tci(vlan:80,pcp:0) mac(80:81:81:81:81:81->82:82:82:82:82:82) type:0800 proto:6 tos:0 ttl:0 ip(83.83.83.83->84.84.84.84) port(85->86) tcp_csum:316d +]) +AT_CLEANUP + AT_SETUP([NXT_SET_FLOW_FORMAT]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 3a7f47b..615eb57 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -215,7 +215,7 @@ cookie=0x9 table=7 in_port=86 actions=mod_tp_dst:86,controller,controller AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) dnl Flow miss. -AT_CHECK([ovs-ofctl monitor br0 65534 --detach --pidfile 2> ofctl_monitor.log]) +AT_CHECK([ovs-ofctl monitor -P openflow10 br0 65534 --detach --pidfile 2> ofctl_monitor.log]) for i in 1 2 3 ; do ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9)' @@ -234,7 +234,7 @@ priority:0,tunnel:0,in_port:0000,tci(0) mac(50:54:00:00:00:05->50:54:00:00:00:07 ]) dnl Singleton controller action. -AT_CHECK([ovs-ofctl monitor br0 65534 --detach --pidfile 2> ofctl_monitor.log]) +AT_CHECK([ovs-ofctl monitor -P openflow10 br0 65534 --detach --pidfile 2> ofctl_monitor.log]) for i in 1 2 3 ; do ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=10:11:11:11:11:11,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=10)' @@ -253,7 +253,7 @@ priority:0,tunnel:0,in_port:0000,tci(0) mac(10:11:11:11:11:11->50:54:00:00:00:07 ]) dnl Modified controller action. -AT_CHECK([ovs-ofctl monitor br0 65534 --detach --pidfile 2> ofctl_monitor.log]) +AT_CHECK([ovs-ofctl monitor -P openflow10 br0 65534 --detach --pidfile 2> ofctl_monitor.log]) for i in 1 2 3 ; do ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=30:33:33:33:33:33,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=10)' @@ -309,7 +309,7 @@ priority:0,tunnel:0,in_port:0000,tci(vlan:80,pcp:0) mac(80:81:81:81:81:81->82:82 ]) dnl Checksum UDP. -AT_CHECK([ovs-ofctl monitor br0 65534 -P nxm --detach --pidfile 2> ofctl_monitor.log]) +AT_CHECK([ovs-ofctl monitor br0 65534 --detach --pidfile 2> ofctl_monitor.log]) for i in 1 ; do ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 20 22 22 22 22 22 08 00 45 00 00 1C 00 00 00 00 00 11 00 00 C0 A8 00 01 C0 A8 00 02 00 08 00 0B 00 00 12 34 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00' diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 26a4072..d692d42 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1064,10 +1064,12 @@ This packet_in format includes flow metadata encoded using the NXM format. . .RE .IP -Usually, \fBovs\-ofctl\fR allows the switch to choose its default packet_in -format. When \fIformat\fR is one of the formats listed in the above table, -\fBovs\-ofctl\fR will insist on the selected format. If the switch does not -support the requested format, \fBovs\-ofctl\fR will report a fatal error. +Usually, \fBovs\-ofctl\fR prefers the \fBnxm\fR packet_in format, but will +allow the switch to choose its default if \fBnxm\fR is unsupported. When +\fIformat\fR is one of the formats listed in the above table, \fBovs\-ofctl\fR +will insist on the selected format. If the switch does not support the +requested format, \fBovs\-ofctl\fR will report a fatal error. This option only +affects the monitor and snoop commands. . .IP "\fB\-m\fR" .IQ "\fB\-\-more\fR" diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 022fd2d..995a6c6 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -802,6 +802,20 @@ monitor_vconn(struct vconn *vconn) if (preferred_packet_in_format >= 0) { set_packet_in_format(vconn, preferred_packet_in_format); + } else { + struct ofpbuf *spif, *reply; + + spif = ofputil_make_set_packet_in_format(NXPIF_NXM); + run(vconn_transact_noreply(vconn, spif, &reply), + "talking to %s", vconn_get_name(vconn)); + if (reply) { + char *s = ofp_to_string(reply->data, reply->size, 2); + VLOG_DBG("%s: failed to set packet in format to nxm, controller" + " replied: %s. Falling back to the switch default.", + vconn_get_name(vconn), s); + free(s); + ofpbuf_delete(reply); + } } /* Daemonization will close stderr but we really want to keep it, so make a -- 1.7.7.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev