Packet-in messages have been a bit of a mess. First, their abstraction
in the form of struct ofputil_packet_in has some fields that are used
in a clear way for incoming and outgoing packet-ins, and others
(packet_len, total_len, buffer_id) have have confusing meanings or
usage pattern depending on their direction.
Second, it's very confusing how a packet-in has both a reason (OFPR_*)
and a miss type (OFPROTO_PACKET_IN_*) and how those add up to the
actual reason that is used "on the wire" for each OpenFlow version (and
even whether the packet-in is sent at all!).
Finally, there's all kind of low-level detail randomly scattered between
connmgr, ofproto-dpif-xlate, and ofp-util.
This commit attempts to clear up some of the confusion. It simplifies
the struct ofputil_packet_in abstraction by removing the members that
didn't have a clear and consistent meaning between incoming and outgoing
packet-ins. It gets rid of OFPROTO_PACKET_IN_*, instead adding a couple
of nonstandard OFPR_* reasons that add up to what OFPROTO_PACKET_IN_*
was meant to say (in what I hope is a clearer way). And it consolidates
the tricky parts into ofp-util, where I hope it will be easier to
understand all in one place.
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
include/openflow/openflow-common.h | 5 +
lib/learning-switch.c | 18 +--
lib/ofp-print.c | 27 +++--
lib/ofp-util.c | 233 +++++++++++++++++++++++++------------
lib/ofp-util.h | 40 ++++---
ofproto/connmgr.c | 140 ++++------------------
ofproto/connmgr.h | 22 +---
ofproto/fail-open.c | 23 ++--
ofproto/ofproto-dpif-xlate.c | 47 ++++----
ofproto/ofproto-dpif.c | 2 +-
ofproto/ofproto.c | 2 +-
ovn/controller/pinctrl.c | 4 +-
12 files changed, 270 insertions(+), 293 deletions(-)
diff --git a/include/openflow/openflow-common.h
b/include/openflow/openflow-common.h
index 3985705..da2b7a5 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -274,6 +274,7 @@ enum ofp_capabilities {
/* Why is this packet being sent to the controller? */
enum ofp_packet_in_reason {
+ /* Standard reasons. */
OFPR_NO_MATCH, /* No matching flow. */
OFPR_ACTION, /* Action explicitly output to controller. */
OFPR_INVALID_TTL, /* Packet has invalid TTL. */
@@ -287,6 +288,10 @@ enum ofp_packet_in_reason {
(OFPR10_BITS | \
(1u << OFPR_ACTION_SET) | (1u << OFPR_GROUP) | (1u << OFPR_PACKET_OUT))
+ /* Nonstandard reason--not exposed via OpenFlow. */
+ OFPR_EXPLICIT_MISS,
+ OFPR_IMPLICIT_MISS,
+
OFPR_N_REASONS
};
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 2b764f6..35c3fef 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -512,6 +512,8 @@ static void
process_packet_in(struct lswitch *sw, const struct ofp_header *oh)
{
struct ofputil_packet_in pi;
+ size_t total_len;
+ uint32_t buffer_id;
uint32_t queue_id;
ofp_port_t out_port;
@@ -524,7 +526,7 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
struct dp_packet pkt;
struct flow flow;
- error = ofputil_decode_packet_in(&pi, oh);
+ error = ofputil_decode_packet_in(oh, &pi, &total_len, &buffer_id);
if (error) {
VLOG_WARN_RL(&rl, "failed to decode packet-in: %s",
ofperr_to_string(error));
@@ -538,8 +540,8 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
return;
}
- /* Extract flow data from 'opi' into 'flow'. */
- dp_packet_use_const(&pkt, pi.packet, pi.packet_len);
+ /* Extract flow data from 'pi' into 'flow'. */
+ dp_packet_use_const(&pkt, pi.packet, pi.len);
flow_extract(&pkt, &flow);
flow.in_port.ofp_port = pi.flow_metadata.flow.in_port.ofp_port;
flow.tunnel.tun_id = pi.flow_metadata.flow.tunnel.tun_id;
@@ -562,8 +564,8 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
}
/* Prepare packet_out in case we need one. */
- po.buffer_id = pi.buffer_id;
- if (po.buffer_id == UINT32_MAX) {
+ po.buffer_id = buffer_id;
+ if (buffer_id == UINT32_MAX) {
po.packet = dp_packet_data(&pkt);
po.packet_len = dp_packet_size(&pkt);
} else {
@@ -583,7 +585,7 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
.table_id = 0xff,
.command = OFPFC_ADD,
.idle_timeout = sw->max_idle,
- .buffer_id = pi.buffer_id,
+ .buffer_id = buffer_id,
.out_port = OFPP_NONE,
.ofpacts = ofpacts.data,
.ofpacts_len = ofpacts.size,
@@ -596,13 +598,13 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
queue_tx(sw, buffer);
/* If the switch didn't buffer the packet, we need to send a copy. */
- if (pi.buffer_id == UINT32_MAX && out_port != OFPP_NONE) {
+ if (buffer_id == UINT32_MAX && out_port != OFPP_NONE) {
queue_tx(sw, ofputil_encode_packet_out(&po, sw->protocol));
}
} else {
/* We don't know that MAC, or we don't set up flows. Send along the
* packet without setting up a flow. */
- if (pi.buffer_id != UINT32_MAX || out_port != OFPP_NONE) {
+ if (buffer_id != UINT32_MAX || out_port != OFPP_NONE) {
queue_tx(sw, ofputil_encode_packet_out(&po, sw->protocol));
}
}
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index bedfc94..5c9a6ab 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -100,9 +100,11 @@ ofp_print_packet_in(struct ds *string, const struct
ofp_header *oh,
{
char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
struct ofputil_packet_in pin;
+ uint32_t buffer_id;
+ size_t total_len;
int error;
- error = ofputil_decode_packet_in(&pin, oh);
+ error = ofputil_decode_packet_in(oh, &pin, &total_len, &buffer_id);
if (error) {
ofp_print_error(string, error);
return;
@@ -116,35 +118,36 @@ ofp_print_packet_in(struct ds *string, const struct
ofp_header *oh,
ds_put_format(string, " cookie=0x%"PRIx64, ntohll(pin.cookie));
}
- ds_put_format(string, " total_len=%"PRIuSIZE" ", pin.total_len);
+ ds_put_format(string, " total_len=%"PRIuSIZE" ", total_len);
match_format(&pin.flow_metadata, string, OFP_DEFAULT_PRIORITY);
ds_put_format(string, " (via %s)",
- ofputil_packet_in_reason_to_string(pin.reason, reasonbuf,
+ ofputil_packet_in_reason_to_string(pin.reason,
+ reasonbuf,
sizeof reasonbuf));
- ds_put_format(string, " data_len=%"PRIuSIZE, pin.packet_len);
- if (pin.buffer_id == UINT32_MAX) {
+ ds_put_format(string, " data_len=%"PRIuSIZE, pin.len);
+ if (buffer_id == UINT32_MAX) {
ds_put_format(string, " (unbuffered)");
- if (pin.total_len != pin.packet_len) {
+ if (total_len != pin.len) {
ds_put_format(string, " (***total_len != data_len***)");
}
} else {
- ds_put_format(string, " buffer=0x%08"PRIx32, pin.buffer_id);
- if (pin.total_len < pin.packet_len) {
+ ds_put_format(string, " buffer=0x%08"PRIx32, buffer_id);
+ if (total_len < pin.len) {
ds_put_format(string, " (***total_len < data_len***)");
}
}
ds_put_char(string, '\n');
if (verbosity > 0) {
- char *packet = ofp_packet_to_string(pin.packet, pin.packet_len);
+ char *packet = ofp_packet_to_string(pin.packet, pin.len);
ds_put_cstr(string, packet);
free(packet);
}
if (verbosity > 2) {
- ds_put_hex_dump(string, pin.packet, pin.packet_len, 0, false);
+ ds_put_hex_dump(string, pin.packet, pin.len, 0, false);
}
}
@@ -2091,7 +2094,9 @@ ofp_print_set_async_config(struct ds *string, const
struct ofp_header *oh,
reason = ofp_async_config_reason_to_string(
j, type, reasonbuf, sizeof reasonbuf);
- ds_put_format(string, " %s", reason);
+ if (reason[0]) {
+ ds_put_format(string, " %s", reason);
+ }
}
}
if (!role) {
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 347cb61..48e2e8e 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -41,6 +41,7 @@
#include "ofpbuf.h"
#include "openflow/netronome-ext.h"
#include "packets.h"
+#include "pktbuf.h"
#include "random.h"
#include "tun-metadata.h"
#include "unaligned.h"
@@ -3311,9 +3312,18 @@ ofputil_encode_flow_removed(const struct
ofputil_flow_removed *fr,
return msg;
}
+/* Decodes the packet-in message starting at 'oh' into '*pin'. Populates
+ * 'pin->packet' and 'pin->len' with the part of the packet actually included
+ * in the message, and '*total_len' with the original length of the packet
+ * (which is larger than 'packet->len' if only part of the packet was
+ * included). Stores the packet's buffer ID in '*buffer_id' (UINT32_MAX if it
+ * was not buffered).
+ *
+ * Returns 0 if successful, otherwise an OpenFlow error code. */
enum ofperr
-ofputil_decode_packet_in(struct ofputil_packet_in *pin,
- const struct ofp_header *oh)
+ofputil_decode_packet_in(const struct ofp_header *oh,
+ struct ofputil_packet_in *pin,
+ size_t *total_len, uint32_t *buffer_id)
{
enum ofpraw raw;
struct ofpbuf b;
@@ -3339,28 +3349,26 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
pin->reason = opi->reason;
pin->table_id = opi->table_id;
- pin->buffer_id = ntohl(opi->buffer_id);
- pin->total_len = ntohs(opi->total_len);
-
- if (cookie) {
- pin->cookie = *cookie;
- }
+ *buffer_id = ntohl(opi->buffer_id);
+ *total_len = ntohs(opi->total_len);
+ pin->cookie = cookie ? *cookie : OVS_BE64_MAX;
pin->packet = b.data;
- pin->packet_len = b.size;
+ pin->len = b.size;
} else if (raw == OFPRAW_OFPT10_PACKET_IN) {
const struct ofp10_packet_in *opi;
opi = ofpbuf_pull(&b, offsetof(struct ofp10_packet_in, data));
pin->packet = opi->data;
- pin->packet_len = b.size;
+ pin->len = b.size;
match_init_catchall(&pin->flow_metadata);
- match_set_in_port(&pin->flow_metadata,
u16_to_ofp(ntohs(opi->in_port)));
+ match_set_in_port(&pin->flow_metadata,
+ u16_to_ofp(ntohs(opi->in_port)));
pin->reason = opi->reason;
- pin->buffer_id = ntohl(opi->buffer_id);
- pin->total_len = ntohs(opi->total_len);
+ *buffer_id = ntohl(opi->buffer_id);
+ *total_len = ntohs(opi->total_len);
} else if (raw == OFPRAW_OFPT11_PACKET_IN) {
const struct ofp11_packet_in *opi;
ofp_port_t in_port;
@@ -3369,16 +3377,16 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
opi = ofpbuf_pull(&b, sizeof *opi);
pin->packet = b.data;
- pin->packet_len = b.size;
+ pin->len = b.size;
- pin->buffer_id = ntohl(opi->buffer_id);
+ *buffer_id = ntohl(opi->buffer_id);
error = ofputil_port_from_ofp11(opi->in_port, &in_port);
if (error) {
return error;
}
match_init_catchall(&pin->flow_metadata);
match_set_in_port(&pin->flow_metadata, in_port);
- pin->total_len = ntohs(opi->total_len);
+ *total_len = ntohs(opi->total_len);
pin->reason = opi->reason;
pin->table_id = opi->table_id;
} else if (raw == OFPRAW_NXT_PACKET_IN) {
@@ -3400,11 +3408,11 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
pin->table_id = npi->table_id;
pin->cookie = npi->cookie;
- pin->buffer_id = ntohl(npi->buffer_id);
- pin->total_len = ntohs(npi->total_len);
+ *buffer_id = ntohl(npi->buffer_id);
+ *total_len = ntohs(npi->total_len);
pin->packet = b.data;
- pin->packet_len = b.size;
+ pin->len = b.size;
} else {
OVS_NOT_REACHED();
}
@@ -3412,77 +3420,103 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
return 0;
}
+static int
+encode_packet_in_reason(enum ofp_packet_in_reason reason,
+ enum ofp_version version)
+{
+ switch (reason) {
+ case OFPR_NO_MATCH:
+ case OFPR_ACTION:
+ case OFPR_INVALID_TTL:
+ return reason;
+
+ case OFPR_ACTION_SET:
+ case OFPR_GROUP:
+ case OFPR_PACKET_OUT:
+ return version < OFP14_VERSION ? OFPR_ACTION : reason;
+
+ case OFPR_EXPLICIT_MISS:
+ return version < OFP13_VERSION ? OFPR_ACTION : OFPR_NO_MATCH;
+
+ case OFPR_IMPLICIT_MISS:
+ return OFPR_NO_MATCH;
+
+ case OFPR_N_REASONS:
+ default:
+ OVS_NOT_REACHED();
+ }
+}
+
static struct ofpbuf *
-ofputil_encode_ofp10_packet_in(const struct ofputil_packet_in *pin)
+ofputil_encode_ofp10_packet_in(const struct ofputil_packet_in *pin,
+ uint32_t buffer_id)
{
struct ofp10_packet_in *opi;
- struct ofpbuf *packet;
+ struct ofpbuf *msg;
- packet = ofpraw_alloc_xid(OFPRAW_OFPT10_PACKET_IN, OFP10_VERSION,
- htonl(0), pin->packet_len);
- opi = ofpbuf_put_zeros(packet, offsetof(struct ofp10_packet_in, data));
- opi->total_len = htons(pin->total_len);
+ msg = ofpraw_alloc_xid(OFPRAW_OFPT10_PACKET_IN, OFP10_VERSION,
+ htonl(0), pin->len);
+ opi = ofpbuf_put_zeros(msg, offsetof(struct ofp10_packet_in, data));
+ opi->total_len = htons(pin->len);
opi->in_port = htons(ofp_to_u16(pin->flow_metadata.flow.in_port.ofp_port));
- opi->reason = pin->reason;
- opi->buffer_id = htonl(pin->buffer_id);
-
- ofpbuf_put(packet, pin->packet, pin->packet_len);
+ opi->reason = encode_packet_in_reason(pin->reason, OFP10_VERSION);
+ opi->buffer_id = htonl(buffer_id);
- return packet;
+ return msg;
}
static struct ofpbuf *
-ofputil_encode_nx_packet_in(const struct ofputil_packet_in *pin)
+ofputil_encode_nx_packet_in(const struct ofputil_packet_in *pin,
+ uint32_t buffer_id)
{
struct nx_packet_in *npi;
- struct ofpbuf *packet;
+ struct ofpbuf *msg;
size_t match_len;
/* The final argument is just an estimate of the space required. */
- packet = ofpraw_alloc_xid(OFPRAW_NXT_PACKET_IN, OFP10_VERSION,
- htonl(0), NXM_TYPICAL_LEN + 2 + pin->packet_len);
- ofpbuf_put_zeros(packet, sizeof *npi);
- match_len = nx_put_match(packet, &pin->flow_metadata, 0, 0);
- ofpbuf_put_zeros(packet, 2);
- ofpbuf_put(packet, pin->packet, pin->packet_len);
-
- npi = packet->msg;
- npi->buffer_id = htonl(pin->buffer_id);
- npi->total_len = htons(pin->total_len);
- npi->reason = pin->reason;
+ msg = ofpraw_alloc_xid(OFPRAW_NXT_PACKET_IN, OFP10_VERSION,
+ htonl(0), NXM_TYPICAL_LEN + 2 + pin->len);
+ ofpbuf_put_zeros(msg, sizeof *npi);
+ match_len = nx_put_match(msg, &pin->flow_metadata, 0, 0);
+ ofpbuf_put_zeros(msg, 2);
+
+ npi = msg->msg;
+ npi->buffer_id = htonl(buffer_id);
+ npi->total_len = htons(pin->len);
+ npi->reason = encode_packet_in_reason(pin->reason, OFP10_VERSION);
npi->table_id = pin->table_id;
npi->cookie = pin->cookie;
npi->match_len = htons(match_len);
- return packet;
+ return msg;
}
static struct ofpbuf *
-ofputil_encode_ofp11_packet_in(const struct ofputil_packet_in *pin)
+ofputil_encode_ofp11_packet_in(const struct ofputil_packet_in *pin,
+ uint32_t buffer_id)
{
struct ofp11_packet_in *opi;
- struct ofpbuf *packet;
+ struct ofpbuf *msg;
- packet = ofpraw_alloc_xid(OFPRAW_OFPT11_PACKET_IN, OFP11_VERSION,
- htonl(0), pin->packet_len);
- opi = ofpbuf_put_zeros(packet, sizeof *opi);
- opi->buffer_id = htonl(pin->buffer_id);
- opi->in_port =
ofputil_port_to_ofp11(pin->flow_metadata.flow.in_port.ofp_port);
+ msg = ofpraw_alloc_xid(OFPRAW_OFPT11_PACKET_IN, OFP11_VERSION,
+ htonl(0), pin->len);
+ opi = ofpbuf_put_zeros(msg, sizeof *opi);
+ opi->buffer_id = htonl(buffer_id);
+ opi->in_port = ofputil_port_to_ofp11(
+ pin->flow_metadata.flow.in_port.ofp_port);
opi->in_phy_port = opi->in_port;
- opi->total_len = htons(pin->total_len);
- opi->reason = pin->reason;
+ opi->total_len = htons(pin->len);
+ opi->reason = encode_packet_in_reason(pin->reason, OFP11_VERSION);
opi->table_id = pin->table_id;
- ofpbuf_put(packet, pin->packet, pin->packet_len);
-
- return packet;
+ return msg;
}
static struct ofpbuf *
ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin,
- enum ofputil_protocol protocol)
+ enum ofp_version version,
+ uint32_t buffer_id)
{
- enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
enum ofpraw raw = (version >= OFP13_VERSION
? OFPRAW_OFPT13_PACKET_IN
: OFPRAW_OFPT12_PACKET_IN);
@@ -3490,13 +3524,14 @@ ofputil_encode_ofp12_packet_in(const struct
ofputil_packet_in *pin,
/* The final argument is just an estimate of the space required. */
msg = ofpraw_alloc_xid(raw, version,
- htonl(0), NXM_TYPICAL_LEN + 2 + pin->packet_len);
+ htonl(0), NXM_TYPICAL_LEN + 2 + pin->len);
struct ofp12_packet_in *opi = ofpbuf_put_zeros(msg, sizeof *opi);
- opi->buffer_id = htonl(pin->buffer_id);
- opi->total_len = htons(pin->total_len);
- opi->reason = pin->reason;
+ opi->buffer_id = htonl(buffer_id);
+ opi->total_len = htons(pin->len);
+ opi->reason = encode_packet_in_reason(pin->reason, version);
opi->table_id = pin->table_id;
+
if (version >= OFP13_VERSION) {
ovs_be64 cookie = pin->cookie;
ofpbuf_put(msg, &cookie, sizeof cookie);
@@ -3504,47 +3539,70 @@ ofputil_encode_ofp12_packet_in(const struct
ofputil_packet_in *pin,
oxm_put_match(msg, &pin->flow_metadata, version);
ofpbuf_put_zeros(msg, 2);
- ofpbuf_put(msg, pin->packet, pin->packet_len);
return msg;
}
-/* Converts abstract ofputil_packet_in 'pin' into a PACKET_IN message
- * in the format specified by 'packet_in_format'. */
+/* Converts abstract ofputil_packet_in 'pin' into a PACKET_IN message for
+ * 'protocol', using the packet-in format specified by 'packet_in_format'.
+ *
+ * If 'pkt_buf' is nonnull and 'max_len' allows the packet to be buffered, this
+ * function will attempt to obtain a buffer ID from 'pktbuf' and truncate the
+ * packet to 'max_len' bytes. Otherwise, or if 'pktbuf' doesn't have a free
+ * buffer, it will send the whole packet without buffering. */
struct ofpbuf *
ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
enum ofputil_protocol protocol,
- enum nx_packet_in_format packet_in_format)
+ enum nx_packet_in_format packet_in_format,
+ uint16_t max_len, struct pktbuf *pktbuf)
{
- struct ofpbuf *packet;
+ enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
+
+ /* Get buffer ID. */
+ ofp_port_t in_port = pin->flow_metadata.flow.in_port.ofp_port;
+ uint32_t buffer_id = (max_len != OFPCML12_NO_BUFFER && pktbuf
+ ? pktbuf_save(pktbuf, pin->packet,
+ pin->len, in_port)
+ : UINT32_MAX);
+ struct ofpbuf *msg;
switch (protocol) {
case OFPUTIL_P_OF10_STD:
case OFPUTIL_P_OF10_STD_TID:
case OFPUTIL_P_OF10_NXM:
case OFPUTIL_P_OF10_NXM_TID:
- packet = (packet_in_format == NXPIF_NXM
- ? ofputil_encode_nx_packet_in(pin)
- : ofputil_encode_ofp10_packet_in(pin));
+ msg = (packet_in_format == NXPIF_NXM
+ ? ofputil_encode_nx_packet_in(pin, buffer_id)
+ : ofputil_encode_ofp10_packet_in(pin, buffer_id));
break;
case OFPUTIL_P_OF11_STD:
- packet = ofputil_encode_ofp11_packet_in(pin);
+ msg = ofputil_encode_ofp11_packet_in(pin, buffer_id);
break;
case OFPUTIL_P_OF12_OXM:
case OFPUTIL_P_OF13_OXM:
case OFPUTIL_P_OF14_OXM:
case OFPUTIL_P_OF15_OXM:
- packet = ofputil_encode_ofp12_packet_in(pin, protocol);
+ msg = ofputil_encode_ofp12_packet_in(pin, version, buffer_id);
break;
default:
OVS_NOT_REACHED();
}
- ofpmsg_update_length(packet);
- return packet;
+ /* Append some of the packet:
+ *
+ * - If not buffered, the whole thing.
+ *
+ * - Otherwise, no more than 'max_len' bytes. */
+ ofpbuf_put(msg, pin->packet,
+ (buffer_id == UINT32_MAX
+ ? pin->len
+ : MIN(max_len, pin->len)));
+
+ ofpmsg_update_length(msg);
+ return msg;
}
/* Returns a string form of 'reason'. The return value is either a statically
@@ -3567,6 +3625,9 @@ ofputil_packet_in_reason_to_string(enum
ofp_packet_in_reason reason,
return "group";
case OFPR_PACKET_OUT:
return "packet_out";
+ case OFPR_EXPLICIT_MISS:
+ case OFPR_IMPLICIT_MISS:
+ return "";
case OFPR_N_REASONS:
default:
@@ -9496,6 +9557,15 @@ decode_async_mask(ovs_be32 src,
}
}
+ if (ap->oam == OAM_PACKET_IN) {
+ if (mask & (1u << OFPR_NO_MATCH)) {
+ mask |= 1u << OFPR_EXPLICIT_MISS;
+ if (version < OFP13_VERSION) {
+ mask |= 1u << OFPR_IMPLICIT_MISS;
+ }
+ }
+ }
+
uint32_t *array = ap->master ? dst->master : dst->slave;
array[ap->oam] = mask;
return 0;
@@ -9723,10 +9793,19 @@ ofputil_encode_set_async_config(const struct
ofputil_async_cfg *ac,
struct ofputil_async_cfg
ofputil_async_cfg_default(enum ofp_version version)
{
+ /* We enable all of the OF1.4 reasons regardless of 'version' because the
+ * reasons added in OF1.4 just are just refinements of the OFPR_ACTION
+ * introduced in OF1.0, breaking it into more specific categories. When we
+ * encode these for earlier OpenFlow versions, we translate them into
+ * OFPR_ACTION. */
+ uint32_t pin = OFPR14_BITS & ~(1u << OFPR_INVALID_TTL);
+ pin |= 1u << OFPR_EXPLICIT_MISS;
+ if (version <= OFP12_VERSION) {
+ pin |= 1u << OFPR_IMPLICIT_MISS;
+ }
+
return (struct ofputil_async_cfg) {
- .master[OAM_PACKET_IN]
- = ((version >= OFP14_VERSION ? OFPR14_BITS : OFPR10_BITS)
- & ~(1u << OFPR_INVALID_TTL)),
+ .master[OAM_PACKET_IN] = pin,
.master[OAM_FLOW_REMOVED]
= (version >= OFP14_VERSION ? OFPRR14_BITS : OFPRR10_BITS),
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 1c359b3..cf77d8a 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -35,6 +35,7 @@
struct ofpbuf;
union ofp_action;
struct ofpact_set_field;
+struct pktbuf;
/* Port numbers. */
enum ofperr ofputil_port_from_ofp11(ovs_be32 ofp11_port,
@@ -411,25 +412,25 @@ enum ofperr ofputil_decode_flow_removed(struct
ofputil_flow_removed *,
struct ofpbuf *ofputil_encode_flow_removed(const struct ofputil_flow_removed *,
enum ofputil_protocol);
-/* Abstract packet-in message. */
+/* Abstract packet-in message.
+ *
+ * This omits the 'total_len' and 'buffer_id' fields, which we handle
+ * differently for encoding and decoding.*/
struct ofputil_packet_in {
/* Packet data and metadata.
*
- * To save bandwidth, in some cases a switch may send only the first
- * several bytes of a packet, indicated by 'packet_len < total_len'. When
- * the full packet is included, 'packet_len == total_len'. */
- const void *packet;
- size_t packet_len; /* Number of bytes in 'packet'. */
- size_t total_len; /* Size of packet, pre-truncation. */
- struct match flow_metadata;
-
- /* Identifies a buffer in the switch that contains the full packet, to
- * allow the controller to reference it later without having to send the
- * entire packet back to the switch.
+ * On encoding, the full packet should be supplied, but depending on its
+ * other parameters ofputil_encode_packet_in() might send only the first
+ * part of the packet.
*
- * UINT32_MAX indicates that the packet is not buffered in the switch. A
- * switch should only use UINT32_MAX when it sends the entire packet. */
- uint32_t buffer_id;
+ * On decoding, the 'len' bytes in 'packet' might only be the first part of
+ * the original packet. ofputil_decode_packet_in() reports the full
+ * original length of the packet using its 'total_len' output parameter. */
+ const void *packet; /* The packet. */
+ size_t len; /* Length of 'packet' in bytes. */
+
+ /* Input port and other metadata for packet. */
+ struct match flow_metadata;
/* Reason that the packet-in is being sent. */
enum ofp_packet_in_reason reason; /* One of OFPR_*. */
@@ -442,11 +443,14 @@ struct ofputil_packet_in {
ovs_be64 cookie; /* Flow's cookie. */
};
-enum ofperr ofputil_decode_packet_in(struct ofputil_packet_in *,
- const struct ofp_header *);
struct ofpbuf *ofputil_encode_packet_in(const struct ofputil_packet_in *,
enum ofputil_protocol protocol,
- enum nx_packet_in_format);
+ enum nx_packet_in_format,
+ uint16_t max_len, struct pktbuf *);
+
+enum ofperr ofputil_decode_packet_in(const struct ofp_header *,
+ struct ofputil_packet_in *,
+ size_t *total_len, uint32_t *buffer_id);
enum { OFPUTIL_PACKET_IN_REASON_BUFSIZE = INT_STRLEN(int) + 1 };
const char *ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason,
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index c3e486c..fb8f251 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1483,33 +1483,6 @@ ofconn_receives_async_msg(const struct ofconn *ofconn,
/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the
* packet rather than to send the packet to the controller.
*
- * This function returns false to indicate the packet should be dropped if
- * the controller action was the result of the default table-miss behaviour
- * and the controller is using OpenFlow1.3+.
- *
- * Otherwise true is returned to indicate the packet should be forwarded to
- * the controller */
-static bool
-ofconn_wants_packet_in_on_miss(struct ofconn *ofconn,
- const struct ofproto_packet_in *pin)
-{
- if (pin->miss_type == OFPROTO_PACKET_IN_MISS_WITHOUT_FLOW) {
- enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
-
- if (protocol != OFPUTIL_P_NONE
- && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION
- && (ofproto_table_get_miss_config(ofconn->connmgr->ofproto,
- pin->up.table_id)
- == OFPUTIL_TABLE_MISS_DEFAULT)) {
- return false;
- }
- }
- return true;
-}
-
-/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the
- * packet rather than to send the packet to the controller.
- *
* This function returns true to indicate that a packet_in message
* for a "table-miss" should be sent to at least one controller.
* That is there is at least one controller with controller_id 0
@@ -1582,9 +1555,6 @@ ofconn_send(const struct ofconn *ofconn, struct ofpbuf
*msg,
/* Sending asynchronous messages. */
-static void schedule_packet_in(struct ofconn *, struct ofproto_packet_in,
- enum ofp_packet_in_reason wire_reason);
-
/* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to appropriate
* controllers managed by 'mgr'. For messages caused by a controller
* OFPT_PORT_MOD, specify 'source' as the controller connection that sent the
@@ -1678,41 +1648,6 @@ connmgr_send_flow_removed(struct connmgr *mgr,
}
}
-/* Normally a send-to-controller action uses reason OFPR_ACTION. However, in
- * OpenFlow 1.3 and later, packet_ins generated by a send-to-controller action
- * in a "table-miss" flow (one with priority 0 and completely wildcarded) are
- * sent as OFPR_NO_MATCH. This function returns the reason that should
- * actually be sent on 'ofconn' for 'pin'. */
-static enum ofp_packet_in_reason
-wire_reason(struct ofconn *ofconn, const struct ofproto_packet_in *pin)
-{
- enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
-
- if (pin->miss_type == OFPROTO_PACKET_IN_MISS_FLOW
- && pin->up.reason == OFPR_ACTION
- && protocol != OFPUTIL_P_NONE
- && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION) {
- return OFPR_NO_MATCH;
- }
-
- switch (pin->up.reason) {
- case OFPR_ACTION_SET:
- case OFPR_GROUP:
- case OFPR_PACKET_OUT:
- if (!(protocol & OFPUTIL_P_OF14_UP)) {
- /* Only supported in OF1.4+ */
- return OFPR_ACTION;
- }
- /* Fall through. */
- case OFPR_NO_MATCH:
- case OFPR_ACTION:
- case OFPR_INVALID_TTL:
- case OFPR_N_REASONS:
- default:
- return pin->up.reason;
- }
-}
-
/* Given 'pin', sends an OFPT_PACKET_IN message to each OpenFlow controller as
* necessary according to their individual configurations.
*
@@ -1724,13 +1659,27 @@ connmgr_send_packet_in(struct connmgr *mgr,
struct ofconn *ofconn;
LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
- enum ofp_packet_in_reason reason = wire_reason(ofconn, pin);
-
- if (ofconn_wants_packet_in_on_miss(ofconn, pin)
- && ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, reason)
- && ofconn->controller_id == pin->controller_id) {
- schedule_packet_in(ofconn, *pin, reason);
+ enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
+ if (protocol == OFPUTIL_P_NONE || !rconn_is_connected(ofconn->rconn)
+ || ofconn->controller_id != pin->controller_id
+ || !ofconn_receives_async_msg(ofconn, OAM_PACKET_IN,
+ pin->up.reason)) {
+ continue;
}
+
+ struct ofpbuf *msg = ofputil_encode_packet_in(
+ &pin->up, protocol, ofconn->packet_in_format,
+ pin->max_len >= 0 ? pin->max_len : ofconn->miss_send_len,
+ ofconn->pktbuf);
+
+ struct ovs_list txq;
+ bool is_miss = (pin->up.reason == OFPR_NO_MATCH ||
+ pin->up.reason == OFPR_EXPLICIT_MISS ||
+ pin->up.reason == OFPR_IMPLICIT_MISS);
+ pinsched_send(ofconn->schedulers[is_miss],
+ pin->up.flow_metadata.flow.in_port.ofp_port /* XXX */,
+ msg, &txq);
+ do_send_packet_ins(ofconn, &txq);
}
}
@@ -1749,55 +1698,6 @@ do_send_packet_ins(struct ofconn *ofconn, struct
ovs_list *txq)
}
}
}
-
-/* Takes 'pin', composes an OpenFlow packet-in message from it, and passes it
- * to 'ofconn''s packet scheduler for sending. */
-static void
-schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin,
- enum ofp_packet_in_reason wire_reason)
-{
- uint16_t controller_max_len;
- struct ovs_list txq;
-
- pin.up.total_len = pin.up.packet_len;
-
- pin.up.reason = wire_reason;
- if (pin.up.reason == OFPR_ACTION) {
- controller_max_len = pin.send_len; /* max_len */
- } else {
- controller_max_len = ofconn->miss_send_len;
- }
-
- /* Get OpenFlow buffer_id.
- * For OpenFlow 1.2+, OFPCML_NO_BUFFER (== UINT16_MAX) specifies
- * unbuffered. This behaviour doesn't violate prior versions, too. */
- if (controller_max_len == UINT16_MAX) {
- pin.up.buffer_id = UINT32_MAX;
- } else if (!ofconn->pktbuf) {
- pin.up.buffer_id = UINT32_MAX;
- } else {
- pin.up.buffer_id = pktbuf_save(ofconn->pktbuf,
- pin.up.packet, pin.up.packet_len,
-
pin.up.flow_metadata.flow.in_port.ofp_port);
- }
-
- /* Figure out how much of the packet to send.
- * If not buffered, send the entire packet. Otherwise, depending on
- * the reason of packet-in, send what requested by the controller. */
- if (pin.up.buffer_id != UINT32_MAX
- && controller_max_len < pin.up.packet_len) {
- pin.up.packet_len = controller_max_len;
- }
-
- /* Make OFPT_PACKET_IN and hand over to packet scheduler. */
- pinsched_send(ofconn->schedulers[pin.up.reason == OFPR_NO_MATCH ? 0 : 1],
- pin.up.flow_metadata.flow.in_port.ofp_port,
- ofputil_encode_packet_in(&pin.up,
- ofconn_get_protocol(ofconn),
- ofconn->packet_in_format),
- &txq);
- do_send_packet_ins(ofconn, &txq);
-}
/* Fail-open settings. */
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index 98821bc..ced6a68 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -54,32 +54,12 @@ enum ofconn_type {
OFCONN_SERVICE /* A service connection, e.g. "ovs-ofctl". */
};
-enum ofproto_packet_in_miss_type {
- /* Not generated by a flow miss or table-miss flow. */
- OFPROTO_PACKET_IN_NO_MISS,
-
- /* The packet_in was generated directly by a table-miss flow, that is, a
- * flow with priority 0 that wildcards all fields. See OF1.3.3 section
- * 5.4.
- *
- * (Our interpretation of "directly" is "not via groups". Packet_ins
- * generated by table-miss flows via groups use
- * OFPROTO_PACKET_IN_NO_MISS.) */
- OFPROTO_PACKET_IN_MISS_FLOW,
-
- /* The packet-in was generated directly by a table-miss, but not a
- * table-miss flow. That is, it was generated by the OpenFlow 1.0, 1.1, or
- * 1.2 table-miss behavior. */
- OFPROTO_PACKET_IN_MISS_WITHOUT_FLOW,
-};
-
/* A packet_in, with extra members to assist in queuing and routing it. */
struct ofproto_packet_in {
struct ofputil_packet_in up;
struct ovs_list list_node; /* For queuing. */
uint16_t controller_id; /* Controller ID to send to. */
- int send_len; /* Length that the action requested sending. */
- enum ofproto_packet_in_miss_type miss_type;
+ int max_len; /* From action, or -1 if none. */
};
/* Basics. */
diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c
index 3c9a9ad..5a692bf 100644
--- a/ofproto/fail-open.c
+++ b/ofproto/fail-open.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -118,7 +118,6 @@ fail_open_is_active(const struct fail_open *fo)
static void
send_bogus_packet_ins(struct fail_open *fo)
{
- struct ofproto_packet_in pin;
struct eth_addr mac;
struct dp_packet b;
@@ -126,14 +125,18 @@ send_bogus_packet_ins(struct fail_open *fo)
eth_addr_nicira_random(&mac);
compose_rarp(&b, mac);
- memset(&pin, 0, sizeof pin);
- pin.up.packet = dp_packet_data(&b);
- pin.up.packet_len = dp_packet_size(&b);
- pin.up.reason = OFPR_NO_MATCH;
- match_init_catchall(&pin.up.flow_metadata);
- match_set_in_port(&pin.up.flow_metadata, OFPP_LOCAL);
- pin.send_len = dp_packet_size(&b);
- pin.miss_type = OFPROTO_PACKET_IN_NO_MISS;
+ struct ofproto_packet_in pin = {
+ .up = {
+ .packet = dp_packet_data(&b),
+ .len = dp_packet_size(&b),
+ .flow_metadata = MATCH_CATCHALL_INITIALIZER,
+ .flow_metadata.flow.in_port.ofp_port = OFPP_LOCAL,
+ .flow_metadata.wc.masks.in_port.ofp_port = u16_to_ofp(UINT16_MAX),
+ .reason = OFPR_NO_MATCH,
+ .cookie = OVS_BE64_MAX,
+ },
+ .max_len = UINT16_MAX,
+ };
connmgr_send_packet_in(fo->connmgr, &pin);
dp_packet_uninit(&b);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e2ca960..c377963 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -3582,33 +3582,30 @@ execute_controller_action(struct xlate_ctx *ctx, int
len,
odp_execute_actions(NULL, &packet, 1, false,
ctx->odp_actions->data, ctx->odp_actions->size, NULL);
- pin = xmalloc(sizeof *pin);
- pin->up.packet_len = dp_packet_size(packet);
- pin->up.packet = dp_packet_steal_data(packet);
- pin->up.reason = reason;
- pin->up.table_id = ctx->table_id;
- pin->up.cookie = ctx->rule_cookie;
+ /* A packet sent by an action in a table-miss rule is considered an
+ * explicit table miss. OpenFlow before 1.3 doesn't have that concept so
+ * it will get translated back to OFPR_ACTION for those versions. */
+ if (reason == OFPR_ACTION
+ && ctx->rule && rule_dpif_is_table_miss(ctx->rule)) {
+ reason = OFPR_EXPLICIT_MISS;
+ }
+
+ size_t packet_len = dp_packet_size(packet);
+ pin = xmalloc(sizeof *pin);
+ *pin = (struct ofproto_packet_in) {
+ .controller_id = controller_id,
+ .up = {
+ .packet = dp_packet_steal_data(packet),
+ .len = packet_len,
+ .reason = reason,
+ .table_id = ctx->table_id,
+ .cookie = ctx->rule_cookie,
+ },
+ .max_len = len,
+ };
flow_get_metadata(&ctx->xin->flow, &pin->up.flow_metadata);
- pin->controller_id = controller_id;
- pin->send_len = len;
- /* If a rule is a table-miss rule then this is
- * a table-miss handled by a table-miss rule.
- *
- * Else, if rule is internal and has a controller action,
- * the later being implied by the rule being processed here,
- * then this is a table-miss handled without a table-miss rule.
- *
- * Otherwise this is not a table-miss. */
- pin->miss_type = OFPROTO_PACKET_IN_NO_MISS;
- if (ctx->rule) {
- if (rule_dpif_is_table_miss(ctx->rule)) {
- pin->miss_type = OFPROTO_PACKET_IN_MISS_FLOW;
- } else if (rule_dpif_is_internal(ctx->rule)) {
- pin->miss_type = OFPROTO_PACKET_IN_MISS_WITHOUT_FLOW;
- }
- }
ofproto_dpif_send_packet_in(ctx->xbridge->ofproto, pin);
dp_packet_delete(packet);
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8186f6b..fbb392f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1406,7 +1406,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
controller = ofpact_put_CONTROLLER(&ofpacts);
controller->max_len = UINT16_MAX;
controller->controller_id = 0;
- controller->reason = OFPR_NO_MATCH;
+ controller->reason = OFPR_IMPLICIT_MISS;
error = add_internal_miss_flow(ofproto, id++, &ofpacts,
&ofproto->miss_rule);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index cbd9c47..3faf42a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3378,7 +3378,7 @@ handle_packet_out(struct ofconn *ofconn, const struct
ofp_header *oh)
/* Get payload. */
if (po.buffer_id != UINT32_MAX) {
error = ofconn_pktbuf_retrieve(ofconn, po.buffer_id, &payload, NULL);
- if (error || !payload) {
+ if (error) {
goto exit_free_ofpacts;
}
} else {
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 8c53c19..360f38b 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -78,8 +78,10 @@ process_packet_in(struct controller_ctx *ctx OVS_UNUSED,
const struct ofp_header *msg)
{
struct ofputil_packet_in pin;
+ uint32_t buffer_id;
+ size_t total_len;
- if (ofputil_decode_packet_in(&pin, msg) != 0) {
+ if (ofputil_decode_packet_in(msg, &pin, &total_len, &buffer_id) != 0) {
return;
}
if (pin.reason != OFPR_ACTION) {
--
2.1.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
