OVS implementation of buffering packets that are sent to the
controller is not compliant with the OpenFlow specifications after
OpenFlow 1.0, which is possibly true since OpenFlow 1.0 is not really
specifying the packet buffering behavior.
OVS implementation executes the buffered packet against the actions of
the modified or added rule, whereas OpenFlow (since 1.1) specifies
that the packet should be matched against the flow table 0 and
processed accordingly.
Rather than fix this behavior, and potentially break OVS users, we
propose to remove the feature altogether. After all, such packet
buffering is an optional OpenFlow feature, and as such any possible
users should continue to work without this feature.
This patch also makes OVS check the received 'buffer_id' values more
rigorously, and fixes some internal users accordingly.
Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
FAQ.md | 41 --------
include/openvswitch/ofp-util.h | 4 +-
lib/automake.mk | 2 -
lib/learn.c | 2 +-
lib/ofp-util.c | 31 +-----
lib/pktbuf.c | 220 -----------------------------------------
lib/pktbuf.h | 40 --------
ofproto/bundles.c | 1 -
ofproto/connmgr.c | 21 +---
ofproto/connmgr.h | 3 -
ofproto/fail-open.c | 1 -
ofproto/ofproto-dpif.c | 2 +
ofproto/ofproto-provider.h | 1 -
ofproto/ofproto.c | 75 +++-----------
ovn/controller/ofctrl.c | 4 +
tests/ofproto.at | 10 +-
tutorial/OVN-Tutorial.md | 4 +-
17 files changed, 35 insertions(+), 427 deletions(-)
delete mode 100644 lib/pktbuf.c
delete mode 100644 lib/pktbuf.h
diff --git a/FAQ.md b/FAQ.md
index f4fd55d..08f84ae 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -1970,47 +1970,6 @@ A: Reconfiguring your bridge can change your bridge's
datapath-id because
ovs-vsctl set bridge br0 other-config:datapath-id=0123456789abcdef
-### Q: My controller is getting errors about "buffers". What's going on?
-
-A: When a switch sends a packet to an OpenFlow controller using a
- "packet-in" message, it can also keep a copy of that packet in a
- "buffer", identified by a 32-bit integer "buffer_id". There are
- two advantages to buffering. First, when the controller wants to
- tell the switch to do something with the buffered packet (with a
- "packet-out" OpenFlow request), it does not need to send another
- copy of the packet back across the OpenFlow connection, which
- reduces the bandwidth cost of the connection and improves latency.
- This enables the second advantage: the switch can optionally send
- only the first part of the packet to the controller (assuming that
- the switch only needs to look at the first few bytes of the
- packet), further reducing bandwidth and improving latency.
-
- However, buffering introduces some issues of its own. First, any
- switch has limited resources, so if the controller does not use a
- buffered packet, the switch has to decide how long to keep it
- buffered. When many packets are sent to a controller and buffered,
- Open vSwitch can discard buffered packets that the controller has
- not used after as little as 5 seconds. This means that
- controllers, if they make use of packet buffering, should use the
- buffered packets promptly. (This includes sending a "packet-out"
- with no actions if the controller does not want to do anything with
- a buffered packet, to clear the packet buffer and effectively
- "drop" its packet.)
-
- Second, packet buffers are one-time-use, meaning that a controller
- cannot use a single packet buffer in two or more "packet-out"
- commands. Open vSwitch will respond with an error to the second
- and subsequent "packet-out"s in such a case.
-
- Finally, a common error early in controller development is to try
- to use buffer_id 0 in a "packet-out" message as if 0 represented
- "no buffered packet". This is incorrect usage: the buffer_id with
- this meaning is actually 0xffffffff.
-
- ovs-vswitchd(8) describes some details of Open vSwitch packet
- buffering that the OpenFlow specification requires implementations
- to document.
-
### Q: How does OVS divide flows among buckets in an OpenFlow "select" group?
A: In Open vSwitch 2.3 and earlier, Open vSwitch used the destination
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 177bf2b..450e739 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -36,7 +36,6 @@
struct ofpbuf;
union ofp_action;
struct ofpact_set_field;
-struct pktbuf;
/* Port numbers. */
enum ofperr ofputil_port_from_ofp11(ovs_be32 ofp11_port,
@@ -498,8 +497,7 @@ struct ofputil_packet_in_private {
struct ofpbuf *ofputil_encode_packet_in_private(
const struct ofputil_packet_in_private *,
enum ofputil_protocol protocol,
- enum nx_packet_in_format,
- uint16_t max_len, struct pktbuf *);
+ enum nx_packet_in_format);
enum ofperr ofputil_decode_packet_in_private(
const struct ofp_header *, bool loose,
diff --git a/lib/automake.mk b/lib/automake.mk
index 165e6a8..b00e90f 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -201,8 +201,6 @@ lib_libopenvswitch_la_SOURCES = \
lib/pcap-file.h \
lib/perf-counter.h \
lib/perf-counter.c \
- lib/pktbuf.c \
- lib/pktbuf.h \
lib/poll-loop.c \
lib/poll-loop.h \
lib/process.c \
diff --git a/lib/learn.c b/lib/learn.c
index a8469fa..d9cef93 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -93,6 +93,7 @@ learn_execute(const struct ofpact_learn *learn, const struct
flow *flow,
const struct ofpact_learn_spec *spec;
match_init_catchall(&fm->match);
+ fm->buffer_id = UINT32_MAX;
fm->priority = learn->priority;
fm->cookie = htonll(0);
fm->cookie_mask = htonll(0);
@@ -103,7 +104,6 @@ learn_execute(const struct ofpact_learn *learn, const
struct flow *flow,
fm->idle_timeout = learn->idle_timeout;
fm->hard_timeout = learn->hard_timeout;
fm->importance = 0;
- fm->buffer_id = UINT32_MAX;
fm->out_port = OFPP_NONE;
fm->flags = 0;
if (learn->flags & NX_LEARN_F_SEND_FLOW_REM) {
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index ccb06fe..a9087a5 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -45,7 +45,6 @@
#include "openvswitch/vlog.h"
#include "openflow/intel-ext.h"
#include "packets.h"
-#include "pktbuf.h"
#include "random.h"
#include "tun-metadata.h"
#include "unaligned.h"
@@ -3857,11 +3856,6 @@ ofputil_encode_ofp12_packet_in(const struct
ofputil_packet_in *pin,
/* Converts abstract ofputil_packet_in_private '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.
- *
* This function is really meant only for use by ovs-vswitchd. To any other
* code, the "continuation" data, i.e. the data that is in struct
* ofputil_packet_in_private but not in struct ofputil_packet_in, is supposed
@@ -3873,27 +3867,10 @@ ofputil_encode_ofp12_packet_in(const struct
ofputil_packet_in *pin,
struct ofpbuf *
ofputil_encode_packet_in_private(const struct ofputil_packet_in_private *pin,
enum ofputil_protocol protocol,
- enum nx_packet_in_format packet_in_format,
- uint16_t max_len, struct pktbuf *pktbuf)
+ enum nx_packet_in_format packet_in_format)
{
enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
-
- /* Get buffer ID. */
- ofp_port_t in_port = pin->public.flow_metadata.flow.in_port.ofp_port;
- uint32_t buffer_id = (max_len != OFPCML12_NO_BUFFER && pktbuf
- ? pktbuf_save(pktbuf, pin->public.packet,
- pin->public.packet_len, in_port)
- : UINT32_MAX);
-
- /* Calculate the number of bytes of the packet to include in the
- * packet-in:
- *
- * - If not buffered, the whole thing.
- *
- * - Otherwise, no more than 'max_len' bytes. */
- size_t include_bytes = (buffer_id == UINT32_MAX
- ? pin->public.packet_len
- : MIN(max_len, pin->public.packet_len));
+ uint32_t buffer_id = UINT32_MAX;
struct ofpbuf *msg;
switch (packet_in_format) {
@@ -3929,13 +3906,13 @@ ofputil_encode_packet_in_private(const struct
ofputil_packet_in_private *pin,
case NXPIF_NXT_PACKET_IN2:
return ofputil_encode_nx_packet_in2(pin, version, buffer_id,
- include_bytes);
+ pin->public.packet_len);
default:
OVS_NOT_REACHED();
}
- ofpbuf_put(msg, pin->public.packet, include_bytes);
+ ofpbuf_put(msg, pin->public.packet, pin->public.packet_len);
ofpmsg_update_length(msg);
return msg;
}
diff --git a/lib/pktbuf.c b/lib/pktbuf.c
deleted file mode 100644
index e2c61fd..0000000
--- a/lib/pktbuf.c
+++ /dev/null
@@ -1,220 +0,0 @@
-/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 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.
- * You may obtain a copy of the License at:
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <config.h>
-#include "pktbuf.h"
-#include <inttypes.h>
-#include <stdlib.h>
-#include "coverage.h"
-#include "openvswitch/ofp-util.h"
-#include "dp-packet.h"
-#include "timeval.h"
-#include "util.h"
-#include "openvswitch/vconn.h"
-#include "openvswitch/vlog.h"
-
-VLOG_DEFINE_THIS_MODULE(pktbuf);
-
-COVERAGE_DEFINE(pktbuf_buffer_unknown);
-COVERAGE_DEFINE(pktbuf_retrieved);
-COVERAGE_DEFINE(pktbuf_reuse_error);
-
-/* Buffers are identified by a 32-bit opaque ID. We divide the ID
- * into a buffer number (low bits) and a cookie (high bits). The buffer number
- * is an index into an array of buffers. The cookie distinguishes between
- * different packets that have occupied a single buffer. Thus, the more
- * buffers we have, the lower-quality the cookie... */
-#define PKTBUF_BITS 8
-#define PKTBUF_MASK (PKTBUF_CNT - 1)
-#define PKTBUF_CNT (1u << PKTBUF_BITS)
-
-#define COOKIE_BITS (32 - PKTBUF_BITS)
-#define COOKIE_MAX ((1u << COOKIE_BITS) - 1)
-
-#define OVERWRITE_MSECS 5000
-
-struct packet {
- struct dp_packet *buffer;
- uint32_t cookie;
- long long int timeout;
- ofp_port_t in_port;
-};
-
-struct pktbuf {
- struct packet packets[PKTBUF_CNT];
- unsigned int buffer_idx;
- unsigned int null_idx;
-};
-
-int
-pktbuf_capacity(void)
-{
- return PKTBUF_CNT;
-}
-
-struct pktbuf *
-pktbuf_create(void)
-{
- return xzalloc(sizeof *pktbuf_create());
-}
-
-void
-pktbuf_destroy(struct pktbuf *pb)
-{
- if (pb) {
- size_t i;
-
- for (i = 0; i < PKTBUF_CNT; i++) {
- dp_packet_delete(pb->packets[i].buffer);
- }
- free(pb);
- }
-}
-
-static unsigned int
-make_id(unsigned int buffer_idx, unsigned int cookie)
-{
- return buffer_idx | (cookie << PKTBUF_BITS);
-}
-
-/* Attempts to allocate an OpenFlow packet buffer id within 'pb'. The packet
- * buffer will store a copy of 'buffer_size' bytes in 'buffer' and the port
- * number 'in_port', which should be the OpenFlow port number on which 'buffer'
- * was received.
- *
- * If successful, returns the packet buffer id (a number other than
- * UINT32_MAX). pktbuf_retrieve() can later be used to retrieve the buffer and
- * its input port number (buffers do expire after a time, so this is not
- * guaranteed to be true forever). On failure, returns UINT32_MAX.
- *
- * The caller retains ownership of 'buffer'. */
-uint32_t
-pktbuf_save(struct pktbuf *pb, const void *buffer, size_t buffer_size,
- ofp_port_t in_port)
-{
- struct packet *p = &pb->packets[pb->buffer_idx];
- pb->buffer_idx = (pb->buffer_idx + 1) & PKTBUF_MASK;
- if (p->buffer) {
- if (time_msec() < p->timeout) {
- return UINT32_MAX;
- }
- dp_packet_delete(p->buffer);
- }
-
- /* Don't use maximum cookie value since all-1-bits ID is special. */
- if (++p->cookie >= COOKIE_MAX) {
- p->cookie = 0;
- }
-
- /* Use 2 bytes of headroom to 32-bit align the L3 header. */
- p->buffer = dp_packet_clone_data_with_headroom(buffer, buffer_size, 2);
-
- p->timeout = time_msec() + OVERWRITE_MSECS;
- p->in_port = in_port;
- return make_id(p - pb->packets, p->cookie);
-}
-
-/* Attempts to retrieve a saved packet with the given 'id' from 'pb'. Returns
- * 0 if successful, otherwise an OpenFlow error code.
- *
- * On success, stores the buffered packet in '*bufferp' and the OpenFlow port
- * number on which the packet was received in '*in_port'. The caller becomes
- * responsible for freeing the buffer.
- *
- * 'in_port' may be NULL if the input port is not of interest.
- *
- * The L3 header of a returned packet will be 32-bit aligned.
- *
- * On failure, stores NULL in in '*bufferp' and UINT16_MAX in '*in_port'. */
-enum ofperr
-pktbuf_retrieve(struct pktbuf *pb, uint32_t id, struct dp_packet **bufferp,
- ofp_port_t *in_port)
-{
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 20);
- struct packet *p;
- enum ofperr error;
-
- if (id == UINT32_MAX) {
- error = 0;
- goto error;
- }
-
- if (!pb) {
- VLOG_WARN_RL(&rl, "attempt to send buffered packet via connection "
- "without buffers");
- error = OFPERR_OFPBRC_BUFFER_UNKNOWN;
- goto error;
- }
-
- p = &pb->packets[id & PKTBUF_MASK];
- if (p->cookie == id >> PKTBUF_BITS) {
- struct dp_packet *buffer = p->buffer;
- if (buffer) {
- *bufferp = buffer;
- if (in_port) {
- *in_port = p->in_port;
- }
- p->buffer = NULL;
- COVERAGE_INC(pktbuf_retrieved);
- return 0;
- } else {
- COVERAGE_INC(pktbuf_reuse_error);
- VLOG_WARN_RL(&rl, "attempt to reuse buffer %08"PRIx32, id);
- error = OFPERR_OFPBRC_BUFFER_EMPTY;
- }
- } else {
- COVERAGE_INC(pktbuf_buffer_unknown);
- VLOG_WARN_RL(&rl, "cookie mismatch: %08"PRIx32" != %08"PRIx32,
- id, (id & PKTBUF_MASK) | (p->cookie << PKTBUF_BITS));
- error = OFPERR_OFPBRC_BUFFER_UNKNOWN;
- }
-error:
- *bufferp = NULL;
- if (in_port) {
- *in_port = OFPP_NONE;
- }
- return error;
-}
-
-void
-pktbuf_discard(struct pktbuf *pb, uint32_t id)
-{
- struct packet *p = &pb->packets[id & PKTBUF_MASK];
- if (p->cookie == id >> PKTBUF_BITS) {
- dp_packet_delete(p->buffer);
- p->buffer = NULL;
- }
-}
-
-/* Returns the number of packets buffered in 'pb'. Returns 0 if 'pb' is
- * null. */
-unsigned int
-pktbuf_count_packets(const struct pktbuf *pb)
-{
- int n = 0;
-
- if (pb) {
- int i;
-
- for (i = 0; i < PKTBUF_CNT; i++) {
- if (pb->packets[i].buffer) {
- n++;
- }
- }
- }
-
- return n;
-}
diff --git a/lib/pktbuf.h b/lib/pktbuf.h
deleted file mode 100644
index ccbaed7..0000000
--- a/lib/pktbuf.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * Copyright (c) 2008, 2009, 2011, 2012, 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.
- * You may obtain a copy of the License at:
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef PKTBUF_H
-#define PKTBUF_H 1
-
-#include <stddef.h>
-#include <stdint.h>
-
-#include "openvswitch/ofp-errors.h"
-
-struct pktbuf;
-struct dp_packet;
-
-int pktbuf_capacity(void);
-
-struct pktbuf *pktbuf_create(void);
-void pktbuf_destroy(struct pktbuf *);
-uint32_t pktbuf_save(struct pktbuf *, const void *buffer, size_t buffer_size,
- ofp_port_t in_port);
-enum ofperr pktbuf_retrieve(struct pktbuf *, uint32_t id,
- struct dp_packet **bufferp, ofp_port_t *in_port);
-void pktbuf_discard(struct pktbuf *, uint32_t id);
-
-unsigned int pktbuf_count_packets(const struct pktbuf *);
-
-#endif /* pktbuf.h */
diff --git a/ofproto/bundles.c b/ofproto/bundles.c
index e8fb7ba..353a3a7 100644
--- a/ofproto/bundles.c
+++ b/ofproto/bundles.c
@@ -32,7 +32,6 @@
#include "openvswitch/vlog.h"
#include "pinsched.h"
#include "poll-loop.h"
-#include "pktbuf.h"
#include "rconn.h"
#include "openvswitch/shash.h"
#include "simap.h"
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index d70d990..d2bedad 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -34,7 +34,6 @@
#include "openvswitch/vlog.h"
#include "pinsched.h"
#include "poll-loop.h"
-#include "pktbuf.h"
#include "rconn.h"
#include "openvswitch/shash.h"
#include "simap.h"
@@ -78,7 +77,6 @@ struct ofconn {
struct rconn_packet_counter *packet_in_counter; /* # queued on 'rconn'. */
#define N_SCHEDULERS 2
struct pinsched *schedulers[N_SCHEDULERS];
- struct pktbuf *pktbuf; /* OpenFlow packet buffers. */
int miss_send_len; /* Bytes to send of buffered packets. */
uint16_t controller_id; /* Connection controller ID. */
@@ -416,7 +414,6 @@ connmgr_get_memory_usage(const struct connmgr *mgr, struct
simap *usage)
pinsched_get_stats(ofconn->schedulers[i], &stats);
packets += stats.n_queued;
}
- packets += pktbuf_count_packets(ofconn->pktbuf);
}
simap_increase(usage, "ofconns", ofconns);
simap_increase(usage, "packets", packets);
@@ -694,7 +691,6 @@ add_controller(struct connmgr *mgr, const char *target,
uint8_t dscp,
ofconn = ofconn_create(mgr, rconn_create(5, 8, dscp, allowed_versions),
OFCONN_PRIMARY, true);
- ofconn->pktbuf = pktbuf_create();
rconn_connect(ofconn->rconn, target, name);
hmap_insert(&mgr->controllers, &ofconn->hmap_node, hash_string(target, 0));
@@ -1113,14 +1109,6 @@ ofconn_send_error(const struct ofconn *ofconn,
ofconn_send_reply(ofconn, reply);
}
-/* Same as pktbuf_retrieve(), using the pktbuf owned by 'ofconn'. */
-enum ofperr
-ofconn_pktbuf_retrieve(struct ofconn *ofconn, uint32_t id,
- struct dp_packet **bufferp, ofp_port_t *in_port)
-{
- return pktbuf_retrieve(ofconn->pktbuf, id, bufferp, in_port);
-}
-
/* Reports that a flow_mod operation of the type specified by 'command' was
* successfully executed by 'ofconn', so that the connmgr can log it. */
void
@@ -1261,10 +1249,6 @@ ofconn_flush(struct ofconn *ofconn)
ofconn->schedulers[i] = pinsched_create(rate, burst);
}
}
- if (ofconn->pktbuf) {
- pktbuf_destroy(ofconn->pktbuf);
- ofconn->pktbuf = pktbuf_create();
- }
ofconn->miss_send_len = (ofconn->type == OFCONN_PRIMARY
? OFP_DEFAULT_MISS_SEND_LEN
: 0);
@@ -1308,7 +1292,6 @@ ofconn_destroy(struct ofconn *ofconn)
rconn_destroy(ofconn->rconn);
rconn_packet_counter_destroy(ofconn->packet_in_counter);
rconn_packet_counter_destroy(ofconn->reply_counter);
- pktbuf_destroy(ofconn->pktbuf);
rconn_packet_counter_destroy(ofconn->monitor_counter);
free(ofconn);
}
@@ -1695,9 +1678,7 @@ connmgr_send_async_msg(struct connmgr *mgr,
}
struct ofpbuf *msg = ofputil_encode_packet_in_private(
- &am->pin.up, protocol, ofconn->packet_in_format,
- am->pin.max_len >= 0 ? am->pin.max_len : ofconn->miss_send_len,
- ofconn->pktbuf);
+ &am->pin.up, protocol, ofconn->packet_in_format);
struct ovs_list txq;
bool is_miss = (am->pin.up.public.reason == OFPR_NO_MATCH ||
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index be4ce28..0768aa0 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -128,9 +128,6 @@ void ofconn_send_replies(const struct ofconn *, struct
ovs_list *);
void ofconn_send_error(const struct ofconn *, const struct ofp_header *request,
enum ofperr);
-enum ofperr ofconn_pktbuf_retrieve(struct ofconn *, uint32_t id,
- struct dp_packet **bufferp, ofp_port_t
*in_port);
-
struct ofp_bundle;
struct ofp_bundle *ofconn_get_bundle(struct ofconn *, uint32_t id);
diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c
index d8ec213..3c3579e 100644
--- a/ofproto/fail-open.c
+++ b/ofproto/fail-open.c
@@ -31,7 +31,6 @@
#include "openvswitch/vlog.h"
#include "ofproto.h"
#include "ofproto-provider.h"
-#include "pktbuf.h"
#include "poll-loop.h"
#include "rconn.h"
#include "timeval.h"
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index fd2c2bd..f8fbe48 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5542,6 +5542,7 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif
*ofproto,
int error;
fm = (struct ofputil_flow_mod) {
+ .buffer_id = UINT32_MAX,
.match = *match,
.priority = priority,
.table_id = TBL_INTERNAL,
@@ -5580,6 +5581,7 @@ ofproto_dpif_delete_internal_flow(struct ofproto_dpif
*ofproto,
int error;
fm = (struct ofputil_flow_mod) {
+ .buffer_id = UINT32_MAX,
.match = *match,
.priority = priority,
.table_id = TBL_INTERNAL,
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 401d1b5..7574a17 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1880,7 +1880,6 @@ struct ofproto_flow_mod {
/* Replicate needed fields from ofputil_flow_mod to not need it after the
* flow has been created. */
uint16_t command;
- uint32_t buffer_id;
bool modify_cookie;
/* Fields derived from ofputil_flow_mod. */
bool modify_may_add_flow;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index aec4837..54b4944 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -50,7 +50,6 @@
#include "ovs-rcu.h"
#include "packets.h"
#include "pinsched.h"
-#include "pktbuf.h"
#include "poll-loop.h"
#include "random.h"
#include "seq.h"
@@ -264,10 +263,6 @@ static void delete_flows__(struct rule_collection *,
const struct openflow_mod_requester *)
OVS_REQUIRES(ofproto_mutex);
-static void send_buffered_packet(const struct openflow_mod_requester *,
- uint32_t buffer_id, struct rule *)
- OVS_REQUIRES(ofproto_mutex);
-
static bool ofproto_group_exists(const struct ofproto *, uint32_t group_id);
static void handle_openflow(struct ofconn *, const struct ofpbuf *);
static enum ofperr ofproto_flow_mod_init(struct ofproto *,
@@ -3353,7 +3348,7 @@ handle_features_request(struct ofconn *ofconn, const
struct ofp_header *oh)
query_switch_features(ofproto, &arp_match_ip, &features.ofpacts);
features.datapath_id = ofproto->datapath_id;
- features.n_buffers = pktbuf_capacity();
+ features.n_buffers = 0;
features.n_tables = ofproto_get_n_visible_tables(ofproto);
features.capabilities = (OFPUTIL_C_FLOW_STATS | OFPUTIL_C_TABLE_STATS |
OFPUTIL_C_PORT_STATS | OFPUTIL_C_QUEUE_STATS |
@@ -3502,14 +3497,11 @@ 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) {
- goto exit_free_ofpacts;
- }
- } else {
- /* Ensure that the L3 header is 32-bit aligned. */
- payload = dp_packet_clone_data_with_headroom(po.packet, po.packet_len,
2);
+ error = OFPERR_OFPBRC_BUFFER_UNKNOWN;
+ goto exit_free_ofpacts;
}
+ /* Ensure that the L3 header is 32-bit aligned. */
+ payload = dp_packet_clone_data_with_headroom(po.packet, po.packet_len, 2);
/* Verify actions against packet, then send packet if successful. */
flow_extract(payload, &flow);
@@ -4807,8 +4799,6 @@ add_flow_finish(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
/* Send Vacancy Events for OF1.4+. */
send_table_status(ofproto, new_rule->table_id);
}
-
- send_buffered_packet(req, ofm->buffer_id, new_rule);
}
/* OFPFC_MODIFY and OFPFC_MODIFY_STRICT. */
@@ -5107,10 +5097,7 @@ modify_flows_init_loose(struct ofproto *ofproto,
}
/* Implements OFPFC_MODIFY. Returns 0 on success or an OpenFlow error code on
- * failure.
- *
- * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
- * if any. */
+ * failure. */
static enum ofperr
modify_flows_start_loose(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
@@ -5173,9 +5160,6 @@ modify_flows_finish(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
}
learned_cookies_flush(ofproto, &dead_cookies);
remove_rules_postponed(old_rules);
-
- send_buffered_packet(req, ofm->buffer_id,
- rule_collection_rules(new_rules)[0]);
rule_collection_destroy(new_rules);
}
}
@@ -7110,7 +7094,6 @@ ofproto_flow_mod_init(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
/* Forward flow mod fields we need later. */
ofm->command = fm->command;
- ofm->buffer_id = fm->buffer_id;
ofm->modify_cookie = fm->modify_cookie;
ofm->modify_may_add_flow = (fm->new_cookie != OVS_BE64_MAX
@@ -7122,14 +7105,19 @@ ofproto_flow_mod_init(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
ofm->conjs = NULL;
ofm->n_conjs = 0;
+ bool check_buffer_id = false;
+
switch (ofm->command) {
case OFPFC_ADD:
+ check_buffer_id = true;
error = add_flow_init(ofproto, ofm, fm);
break;
case OFPFC_MODIFY:
+ check_buffer_id = true;
error = modify_flows_init_loose(ofproto, ofm, fm);
break;
case OFPFC_MODIFY_STRICT:
+ check_buffer_id = true;
error = modify_flow_init_strict(ofproto, ofm, fm);
break;
case OFPFC_DELETE:
@@ -7142,6 +7130,10 @@ ofproto_flow_mod_init(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
error = OFPERR_OFPFMFC_BAD_COMMAND;
break;
}
+ if (check_buffer_id && fm->buffer_id != UINT32_MAX) {
+ error = OFPERR_OFPBRC_BUFFER_UNKNOWN;
+ }
+
if (error) {
ofproto_flow_mod_uninit(ofm);
}
@@ -7751,43 +7743,6 @@ handle_openflow(struct ofconn *ofconn, const struct
ofpbuf *ofp_msg)
COVERAGE_INC(ofproto_recv_openflow);
}
-/* Asynchronous operations. */
-
-static void
-send_buffered_packet(const struct openflow_mod_requester *req,
- uint32_t buffer_id, struct rule *rule)
- OVS_REQUIRES(ofproto_mutex)
-{
- if (req && req->ofconn && buffer_id != UINT32_MAX) {
- struct ofproto *ofproto = ofconn_get_ofproto(req->ofconn);
- struct dp_packet *packet;
- ofp_port_t in_port;
- enum ofperr error;
-
- error = ofconn_pktbuf_retrieve(req->ofconn, buffer_id, &packet,
- &in_port);
- if (packet) {
- struct rule_execute *re;
-
- ofproto_rule_ref(rule);
-
- re = xmalloc(sizeof *re);
- re->rule = rule;
- re->in_port = in_port;
- re->packet = packet;
-
- if (!guarded_list_push_back(&ofproto->rule_executes,
- &re->list_node, 1024)) {
- ofproto_rule_unref(rule);
- dp_packet_delete(re->packet);
- free(re);
- }
- } else {
- ofconn_send_error(req->ofconn, req->request, error);
- }
- }
-}
-
static uint64_t
pick_datapath_id(const struct ofproto *ofproto)
{
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index d7b3d3e..bdf2cdd 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -350,6 +350,7 @@ run_S_CLEAR_FLOWS(void)
{
/* Send a flow_mod to delete all flows. */
struct ofputil_flow_mod fm = {
+ .buffer_id = UINT32_MAX,
.match = MATCH_CATCHALL_INITIALIZER,
.table_id = OFPTT_ALL,
.command = OFPFC_DELETE,
@@ -967,6 +968,7 @@ ofctrl_put(int64_t nb_cfg)
/* Installed flow is no longer desirable. Delete it from the
* switch and from installed_flows. */
struct ofputil_flow_mod fm = {
+ .buffer_id = UINT32_MAX,
.match = i->match,
.priority = i->priority,
.table_id = i->table_id,
@@ -992,6 +994,7 @@ ofctrl_put(int64_t nb_cfg)
d->ofpacts, d->ofpacts_len)) {
/* Update actions in installed flow. */
struct ofputil_flow_mod fm = {
+ .buffer_id = UINT32_MAX,
.match = i->match,
.priority = i->priority,
.table_id = i->table_id,
@@ -1025,6 +1028,7 @@ ofctrl_put(int64_t nb_cfg)
struct ovn_flow *d = ovn_flow_select_from_list(&candidates);
/* Send flow_mod to add flow. */
struct ofputil_flow_mod fm = {
+ .buffer_id = UINT32_MAX,
.match = d->match,
.priority = d->priority,
.table_id = d->table_id,
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 147ac23..a849c43 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -34,7 +34,7 @@ OVS_VSWITCHD_START
AT_CHECK([ovs-ofctl -vwarn show br0], [0], [stdout])
AT_CHECK([strip_xids < stdout], [0], [dnl
OFPT_FEATURES_REPLY: dpid:fedcba9876543210
-n_tables:254, n_buffers:256
+n_tables:254, n_buffers:0
capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS ARP_MATCH_IP
actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src
mod_dl_dst mod_nw_src mod_nw_dst mod_nw_tos mod_tp_src mod_tp_dst
LOCAL(br0): addr:aa:55:aa:55:00:00
@@ -56,7 +56,7 @@ s/ (xid=0x[0-9a-fA-F]*)//
s/00:0.$/00:0x/' < stdout]],
[0], [dnl
OFPT_FEATURES_REPLY: dpid:fedcba9876543210
-n_tables:254, n_buffers:256
+n_tables:254, n_buffers:0
capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS ARP_MATCH_IP
actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src
mod_dl_dst mod_nw_src mod_nw_dst mod_nw_tos mod_tp_src mod_tp_dst
1(p1): addr:aa:55:aa:55:00:0x
@@ -1174,7 +1174,7 @@ do
AT_CHECK([ovs-ofctl -vwarn show br0], [0], [stdout])
AT_CHECK_UNQUOTED([strip_xids < stdout], [0], [dnl
OFPT_FEATURES_REPLY: dpid:fedcba9876543210
-n_tables:254, n_buffers:256
+n_tables:254, n_buffers:0
capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS ARP_MATCH_IP
actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src
mod_dl_dst mod_nw_src mod_nw_dst mod_nw_tos mod_tp_src mod_tp_dst
LOCAL(br0): addr:aa:55:aa:55:00:00
@@ -1206,7 +1206,7 @@ do
AT_CHECK([ovs-ofctl -O OpenFlow12 -vwarn show br0], [0], [stdout])
AT_CHECK_UNQUOTED([strip_xids < stdout], [0], [dnl
OFPT_FEATURES_REPLY (OF1.2): dpid:fedcba9876543210
-n_tables:254, n_buffers:256
+n_tables:254, n_buffers:0
capabilities: FLOW_STATS TABLE_STATS PORT_STATS GROUP_STATS QUEUE_STATS
LOCAL(br0): addr:aa:55:aa:55:00:00
config: $config
@@ -1237,7 +1237,7 @@ do
AT_CHECK([ovs-ofctl -O OpenFlow14 -vwarn show br0], [0], [stdout])
AT_CHECK_UNQUOTED([strip_xids < stdout], [0], [dnl
OFPT_FEATURES_REPLY (OF1.4): dpid:fedcba9876543210
-n_tables:254, n_buffers:256
+n_tables:254, n_buffers:0
capabilities: FLOW_STATS TABLE_STATS PORT_STATS GROUP_STATS QUEUE_STATS
OFPST_PORT_DESC reply (OF1.4):
LOCAL(br0): addr:aa:55:aa:55:00:00
diff --git a/tutorial/OVN-Tutorial.md b/tutorial/OVN-Tutorial.md
index be9805c..8ef40f1 100644
--- a/tutorial/OVN-Tutorial.md
+++ b/tutorial/OVN-Tutorial.md
@@ -144,7 +144,7 @@ OpenFlow port number of `1`. Similarly, `lport2` has an
OpenFlow port number of
$ ovs-ofctl show br-int
OFPT_FEATURES_REPLY (xid=0x2): dpid:00003e1ba878364d
- n_tables:254, n_buffers:256
+ n_tables:254, n_buffers:0
capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS ARP_MATCH_IP
actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src
mod_dl_dst mod_nw_src mod_nw_dst mod_nw_tos mod_tp_src mod_tp_dst
1(lport1): addr:aa:55:aa:55:00:07
@@ -624,7 +624,7 @@ We get those port numbers using `ovs-ofctl`:
$ ovs-ofctl show br-int
OFPT_FEATURES_REPLY (xid=0x2): dpid:00002a84824b0d40
- n_tables:254, n_buffers:256
+ n_tables:254, n_buffers:0
capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS ARP_MATCH_IP
actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src
mod_dl_dst
1(ovn-fakech-0): addr:aa:55:aa:55:00:0e
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
