[ovs-dev] Buffer in openvswitch
Hello, For what I understand the openvSwitch works in store-and-forward mode using linux kernel. Can someone tell me where is the buffer and the files that are connected and do or treats the store-and-forward on openvswitch sources. Where is this done? Can someone tell me if is in mind an cut-through implementation for openvSwitch? I was thinking in do something like that. Regards, Marco ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Buffer in openvswitch
On Tue, Aug 07, 2012 at 02:21:20PM +0100, mvp...@iol.pt wrote: > For what I understand the openvSwitch works in store-and-forward mode > using linux kernel. > Can someone tell me where is the buffer and the files that are connected > and do or treats the store-and-forward on openvswitch sources. Where is > this done? > > Can someone tell me if is in mind an cut-through implementation for > openvSwitch? I was thinking in do something like that. Open vSwitch is a software switch. As far as I know your characterizations do not apply to software switches. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Buffer in openvswitch
Hello, but in finction: netdev_linux_listen(struct netdev *netdev_) on netdev_linux.c file inside lib, they have sockets open to the interfaces. Where it receive a packet and copy to memory. It reads the memory content only when packet reach the exact length of the packet, for example 1500 bytes, and if reach this number send the packet - like store-and-forward. Using this I think may be possible to implement cut-trhough, because we may start sending in the exact moment we receive. Advises will be very appreciated. Regards, Marco Citando Ben Pfaff : On Tue, Aug 07, 2012 at 02:21:20PM +0100, mvp...@iol.pt wrote: > For what I understand the openvSwitch works in store-and-forward mode using linux kernel. Can someone tell me where is the buffer and the files that are connected and do or treats the store-and-forward on openvswitch sources. Where is this done? Can someone tell me if is in mind an cut-through implementation for openvSwitch? I was thinking in do something like that. Open vSwitch is a software switch. As far as I know yourcharacterizations do not apply to software switches. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Buffer in openvswitch
OK, it sounds like you answered your own questions. Fine. On Tue, Aug 07, 2012 at 04:01:05PM +0100, mvp...@iol.pt wrote: > Hello, > >but in finction: netdev_linux_listen(struct netdev *netdev_) on > netdev_linux.c file inside lib, they have sockets open to the interfaces. > Where it receive a packet and copy to memory. It reads the memory content > only when packet reach the exact length of the packet, for example 1500 > bytes, and if reach this number send the packet - like store-and-forward. > >Using this I think may be possible to implement cut-trhough, because we > may start sending in the exact moment we receive. > >Advises will be very appreciated. > >Regards, >Marco > >Citando Ben Pfaff : > >On Tue, Aug 07, 2012 at 02:21:20PM +0100, mvp...@iol.pt wrote: > > >For what I understand the openvSwitch works in store-and-forward mode > >> using linux kernel. > >> Can someone tell me where is the buffer and the files that are connected > >> and do or treats the store-and-forward on openvswitch sources. Where is > >> this done? > >> > >> Can someone tell me if is in mind an cut-through implementation for > >> openvSwitch? I was thinking in do something like that. > > Open vSwitch is a software switch. As far as I know > >yourcharacterizations do not apply to software switches. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Adding checksum to ICMP packets created by OVS for testing.
On Mon, Aug 06, 2012 at 11:55:15PM -0700, Mehak Mahajan wrote: > OVS provides a utility to create ICMP packets for the purpose of > testing using ovs-appctl netdev-dummy/receive. These packets created > by flow_compose() earlier did not have the ICMP checksum in them. > With this commit, the checksum will be added to these test ICMP > packets. > > Signed-off-by: Mehak Mahajan Looks good to me, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [controller 2/6] vconn: Fix vconn_get_version().
It's documented to return -1 if the version isn't yet known, but in fact
it returned 0.
Signed-off-by: Ben Pfaff
---
lib/vconn.c |4 ++--
lib/vconn.h |3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/lib/vconn.c b/lib/vconn.c
index eb848ba..8b53b06 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -368,10 +368,10 @@ vconn_get_local_port(const struct vconn *vconn)
*
* A vconn that has successfully connected (that is, vconn_connect() or
* vconn_send() or vconn_recv() has returned 0) always negotiated a version. */
-enum ofp_version
+int
vconn_get_version(const struct vconn *vconn)
{
-return vconn->version;
+return vconn->version ? vconn->version : -1;
}
static void
diff --git a/lib/vconn.h b/lib/vconn.h
index 3bb4450..1a0bc60 100644
--- a/lib/vconn.h
+++ b/lib/vconn.h
@@ -19,6 +19,7 @@
#include
#include "openvswitch/types.h"
+#include "openflow/openflow.h"
#ifdef __cplusplus
extern "C" {
@@ -41,7 +42,7 @@ ovs_be32 vconn_get_remote_ip(const struct vconn *);
ovs_be16 vconn_get_remote_port(const struct vconn *);
ovs_be32 vconn_get_local_ip(const struct vconn *);
ovs_be16 vconn_get_local_port(const struct vconn *);
-enum ofp_version vconn_get_version(const struct vconn *);
+int vconn_get_version(const struct vconn *);
int vconn_connect(struct vconn *);
int vconn_recv(struct vconn *, struct ofpbuf **);
int vconn_send(struct vconn *, struct ofpbuf *);
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [controller 4/6] learning-switch: Delay sending handshake until version negotiation is done.
The learning-switch implementation needs to know the OpenFlow version in
use to send the initial handshake messages (e.g. the feature request), but
the version is not always available at the time that the code currently
sends the handshake. This can cause an assertion failure later when
ofputil_encode_flow_mod() checks the protocol, which will be 0 if the
version wasn't known.
This commit fixes the problem by introducing a state machine that sends the
handshake messages only after version negotiation has finished.
Reported-by: Simon Horman
Signed-off-by: Ben Pfaff
---
lib/learning-switch.c | 96 +++-
1 files changed, 62 insertions(+), 34 deletions(-)
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index d53f147..f52d3c9 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -52,8 +52,15 @@ struct lswitch_port {
uint32_t queue_id; /* OpenFlow queue number. */
};
+enum lswitch_state {
+S_CONNECTING, /* Waiting for connection to complete. */
+S_FEATURES_REPLY, /* Waiting for features reply. */
+S_SWITCHING,/* Switching flows. */
+};
+
struct lswitch {
struct rconn *rconn;
+enum lswitch_state state;
/* If nonnegative, the switch sets up flows that expire after the given
* number of seconds (or never expire, if the value is OFP_FLOW_PERMANENT).
@@ -62,7 +69,6 @@ struct lswitch {
enum ofputil_protocol protocol;
unsigned long long int datapath_id;
-time_t last_features_request;
struct mac_learning *ml;/* NULL to act as hub instead of switch. */
struct flow_wildcards wc; /* Wildcards to apply to flows. */
bool action_normal; /* Use OFPP_NORMAL? */
@@ -78,6 +84,11 @@ struct lswitch {
/* If true, do not reply to any messages from the switch (for debugging
* fail-open mode). */
bool mute;
+
+/* Optional "flow mod" requests to send to the switch at connection time,
+ * to set up the flow table. */
+const struct ofputil_flow_mod *default_flows;
+size_t n_default_flows;
};
/* The log messages here could actually be useful in debugging, so keep the
@@ -100,14 +111,13 @@ static void process_echo_request(struct lswitch *, const
struct ofp_header *);
struct lswitch *
lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
{
-enum ofputil_protocol protocol;
struct lswitch *sw;
sw = xzalloc(sizeof *sw);
sw->rconn = rconn;
+sw->state = S_CONNECTING;
sw->max_idle = cfg->max_idle;
sw->datapath_id = 0;
-sw->last_features_request = time_now() - 1;
sw->ml = (cfg->mode == LSW_LEARN
? mac_learning_create(MAC_ENTRY_DEFAULT_IDLE_TIME)
: NULL);
@@ -146,11 +156,23 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
}
}
+sw->default_flows = cfg->default_flows;
+sw->n_default_flows = cfg->n_default_flows;
+
sw->queued = rconn_packet_counter_create();
+
+return sw;
+}
+
+static void
+lswitch_handshake(struct lswitch *sw)
+{
+enum ofputil_protocol protocol;
+
send_features_request(sw);
-protocol = ofputil_protocol_from_ofp_version(rconn_get_version(rconn));
-if (cfg->default_flows) {
+protocol = ofputil_protocol_from_ofp_version(rconn_get_version(sw->rconn));
+if (sw->default_flows) {
enum ofputil_protocol usable_protocols;
struct ofpbuf *msg = NULL;
int error = 0;
@@ -164,7 +186,7 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
* flow format with the switch, but that would require an asynchronous
* state machine. This version ought to work fine in practice. */
usable_protocols = ofputil_flow_mod_usable_protocols(
-cfg->default_flows, cfg->n_default_flows);
+sw->default_flows, sw->n_default_flows);
if (!(protocol & usable_protocols)) {
enum ofputil_protocol want = rightmost_1bit(usable_protocols);
while (!error) {
@@ -172,23 +194,21 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
if (!msg) {
break;
}
-error = rconn_send(rconn, msg, NULL);
+error = rconn_send(sw->rconn, msg, NULL);
}
}
-for (i = 0; !error && i < cfg->n_default_flows; i++) {
-msg = ofputil_encode_flow_mod(&cfg->default_flows[i], protocol);
-error = rconn_send(rconn, msg, NULL);
+for (i = 0; !error && i < sw->n_default_flows; i++) {
+msg = ofputil_encode_flow_mod(&sw->default_flows[i], protocol);
+error = rconn_send(sw->rconn, msg, NULL);
}
if (error) {
VLOG_INFO_RL(&rl, "%s: failed to queue default flows (%s)",
- rconn_get_name(rconn), strerror(error));
+
[ovs-dev] [controller 1/6] vconn: Ensure that vconn_run() is enough to complete a connection.
Until now, it seems that all vconn users have immediately started reading
messages from the connection. Today, however, I added a new user that
only wants to read packets after the OpenFlow version is negotiated, so
it never called vconn_recv() before that happened. It turns out that if
you do this, the version never gets negotiated at all.
This commit fixes the problem by ensuring that vconn_run() will continue
version negotiation if it isn't done yet.
This changes the error return that I get for Unix sockets in the
test-vconn "accept-then-close" test from EPIPE to ECONNRESET, so this
commit also adjusts that test to accept either error code; both of them
seem reasonable enough to me.
Signed-off-by: Ben Pfaff
---
lib/vconn.c| 12
tests/test-vconn.c | 18 --
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/lib/vconn.c b/lib/vconn.c
index 6fd73d6..eb848ba 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -262,6 +262,12 @@ error:
void
vconn_run(struct vconn *vconn)
{
+if (vconn->state == VCS_CONNECTING ||
+vconn->state == VCS_SEND_HELLO ||
+vconn->state == VCS_RECV_HELLO) {
+vconn_connect(vconn);
+}
+
if (vconn->class->run) {
(vconn->class->run)(vconn);
}
@@ -272,6 +278,12 @@ vconn_run(struct vconn *vconn)
void
vconn_run_wait(struct vconn *vconn)
{
+if (vconn->state == VCS_CONNECTING ||
+vconn->state == VCS_SEND_HELLO ||
+vconn->state == VCS_RECV_HELLO) {
+vconn_connect_wait(vconn);
+}
+
if (vconn->class->run_wait) {
(vconn->class->run_wait)(vconn);
}
diff --git a/tests/test-vconn.c b/tests/test-vconn.c
index 377f9fa..5dd38f0 100644
--- a/tests/test-vconn.c
+++ b/tests/test-vconn.c
@@ -174,13 +174,9 @@ static void
test_accept_then_close(int argc OVS_UNUSED, char *argv[])
{
const char *type = argv[1];
-int expected_error;
struct fake_pvconn fpv;
struct vconn *vconn;
-
-expected_error = (!strcmp(type, "unix") ? EPIPE
- : !strcmp(type, "tcp") ? ECONNRESET
- : EPROTO);
+int error;
fpv_create(type, &fpv);
CHECK_ERRNO(vconn_open(fpv.vconn_name, OFP10_VERSION, &vconn,
@@ -188,7 +184,17 @@ test_accept_then_close(int argc OVS_UNUSED, char *argv[])
vconn_run(vconn);
stream_close(fpv_accept(&fpv));
fpv_close(&fpv);
-CHECK_ERRNO(vconn_connect(vconn), expected_error);
+
+error = vconn_connect_block(vconn);
+if (!strcmp(type, "tcp") || !strcmp(type, "unix")) {
+if (error != ECONNRESET && error != EPIPE) {
+ovs_fatal(0, "unexpected vconn_connect() return value %d (%s)",
+ error, strerror(error));
+}
+} else {
+CHECK_ERRNO(error, EPROTO);
+}
+
vconn_close(vconn);
fpv_destroy(&fpv);
}
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [controller 5/6] learning-switch: Don't use exact-match on every field by default.
OVS has all kinds of odd fields, e.g. registers, and it doesn't make sense
to try to match on all of them. This commit changes learning-switch to
only try to match on the fields defined by OpenFlow 1.0. That's still not
minimal, but it's more reasonable.
This commit should not have an immediately visible effect since
ovs-controller always sends OF1.0 format flows to the switch, and OF1.0
format flows don't have these extra fields. But in the future when we
add support for new protocols and flow formats to ovs-controller, it
will make a difference.
Signed-off-by: Ben Pfaff
---
lib/learning-switch.c | 37 ++---
lib/ofp-util.c| 54
lib/ofp-util.h|1 +
3 files changed, 57 insertions(+), 35 deletions(-)
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index f52d3c9..e4287c4 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -112,6 +112,7 @@ struct lswitch *
lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
{
struct lswitch *sw;
+uint32_t ofpfw;
sw = xzalloc(sizeof *sw);
sw->rconn = rconn;
@@ -123,24 +124,26 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
: NULL);
sw->action_normal = cfg->mode == LSW_NORMAL;
-flow_wildcards_init_exact(&sw->wc);
-if (cfg->wildcards) {
-uint32_t ofpfw;
-
-if (cfg->wildcards == UINT32_MAX) {
-/* Try to wildcard as many fields as possible, but we cannot
- * wildcard all fields. We need in_port to detect moves. We need
- * Ethernet source and dest and VLAN VID to do L2 learning. */
-ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP
- | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL
- | OFPFW10_NW_TOS | OFPFW10_NW_PROTO
- | OFPFW10_TP_SRC | OFPFW10_TP_DST);
-} else {
-ofpfw = cfg->wildcards;
-}
+switch (cfg->wildcards) {
+case 0:
+ofpfw = 0;
+break;
-ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc);
+case UINT32_MAX:
+/* Try to wildcard as many fields as possible, but we cannot
+ * wildcard all fields. We need in_port to detect moves. We need
+ * Ethernet source and dest and VLAN VID to do L2 learning. */
+ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP
+ | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL
+ | OFPFW10_NW_TOS | OFPFW10_NW_PROTO
+ | OFPFW10_TP_SRC | OFPFW10_TP_DST);
+break;
+
+default:
+ofpfw = cfg->wildcards;
+break;
}
+ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc);
sw->default_queue = cfg->default_queue;
hmap_init(&sw->queue_numbers);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1fe30b4..030c812 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3319,23 +3319,8 @@ ofputil_put_action(enum ofputil_action_code code, struct
ofpbuf *buf)
}
#include "ofp-util.def"
-/* "Normalizes" the wildcards in 'rule'. That means:
- *
- *1. If the type of level N is known, then only the valid fields for that
- * level may be specified. For example, ARP does not have a TOS field,
- * so nw_tos must be wildcarded if 'rule' specifies an ARP flow.
- * Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and
- * ipv6_dst (and other fields) must be wildcarded if 'rule' specifies an
- * IPv4 flow.
- *
- *2. If the type of level N is not known (or not understood by Open
- * vSwitch), then no fields at all for that level may be specified. For
- * example, Open vSwitch does not understand SCTP, an L4 protocol, so the
- * L4 fields tp_src and tp_dst must be wildcarded if 'rule' specifies an
- * SCTP flow.
- */
-void
-ofputil_normalize_rule(struct cls_rule *rule)
+static void
+ofputil_normalize_rule__(struct cls_rule *rule, bool may_log)
{
enum {
MAY_NW_ADDR = 1 << 0, /* nw_src, nw_dst */
@@ -3409,7 +3394,7 @@ ofputil_normalize_rule(struct cls_rule *rule)
/* Log any changes. */
if (!flow_wildcards_equal(&wc, &rule->wc)) {
-bool log = !VLOG_DROP_INFO(&bad_ofmsg_rl);
+bool log = may_log && !VLOG_DROP_INFO(&bad_ofmsg_rl);
char *pre = log ? cls_rule_to_string(rule) : NULL;
rule->wc = wc;
@@ -3426,6 +3411,39 @@ ofputil_normalize_rule(struct cls_rule *rule)
}
}
+/* "Normalizes" the wildcards in 'rule'. That means:
+ *
+ *1. If the type of level N is known, then only the valid fields for that
+ * level may be specified. For example, ARP does not have a TOS field,
+ * so nw_tos must be wildcarded if 'rule' specifies an ARP flow.
+ * Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and
+ * ipv6_dst (and other fields) must be wildcarded if 'rule' specifies an
+ * IPv4 flow.
+
[ovs-dev] [controller 3/6] learning-switch: Make lswitch own its rconn.
Until now, ovs-controller and the learning-switch code split responsibility
for the OpenFlow connection. This commit moves all the responsibility into
the learning-switch code.
The rationale here is twofold. First, the split itself seems odd; I think
there must have been a reason for it at one time, but I don't remember it
and don't see one anymore. Second, I intend to make the lswitch code more
stateful in upcoming commits, and it seems odd to have the lswitch manage
quite a bit of state but not the entity that that state applies to.
Signed-off-by: Ben Pfaff
---
lib/learning-switch.c | 88 +--
lib/learning-switch.h |8 +++-
utilities/ovs-controller.c | 68 ++---
3 files changed, 78 insertions(+), 86 deletions(-)
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index b7a435c..d53f147 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -53,6 +53,8 @@ struct lswitch_port {
};
struct lswitch {
+struct rconn *rconn;
+
/* If nonnegative, the switch sets up flows that expire after the given
* number of seconds (or never expire, if the value is OFP_FLOW_PERMANENT).
* Otherwise, the switch processes every packet. */
@@ -72,21 +74,24 @@ struct lswitch {
/* Number of outgoing queued packets on the rconn. */
struct rconn_packet_counter *queued;
+
+/* If true, do not reply to any messages from the switch (for debugging
+ * fail-open mode). */
+bool mute;
};
/* The log messages here could actually be useful in debugging, so keep the
* rate limit relatively high. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
-static void queue_tx(struct lswitch *, struct rconn *, struct ofpbuf *);
-static void send_features_request(struct lswitch *, struct rconn *);
+static void queue_tx(struct lswitch *, struct ofpbuf *);
+static void send_features_request(struct lswitch *);
+static void lswitch_process_packet(struct lswitch *, const struct ofpbuf *);
static enum ofperr process_switch_features(struct lswitch *,
struct ofp_header *);
-static void process_packet_in(struct lswitch *, struct rconn *,
- const struct ofp_header *);
-static void process_echo_request(struct lswitch *, struct rconn *,
- const struct ofp_header *);
+static void process_packet_in(struct lswitch *, const struct ofp_header *);
+static void process_echo_request(struct lswitch *, const struct ofp_header *);
/* Creates and returns a new learning switch whose configuration is given by
* 'cfg'.
@@ -99,6 +104,7 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
struct lswitch *sw;
sw = xzalloc(sizeof *sw);
+sw->rconn = rconn;
sw->max_idle = cfg->max_idle;
sw->datapath_id = 0;
sw->last_features_request = time_now() - 1;
@@ -141,7 +147,7 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
}
sw->queued = rconn_packet_counter_create();
-send_features_request(sw, rconn);
+send_features_request(sw);
protocol = ofputil_protocol_from_ofp_version(rconn_get_version(rconn));
if (cfg->default_flows) {
@@ -185,6 +191,12 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
return sw;
}
+bool
+lswitch_is_alive(const struct lswitch *sw)
+{
+return rconn_is_alive(sw->rconn);
+}
+
/* Destroys 'sw'. */
void
lswitch_destroy(struct lswitch *sw)
@@ -192,6 +204,7 @@ lswitch_destroy(struct lswitch *sw)
if (sw) {
struct lswitch_port *node, *next;
+rconn_destroy(sw->rconn);
HMAP_FOR_EACH_SAFE (node, next, hmap_node, &sw->queue_numbers) {
hmap_remove(&sw->queue_numbers, &node->hmap_node);
free(node);
@@ -208,9 +221,27 @@ lswitch_destroy(struct lswitch *sw)
void
lswitch_run(struct lswitch *sw)
{
+int i;
+
if (sw->ml) {
mac_learning_run(sw->ml, NULL);
}
+
+rconn_run(sw->rconn);
+
+for (i = 0; i < 50; i++) {
+struct ofpbuf *msg;
+
+msg = rconn_recv(sw->rconn);
+if (!msg) {
+break;
+}
+
+if (!sw->mute) {
+lswitch_process_packet(sw, msg);
+}
+ofpbuf_delete(msg);
+}
}
void
@@ -219,15 +250,16 @@ lswitch_wait(struct lswitch *sw)
if (sw->ml) {
mac_learning_wait(sw->ml);
}
+rconn_run(sw->rconn);
+rconn_recv_wait(sw->rconn);
}
/* Processes 'msg', which should be an OpenFlow received on 'rconn', according
* to the learning switch state in 'sw'. The most likely result of processing
* is that flow-setup and packet-out OpenFlow messages will be sent out on
* 'rconn'. */
-void
-lswitch_process_packet(struct lswitch *sw, struct rconn *rconn,
- const struct ofpbuf *msg)
+static void
+lswitch_process_packet(struct lswitch *sw, cons
[ovs-dev] [controller 6/6] learning-switch: Normalize the flows that are sent to the switch.
This suppresses a long-standing warning from ovs-vswitchd about non-normalized flows. Signed-off-by: Ben Pfaff --- lib/learning-switch.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/lib/learning-switch.c b/lib/learning-switch.c index e4287c4..c1cd909 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -576,6 +576,7 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) * new flow. */ memset(&fm, 0, sizeof fm); cls_rule_init(&flow, &sw->wc, 0, &fm.cr); +ofputil_normalize_rule_quiet(&fm.cr); fm.table_id = 0xff; fm.command = OFPFC_ADD; fm.idle_timeout = sw->max_idle; -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [controller 0/6] controller bugfixes
This fixes a crashing bug in ovs-controller reported by Simon Horman, as well as a long-standing annoyance with lack of flow normalization. Ben Pfaff (6): vconn: Ensure that vconn_run() is enough to complete a connection. vconn: Fix vconn_get_version(). learning-switch: Make lswitch own its rconn. learning-switch: Delay sending handshake until version negotiation is done. learning-switch: Don't use exact-match on every field by default. learning-switch: Normalize the flows that are sent to the switch. lib/learning-switch.c | 208 --- lib/learning-switch.h |8 ++- lib/ofp-util.c | 54 lib/ofp-util.h |1 + lib/vconn.c| 16 +++- lib/vconn.h|3 +- tests/test-vconn.c | 18 +++-- utilities/ovs-controller.c | 68 +++ 8 files changed, 219 insertions(+), 157 deletions(-) -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Buffer in openvswitch
On 8/7/12 8:21 AM, mvp...@iol.pt wrote: Can someone tell me if is in mind an cut-through implementation for openvSwitch? I was thinking in do something like that. AFAIK, NICs only trigger an interrupt after the packet has been completely DMAed into memory, which makes cut-through impossible to implement on today's servers. There may be some possible optimizations that would make OVS faster, but they would have to be something other than cut-through. I would assume OVS is already zero-copy, so reducing copies probably won't help either. -- Wes Felter IBM Research - Austin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [controller 2/6] vconn: Fix vconn_get_version().
On Aug 7, 2012, at 1:50 PM, Ben Pfaff wrote: > It's documented to return -1 if the version isn't yet known, but in fact > it returned 0. > > Signed-off-by: Ben Pfaff Looks good to me. Thanks, Kyle ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [controller 1/6] vconn: Ensure that vconn_run() is enough to complete a connection.
On Aug 7, 2012, at 1:50 PM, Ben Pfaff wrote: > Until now, it seems that all vconn users have immediately started reading > messages from the connection. Today, however, I added a new user that > only wants to read packets after the OpenFlow version is negotiated, so > it never called vconn_recv() before that happened. It turns out that if > you do this, the version never gets negotiated at all. > > This commit fixes the problem by ensuring that vconn_run() will continue > version negotiation if it isn't done yet. > > This changes the error return that I get for Unix sockets in the > test-vconn "accept-then-close" test from EPIPE to ECONNRESET, so this > commit also adjusts that test to accept either error code; both of them > seem reasonable enough to me. > > Signed-off-by: Ben Pfaff Looks good to me. Thanks, Kyle ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [controller 3/6] learning-switch: Make lswitch own its rconn.
On Aug 7, 2012, at 1:50 PM, Ben Pfaff wrote: > Until now, ovs-controller and the learning-switch code split responsibility > for the OpenFlow connection. This commit moves all the responsibility into > the learning-switch code. > > The rationale here is twofold. First, the split itself seems odd; I think > there must have been a reason for it at one time, but I don't remember it > and don't see one anymore. Second, I intend to make the lswitch code more > stateful in upcoming commits, and it seems odd to have the lswitch manage > quite a bit of state but not the entity that that state applies to. > > Signed-off-by: Ben Pfaff Looks good. Thanks, Kyle ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [controller 4/6] learning-switch: Delay sending handshake until version negotiation is done.
On Aug 7, 2012, at 1:50 PM, Ben Pfaff wrote: > The learning-switch implementation needs to know the OpenFlow version in > use to send the initial handshake messages (e.g. the feature request), but > the version is not always available at the time that the code currently > sends the handshake. This can cause an assertion failure later when > ofputil_encode_flow_mod() checks the protocol, which will be 0 if the > version wasn't known. > > This commit fixes the problem by introducing a state machine that sends the > handshake messages only after version negotiation has finished. > > Reported-by: Simon Horman > Signed-off-by: Ben Pfaff Looks good. Thanks, Kyle ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [controller 5/6] learning-switch: Don't use exact-match on every field by default.
On Aug 7, 2012, at 1:50 PM, Ben Pfaff wrote:
> OVS has all kinds of odd fields, e.g. registers, and it doesn't make sense
> to try to match on all of them. This commit changes learning-switch to
> only try to match on the fields defined by OpenFlow 1.0. That's still not
> minimal, but it's more reasonable.
>
> This commit should not have an immediately visible effect since
> ovs-controller always sends OF1.0 format flows to the switch, and OF1.0
> format flows don't have these extra fields. But in the future when we
> add support for new protocols and flow formats to ovs-controller, it
> will make a difference.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/learning-switch.c | 37 ++---
> lib/ofp-util.c| 54
> lib/ofp-util.h|1 +
> 3 files changed, 57 insertions(+), 35 deletions(-)
>
> diff --git a/lib/learning-switch.c b/lib/learning-switch.c
> index f52d3c9..e4287c4 100644
> --- a/lib/learning-switch.c
> +++ b/lib/learning-switch.c
> @@ -112,6 +112,7 @@ struct lswitch *
> lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
> {
> struct lswitch *sw;
> +uint32_t ofpfw;
>
> sw = xzalloc(sizeof *sw);
> sw->rconn = rconn;
> @@ -123,24 +124,26 @@ lswitch_create(struct rconn *rconn, const struct
> lswitch_config *cfg)
> : NULL);
> sw->action_normal = cfg->mode == LSW_NORMAL;
>
> -flow_wildcards_init_exact(&sw->wc);
> -if (cfg->wildcards) {
> -uint32_t ofpfw;
> -
> -if (cfg->wildcards == UINT32_MAX) {
> -/* Try to wildcard as many fields as possible, but we cannot
> - * wildcard all fields. We need in_port to detect moves. We
> need
> - * Ethernet source and dest and VLAN VID to do L2 learning. */
> -ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP
> - | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL
> - | OFPFW10_NW_TOS | OFPFW10_NW_PROTO
> - | OFPFW10_TP_SRC | OFPFW10_TP_DST);
> -} else {
> -ofpfw = cfg->wildcards;
> -}
> +switch (cfg->wildcards) {
> +case 0:
> +ofpfw = 0;
> +break;
>
> -ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc);
> +case UINT32_MAX:
> +/* Try to wildcard as many fields as possible, but we cannot
> + * wildcard all fields. We need in_port to detect moves. We need
> + * Ethernet source and dest and VLAN VID to do L2 learning. */
> +ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP
> + | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL
> + | OFPFW10_NW_TOS | OFPFW10_NW_PROTO
> + | OFPFW10_TP_SRC | OFPFW10_TP_DST);
> +break;
> +
> +default:
> +ofpfw = cfg->wildcards;
> +break;
> }
> +ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc);
>
This is nice, I like the look of this better as well.
Everything looks good to me.
Thanks,
Kyle
> sw->default_queue = cfg->default_queue;
> hmap_init(&sw->queue_numbers);
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 1fe30b4..030c812 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -3319,23 +3319,8 @@ ofputil_put_action(enum ofputil_action_code code,
> struct ofpbuf *buf)
> }
> #include "ofp-util.def"
>
> -/* "Normalizes" the wildcards in 'rule'. That means:
> - *
> - *1. If the type of level N is known, then only the valid fields for that
> - * level may be specified. For example, ARP does not have a TOS field,
> - * so nw_tos must be wildcarded if 'rule' specifies an ARP flow.
> - * Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and
> - * ipv6_dst (and other fields) must be wildcarded if 'rule' specifies
> an
> - * IPv4 flow.
> - *
> - *2. If the type of level N is not known (or not understood by Open
> - * vSwitch), then no fields at all for that level may be specified.
> For
> - * example, Open vSwitch does not understand SCTP, an L4 protocol, so
> the
> - * L4 fields tp_src and tp_dst must be wildcarded if 'rule' specifies
> an
> - * SCTP flow.
> - */
> -void
> -ofputil_normalize_rule(struct cls_rule *rule)
> +static void
> +ofputil_normalize_rule__(struct cls_rule *rule, bool may_log)
> {
> enum {
> MAY_NW_ADDR = 1 << 0, /* nw_src, nw_dst */
> @@ -3409,7 +3394,7 @@ ofputil_normalize_rule(struct cls_rule *rule)
>
> /* Log any changes. */
> if (!flow_wildcards_equal(&wc, &rule->wc)) {
> -bool log = !VLOG_DROP_INFO(&bad_ofmsg_rl);
> +bool log = may_log && !VLOG_DROP_INFO(&bad_ofmsg_rl);
> char *pre = log ? cls_rule_to_string(rule) : NULL;
>
> rule->wc = wc;
> @@ -3426,6 +3411,39 @@ ofputil_normalize_rule(struct cls_rule *rule)
> }
> }
>
> +/* "Normalizes" the wildcards in 'rule'. That means:
> + *
> + *1. If the type of level N is kno
Re: [ovs-dev] [controller 6/6] learning-switch: Normalize the flows that are sent to the switch.
On Aug 7, 2012, at 1:50 PM, Ben Pfaff wrote: > This suppresses a long-standing warning from ovs-vswitchd about > non-normalized flows. > > Signed-off-by: Ben Pfaff Looks good to me. Thanks, Kyle ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 10/28] ofproto: Move ofpacts_check() calls from ofproto-dpif to ofproto.
I think you and I might have slightly different models in mind. The
model that I'm after at the moment is that the ofproto generic layer
checks that a set of actions is valid (which can be done without
knowing anything about the hardware) and then the implementation
checks that it can actually implement those valid actions.
I agree that the approach you suggest would work too, and that it
could work with either the model I have in mind or a model where the
implementation has to do all of the validation as well as the
implement-ability checking.
So, I'm inclined to leave it the way I have it, but I remain open to
further discussion.
On Mon, Jul 30, 2012 at 02:45:26PM -0700, Ethan Jackson wrote:
> Looks good to me.
>
> I had a thought but I don't feel strongly about it. What if we made a
> new hook for ofproto implementations:
>
> enum ofperr (*validate_actions)(const struct ofpact ofpacts[], size_t
> ofpacts_len, const struct flow *, int max_ports).
>
> In the ofproto-dpif implementation this would simply pass through to
> ofpacts_check(), but in other implementations it could reject action
> lists which are not implementable by their dpif. This would allow us
> to make *packet_out() and *rule_construct() assume that the actions
> are both valid and implementable. Then they could return void instead
> of an ofperr. Also if we did this, rule_modify_actions() may be
> completely unnecessary. The complete_operation() calls could be
> pulled into ofproto.
>
> At any rate, this seems marginally cleaner to me, but you may feel
> strongly in the other direction in which case the current approach is
> fine.
>
> Ethan
>
> On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
> > Signed-off-by: Ben Pfaff
> > ---
> > ofproto/ofproto-dpif.c | 58
> > ++-
> > ofproto/ofproto-provider.h | 14 ---
> > ofproto/ofproto.c | 16 +---
> > 3 files changed, 36 insertions(+), 52 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index f1d2ae0..44cbd17 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -4592,13 +4592,6 @@ rule_construct(struct rule *rule_)
> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> > struct rule_dpif *victim;
> > uint8_t table_id;
> > -enum ofperr error;
> > -
> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
> > - &rule->up.cr.flow, ofproto->up.max_ports);
> > -if (error) {
> > -return error;
> > -}
> >
> > rule->packet_count = 0;
> > rule->byte_count = 0;
> > @@ -4701,15 +4694,6 @@ static void
> > rule_modify_actions(struct rule *rule_)
> > {
> > struct rule_dpif *rule = rule_dpif_cast(rule_);
> > -struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> > -enum ofperr error;
> > -
> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
> > - &rule->up.cr.flow, ofproto->up.max_ports);
> > -if (error) {
> > -ofoperation_complete(rule->up.pending, error);
> > -return;
> > -}
> >
> > complete_operation(rule);
> > }
> > @@ -6359,36 +6343,32 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf
> > *packet,
> > const struct ofpact *ofpacts, size_t ofpacts_len)
> > {
> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> > -enum ofperr error;
> > +struct odputil_keybuf keybuf;
> > +struct dpif_flow_stats stats;
> >
> > -error = ofpacts_check(ofpacts, ofpacts_len, flow,
> > ofproto->up.max_ports);
> > -if (!error) {
> > -struct odputil_keybuf keybuf;
> > -struct dpif_flow_stats stats;
> > +struct ofpbuf key;
> >
> > -struct ofpbuf key;
> > +struct action_xlate_ctx ctx;
> > +uint64_t odp_actions_stub[1024 / 8];
> > +struct ofpbuf odp_actions;
> >
> > -struct action_xlate_ctx ctx;
> > -uint64_t odp_actions_stub[1024 / 8];
> > -struct ofpbuf odp_actions;
> > +ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> > +odp_flow_key_from_flow(&key, flow);
> >
> > -ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> > -odp_flow_key_from_flow(&key, flow);
> > +dpif_flow_stats_extract(flow, packet, &stats);
> >
> > -dpif_flow_stats_extract(flow, packet, &stats);
> > +action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, NULL,
> > + packet_get_tcp_flags(packet, flow), packet);
> > +ctx.resubmit_stats = &stats;
> >
> > -action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, NULL,
> > - packet_get_tcp_flags(packet, flow), packet);
> > -ctx.resubmit_stats = &stats;
> > +ofpbuf_use_stub(&odp_actions,
> > +odp_actions_stub, sizeof odp_actions_stub);
> > +xlate_actions(&ctx, ofpacts, ofpacts_len, &odp_actions);
> >
Re: [ovs-dev] [controller 6/6] learning-switch: Normalize the flows that are sent to the switch.
On Tue, Aug 07, 2012 at 07:34:19PM +, Kyle Mestery (kmestery) wrote: > On Aug 7, 2012, at 1:50 PM, Ben Pfaff wrote: > > This suppresses a long-standing warning from ovs-vswitchd about > > non-normalized flows. > > > > Signed-off-by: Ben Pfaff > > > Looks good to me. Thanks for the reviews. I pushed this series to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 10/28] ofproto: Move ofpacts_check() calls from ofproto-dpif to ofproto.
I don't feel particularly strongly about it. The current
implementation is fine with me.
Ethan
On Tue, Aug 7, 2012 at 12:40 PM, Ben Pfaff wrote:
> I think you and I might have slightly different models in mind. The
> model that I'm after at the moment is that the ofproto generic layer
> checks that a set of actions is valid (which can be done without
> knowing anything about the hardware) and then the implementation
> checks that it can actually implement those valid actions.
>
> I agree that the approach you suggest would work too, and that it
> could work with either the model I have in mind or a model where the
> implementation has to do all of the validation as well as the
> implement-ability checking.
>
> So, I'm inclined to leave it the way I have it, but I remain open to
> further discussion.
>
> On Mon, Jul 30, 2012 at 02:45:26PM -0700, Ethan Jackson wrote:
>> Looks good to me.
>>
>> I had a thought but I don't feel strongly about it. What if we made a
>> new hook for ofproto implementations:
>>
>> enum ofperr (*validate_actions)(const struct ofpact ofpacts[], size_t
>> ofpacts_len, const struct flow *, int max_ports).
>>
>> In the ofproto-dpif implementation this would simply pass through to
>> ofpacts_check(), but in other implementations it could reject action
>> lists which are not implementable by their dpif. This would allow us
>> to make *packet_out() and *rule_construct() assume that the actions
>> are both valid and implementable. Then they could return void instead
>> of an ofperr. Also if we did this, rule_modify_actions() may be
>> completely unnecessary. The complete_operation() calls could be
>> pulled into ofproto.
>>
>> At any rate, this seems marginally cleaner to me, but you may feel
>> strongly in the other direction in which case the current approach is
>> fine.
>>
>> Ethan
>>
>> On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
>> > Signed-off-by: Ben Pfaff
>> > ---
>> > ofproto/ofproto-dpif.c | 58
>> > ++-
>> > ofproto/ofproto-provider.h | 14 ---
>> > ofproto/ofproto.c | 16 +---
>> > 3 files changed, 36 insertions(+), 52 deletions(-)
>> >
>> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> > index f1d2ae0..44cbd17 100644
>> > --- a/ofproto/ofproto-dpif.c
>> > +++ b/ofproto/ofproto-dpif.c
>> > @@ -4592,13 +4592,6 @@ rule_construct(struct rule *rule_)
>> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
>> > struct rule_dpif *victim;
>> > uint8_t table_id;
>> > -enum ofperr error;
>> > -
>> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
>> > - &rule->up.cr.flow, ofproto->up.max_ports);
>> > -if (error) {
>> > -return error;
>> > -}
>> >
>> > rule->packet_count = 0;
>> > rule->byte_count = 0;
>> > @@ -4701,15 +4694,6 @@ static void
>> > rule_modify_actions(struct rule *rule_)
>> > {
>> > struct rule_dpif *rule = rule_dpif_cast(rule_);
>> > -struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
>> > -enum ofperr error;
>> > -
>> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
>> > - &rule->up.cr.flow, ofproto->up.max_ports);
>> > -if (error) {
>> > -ofoperation_complete(rule->up.pending, error);
>> > -return;
>> > -}
>> >
>> > complete_operation(rule);
>> > }
>> > @@ -6359,36 +6343,32 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf
>> > *packet,
>> > const struct ofpact *ofpacts, size_t ofpacts_len)
>> > {
>> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
>> > -enum ofperr error;
>> > +struct odputil_keybuf keybuf;
>> > +struct dpif_flow_stats stats;
>> >
>> > -error = ofpacts_check(ofpacts, ofpacts_len, flow,
>> > ofproto->up.max_ports);
>> > -if (!error) {
>> > -struct odputil_keybuf keybuf;
>> > -struct dpif_flow_stats stats;
>> > +struct ofpbuf key;
>> >
>> > -struct ofpbuf key;
>> > +struct action_xlate_ctx ctx;
>> > +uint64_t odp_actions_stub[1024 / 8];
>> > +struct ofpbuf odp_actions;
>> >
>> > -struct action_xlate_ctx ctx;
>> > -uint64_t odp_actions_stub[1024 / 8];
>> > -struct ofpbuf odp_actions;
>> > +ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
>> > +odp_flow_key_from_flow(&key, flow);
>> >
>> > -ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
>> > -odp_flow_key_from_flow(&key, flow);
>> > +dpif_flow_stats_extract(flow, packet, &stats);
>> >
>> > -dpif_flow_stats_extract(flow, packet, &stats);
>> > +action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, NULL,
>> > + packet_get_tcp_flags(packet, flow), packet);
>> > +ctx.resubmit_stats = &stats;
>> >
>> > -action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, NULL,
>> > -
Re: [ovs-dev] [PATCH 11/24] ovs-controller: Make sure vconn is connected before making messages
On Wed, Jul 25, 2012 at 09:05:58AM +0900, Simon Horman wrote: > On Tue, Jul 24, 2012 at 04:03:59PM -0700, Ben Pfaff wrote: > > On Mon, Jul 23, 2012 at 03:16:40PM +0900, Simon Horman wrote: > > > This is a rather unpleasant hack but I am unsure of how to rework things > > > such that ovs-controller will work with a version of make_openflow() which > > > uses prevailing OpenFlow version in the header of OpenFlow messages. > > > > > > Signed-off-by: Simon Horman > > > > Oh, OK, I see the problem that the previous patch pointed out. > > > > I'll send out some proper fixes in a while. > > Thanks. The fixes were in the series starting here: http://openvswitch.org/pipermail/dev/2012-August/020071.html which I've now applied to master. If you still see problems with ovs-controller then please raise the issue again. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 10/28] ofproto: Move ofpacts_check() calls from ofproto-dpif to ofproto.
OK, thanks.
Perhaps when (if?) someone submits another ofproto provider to master,
we can figure out what best suits the set of providers as a whole.
On Tue, Aug 07, 2012 at 12:44:52PM -0700, Ethan Jackson wrote:
> I don't feel particularly strongly about it. The current
> implementation is fine with me.
>
> Ethan
>
> On Tue, Aug 7, 2012 at 12:40 PM, Ben Pfaff wrote:
> > I think you and I might have slightly different models in mind. The
> > model that I'm after at the moment is that the ofproto generic layer
> > checks that a set of actions is valid (which can be done without
> > knowing anything about the hardware) and then the implementation
> > checks that it can actually implement those valid actions.
> >
> > I agree that the approach you suggest would work too, and that it
> > could work with either the model I have in mind or a model where the
> > implementation has to do all of the validation as well as the
> > implement-ability checking.
> >
> > So, I'm inclined to leave it the way I have it, but I remain open to
> > further discussion.
> >
> > On Mon, Jul 30, 2012 at 02:45:26PM -0700, Ethan Jackson wrote:
> >> Looks good to me.
> >>
> >> I had a thought but I don't feel strongly about it. What if we made a
> >> new hook for ofproto implementations:
> >>
> >> enum ofperr (*validate_actions)(const struct ofpact ofpacts[], size_t
> >> ofpacts_len, const struct flow *, int max_ports).
> >>
> >> In the ofproto-dpif implementation this would simply pass through to
> >> ofpacts_check(), but in other implementations it could reject action
> >> lists which are not implementable by their dpif. This would allow us
> >> to make *packet_out() and *rule_construct() assume that the actions
> >> are both valid and implementable. Then they could return void instead
> >> of an ofperr. Also if we did this, rule_modify_actions() may be
> >> completely unnecessary. The complete_operation() calls could be
> >> pulled into ofproto.
> >>
> >> At any rate, this seems marginally cleaner to me, but you may feel
> >> strongly in the other direction in which case the current approach is
> >> fine.
> >>
> >> Ethan
> >>
> >> On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
> >> > Signed-off-by: Ben Pfaff
> >> > ---
> >> > ofproto/ofproto-dpif.c | 58
> >> > ++-
> >> > ofproto/ofproto-provider.h | 14 ---
> >> > ofproto/ofproto.c | 16 +---
> >> > 3 files changed, 36 insertions(+), 52 deletions(-)
> >> >
> >> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> >> > index f1d2ae0..44cbd17 100644
> >> > --- a/ofproto/ofproto-dpif.c
> >> > +++ b/ofproto/ofproto-dpif.c
> >> > @@ -4592,13 +4592,6 @@ rule_construct(struct rule *rule_)
> >> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> >> > struct rule_dpif *victim;
> >> > uint8_t table_id;
> >> > -enum ofperr error;
> >> > -
> >> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
> >> > - &rule->up.cr.flow, ofproto->up.max_ports);
> >> > -if (error) {
> >> > -return error;
> >> > -}
> >> >
> >> > rule->packet_count = 0;
> >> > rule->byte_count = 0;
> >> > @@ -4701,15 +4694,6 @@ static void
> >> > rule_modify_actions(struct rule *rule_)
> >> > {
> >> > struct rule_dpif *rule = rule_dpif_cast(rule_);
> >> > -struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> >> > -enum ofperr error;
> >> > -
> >> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
> >> > - &rule->up.cr.flow, ofproto->up.max_ports);
> >> > -if (error) {
> >> > -ofoperation_complete(rule->up.pending, error);
> >> > -return;
> >> > -}
> >> >
> >> > complete_operation(rule);
> >> > }
> >> > @@ -6359,36 +6343,32 @@ packet_out(struct ofproto *ofproto_, struct
> >> > ofpbuf *packet,
> >> > const struct ofpact *ofpacts, size_t ofpacts_len)
> >> > {
> >> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> >> > -enum ofperr error;
> >> > +struct odputil_keybuf keybuf;
> >> > +struct dpif_flow_stats stats;
> >> >
> >> > -error = ofpacts_check(ofpacts, ofpacts_len, flow,
> >> > ofproto->up.max_ports);
> >> > -if (!error) {
> >> > -struct odputil_keybuf keybuf;
> >> > -struct dpif_flow_stats stats;
> >> > +struct ofpbuf key;
> >> >
> >> > -struct ofpbuf key;
> >> > +struct action_xlate_ctx ctx;
> >> > +uint64_t odp_actions_stub[1024 / 8];
> >> > +struct ofpbuf odp_actions;
> >> >
> >> > -struct action_xlate_ctx ctx;
> >> > -uint64_t odp_actions_stub[1024 / 8];
> >> > -struct ofpbuf odp_actions;
> >> > +ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> >> > +odp_flow_key_from_flow(&key, flow);
> >> >
> >> > -ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> >> > -odp_flow_key_
Re: [ovs-dev] [classifier-opt 10/28] ofproto: Move ofpacts_check() calls from ofproto-dpif to ofproto.
> Perhaps when (if?) someone submits another ofproto provider to master,
> we can figure out what best suits the set of providers as a whole.
Yah it's hard to get the abstraction right when we only have deep
knowledge of one implementation. Hopefully a significant open source
secondary implementation will pop up and we will learn a lot from it.
Ethan
>
> On Tue, Aug 07, 2012 at 12:44:52PM -0700, Ethan Jackson wrote:
>> I don't feel particularly strongly about it. The current
>> implementation is fine with me.
>>
>> Ethan
>>
>> On Tue, Aug 7, 2012 at 12:40 PM, Ben Pfaff wrote:
>> > I think you and I might have slightly different models in mind. The
>> > model that I'm after at the moment is that the ofproto generic layer
>> > checks that a set of actions is valid (which can be done without
>> > knowing anything about the hardware) and then the implementation
>> > checks that it can actually implement those valid actions.
>> >
>> > I agree that the approach you suggest would work too, and that it
>> > could work with either the model I have in mind or a model where the
>> > implementation has to do all of the validation as well as the
>> > implement-ability checking.
>> >
>> > So, I'm inclined to leave it the way I have it, but I remain open to
>> > further discussion.
>> >
>> > On Mon, Jul 30, 2012 at 02:45:26PM -0700, Ethan Jackson wrote:
>> >> Looks good to me.
>> >>
>> >> I had a thought but I don't feel strongly about it. What if we made a
>> >> new hook for ofproto implementations:
>> >>
>> >> enum ofperr (*validate_actions)(const struct ofpact ofpacts[], size_t
>> >> ofpacts_len, const struct flow *, int max_ports).
>> >>
>> >> In the ofproto-dpif implementation this would simply pass through to
>> >> ofpacts_check(), but in other implementations it could reject action
>> >> lists which are not implementable by their dpif. This would allow us
>> >> to make *packet_out() and *rule_construct() assume that the actions
>> >> are both valid and implementable. Then they could return void instead
>> >> of an ofperr. Also if we did this, rule_modify_actions() may be
>> >> completely unnecessary. The complete_operation() calls could be
>> >> pulled into ofproto.
>> >>
>> >> At any rate, this seems marginally cleaner to me, but you may feel
>> >> strongly in the other direction in which case the current approach is
>> >> fine.
>> >>
>> >> Ethan
>> >>
>> >> On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
>> >> > Signed-off-by: Ben Pfaff
>> >> > ---
>> >> > ofproto/ofproto-dpif.c | 58
>> >> > ++-
>> >> > ofproto/ofproto-provider.h | 14 ---
>> >> > ofproto/ofproto.c | 16 +---
>> >> > 3 files changed, 36 insertions(+), 52 deletions(-)
>> >> >
>> >> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> >> > index f1d2ae0..44cbd17 100644
>> >> > --- a/ofproto/ofproto-dpif.c
>> >> > +++ b/ofproto/ofproto-dpif.c
>> >> > @@ -4592,13 +4592,6 @@ rule_construct(struct rule *rule_)
>> >> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
>> >> > struct rule_dpif *victim;
>> >> > uint8_t table_id;
>> >> > -enum ofperr error;
>> >> > -
>> >> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
>> >> > - &rule->up.cr.flow, ofproto->up.max_ports);
>> >> > -if (error) {
>> >> > -return error;
>> >> > -}
>> >> >
>> >> > rule->packet_count = 0;
>> >> > rule->byte_count = 0;
>> >> > @@ -4701,15 +4694,6 @@ static void
>> >> > rule_modify_actions(struct rule *rule_)
>> >> > {
>> >> > struct rule_dpif *rule = rule_dpif_cast(rule_);
>> >> > -struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
>> >> > -enum ofperr error;
>> >> > -
>> >> > -error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
>> >> > - &rule->up.cr.flow, ofproto->up.max_ports);
>> >> > -if (error) {
>> >> > -ofoperation_complete(rule->up.pending, error);
>> >> > -return;
>> >> > -}
>> >> >
>> >> > complete_operation(rule);
>> >> > }
>> >> > @@ -6359,36 +6343,32 @@ packet_out(struct ofproto *ofproto_, struct
>> >> > ofpbuf *packet,
>> >> > const struct ofpact *ofpacts, size_t ofpacts_len)
>> >> > {
>> >> > struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
>> >> > -enum ofperr error;
>> >> > +struct odputil_keybuf keybuf;
>> >> > +struct dpif_flow_stats stats;
>> >> >
>> >> > -error = ofpacts_check(ofpacts, ofpacts_len, flow,
>> >> > ofproto->up.max_ports);
>> >> > -if (!error) {
>> >> > -struct odputil_keybuf keybuf;
>> >> > -struct dpif_flow_stats stats;
>> >> > +struct ofpbuf key;
>> >> >
>> >> > -struct ofpbuf key;
>> >> > +struct action_xlate_ctx ctx;
>> >> > +uint64_t odp_actions_stub[1024 / 8];
>> >> > +struct ofpbuf odp_actions;
>> >> >
>> >> > -struct action_xl
Re: [ovs-dev] [classifier-opt 10/28] ofproto: Move ofpacts_check() calls from ofproto-dpif to ofproto.
On Tue, Aug 07, 2012 at 12:53:52PM -0700, Ethan Jackson wrote: > > Perhaps when (if?) someone submits another ofproto provider to master, > > we can figure out what best suits the set of providers as a whole. > > Yah it's hard to get the abstraction right when we only have deep > knowledge of one implementation. Hopefully a significant open source > secondary implementation will pop up and we will learn a lot from it. Perhaps someone could write a provider for, for example, the Intel FM2112 chip, for which full register-level specs are published. But I'm not holding my breath. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 12/28] flow: Use bit-mask for DSCP and ECN bits, instead of FWW_* flags.
I think you might be right about which way is a little prettier, but
all this code gets deleted a few commits later so I don't think I'm
going to tweak the style.
Thanks,
Ben.
On Mon, Jul 30, 2012 at 03:12:37PM -0700, Ethan Jackson wrote:
> In flow_wildcards_combine(), does it make sense to move the new line
> above the eth_addr_bitands() so it's with the similar code? Totally
> aesthetic, so it doesn't matter much. Same in flow_wildcards_hash().
>
> Looks like a big win to me, thanks.
> Ethan
>
> On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/classifier.c| 11 +--
> > lib/flow.c | 24
> > lib/flow.h | 10 --
> > lib/meta-flow.c | 24
> > lib/nx-match.c |4 ++--
> > lib/ofp-util.c | 16
> > tests/test-classifier.c |7 +--
> > 7 files changed, 52 insertions(+), 44 deletions(-)
> >
> > diff --git a/lib/classifier.c b/lib/classifier.c
> > index 33245ae..446ca4e 100644
> > --- a/lib/classifier.c
> > +++ b/lib/classifier.c
> > @@ -362,7 +362,7 @@ cls_rule_set_nw_dst_masked(struct cls_rule *rule,
> > ovs_be32 ip, ovs_be32 mask)
> > void
> > cls_rule_set_nw_dscp(struct cls_rule *rule, uint8_t nw_dscp)
> > {
> > -rule->wc.wildcards &= ~FWW_NW_DSCP;
> > +rule->wc.nw_tos_mask |= IP_DSCP_MASK;
> > rule->flow.nw_tos &= ~IP_DSCP_MASK;
> > rule->flow.nw_tos |= nw_dscp & IP_DSCP_MASK;
> > }
> > @@ -370,7 +370,7 @@ cls_rule_set_nw_dscp(struct cls_rule *rule, uint8_t
> > nw_dscp)
> > void
> > cls_rule_set_nw_ecn(struct cls_rule *rule, uint8_t nw_ecn)
> > {
> > -rule->wc.wildcards &= ~FWW_NW_ECN;
> > +rule->wc.nw_tos_mask |= IP_ECN_MASK;
> > rule->flow.nw_tos &= ~IP_ECN_MASK;
> > rule->flow.nw_tos |= nw_ecn & IP_ECN_MASK;
> > }
> > @@ -723,10 +723,10 @@ cls_rule_format(const struct cls_rule *rule, struct
> > ds *s)
> > format_eth_masked(s, "arp_sha", f->arp_sha, wc->arp_sha_mask);
> > format_eth_masked(s, "arp_tha", f->arp_tha, wc->arp_tha_mask);
> > }
> > -if (!(w & FWW_NW_DSCP)) {
> > +if (wc->nw_tos_mask & IP_DSCP_MASK) {
> > ds_put_format(s, "nw_tos=%"PRIu8",", f->nw_tos & IP_DSCP_MASK);
> > }
> > -if (!(w & FWW_NW_ECN)) {
> > +if (wc->nw_tos_mask & IP_ECN_MASK) {
> > ds_put_format(s, "nw_ecn=%"PRIu8",", f->nw_tos & IP_ECN_MASK);
> > }
> > if (!(w & FWW_NW_TTL)) {
> > @@ -1289,8 +1289,7 @@ flow_equal_except(const struct flow *a, const struct
> > flow *b,
> > wildcards->dl_dst_mask)
> > && (wc & FWW_NW_PROTO || a->nw_proto == b->nw_proto)
> > && (wc & FWW_NW_TTL || a->nw_ttl == b->nw_ttl)
> > -&& (wc & FWW_NW_DSCP || !((a->nw_tos ^ b->nw_tos) &
> > IP_DSCP_MASK))
> > -&& (wc & FWW_NW_ECN || !((a->nw_tos ^ b->nw_tos) &
> > IP_ECN_MASK))
> > +&& !((a->nw_tos ^ b->nw_tos) & wildcards->nw_tos_mask)
> > && !((a->nw_frag ^ b->nw_frag) & wildcards->nw_frag_mask)
> > && eth_addr_equal_except(a->arp_sha, b->arp_sha,
> > wildcards->arp_sha_mask)
> > diff --git a/lib/flow.c b/lib/flow.c
> > index 222983a..c0d5a47 100644
> > --- a/lib/flow.c
> > +++ b/lib/flow.c
> > @@ -468,12 +468,7 @@ flow_zero_wildcards(struct flow *flow, const struct
> > flow_wildcards *wildcards)
> > flow->nw_proto = 0;
> > }
> > flow->ipv6_label &= wildcards->ipv6_label_mask;
> > -if (wc & FWW_NW_DSCP) {
> > -flow->nw_tos &= ~IP_DSCP_MASK;
> > -}
> > -if (wc & FWW_NW_ECN) {
> > -flow->nw_tos &= ~IP_ECN_MASK;
> > -}
> > +flow->nw_tos &= wildcards->nw_tos_mask;
> > if (wc & FWW_NW_TTL) {
> > flow->nw_ttl = 0;
> > }
> > @@ -607,7 +602,7 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc)
> > memset(wc->dl_dst_mask, 0, ETH_ADDR_LEN);
> > memset(wc->arp_sha_mask, 0, ETH_ADDR_LEN);
> > memset(wc->arp_tha_mask, 0, ETH_ADDR_LEN);
> > -memset(wc->zeros, 0, sizeof wc->zeros);
> > +wc->nw_tos_mask = 0;
> > }
> >
> > /* Initializes 'wc' as an exact-match set of wildcards; that is, 'wc' does
> > not
> > @@ -635,7 +630,7 @@ flow_wildcards_init_exact(struct flow_wildcards *wc)
> > memset(wc->dl_dst_mask, 0xff, ETH_ADDR_LEN);
> > memset(wc->arp_sha_mask, 0xff, ETH_ADDR_LEN);
> > memset(wc->arp_tha_mask, 0xff, ETH_ADDR_LEN);
> > -memset(wc->zeros, 0, sizeof wc->zeros);
> > +wc->nw_tos_mask = UINT8_MAX;
> > }
> >
> > /* Returns true if 'wc' is exact-match, false if 'wc' wildcards any bits or
> > @@ -663,7 +658,8 @@ flow_wildcards_is_exact(const struct flow_wildcards *wc)
> > || !ipv6_mask_is_exact(&wc->ipv6_dst_mask)
> > || wc->ipv6_label_mask != htonl(UINT32_MAX)
> > || !ipv6_mask_is_exact(&wc->nd_target_mask)
> > -
Re: [ovs-dev] [classifier-opt 19/28] flow: Replace flow_wildcards members by a single "struct flow".
On Mon, Jul 30, 2012 at 06:10:22PM -0700, Ethan Jackson wrote: > In flow_wildcards_init_catchall() and flow_wildcards_init_exact() why > not just memset? Perhaps the appropriate time to make that change > would have been when in_port was changed to a mask come to think of > it. It's fine to leave it if you're going to resolve it in a future > patch of the series. That (and other simplifications) happen in patch 21/28, so I guess I'll leave them there. > The indentation isn't quite right in flow_wildcards_combine(). It was > incorrect before this patch as well, but this may be a good time to > clean it up. OK, fixed. > I suspect you're going to switch flow_wildcards_equal() to using > memcmp() in a future patch? Yes. > Do we still need "struct flow_wildcards" at all? We could just use > struct flow directly. Again, perhaps this will make more sense once > I've seen the future patches. We don't need flow_wildcards. I kept it on the notion that it was useful to readers of code to be able to distinguish a flow from a set of wildcards for a flow, and useful from a type system perspective for the same reason. I may be wrong; I don't know. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 21/28] flow: Simplify many functions for working with flows and wildcards.
Good point, thanks, I've now removed the functions that are generic.
On Mon, Jul 30, 2012 at 06:43:20PM -0700, Ethan Jackson wrote:
> Oh forgot to mention. We should probably remove the FLOW_WC_SEQ build
> assertions from a lot of these functions as they no longer need to
> change when new fields are added.
>
> Ethan
>
> On Mon, Jul 30, 2012 at 6:42 PM, Ethan Jackson wrote:
> > Looks good thanks.
> >
> > Ethan
> >
> > On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
> >> Now that "struct flow" and "struct flow_wildcards" have the same simple
> >> and uniform structure, it's easy to handle common operations by just
> >> iterating over the bits inside them.
> >>
> >> Signed-off-by: Ben Pfaff
> >> ---
> >> lib/classifier.c | 71 ---
> >> lib/flow.c | 262
> >> +-
> >> lib/flow.h |7 ++-
> >> 3 files changed, 48 insertions(+), 292 deletions(-)
> >>
> >> diff --git a/lib/classifier.c b/lib/classifier.c
> >> index 486b3d5..88c8e1a 100644
> >> --- a/lib/classifier.c
> >> +++ b/lib/classifier.c
> >> @@ -40,9 +40,6 @@ static struct cls_rule *find_equal(struct cls_table *,
> >> const struct flow *,
> >> uint32_t hash);
> >> static struct cls_rule *insert_rule(struct cls_table *, struct cls_rule
> >> *);
> >>
> >> -static bool flow_equal_except(const struct flow *, const struct flow *,
> >> -const struct flow_wildcards *);
> >> -
> >> /* Iterates RULE over HEAD and all of the cls_rules on HEAD->list. */
> >> #define FOR_EACH_RULE_IN_LIST(RULE, HEAD) \
> >> for ((RULE) = (HEAD); (RULE) != NULL; (RULE) =
> >> next_rule_in_list(RULE))
> >> @@ -1233,71 +1230,3 @@ next_rule_in_list(struct cls_rule *rule)
> >> struct cls_rule *next = next_rule_in_list__(rule);
> >> return next->priority < rule->priority ? next : NULL;
> >> }
> >> -
> >> -static bool
> >> -ipv6_equal_except(const struct in6_addr *a, const struct in6_addr *b,
> >> - const struct in6_addr *mask)
> >> -{
> >> -int i;
> >> -
> >> -#ifdef s6_addr32
> >> -for (i=0; i<4; i++) {
> >> -if ((a->s6_addr32[i] ^ b->s6_addr32[i]) & mask->s6_addr32[i]) {
> >> -return false;
> >> -}
> >> -}
> >> -#else
> >> -for (i=0; i<16; i++) {
> >> -if ((a->s6_addr[i] ^ b->s6_addr[i]) & mask->s6_addr[i]) {
> >> -return false;
> >> -}
> >> -}
> >> -#endif
> >> -
> >> -return true;
> >> -}
> >> -
> >> -
> >> -static bool
> >> -flow_equal_except(const struct flow *a, const struct flow *b,
> >> - const struct flow_wildcards *wildcards)
> >> -{
> >> -int i;
> >> -
> >> -BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
> >> -
> >> -for (i = 0; i < FLOW_N_REGS; i++) {
> >> -if ((a->regs[i] ^ b->regs[i]) & wildcards->masks.regs[i]) {
> >> -return false;
> >> -}
> >> -}
> >> -
> >> -return (!((a->tun_id ^ b->tun_id) & wildcards->masks.tun_id)
> >> -&& !((a->metadata ^ b->metadata) & wildcards->masks.metadata)
> >> -&& !((a->nw_src ^ b->nw_src) & wildcards->masks.nw_src)
> >> -&& !((a->nw_dst ^ b->nw_dst) & wildcards->masks.nw_dst)
> >> -&& !((a->in_port ^ b->in_port) & wildcards->masks.in_port)
> >> -&& !((a->vlan_tci ^ b->vlan_tci) & wildcards->masks.vlan_tci)
> >> -&& !((a->dl_type ^ b->dl_type) & wildcards->masks.dl_type)
> >> -&& !((a->tp_src ^ b->tp_src) & wildcards->masks.tp_src)
> >> -&& !((a->tp_dst ^ b->tp_dst) & wildcards->masks.tp_dst)
> >> -&& eth_addr_equal_except(a->dl_src, b->dl_src,
> >> - wildcards->masks.dl_src)
> >> -&& eth_addr_equal_except(a->dl_dst, b->dl_dst,
> >> - wildcards->masks.dl_dst)
> >> -&& !((a->nw_proto ^ b->nw_proto) & wildcards->masks.nw_proto)
> >> -&& !((a->nw_ttl ^ b->nw_ttl) & wildcards->masks.nw_ttl)
> >> -&& !((a->nw_tos ^ b->nw_tos) & wildcards->masks.nw_tos)
> >> -&& !((a->nw_frag ^ b->nw_frag) & wildcards->masks.nw_frag)
> >> -&& eth_addr_equal_except(a->arp_sha, b->arp_sha,
> >> - wildcards->masks.arp_sha)
> >> -&& eth_addr_equal_except(a->arp_tha, b->arp_tha,
> >> - wildcards->masks.arp_tha)
> >> -&& !((a->ipv6_label ^ b->ipv6_label) &
> >> wildcards->masks.ipv6_label)
> >> -&& ipv6_equal_except(&a->ipv6_src, &b->ipv6_src,
> >> -&wildcards->masks.ipv6_src)
> >> -&& ipv6_equal_except(&a->ipv6_dst, &b->ipv6_dst,
> >> -&wildcards->masks.ipv6_dst)
> >> -&& ipv6_equal_except(&a->nd_target, &b->nd_target,
> >> - &wildcards->masks.nd_target));
> >> -
Re: [ovs-dev] [classifier-opt 12/28] flow: Use bit-mask for DSCP and ECN bits, instead of FWW_* flags.
Sounds good.
Ethan
On Tue, Aug 7, 2012 at 1:31 PM, Ben Pfaff wrote:
> I think you might be right about which way is a little prettier, but
> all this code gets deleted a few commits later so I don't think I'm
> going to tweak the style.
>
> Thanks,
>
> Ben.
>
> On Mon, Jul 30, 2012 at 03:12:37PM -0700, Ethan Jackson wrote:
>> In flow_wildcards_combine(), does it make sense to move the new line
>> above the eth_addr_bitands() so it's with the similar code? Totally
>> aesthetic, so it doesn't matter much. Same in flow_wildcards_hash().
>>
>> Looks like a big win to me, thanks.
>> Ethan
>>
>> On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
>> > Signed-off-by: Ben Pfaff
>> > ---
>> > lib/classifier.c| 11 +--
>> > lib/flow.c | 24
>> > lib/flow.h | 10 --
>> > lib/meta-flow.c | 24
>> > lib/nx-match.c |4 ++--
>> > lib/ofp-util.c | 16
>> > tests/test-classifier.c |7 +--
>> > 7 files changed, 52 insertions(+), 44 deletions(-)
>> >
>> > diff --git a/lib/classifier.c b/lib/classifier.c
>> > index 33245ae..446ca4e 100644
>> > --- a/lib/classifier.c
>> > +++ b/lib/classifier.c
>> > @@ -362,7 +362,7 @@ cls_rule_set_nw_dst_masked(struct cls_rule *rule,
>> > ovs_be32 ip, ovs_be32 mask)
>> > void
>> > cls_rule_set_nw_dscp(struct cls_rule *rule, uint8_t nw_dscp)
>> > {
>> > -rule->wc.wildcards &= ~FWW_NW_DSCP;
>> > +rule->wc.nw_tos_mask |= IP_DSCP_MASK;
>> > rule->flow.nw_tos &= ~IP_DSCP_MASK;
>> > rule->flow.nw_tos |= nw_dscp & IP_DSCP_MASK;
>> > }
>> > @@ -370,7 +370,7 @@ cls_rule_set_nw_dscp(struct cls_rule *rule, uint8_t
>> > nw_dscp)
>> > void
>> > cls_rule_set_nw_ecn(struct cls_rule *rule, uint8_t nw_ecn)
>> > {
>> > -rule->wc.wildcards &= ~FWW_NW_ECN;
>> > +rule->wc.nw_tos_mask |= IP_ECN_MASK;
>> > rule->flow.nw_tos &= ~IP_ECN_MASK;
>> > rule->flow.nw_tos |= nw_ecn & IP_ECN_MASK;
>> > }
>> > @@ -723,10 +723,10 @@ cls_rule_format(const struct cls_rule *rule, struct
>> > ds *s)
>> > format_eth_masked(s, "arp_sha", f->arp_sha, wc->arp_sha_mask);
>> > format_eth_masked(s, "arp_tha", f->arp_tha, wc->arp_tha_mask);
>> > }
>> > -if (!(w & FWW_NW_DSCP)) {
>> > +if (wc->nw_tos_mask & IP_DSCP_MASK) {
>> > ds_put_format(s, "nw_tos=%"PRIu8",", f->nw_tos & IP_DSCP_MASK);
>> > }
>> > -if (!(w & FWW_NW_ECN)) {
>> > +if (wc->nw_tos_mask & IP_ECN_MASK) {
>> > ds_put_format(s, "nw_ecn=%"PRIu8",", f->nw_tos & IP_ECN_MASK);
>> > }
>> > if (!(w & FWW_NW_TTL)) {
>> > @@ -1289,8 +1289,7 @@ flow_equal_except(const struct flow *a, const struct
>> > flow *b,
>> > wildcards->dl_dst_mask)
>> > && (wc & FWW_NW_PROTO || a->nw_proto == b->nw_proto)
>> > && (wc & FWW_NW_TTL || a->nw_ttl == b->nw_ttl)
>> > -&& (wc & FWW_NW_DSCP || !((a->nw_tos ^ b->nw_tos) &
>> > IP_DSCP_MASK))
>> > -&& (wc & FWW_NW_ECN || !((a->nw_tos ^ b->nw_tos) &
>> > IP_ECN_MASK))
>> > +&& !((a->nw_tos ^ b->nw_tos) & wildcards->nw_tos_mask)
>> > && !((a->nw_frag ^ b->nw_frag) & wildcards->nw_frag_mask)
>> > && eth_addr_equal_except(a->arp_sha, b->arp_sha,
>> > wildcards->arp_sha_mask)
>> > diff --git a/lib/flow.c b/lib/flow.c
>> > index 222983a..c0d5a47 100644
>> > --- a/lib/flow.c
>> > +++ b/lib/flow.c
>> > @@ -468,12 +468,7 @@ flow_zero_wildcards(struct flow *flow, const struct
>> > flow_wildcards *wildcards)
>> > flow->nw_proto = 0;
>> > }
>> > flow->ipv6_label &= wildcards->ipv6_label_mask;
>> > -if (wc & FWW_NW_DSCP) {
>> > -flow->nw_tos &= ~IP_DSCP_MASK;
>> > -}
>> > -if (wc & FWW_NW_ECN) {
>> > -flow->nw_tos &= ~IP_ECN_MASK;
>> > -}
>> > +flow->nw_tos &= wildcards->nw_tos_mask;
>> > if (wc & FWW_NW_TTL) {
>> > flow->nw_ttl = 0;
>> > }
>> > @@ -607,7 +602,7 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc)
>> > memset(wc->dl_dst_mask, 0, ETH_ADDR_LEN);
>> > memset(wc->arp_sha_mask, 0, ETH_ADDR_LEN);
>> > memset(wc->arp_tha_mask, 0, ETH_ADDR_LEN);
>> > -memset(wc->zeros, 0, sizeof wc->zeros);
>> > +wc->nw_tos_mask = 0;
>> > }
>> >
>> > /* Initializes 'wc' as an exact-match set of wildcards; that is, 'wc'
>> > does not
>> > @@ -635,7 +630,7 @@ flow_wildcards_init_exact(struct flow_wildcards *wc)
>> > memset(wc->dl_dst_mask, 0xff, ETH_ADDR_LEN);
>> > memset(wc->arp_sha_mask, 0xff, ETH_ADDR_LEN);
>> > memset(wc->arp_tha_mask, 0xff, ETH_ADDR_LEN);
>> > -memset(wc->zeros, 0, sizeof wc->zeros);
>> > +wc->nw_tos_mask = UINT8_MAX;
>> > }
>> >
>> > /* Returns true if 'wc' is exact-match, false if 'wc' wildcards any bits
>> > or
>> > @@ -663,7 +658,8 @@ flow_wildcards_is_exact(const struct
Re: [ovs-dev] [classifier-opt 19/28] flow: Replace flow_wildcards members by a single "struct flow".
Sounds reasonable. Ethan On Tue, Aug 7, 2012 at 1:41 PM, Ben Pfaff wrote: > On Mon, Jul 30, 2012 at 06:10:22PM -0700, Ethan Jackson wrote: >> In flow_wildcards_init_catchall() and flow_wildcards_init_exact() why >> not just memset? Perhaps the appropriate time to make that change >> would have been when in_port was changed to a mask come to think of >> it. It's fine to leave it if you're going to resolve it in a future >> patch of the series. > > That (and other simplifications) happen in patch 21/28, so I guess > I'll leave them there. > >> The indentation isn't quite right in flow_wildcards_combine(). It was >> incorrect before this patch as well, but this may be a good time to >> clean it up. > > OK, fixed. > >> I suspect you're going to switch flow_wildcards_equal() to using >> memcmp() in a future patch? > > Yes. > >> Do we still need "struct flow_wildcards" at all? We could just use >> struct flow directly. Again, perhaps this will make more sense once >> I've seen the future patches. > > We don't need flow_wildcards. I kept it on the notion that it was > useful to readers of code to be able to distinguish a flow from a set > of wildcards for a flow, and useful from a type system perspective for > the same reason. I may be wrong; I don't know. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 19/28] flow: Replace flow_wildcards members by a single "struct flow".
One more thought: it's pretty easy to get rid of flow_wildcards later if we don't want it; it's harder to reintroduce it. On Tue, Aug 07, 2012 at 01:45:04PM -0700, Ethan Jackson wrote: > Sounds reasonable. > > Ethan > > On Tue, Aug 7, 2012 at 1:41 PM, Ben Pfaff wrote: > > On Mon, Jul 30, 2012 at 06:10:22PM -0700, Ethan Jackson wrote: > >> In flow_wildcards_init_catchall() and flow_wildcards_init_exact() why > >> not just memset? Perhaps the appropriate time to make that change > >> would have been when in_port was changed to a mask come to think of > >> it. It's fine to leave it if you're going to resolve it in a future > >> patch of the series. > > > > That (and other simplifications) happen in patch 21/28, so I guess > > I'll leave them there. > > > >> The indentation isn't quite right in flow_wildcards_combine(). It was > >> incorrect before this patch as well, but this may be a good time to > >> clean it up. > > > > OK, fixed. > > > >> I suspect you're going to switch flow_wildcards_equal() to using > >> memcmp() in a future patch? > > > > Yes. > > > >> Do we still need "struct flow_wildcards" at all? We could just use > >> struct flow directly. Again, perhaps this will make more sense once > >> I've seen the future patches. > > > > We don't need flow_wildcards. I kept it on the notion that it was > > useful to readers of code to be able to distinguish a flow from a set > > of wildcards for a flow, and useful from a type system perspective for > > the same reason. I may be wrong; I don't know. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 19/28] flow: Replace flow_wildcards members by a single "struct flow".
> One more thought: it's pretty easy to get rid of flow_wildcards later > if we don't want it; it's harder to reintroduce it. Yep Yep, don't care that much either way, just wondered if you thought about it. You obviously have, so lets leave it for now. Ethan > > On Tue, Aug 07, 2012 at 01:45:04PM -0700, Ethan Jackson wrote: >> Sounds reasonable. >> >> Ethan >> >> On Tue, Aug 7, 2012 at 1:41 PM, Ben Pfaff wrote: >> > On Mon, Jul 30, 2012 at 06:10:22PM -0700, Ethan Jackson wrote: >> >> In flow_wildcards_init_catchall() and flow_wildcards_init_exact() why >> >> not just memset? Perhaps the appropriate time to make that change >> >> would have been when in_port was changed to a mask come to think of >> >> it. It's fine to leave it if you're going to resolve it in a future >> >> patch of the series. >> > >> > That (and other simplifications) happen in patch 21/28, so I guess >> > I'll leave them there. >> > >> >> The indentation isn't quite right in flow_wildcards_combine(). It was >> >> incorrect before this patch as well, but this may be a good time to >> >> clean it up. >> > >> > OK, fixed. >> > >> >> I suspect you're going to switch flow_wildcards_equal() to using >> >> memcmp() in a future patch? >> > >> > Yes. >> > >> >> Do we still need "struct flow_wildcards" at all? We could just use >> >> struct flow directly. Again, perhaps this will make more sense once >> >> I've seen the future patches. >> > >> > We don't need flow_wildcards. I kept it on the notion that it was >> > useful to readers of code to be able to distinguish a flow from a set >> > of wildcards for a flow, and useful from a type system perspective for >> > the same reason. I may be wrong; I don't know. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 23/28] util: New function popcount().
On Tue, Jul 31, 2012 at 10:40:13AM -0700, Ethan Jackson wrote:
> Oh one more thing:
>
> This seems like the kind of thing that's both easy to get wrong, and
> easy to unit test. Does it make sense to add one?
Yes, I folded this in, thanks:
diff --git a/tests/library.at b/tests/library.at
index 70660a2..0765c3f 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -103,12 +103,14 @@ AT_CLEANUP
m4_foreach(
[testname],
[[ctz],
+ [popcount],
[log_2_floor],
[bitwise_copy],
[bitwise_zero],
[bitwise_one],
[bitwise_is_all_zeros]],
[AT_SETUP([testname[()] function])
+ AT_KEYWORDS([testname])
AT_CHECK([test-util testname], [0], [], [])
AT_CLEANUP])
diff --git a/tests/test-util.c b/tests/test-util.c
index 71a7f21..b98fc79 100644
--- a/tests/test-util.c
+++ b/tests/test-util.c
@@ -26,6 +26,9 @@
#include "random.h"
#include "util.h"
+#undef NDEBUG
+#include
+
static void
check_log_2_floor(uint32_t x, int n)
{
@@ -85,6 +88,59 @@ test_ctz(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
check_ctz(0, 32);
}
+static void
+shuffle(unsigned int *p, size_t n)
+{
+for (; n > 1; n--, p++) {
+unsigned int *q = &p[rand() % n];
+unsigned int tmp = *p;
+*p = *q;
+*q = tmp;
+}
+}
+
+static void
+check_popcount(uint32_t x, int n)
+{
+if (popcount(x) != n) {
+fprintf(stderr, "popcount(%#"PRIx32") is %d but should be %d\n",
+x, popcount(x), n);
+abort();
+}
+}
+
+static void
+test_popcount(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
+{
+unsigned int bits[32];
+int i;
+
+for (i = 0; i < ARRAY_SIZE(bits); i++) {
+bits[i] = 1u << i;
+}
+
+check_popcount(0, 0);
+
+for (i = 0; i < 1000; i++) {
+uint32_t x = 0;
+int j;
+
+shuffle(bits, ARRAY_SIZE(bits));
+for (j = 0; j < 32; j++) {
+x |= bits[j];
+check_popcount(x, j + 1);
+}
+assert(x == UINT32_MAX);
+
+shuffle(bits, ARRAY_SIZE(bits));
+for (j = 31; j >= 0; j--) {
+x &= ~bits[j];
+check_popcount(x, j);
+}
+assert(x == 0);
+}
+}
+
/* Returns the sum of the squares of the first 'n' positive integers. */
static unsigned int
sum_of_squares(int n)
@@ -282,6 +338,7 @@ test_follow_symlinks(int argc, char *argv[])
static const struct command commands[] = {
{"ctz", 0, 0, test_ctz},
+{"popcount", 0, 0, test_popcount},
{"log_2_floor", 0, 0, test_log_2_floor},
{"bitwise_copy", 0, 0, test_bitwise_copy},
{"bitwise_zero", 0, 0, test_bitwise_zero},
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 23/28] util: New function popcount().
Awesome, thanks!
Ethan
On Tue, Aug 7, 2012 at 2:06 PM, Ben Pfaff wrote:
> On Tue, Jul 31, 2012 at 10:40:13AM -0700, Ethan Jackson wrote:
>> Oh one more thing:
>>
>> This seems like the kind of thing that's both easy to get wrong, and
>> easy to unit test. Does it make sense to add one?
>
> Yes, I folded this in, thanks:
>
> diff --git a/tests/library.at b/tests/library.at
> index 70660a2..0765c3f 100644
> --- a/tests/library.at
> +++ b/tests/library.at
> @@ -103,12 +103,14 @@ AT_CLEANUP
> m4_foreach(
>[testname],
>[[ctz],
> + [popcount],
> [log_2_floor],
> [bitwise_copy],
> [bitwise_zero],
> [bitwise_one],
> [bitwise_is_all_zeros]],
>[AT_SETUP([testname[()] function])
> + AT_KEYWORDS([testname])
> AT_CHECK([test-util testname], [0], [], [])
> AT_CLEANUP])
>
> diff --git a/tests/test-util.c b/tests/test-util.c
> index 71a7f21..b98fc79 100644
> --- a/tests/test-util.c
> +++ b/tests/test-util.c
> @@ -26,6 +26,9 @@
> #include "random.h"
> #include "util.h"
>
> +#undef NDEBUG
> +#include
> +
> static void
> check_log_2_floor(uint32_t x, int n)
> {
> @@ -85,6 +88,59 @@ test_ctz(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> check_ctz(0, 32);
> }
>
> +static void
> +shuffle(unsigned int *p, size_t n)
> +{
> +for (; n > 1; n--, p++) {
> +unsigned int *q = &p[rand() % n];
> +unsigned int tmp = *p;
> +*p = *q;
> +*q = tmp;
> +}
> +}
> +
> +static void
> +check_popcount(uint32_t x, int n)
> +{
> +if (popcount(x) != n) {
> +fprintf(stderr, "popcount(%#"PRIx32") is %d but should be %d\n",
> +x, popcount(x), n);
> +abort();
> +}
> +}
> +
> +static void
> +test_popcount(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> +{
> +unsigned int bits[32];
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(bits); i++) {
> +bits[i] = 1u << i;
> +}
> +
> +check_popcount(0, 0);
> +
> +for (i = 0; i < 1000; i++) {
> +uint32_t x = 0;
> +int j;
> +
> +shuffle(bits, ARRAY_SIZE(bits));
> +for (j = 0; j < 32; j++) {
> +x |= bits[j];
> +check_popcount(x, j + 1);
> +}
> +assert(x == UINT32_MAX);
> +
> +shuffle(bits, ARRAY_SIZE(bits));
> +for (j = 31; j >= 0; j--) {
> +x &= ~bits[j];
> +check_popcount(x, j);
> +}
> +assert(x == 0);
> +}
> +}
> +
> /* Returns the sum of the squares of the first 'n' positive integers. */
> static unsigned int
> sum_of_squares(int n)
> @@ -282,6 +338,7 @@ test_follow_symlinks(int argc, char *argv[])
>
> static const struct command commands[] = {
> {"ctz", 0, 0, test_ctz},
> +{"popcount", 0, 0, test_popcount},
> {"log_2_floor", 0, 0, test_log_2_floor},
> {"bitwise_copy", 0, 0, test_bitwise_copy},
> {"bitwise_zero", 0, 0, test_bitwise_zero},
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 25/28] classifier: Break cls_rule 'flow' and 'wc' members into new "struct match".
On Mon, Aug 06, 2012 at 02:29:30PM -0700, Ethan Jackson wrote: > lib/match.c: > s/clr_match/match Thanks, fixed. > match_set_metadata_masked()'s prototype is too long by a character. Oops, I've broken the line now. > lib/ofp-util.c: > It's strange to me that ofputil_match_from_ofp10_match() has to take a > priority at all. Only a couple of callers use the returned > value. I would be inclined to remove the priority argument and have > it return nothing. The affected callers could either do the > calculation by hand, or we could give them a separate helper that > takes an ofp10_match and a priority and returns a priority. If you > decide to leave it as is, please documente the return value. You're right, this is a wart. I dropped the parameter and return value from ofputil_match_from_ofp10_match(). After some thought, it seemed that I only needed the priority calculation in one place (in ofputil_decode_flow_mod()) so I inlined it there. > The indentation is incorrect in ofputil_match_from_ofp11_match(). OK, fixed. > Everything else looks good, thanks. Thank you. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 25/28] classifier: Break cls_rule 'flow' and 'wc' members into new "struct match".
> I dropped the parameter and return value from > ofputil_match_from_ofp10_match(). After some thought, it seemed that > I only needed the priority calculation in one place (in > ofputil_decode_flow_mod()) so I inlined it there. The other caller didn't need to calculate it? Ethan > >> The indentation is incorrect in ofputil_match_from_ofp11_match(). > > OK, fixed. > >> Everything else looks good, thanks. > > Thank you. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 06/41] ofp-actions: Return action size
Modify ofpacts_put_openflow11_actions() to return the length of actions
appended. This will be used when encoding Packet Out messages for
Open Flow 1.1 and 1.2. The motivation for this is to avoid open coding
the size calculation which may end up being needed elsewhere too.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
lib/ofp-actions.c | 5 -
lib/ofp-actions.h | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 0874cc4..11642d1 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1384,15 +1384,18 @@ ofpact_to_openflow11(const struct ofpact *a, struct
ofpbuf *out)
/* Converts the ofpacts in 'ofpacts' (terminated by OFPACT_END) into OpenFlow
* 1.1 actions in 'openflow', appending the actions to any existing data in
* 'openflow'. */
-void
+size_t
ofpacts_put_openflow11_actions(const struct ofpact ofpacts[],
size_t ofpacts_len, struct ofpbuf *openflow)
{
const struct ofpact *a;
+size_t start_size = openflow->size;
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
ofpact_to_openflow11(a, openflow);
}
+
+return openflow->size - start_size;
}
void
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 7c9cb05..d5497a8 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -396,8 +396,8 @@ enum ofperr ofpacts_check(const struct ofpact[], size_t
ofpacts_len,
/* Converting ofpacts to OpenFlow. */
void ofpacts_put_openflow10(const struct ofpact[], size_t ofpacts_len,
struct ofpbuf *openflow);
-void ofpacts_put_openflow11_actions(const struct ofpact[], size_t ofpacts_len,
-struct ofpbuf *openflow);
+size_t ofpacts_put_openflow11_actions(const struct ofpact[], size_t
ofpacts_len,
+ struct ofpbuf *openflow);
void ofpacts_put_openflow11_instructions(const struct ofpact[],
size_t ofpacts_len,
struct ofpbuf *openflow);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 02/41] ofp-print: Open Flow 1.2 Flow Mod message tests
Signed-off-by: Simon Horman --- v10 * Remove non-test portions as they have already been merged. Ben, did you intentionally omit the tests? If so, feel free to drop this patch. * Rename from "ofp-util: Allow encoding of Open Flow 1.2 Flow Mod Messages" to "ofp-print: Open Flow 1.2 Flow Mod message tests" v9 * No change v8 * Manual rebase * Add ofp-print test v7 * Manual rebase v6 * No change v5 * Manual rebase v4 * No change v3 * No change v2 * No change --- tests/ofp-print.at | 42 -- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 79516e0..05e6cac 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -407,7 +407,7 @@ OFPT_PACKET_OUT (xid=0x0): in_port=1 actions=output:3 buffer=0x0114 AT_CLEANUP # The flow is formatted with cls_rule_format() for the low-verbosity case. -AT_SETUP([OFPT_FLOW_MOD - low verbosity]) +AT_SETUP([OFPT_FLOW_MOD - OF1.0 - low verbosity]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\ 01 0e 00 50 00 00 00 00 00 00 00 00 00 01 50 54 \ @@ -424,9 +424,28 @@ ofp_util|INFO|post: priority=65535,arp,in_port=1,vlan_tci=0x,dl_src=50:54:00 ]) AT_CLEANUP +# The flow is formatted with cls_rule_format() for the low-verbosity case. +AT_SETUP([OFPT_FLOW_MOD - OF1.2 - low verbosity]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\ +03 0e 00 90 00 00 00 02 00 00 00 00 00 00 00 00 \ +00 00 00 00 00 00 00 00 ff 00 00 00 00 00 ff ff \ +ff ff ff ff ff ff ff ff ff ff ff ff 00 00 00 00 \ +00 01 00 42 80 00 00 04 00 00 00 01 80 00 08 06 \ +50 54 00 00 00 06 80 00 06 06 50 54 00 00 00 05 \ +80 00 0a 02 08 06 80 00 0c 02 00 00 80 00 2a 02 \ +00 02 80 00 2c 04 c0 a8 00 02 80 00 2e 04 c0 a8 \ +00 01 00 00 00 00 00 00 00 04 00 18 00 00 00 00 \ +00 00 00 10 00 00 00 03 00 00 00 00 00 00 00 00 \ +" 2], [0], [dnl +OFPT_FLOW_MOD (OF1.2) (xid=0x2): ADD table:255 priority=65535,arp,in_port=1,vlan_tci=0x/0x1fff,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,arp_op=2 actions=output:3 +], [dnl +]) +AT_CLEANUP + # The flow is formatted with ofp10_match_to_string() for the # high-verbosity case. -AT_SETUP([OFPT_FLOW_MOD - high verbosity]) +AT_SETUP([OFPT_FLOW_MOD - OF1.0 - high verbosity]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\ 01 0e 00 50 00 00 00 00 00 00 00 00 00 01 50 54 \ @@ -443,6 +462,25 @@ ofp_util|INFO|post: priority=65535,arp,in_port=1,vlan_tci=0x,dl_src=50:54:00 ]) AT_CLEANUP +# The flow is formatted with cls_rule_format() for the low-verbosity case. +AT_SETUP([OFPT_FLOW_MOD - OF1.2 - low verbosity]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\ +03 0e 00 90 00 00 00 02 00 00 00 00 00 00 00 00 \ +00 00 00 00 00 00 00 00 ff 00 00 00 00 00 ff ff \ +ff ff ff ff ff ff ff ff ff ff ff ff 00 00 00 00 \ +00 01 00 42 80 00 00 04 00 00 00 01 80 00 08 06 \ +50 54 00 00 00 06 80 00 06 06 50 54 00 00 00 05 \ +80 00 0a 02 08 06 80 00 0c 02 00 00 80 00 2a 02 \ +00 02 80 00 2c 04 c0 a8 00 02 80 00 2e 04 c0 a8 \ +00 01 00 00 00 00 00 00 00 04 00 18 00 00 00 00 \ +00 00 00 10 00 00 00 03 00 00 00 00 00 00 00 00 \ +" 2], [0], [dnl +OFPT_FLOW_MOD (OF1.2) (xid=0x2): ADD table:255 priority=65535,arp,in_port=1,vlan_tci=0x/0x1fff,dl_src=50:54:00:00:00:06,dl_dst=50:54:00:00:00:05,nw_src=192.168.0.2,nw_dst=192.168.0.1,arp_op=2 actions=output:3 +], [dnl +]) +AT_CLEANUP + AT_SETUP([OFPT_PORT_MOD - OF1.0]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ -- 1.7.10.2.484.gcd07cc5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 03/41] ofp-util: Allow encoding of Open Flow 1.2 Packet In Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Manual Rebase
v7
* Manual Rebase
* Use struct ofp12_packet_in instead of struct ofp11_packet_in
v6
* No change
v5
* No change
v4
* No change
v3
* Add protocol parameter to ofputil_encode_packet_in().
This allows packet_in_format to be ignored for Open Flow 1.2.
v2
* Update for new ofputil_put_match() implementation
---
lib/ofp-msgs.h| 3 +++
lib/ofp-util.c| 58 +--
lib/ofp-util.h| 1 +
ofproto/connmgr.c | 3 ++-
4 files changed, 49 insertions(+), 16 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 5ff5976..1639cd9 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -127,6 +127,8 @@ enum ofpraw {
OFPRAW_OFPT10_PACKET_IN,
/* OFPT 1.1 (10): struct ofp11_packet_in up to data, uint8_t[]. */
OFPRAW_OFPT11_PACKET_IN,
+/* OFPT 1.2 (10): struct ofp12_packet_in, uint8_t[]. */
+OFPRAW_OFPT12_PACKET_IN,
/* NXT 1.0+ (17): struct nx_packet_in, uint8_t[]. */
OFPRAW_NXT_PACKET_IN,
@@ -327,6 +329,7 @@ enum ofptype {
/* Asynchronous messages. */
OFPTYPE_PACKET_IN, /* OFPRAW_OFPT10_PACKET_IN.
* OFPRAW_OFPT11_PACKET_IN.
+ * OFPRAW_OFPT12_PACKET_IN.
* OFPRAW_NXT_PACKET_IN. */
OFPTYPE_FLOW_REMOVED,/* OFPRAW_OFPT10_FLOW_REMOVED.
* OFPRAW_NXT_FLOW_REMOVED. */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 59a5bab..6b67e89 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1977,17 +1977,58 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
return 0;
}
+static void
+ofputil_packet_in_to_rule(const struct ofputil_packet_in *pin,
+ struct cls_rule *rule)
+{
+int i;
+
+cls_rule_init_catchall(rule, 0);
+cls_rule_set_tun_id_masked(rule, pin->fmd.tun_id,
+ pin->fmd.tun_id_mask);
+cls_rule_set_metadata_masked(rule, pin->fmd.metadata,
+ pin->fmd.metadata_mask);
+
+for (i = 0; i < FLOW_N_REGS; i++) {
+cls_rule_set_reg_masked(rule, i, pin->fmd.regs[i],
+pin->fmd.reg_masks[i]);
+}
+
+cls_rule_set_in_port(rule, pin->fmd.in_port);
+}
+
/* Converts abstract ofputil_packet_in 'pin' into a PACKET_IN message
* in the format specified by 'packet_in_format'. */
struct ofpbuf *
ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
+ enum ofputil_protocol protocol,
enum nx_packet_in_format packet_in_format)
{
size_t send_len = MIN(pin->send_len, pin->packet_len);
struct ofpbuf *packet;
/* Add OFPT_PACKET_IN. */
-if (packet_in_format == NXPIF_OPENFLOW10) {
+if (protocol == OFPUTIL_P_OF12) {
+struct ofp12_packet_in *opi;
+struct cls_rule rule;
+
+ofputil_packet_in_to_rule(pin, &rule);
+
+/* The final argument is just an estimate of the space required. */
+packet = ofpraw_alloc_xid(OFPRAW_OFPT12_PACKET_IN, OFP12_VERSION,
+ htonl(0), (sizeof(struct flow_metadata) * 2
+ + 2 + send_len));
+ofpbuf_put_zeros(packet, sizeof *opi);
+oxm_put_match(packet, &rule);
+ofpbuf_put_zeros(packet, 2);
+ofpbuf_put(packet, pin->packet, send_len);
+
+opi = packet->l3;
+opi->buffer_id = htonl(pin->buffer_id);
+opi->total_len = htons(pin->total_len);
+opi->reason = pin->reason;
+opi->table_id = pin->table_id;
+ } else if (packet_in_format == NXPIF_OPENFLOW10) {
struct ofp_packet_in *opi;
packet = ofpraw_alloc_xid(OFPRAW_OFPT10_PACKET_IN, OFP10_VERSION,
@@ -2003,21 +2044,8 @@ ofputil_encode_packet_in(const struct ofputil_packet_in
*pin,
struct nx_packet_in *npi;
struct cls_rule rule;
size_t match_len;
-size_t i;
-
-cls_rule_init_catchall(&rule, 0);
-cls_rule_set_tun_id_masked(&rule, pin->fmd.tun_id,
- pin->fmd.tun_id_mask);
-cls_rule_set_metadata_masked(&rule, pin->fmd.metadata,
- pin->fmd.metadata_mask);
-
-
-for (i = 0; i < FLOW_N_REGS; i++) {
-cls_rule_set_reg_masked(&rule, i, pin->fmd.regs[i],
-pin->fmd.reg_masks[i]);
-}
-cls_rule_set_in_port(&rule, pin->fmd.in_port);
+ofputil_packet_in_to_rule(pin, &rule);
/* The final argument is just an estimate of the space required. */
packet = ofpraw_alloc_xid(OFPRAW_NXT_PACKET_IN, OFP10_VERSION,
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index f6c2368..3e0d4b6 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -276,6 +276,7 @@ struct ofputil_packet_in {
enum ofperr ofputi
[ovs-dev] [PATCH 01/41] ofp-util: ofputil_pull_ofp11_match: Allow OXM match
* Allow OXM matches which specified in OpenFlow 1.2.
Also allow them for OpenFlow 1.1 as there seems little reason not to.
* Pass padded_match_len parameter which if on NULL will be set to
the padded match len. This will be used when decoding flow statistics
response messages.
Signed-off-by: Simon Horman
---
v10
* Allow OXM for OpenFlow 1.1, as suggested by Ben Pfaff
- Do not add ofputil_pull_ofp12_match(), rather,
allow an enhanced ofputil_pull_ofp11_match() to handle
both OpenFlow 1.1 and 1.2.
- Rename from "ofp-util: Add ofputil_pull_ofp12_match()" to
"ofp-util: ofputil_pull_ofp11_match: Allow OXM match"
v9
* No change
v8
* Manual Rebase
- Remove cookie and cookie mask parameters
* Use '__' as a suffix rather than a prefix
v7
* Make use of oxm_pull_match()
Change subject from "ofp-util: Add ofputil_ofp12_decode_match()" to
"ofp-util: Add ofputil_pull_ofp12_match()"
v6
* No change
v5
* Add padded_match_len parameter to
ofputil_pull_ofp12_match() and __ofputil_pull_ofp11_match().
This will be used when decoding messages that need
to know the match length.
* Read mach_length from omh after the length of buf has been verified,
else an over-run may occur.
v4
* No change
v3
* Do not pull ofpbuf_pull() in the case of a STANDARD match.
This simplifies things a little.
v2
* No change
---
lib/ofp-util.c | 30 ++
lib/ofp-util.h | 3 ++-
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1fe30b4..59a5bab 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -267,23 +267,36 @@ ofputil_cls_rule_to_ofp10_match(const struct cls_rule
*rule,
enum ofperr
ofputil_pull_ofp11_match(struct ofpbuf *buf, unsigned int priority,
- struct cls_rule *rule)
+ struct cls_rule *rule, uint16_t *padded_match_len)
{
-struct ofp11_match_header *omh;
-struct ofp11_match *om;
+struct ofp11_match_header *omh = buf->data;
+uint16_t match_len;
-if (buf->size < sizeof(struct ofp11_match_header)) {
+if (buf->size < sizeof *omh) {
return OFPERR_OFPBMC_BAD_LEN;
}
-omh = buf->data;
+match_len = ntohs(omh->length);
+
switch (ntohs(omh->type)) {
-case OFPMT_STANDARD:
-if (omh->length != htons(sizeof *om) || buf->size < sizeof *om) {
+case OFPMT_STANDARD: {
+struct ofp11_match *om;
+
+if (match_len != sizeof *om || buf->size < sizeof *om) {
return OFPERR_OFPBMC_BAD_LEN;
}
om = ofpbuf_pull(buf, sizeof *om);
+if (padded_match_len) {
+*padded_match_len = match_len;
+}
return ofputil_cls_rule_from_ofp11_match(om, priority, rule);
+}
+
+case OFPMT_OXM:
+if (padded_match_len) {
+*padded_match_len = ROUND_UP(match_len, 8);
+}
+return oxm_pull_match(buf, priority, rule);
default:
return OFPERR_OFPBMC_BAD_TYPE;
@@ -1137,7 +1150,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
ofm = ofpbuf_pull(&b, sizeof *ofm);
-error = ofputil_pull_ofp11_match(&b, ntohs(ofm->priority), &fm->cr);
+error = ofputil_pull_ofp11_match(&b, ntohs(ofm->priority), &fm->cr,
+ NULL);
if (error) {
return error;
}
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 64f6c39..f6c2368 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -119,7 +119,8 @@ void ofputil_cls_rule_to_ofp10_match(const struct cls_rule
*,
/* Work with ofp11_match. */
enum ofperr ofputil_pull_ofp11_match(struct ofpbuf *, unsigned int priority,
- struct cls_rule *);
+ struct cls_rule *,
+ uint16_t *padded_match_len);
enum ofperr ofputil_cls_rule_from_ofp11_match(const struct ofp11_match *,
unsigned int priority,
struct cls_rule *);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 00/41 v10] Preliminary Open Flow 1.2 Message Support
Hi Ben, Hi All,
this series adds infrastructure and message encoding and decoding
to allow OpenFlow 1.2 to be used to the extent that ovs-controller
works as a learning-switch and similar logic implemented using Ryu[1].
Note that some previously posted hacks are required in order for Open
vSwtich to actually use OpenFlow 1.2.
[1] Ryu is an OpenFlow controller implemented in Python
http://osrg.github.com/ryu/
Differing from the previous posting, this series supports encoding and
decoding of Queue Status request and reply messages.
This series also allows OXM for OpenFlow 1.1, as suggested by
Ben Pfaff when reviewing v9 of this series.
Base
This series is based the current master branch
Availability
This series is available in the devel/of1.2 (n.b: not devel/of12) branch
of git://github.com/horms/openvswitch.git
(ef8848df4aca375913d0829554effa260064d74b "ovs-ofpctl: Enable queue-stats
for Open Flow 1.1 & 1.2")
Patches
---
[PATCH 01/41] ofp-util: ofputil_pull_ofp11_match: Allow OXM match
[PATCH 02/41] ofp-print: Open Flow 1.2 Flow Mod message tests
[PATCH 03/41] ofp-util: Allow encoding of Open Flow 1.2 Packet In
[PATCH 04/41] ofp-util: Allow decoding of Open Flow 1.2 Packet In
[PATCH 05/41] ofp-msgs: Update OFPRAW_OFPT_SET_CONFIG for OpenFlow
[PATCH 06/41] ofp-actions: Return action size
[PATCH 07/41] ofp-util: Prepare Packet Out encoder for other Open
[PATCH 08/41] ofp-util: Allow encoding of Open Flow 1.1 and 1.2
[PATCH 09/41] ofp-util: Prepare Packet Out decoder for other Open
[PATCH 10/41] ofp-util: Allow decoding of Open Flow 1.1 and 1.2
[PATCH 11/41] ofp-util: Allow encoding of Open Flow 1.2 Flow Removed
[PATCH 12/41] ofp-util: Allow encoding Open Flow 1.2 Flow Stats
[PATCH 13/41] ofp-util: Allow use of OF12 flow format
[PATCH 14/41] ofp-util: Allow encoding of Open Flow 1.2 Port Mod
[PATCH 15/41] ofp-util: Allow decoding of Open Flow 1.2 Port Mod
[PATCH 16/41] ofp-msgs: Split OFPRAW_OFPST_FLOW_{REQUEST,REPLY}
[PATCH 17/41] ofp-util: Prepare Flow Statistics Request Decoder for
[PATCH 18/41] ofp-util: Allow decoding of Open Flow 1.2 Flow
[PATCH 19/41] ofp-util: Allow encoding of Open Flow 1.2 Flow
[PATCH 20/41] ofp-util: Allow decoding of Open Flow 1.2 Flow
[PATCH 21/41] ofp-msgs: Split OFPRAW_OFPST_AGGREGATE_REQUEST
[PATCH 22/41] ofp-msgs: Allow 1.0-1.2 range
[PATCH 23/41] ofp-msgs: Split OFPRAW_OFPST_TABLE_REPLY
[PATCH 24/41] ofp-print: Enable display of Open Flow 1.1 & 1.2 Table
[PATCH 25/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table
[PATCH 26/41] ovs-ofctl: Use vconn as a parameter of
[PATCH 27/41] ofp-msgs: Allow encoding and decoding of Open Flow 1.1
[PATCH 28/41] ofp-msgs: Split OFPRAW_OFPST_PORT_{REQUEST,REPLY}
[PATCH 29/41] ovs-ofputil: Make str_to_port_no() aware of invalid
[PATCH 30/41] ofp-util: Pass vconn to fetch_port_by_features()
[PATCH 31/41] ofp-util: Allow encoding of Open Flow 1.1 & 1.2 Port
[PATCH 32/41] ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port
[PATCH 33/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Port
[PATCH 34/41] ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port
[PATCH 35/41] ovs-ofctl: Teach dump-ports about Open Flow 1.1 & 1.2
[PATCH 36/41] ofp-msgs: Split OFPRAW_OFPST_QUEUE_{REQUEST,REPLY}
[PATCH 37/41] ofp-print: Enable display of Open Flow 1.1 & 1.2 Queue
[PATCH 38/41] ofp-util: Allow encoding of Open Flow 1.1 & 1.2 Queue
[PATCH 39/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Queue
[PATCH 40/41] ovs-print: Enable display of Open Flow 1.1 & 1.2 Queue
[PATCH 41/41] ovs-ofpctl: Enable queue-stats for Open Flow 1.1 & 1.2
Overall Diffstat
build-aux/extract-ofp-msgs |3
include/openflow/openflow-1.1.h |4
include/openflow/openflow-1.2.h |5
lib/learning-switch.c |4
lib/ofp-actions.c |5
lib/ofp-actions.h |4
lib/ofp-msgs.h | 77 +++--
lib/ofp-print.c | 357 +
lib/ofp-util.c | 426 --
lib/ofp-util.h | 11
ofproto/connmgr.c |3
ofproto/ofproto-dpif.c | 32 +-
ofproto/ofproto-provider.h | 71 -
ofproto/ofproto.c | 295 ++--
tests/learn.at |2
tests/ofp-print.at | 405 +++-
tests/ofproto.at| 12
utilities/ovs-ofctl.c | 191 +
18 files changed, 1617 insertions(+), 290 deletions(-)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 09/41] ofp-util: Prepare Packet Out decoder for other Open Flow versions
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* No change
v7
* Manual Rebase
* Only use OFPERR_NXBRC_BAD_IN_PORT in the case of OFPRAW_OFPT10_PACKET_OUT
v6
* No change
v5
* No change
v4
* No change
v3
* Differentiate versions using oh->version rather than relying
on a separate code for each version of the Packet Out message.
v2
* No change
---
lib/ofp-util.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 593027a..1972b25 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2150,29 +2150,36 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po,
const struct ofp_header *oh,
struct ofpbuf *ofpacts)
{
-const struct ofp_packet_out *opo;
-enum ofperr error;
enum ofpraw raw;
struct ofpbuf b;
+enum ofperr bad_in_port_err = OFPERR_OFPET_BAD_REQUEST;
ofpbuf_use_const(&b, oh, ntohs(oh->length));
raw = ofpraw_pull_assert(&b);
-assert(raw == OFPRAW_OFPT10_PACKET_OUT);
-opo = ofpbuf_pull(&b, sizeof *opo);
-po->buffer_id = ntohl(opo->buffer_id);
-po->in_port = ntohs(opo->in_port);
+if (raw == OFPRAW_OFPT10_PACKET_OUT) {
+enum ofperr error;
+const struct ofp_packet_out *opo = ofpbuf_pull(&b, sizeof *opo);
+
+po->buffer_id = ntohl(opo->buffer_id);
+po->in_port = ntohs(opo->in_port);
+
+error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), ofpacts);
+if (error) {
+return error;
+}
+bad_in_port_err = OFPERR_NXBRC_BAD_IN_PORT;
+} else {
+NOT_REACHED();
+}
+
if (po->in_port >= OFPP_MAX && po->in_port != OFPP_LOCAL
&& po->in_port != OFPP_NONE && po->in_port != OFPP_CONTROLLER) {
VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out has bad input port %#"PRIx16,
po->in_port);
-return OFPERR_NXBRC_BAD_IN_PORT;
+return bad_in_port_err;
}
-error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), ofpacts);
-if (error) {
-return error;
-}
po->ofpacts = ofpacts->data;
po->ofpacts_len = ofpacts->size;
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 05/41] ofp-msgs: Update OFPRAW_OFPT_SET_CONFIG for OpenFlow 1.2
This is sufficient to allow encoding and decoding of
OpenFlow 1.2 Set Config messages as the format is the same
as OpenFlow 1.0 and OpenFlow 1.2.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* No change
v7
* Initial Post
---
lib/ofp-msgs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 1639cd9..62cdf66 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -120,7 +120,7 @@ enum ofpraw {
/* OFPT 1.0+ (8): struct ofp_switch_config. */
OFPRAW_OFPT_GET_CONFIG_REPLY,
-/* OFPT 1.0-1.1 (9): struct ofp_switch_config. */
+/* OFPT 1.0+ (9): struct ofp_switch_config. */
OFPRAW_OFPT_SET_CONFIG,
/* OFPT 1.0 (10): struct ofp_packet_in up to data, uint8_t[]. */
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 08/41] ofp-util: Allow encoding of Open Flow 1.1 and 1.2 Packet Out Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Manual Rebase
* ofpacts_put_openflow11_actions() now returns the length of encoded
actions, make use of this.
v7
* Manual Rebase
v6
* No change
v5
* No change
v4
* Manual rebase
v3
* Correct title: this patch relates to Packet Out not Packet In
* Correct decoding of buffer_id, it is 32bits wide not 16bits wide
* Add decoding of stats reply messages
v2
* No change
ofp-actions: Return action size
Modify ofpacts_put_openflow11_actions() to return the length of actions
appended. This will be used when encoding Packet Out messages for
Open Flow 1.1 and 1.2. The motivation for this is to avoid open coding
the size calculation which may end up being needed elsewhere too.
Signed-off-by: Simon Horman
---
lib/ofp-util.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index eb01f10..593027a 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3074,7 +3074,21 @@ ofputil_encode_packet_out(const struct
ofputil_packet_out *po,
}
case OFP11_VERSION:
-case OFP12_VERSION:
+case OFP12_VERSION: {
+struct ofp11_packet_out *opo;
+size_t len;
+
+msg = ofpraw_alloc(OFPRAW_OFPT11_PACKET_OUT, ofp_version, size);
+ofpbuf_put_zeros(msg, sizeof *opo);
+len = ofpacts_put_openflow11_actions(po->ofpacts, po->ofpacts_len,
msg);
+
+opo = msg->l3;
+opo->buffer_id = htonl(po->buffer_id);
+opo->in_port = ofputil_port_to_ofp11(po->in_port);
+opo->actions_len = htons(len);
+break;
+}
+
default:
NOT_REACHED();
}
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 12/41] ofp-util: Allow encoding Open Flow 1.2 Flow Stats Request Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Manual rebase
v7
* Omitted
v6
* No change
v5
* No change
v4
* Manual rebase
v3
* No change
v2
* Use ofputil_put_match()
Fix ofputil_encode_flow_stats_request
---
lib/ofp-util.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 462db2a..ad39378 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1482,6 +1482,23 @@ ofputil_encode_flow_stats_request(const struct
ofputil_flow_stats_request *fsr,
enum ofpraw raw;
switch (protocol) {
+case OFPUTIL_P_OF12: {
+struct ofp11_flow_stats_request *ofsr;
+
+raw = (fsr->aggregate
+ ? OFPRAW_OFPST_AGGREGATE_REQUEST
+ : OFPRAW_OFPST_FLOW_REQUEST);
+msg = ofpraw_alloc(raw, OFP12_VERSION, 0);
+ofsr = ofpbuf_put_zeros(msg, sizeof *ofsr);
+ofsr->table_id = fsr->table_id;
+ofsr->out_port = ofputil_port_to_ofp11(fsr->out_port);
+ofsr->out_group = htonl(OFPG11_ANY);
+ofsr->cookie = fsr->cookie;
+ofsr->cookie_mask = fsr->cookie_mask;
+oxm_put_match(msg, &fsr->match);
+break;
+}
+
case OFPUTIL_P_OF10:
case OFPUTIL_P_OF10_TID: {
struct ofp10_flow_stats_request *ofsr;
@@ -1517,7 +1534,6 @@ ofputil_encode_flow_stats_request(const struct
ofputil_flow_stats_request *fsr,
break;
}
-case OFPUTIL_P_OF12:
default:
NOT_REACHED();
}
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 13/41] ofp-util: Allow use of OF12 flow format
This enables the use of the OF12 format, the prerequisites of which were
added by "Add OFPUTIL_P_OF12 and NXFF_OPENFLOW12" and patches to fill out
other functions that use OFPUTIL_P_OF12 directly.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
lib/ofp-util.c | 2 +-
lib/ofp-util.h | 4 ++--
tests/learn.at | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index ad39378..b11cbc8 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -734,7 +734,7 @@ ofputil_protocol_to_string(enum ofputil_protocol protocol)
return "OpenFlow10+table_id";
case OFPUTIL_P_OF12:
-return NULL;
+return "OpenFlow12";
}
/* Check abbreviations. */
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index b274ad6..cbf24a3 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -72,10 +72,10 @@ enum ofputil_protocol {
OFPUTIL_P_OF12 = 1 << 4, /* OpenFlow 1.2 flow format. */
/* All protocols. */
-#define OFPUTIL_P_ANY (OFPUTIL_P_OF10_ANY | OFPUTIL_P_NXM_ANY)
+#define OFPUTIL_P_ANY (OFPUTIL_P_OF10_ANY | OFPUTIL_P_NXM_ANY | OFPUTIL_P_OF12)
/* Protocols in which a specific table may be specified in flow_mods. */
-#define OFPUTIL_P_TID (OFPUTIL_P_OF10_TID | OFPUTIL_P_NXM_TID)
+#define OFPUTIL_P_TID (OFPUTIL_P_OF10_TID | OFPUTIL_P_NXM_TID | OFPUTIL_P_OF12)
};
/* Protocols to use for flow dumps, from most to least preferred. */
diff --git a/tests/learn.at b/tests/learn.at
index da82f51..b527276 100644
--- a/tests/learn.at
+++ b/tests/learn.at
@@ -24,7 +24,7 @@ table=0 actions=learn(table=1,hard_timeout=10,
NXM_OF_VLAN_TCI[0..11],output:NXM
table=1 priority=0 actions=flood
]])
AT_CHECK([ovs-ofctl parse-flows flows.txt], [0],
-[[usable protocols: OpenFlow10+table_id,NXM+table_id
+[[usable protocols: OpenFlow10+table_id,NXM+table_id,OpenFlow12
chosen protocol: OpenFlow10+table_id
OFPT_FLOW_MOD (xid=0x1): ADD table:255
actions=learn(table=1,in_port=99,NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],load:NXM_OF_IN_PORT[]->NXM_NX_REG1[16..31])
OFPT_FLOW_MOD (xid=0x2): ADD table:255
actions=learn(table=1,NXM_OF_VLAN_TCI[0..11],NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],output:NXM_OF_IN_PORT[])
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 11/41] ofp-util: Allow encoding of Open Flow 1.2 Flow Removed messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Manual Rebase
v7
* Omitted
v6
* No change
v5
* No change
v4
* Manual rebase
v3
* No change
v2
* Use ofputil_put_match()
---
lib/ofp-msgs.h | 3 +++
lib/ofp-util.c | 21 -
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 62cdf66..05ef334 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -134,6 +134,8 @@ enum ofpraw {
/* OFPT 1.0 (11): struct ofp_flow_removed. */
OFPRAW_OFPT10_FLOW_REMOVED,
+/* OFPT 1.1+ (11): struct ofp11_flow_removed, uint8_t[8][]. */
+OFPRAW_OFPT11_FLOW_REMOVED,
/* NXT 1.0+ (14): struct nx_flow_removed, uint8_t[8][]. */
OFPRAW_NXT_FLOW_REMOVED,
@@ -332,6 +334,7 @@ enum ofptype {
* OFPRAW_OFPT12_PACKET_IN.
* OFPRAW_NXT_PACKET_IN. */
OFPTYPE_FLOW_REMOVED,/* OFPRAW_OFPT10_FLOW_REMOVED.
+ * OFPRAW_OFPT11_FLOW_REMOVED.
* OFPRAW_NXT_FLOW_REMOVED. */
OFPTYPE_PORT_STATUS, /* OFPRAW_OFPT10_PORT_STATUS.
* OFPRAW_OFPT11_PORT_STATUS. */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 10b4555..462db2a 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1861,6 +1861,26 @@ ofputil_encode_flow_removed(const struct
ofputil_flow_removed *fr,
struct ofpbuf *msg;
switch (protocol) {
+case OFPUTIL_P_OF12: {
+struct ofp12_flow_removed *ofr;
+
+msg = ofpraw_alloc_xid(OFPRAW_OFPT11_FLOW_REMOVED,
+ ofputil_protocol_to_ofp_version(protocol),
+ htonl(0), NXM_TYPICAL_LEN);
+ofr = ofpbuf_put_zeros(msg, sizeof *ofr);
+ofr->cookie = fr->cookie;
+ofr->priority = htons(fr->rule.priority);
+ofr->reason = fr->reason;
+ofr->table_id = 0;
+ofr->duration_sec = htonl(fr->duration_sec);
+ofr->duration_nsec = htonl(fr->duration_nsec);
+ofr->idle_timeout = htons(fr->idle_timeout);
+ofr->packet_count = htonll(fr->packet_count);
+ofr->byte_count = htonll(fr->byte_count);
+oxm_put_match(msg, &fr->rule);
+break;
+}
+
case OFPUTIL_P_OF10:
case OFPUTIL_P_OF10_TID: {
struct ofp_flow_removed *ofr;
@@ -1903,7 +1923,6 @@ ofputil_encode_flow_removed(const struct
ofputil_flow_removed *fr,
break;
}
-case OFPUTIL_P_OF12:
default:
NOT_REACHED();
}
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 10/41] ofp-util: Allow decoding of Open Flow 1.1 and 1.2 Packet Out Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Return error value returned by ofputil_port_from_ofp11()
rather than OFPERR_OFPBMC_BAD_VALUE.
- This fix, from v3, disappeared somewhere along the way
* Add ofp-print test for Open Flow 1.2
- I'm not sure how to generate an Open Flow 1.1 message at this point
v7
* Manual rebase
v6
* No change
v5
* No change
v4
* No change
v3
* Manual rebase
* Correct title: This patch relates to Packet Out not Packet In
* Use OFPT_PACKET_OUT instead of OFPT11_PACKET_OUT, it seems
easier to use a single code and differentiate using the version
in the ofp_header
* Return error value returned by ofputil_port_from_ofp11()
rather than OFPERR_OFPBMC_BAD_VALUE.
v2
* No change
---
lib/ofp-util.c | 19 ++-
tests/ofp-print.at | 13 -
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1972b25..10b4555 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2157,7 +2157,24 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po,
ofpbuf_use_const(&b, oh, ntohs(oh->length));
raw = ofpraw_pull_assert(&b);
-if (raw == OFPRAW_OFPT10_PACKET_OUT) {
+if (raw == OFPRAW_OFPT11_PACKET_OUT) {
+enum ofperr error;
+const struct ofp11_packet_out *opo = ofpbuf_pull(&b, sizeof *opo);
+
+po->buffer_id = ntohl(opo->buffer_id);
+error = ofputil_port_from_ofp11(opo->in_port, &po->in_port);
+if (error) {
+return error;
+}
+
+error = ofpacts_pull_openflow11_actions(&b, ntohs(opo->actions_len),
+ofpacts);
+if (error) {
+return error;
+}
+
+bad_in_port_err = OFPERR_OFPBMC_BAD_VALUE;
+} else if (raw == OFPRAW_OFPT10_PACKET_OUT) {
enum ofperr error;
const struct ofp_packet_out *opo = ofpbuf_pull(&b, sizeof *opo);
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 2c4e6c5..842869c 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -406,7 +406,7 @@ OFPT_PORT_STATUS (OF1.1) (xid=0x0): MOD: 3(eth0):
addr:50:54:00:00:00:01
])
AT_CLEANUP
-AT_SETUP([OFPT_PACKET_OUT])
+AT_SETUP([OFPT_PACKET_OUT - OF1.0])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print "\
01 0d 00 54 00 00 00 00 00 00 01 14 00 01 00 08 \
@@ -420,6 +420,17 @@ OFPT_PACKET_OUT (xid=0x0): in_port=1 actions=output:3
buffer=0x0114
])
AT_CLEANUP
+AT_SETUP([OFPT_PACKET_OUT - OF1.1])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 0d 00 28 88 58 df c5 ff ff ff 00 ff ff ff fe \
+00 10 00 00 00 00 00 00 00 00 00 10 ff ff ff fb \
+05 dc 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPT_PACKET_OUT (OF1.2) (xid=0x8858dfc5): in_port=LOCAL actions=FLOOD
buffer=0xff00
+])
+AT_CLEANUP
+
# The flow is formatted with cls_rule_format() for the low-verbosity case.
AT_SETUP([OFPT_FLOW_MOD - OF1.0 - low verbosity])
AT_KEYWORDS([ofp-print])
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 14/41] ofp-util: Allow encoding of Open Flow 1.2 Port Mod Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Manual Rebase
v7
* Omitted
v6
* No change
v5
* No change
v4
* Manual rebase
v3
* No change
v2
* No change
---
lib/ofp-util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index b11cbc8..716e6bc 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2822,7 +2822,8 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
break;
}
-case OFP11_VERSION: {
+case OFP11_VERSION:
+case OFP12_VERSION: {
struct ofp11_port_mod *opm;
b = ofpraw_alloc(OFPRAW_OFPT11_PORT_MOD, ofp_version, 0);
@@ -2835,7 +2836,6 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
break;
}
-case OFP12_VERSION:
default:
NOT_REACHED();
}
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 04/41] ofp-util: Allow decoding of Open Flow 1.2 Packet In Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Manual rebase
* Add ofp-print test
v7
* Manual rebase
* Use struct ofp12_packet_in instead of struct ofp11_packet_in
v6
* No change
v5
* Manual rebase
v4
* No change
v3
* Add OF1.2 entry to to ofputil_msg_types
* Make OF1.2 the first option in the if statement in ofputil_decode_packet_in()
* Rename o11pi as opi, which is consistent with the style used elsewhere
* Rebase
v2
* No change
---
lib/ofp-util.c | 61 --
tests/ofp-print.at | 16 +-
2 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6b67e89..f305952 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1911,6 +1911,27 @@ ofputil_encode_flow_removed(const struct
ofputil_flow_removed *fr,
return msg;
}
+static void
+ofputil_decode_packet_in_finish(struct ofputil_packet_in *pin,
+struct cls_rule *rule,
+struct ofpbuf *b)
+{
+pin->packet = b->data;
+pin->packet_len = b->size;
+
+pin->fmd.in_port = rule->flow.in_port;
+
+pin->fmd.tun_id = rule->flow.tun_id;
+pin->fmd.tun_id_mask = rule->wc.tun_id_mask;
+
+pin->fmd.metadata = rule->flow.metadata;
+pin->fmd.metadata_mask = rule->wc.metadata_mask;
+
+memcpy(pin->fmd.regs, rule->flow.regs, sizeof pin->fmd.regs);
+memcpy(pin->fmd.reg_masks, rule->wc.reg_masks,
+ sizeof pin->fmd.reg_masks);
+}
+
enum ofperr
ofputil_decode_packet_in(struct ofputil_packet_in *pin,
const struct ofp_header *oh)
@@ -1922,7 +1943,29 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
ofpbuf_use_const(&b, oh, ntohs(oh->length));
raw = ofpraw_pull_assert(&b);
-if (raw == OFPRAW_OFPT10_PACKET_IN) {
+if (raw == OFPRAW_OFPT12_PACKET_IN) {
+const struct ofp12_packet_in *opi;
+struct cls_rule rule;
+int error;
+
+opi = ofpbuf_pull(&b, sizeof *opi);
+error = oxm_pull_match_loose(&b, 0, &rule);
+if (error) {
+return error;
+}
+
+if (!ofpbuf_try_pull(&b, 2)) {
+return OFPERR_OFPBRC_BAD_LEN;
+}
+
+pin->reason = opi->reason;
+pin->table_id = opi->table_id;
+
+pin->buffer_id = ntohl(opi->buffer_id);
+pin->total_len = ntohs(opi->total_len);
+
+ofputil_decode_packet_in_finish(pin, &rule, &b);
+} else if (raw == OFPRAW_OFPT10_PACKET_IN) {
const struct ofp_packet_in *opi;
opi = ofpbuf_pull(&b, offsetof(struct ofp_packet_in, data));
@@ -1950,26 +1993,14 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
return OFPERR_OFPBRC_BAD_LEN;
}
-pin->packet = b.data;
-pin->packet_len = b.size;
pin->reason = npi->reason;
pin->table_id = npi->table_id;
pin->cookie = npi->cookie;
-pin->fmd.in_port = rule.flow.in_port;
-
-pin->fmd.tun_id = rule.flow.tun_id;
-pin->fmd.tun_id_mask = rule.wc.tun_id_mask;
-
-pin->fmd.metadata = rule.flow.metadata;
-pin->fmd.metadata_mask = rule.wc.metadata_mask;
-
-memcpy(pin->fmd.regs, rule.flow.regs, sizeof pin->fmd.regs);
-memcpy(pin->fmd.reg_masks, rule.wc.reg_masks,
- sizeof pin->fmd.reg_masks);
-
pin->buffer_id = ntohl(npi->buffer_id);
pin->total_len = ntohs(npi->total_len);
+
+ofputil_decode_packet_in_finish(pin, &rule, &b);
} else {
NOT_REACHED();
}
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 05e6cac..2c4e6c5 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -327,7 +327,7 @@ OFPT_GET_CONFIG_REPLY (xid=0x3): frags=reassemble
miss_send_len=255
])
AT_CLEANUP
-AT_SETUP([OFPT_PACKET_IN])
+AT_SETUP([OFPT_PACKET_IN - OF1.0])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print "\
01 0a 00 4e 00 00 00 00 00 00 01 11 00 3c 00 03 \
@@ -341,6 +341,20 @@ priority:0,tunnel:0,metadata:0,in_port:,tci(0)
mac(50:54:00:00:00:05->50:54:
])
AT_CLEANUP
+AT_SETUP([OFPT_PACKET_IN - OF1.2])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 0a 00 4c 00 00 00 00 ff ff ff 00 00 2a 00 00 \
+00 01 00 0c 80 00 00 04 ff ff ff fe 00 00 00 00 \
+00 00 ff ff ff ff ff ff 00 23 20 83 c1 5f 80 35 \
+00 01 08 00 06 04 00 03 00 23 20 83 c1 5f 00 00 \
+00 00 00 23 20 83 c1 5f 00 00 00 00 \
+"], [0], [dnl
+OFPT_PACKET_IN (OF1.2) (xid=0x0): total_len=42 in_port=LOCAL (via no_match)
data_len=42 buffer=0xff00
+priority:0,tunnel:0,metadata:0,in_port:,tci(0)
mac(00:23:20:83:c1:5f->ff:ff:ff:ff:ff:ff) type:8035 proto:0 tos:0 ttl:0
ip(0.0.0.0->0.0.0.0)
+])
+AT_CLEANUP
+
AT_SETUP([OFPT_FLOW_REMOVED])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print "\
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mail
[ovs-dev] [PATCH 07/41] ofp-util: Prepare Packet Out encoder for other Open Flow versions
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Resolve breakage caused by reordering patches in v8
v8
* Make use of enum ofp_version
v7
* Manual Rebase
v6
* No change
v5
* No change
v4
* Manual rebase
v3
* Add protocol variable to do_probe().
Previously this was added by a separate patch.
v2
* No change
---
lib/learning-switch.c | 4 ++--
lib/ofp-util.c| 39 ++-
lib/ofp-util.h| 3 ++-
utilities/ovs-ofctl.c | 5 +++--
4 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index b7a435c..cb6ec33 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -527,13 +527,13 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn,
/* If the switch didn't buffer the packet, we need to send a copy. */
if (pi.buffer_id == UINT32_MAX && out_port != OFPP_NONE) {
-queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
+queue_tx(sw, rconn, 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) {
-queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
+queue_tx(sw, rconn, ofputil_encode_packet_out(&po, sw->protocol));
}
}
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index f305952..eb01f10 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3044,27 +3044,40 @@ ofputil_append_flow_update(const struct
ofputil_flow_update *update,
}
struct ofpbuf *
-ofputil_encode_packet_out(const struct ofputil_packet_out *po)
+ofputil_encode_packet_out(const struct ofputil_packet_out *po,
+ enum ofputil_protocol protocol)
{
-struct ofp_packet_out *opo;
-size_t actions_ofs;
+enum ofp_version ofp_version = ofputil_protocol_to_ofp_version(protocol);
struct ofpbuf *msg;
-size_t size;
+size_t size = 0;
size = po->ofpacts_len;
if (po->buffer_id == UINT32_MAX) {
-size += po->packet_len;
+size = po->packet_len;
}
-msg = ofpraw_alloc(OFPRAW_OFPT10_PACKET_OUT, OFP10_VERSION, size);
-ofpbuf_put_zeros(msg, sizeof *opo);
-actions_ofs = msg->size;
-ofpacts_put_openflow10(po->ofpacts, po->ofpacts_len, msg);
+switch (ofp_version) {
+case OFP10_VERSION: {
+struct ofp_packet_out *opo;
+size_t actions_ofs;
+
+msg = ofpraw_alloc(OFPRAW_OFPT10_PACKET_OUT, OFP10_VERSION, size);
+ofpbuf_put_zeros(msg, sizeof *opo);
+actions_ofs = msg->size;
+ofpacts_put_openflow10(po->ofpacts, po->ofpacts_len, msg);
+
+opo = msg->l3;
+opo->buffer_id = htonl(po->buffer_id);
+opo->in_port = htons(po->in_port);
+opo->actions_len = htons(msg->size - actions_ofs);
+break;
+}
-opo = msg->l3;
-opo->buffer_id = htonl(po->buffer_id);
-opo->in_port = htons(po->in_port);
-opo->actions_len = htons(msg->size - actions_ofs);
+case OFP11_VERSION:
+case OFP12_VERSION:
+default:
+NOT_REACHED();
+}
if (po->buffer_id == UINT32_MAX) {
ofpbuf_put(msg, po->packet, po->packet_len);
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 3e0d4b6..b274ad6 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -299,7 +299,8 @@ struct ofputil_packet_out {
enum ofperr ofputil_decode_packet_out(struct ofputil_packet_out *,
const struct ofp_header *,
struct ofpbuf *ofpacts);
-struct ofpbuf *ofputil_encode_packet_out(const struct ofputil_packet_out *);
+struct ofpbuf *ofputil_encode_packet_out(const struct ofputil_packet_out *,
+ enum ofputil_protocol protocol);
enum ofputil_port_config {
/* OpenFlow 1.0 and 1.1 share these values for these port config bits. */
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 2343240..4bbd536 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1453,6 +1453,7 @@ ofctl_probe(int argc OVS_UNUSED, char *argv[])
static void
ofctl_packet_out(int argc, char *argv[])
{
+enum ofputil_protocol protocol;
struct ofputil_packet_out po;
struct ofpbuf ofpacts;
struct vconn *vconn;
@@ -1468,7 +1469,7 @@ ofctl_packet_out(int argc, char *argv[])
po.ofpacts = ofpacts.data;
po.ofpacts_len = ofpacts.size;
-open_vconn(argv[1], &vconn);
+protocol = open_vconn(argv[1], &vconn);
for (i = 4; i < argc; i++) {
struct ofpbuf *packet, *opo;
const char *error_msg;
@@ -1480,7 +1481,7 @@ ofctl_packet_out(int argc, char *argv[])
po.packet = packet->data;
po.packet_len = packet->size;
-opo = ofputil_encode_packet_out(&po);
+opo = ofputil_encode_packet_out(&po, protocol);
[ovs-dev] [PATCH 15/41] ofp-util: Allow decoding of Open Flow 1.2 Port Mod Message
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
lib/ofp-msgs.h | 2 +-
tests/ofp-print.at | 14 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 05ef334..0683bc5 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -158,7 +158,7 @@ enum ofpraw {
/* OFPT 1.0 (15): struct ofp10_port_mod. */
OFPRAW_OFPT10_PORT_MOD,
-/* OFPT 1.1 (16): struct ofp11_port_mod. */
+/* OFPT 1.1+ (16): struct ofp11_port_mod. */
OFPRAW_OFPT11_PORT_MOD,
/* OFPT 1.0 (18): void. */
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 842869c..827d4f3 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -533,6 +533,20 @@ OFPT_PORT_MOD (OF1.1) (xid=0x3):port: 3:
addr:50:54:00:00:00:01
])
AT_CLEANUP
+AT_SETUP([OFPT_PORT_MOD - OF1.2])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 10 00 28 00 00 00 03 00 00 00 03 00 00 00 00 \
+50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \
+00 00 00 00 00 00 00 00 \
+" 3], [0], [dnl
+OFPT_PORT_MOD (OF1.2) (xid=0x3):port: 3: addr:50:54:00:00:00:01
+ config: PORT_DOWN
+ mask: PORT_DOWN
+ advertise: UNCHANGED
+])
+AT_CLEANUP
+
AT_SETUP([OFPST_DESC request])
AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
AT_CHECK([ovs-ofctl ofp-print "011c0001"], [0], [dnl
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 17/41] ofp-util: Prepare Flow Statistics Request Decoder for other Open Flow versions
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
lib/ofp-util.c | 35 +++
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 878d307..d5e8bbf 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1404,14 +1404,33 @@ ofputil_flow_mod_usable_protocols(const struct
ofputil_flow_mod *fms,
static enum ofperr
ofputil_decode_ofpst_flow_request(struct ofputil_flow_stats_request *fsr,
- const struct ofp10_flow_stats_request *ofsr,
- bool aggregate)
+ struct ofpbuf *msg, bool aggregate)
{
+enum ofperr error;
+enum ofpraw raw;
+
+error = (msg->l2
+ ? ofpraw_decode(&raw, msg->l2)
+ : ofpraw_pull(&raw, msg));
+if (error) {
+return error;
+}
+
fsr->aggregate = aggregate;
-ofputil_cls_rule_from_ofp10_match(&ofsr->match, 0, &fsr->match);
-fsr->out_port = ntohs(ofsr->out_port);
-fsr->table_id = ofsr->table_id;
-fsr->cookie = fsr->cookie_mask = htonll(0);
+
+if (!msg->size) {
+return EOF;
+} else if (raw == OFPRAW_OFPST10_FLOW_REQUEST ||
+ raw == OFPRAW_OFPST_AGGREGATE_REQUEST) {
+const struct ofp10_flow_stats_request *ofsr = msg->data;
+
+ofputil_cls_rule_from_ofp10_match(&ofsr->match, 0, &fsr->match);
+fsr->out_port = ntohs(ofsr->out_port);
+fsr->table_id = ofsr->table_id;
+fsr->cookie = fsr->cookie_mask = htonll(0);
+} else {
+NOT_REACHED();
+}
return 0;
}
@@ -1454,10 +1473,10 @@ ofputil_decode_flow_stats_request(struct
ofputil_flow_stats_request *fsr,
raw = ofpraw_pull_assert(&b);
switch ((int) raw) {
case OFPRAW_OFPST10_FLOW_REQUEST:
-return ofputil_decode_ofpst_flow_request(fsr, b.data, false);
+return ofputil_decode_ofpst_flow_request(fsr, &b, false);
case OFPRAW_OFPST_AGGREGATE_REQUEST:
-return ofputil_decode_ofpst_flow_request(fsr, b.data, true);
+return ofputil_decode_ofpst_flow_request(fsr, &b, true);
case OFPRAW_NXST_FLOW_REQUEST:
return ofputil_decode_nxst_flow_request(fsr, &b, false);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 19/41] ofp-util: Allow encoding of Open Flow 1.2 Flow Statistics Response Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Manual Rebase
v7
* Omitted
v6
* No change
v5
* No change
v4
* Do not manually add header pad, it is handled by
ofputil_postappend_stats_reply()
v3
* Initial post
fix
---
lib/ofp-util.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1afd29c..2a12d95 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1747,7 +1747,28 @@ ofputil_append_flow_stats_reply(const struct
ofputil_flow_stats *fs,
enum ofpraw raw;
ofpraw_decode_partial(&raw, reply->data, reply->size);
-if (raw == OFPRAW_OFPST10_FLOW_REPLY) {
+if (raw == OFPRAW_OFPST11_FLOW_REPLY) {
+struct ofp11_flow_stats *ofs;
+
+ofpbuf_put_uninit(reply, sizeof *ofs);
+oxm_put_match(reply, &fs->rule);
+ofpacts_put_openflow11_instructions(fs->ofpacts, fs->ofpacts_len,
+reply);
+
+ofs = ofpbuf_at_assert(reply, start_ofs, sizeof *ofs);
+ofs->length = htons(reply->size - start_ofs);
+ofs->table_id = fs->table_id;
+ofs->pad = 0;
+ofs->duration_sec = htonl(fs->duration_sec);
+ofs->duration_nsec = htonl(fs->duration_nsec);
+ofs->priority = htons(fs->rule.priority);
+ofs->idle_timeout = htons(fs->idle_timeout);
+ofs->hard_timeout = htons(fs->hard_timeout);
+memset(ofs->pad2, 0, sizeof ofs->pad2);
+ofs->cookie = fs->cookie;
+ofs->packet_count = htonll(unknown_to_zero(fs->packet_count));
+ofs->byte_count = htonll(unknown_to_zero(fs->byte_count));
+} else if (raw == OFPRAW_OFPST10_FLOW_REPLY) {
struct ofp10_flow_stats *ofs;
ofpbuf_put_uninit(reply, sizeof *ofs);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 22/41] ofp-msgs: Allow 1.0-1.2 range
This is intended for use with OFPRAW_OFPST_TABLE_REQUEST
in order for it to be symmetric with OpenFlow 1.0, 1.1 and 1.2
versions of OFPRAW_OFPST1TABLE_REPLY.
OpenFlow 1.3 introduces yet another format for OFPRAW_OFPST1TABLE_REPLY.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
build-aux/extract-ofp-msgs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/build-aux/extract-ofp-msgs b/build-aux/extract-ofp-msgs
index fe92dae..ad1a8a8 100755
--- a/build-aux/extract-ofp-msgs
+++ b/build-aux/extract-ofp-msgs
@@ -28,7 +28,8 @@ version_map = {"1.0": (OFP10_VERSION, OFP10_VERSION),
"1.1+":(OFP11_VERSION, OFP13_VERSION),
"1.2+":(OFP12_VERSION, OFP13_VERSION),
"1.3+":(OFP13_VERSION, OFP13_VERSION),
- "1.0-1.1": (OFP10_VERSION, OFP11_VERSION)}
+ "1.0-1.1": (OFP10_VERSION, OFP11_VERSION),
+ "1.0-1.2": (OFP10_VERSION, OFP12_VERSION)}
def get_line():
global line
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 23/41] ofp-msgs: Split OFPRAW_OFPST_TABLE_REPLY
Split OFPRAW_OFPST_TABLE_REPLY into OpenFlow 1.0, 1.1 and 1.2 versions.
This is preparation for allowing encoding and decoding of Open Flow 1.1
and 1.2 Table Stats Reply messages.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
lib/ofp-msgs.h | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index dfd646d..7ef7c5a 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -205,11 +205,15 @@ enum ofpraw {
/* NXST 1.0 (1): struct ofp_aggregate_stats_reply. */
OFPRAW_NXST_AGGREGATE_REPLY,
-/* OFPST 1.0 (3): void. */
+/* OFPST 1.0-1.2 (3): void. */
OFPRAW_OFPST_TABLE_REQUEST,
/* OFPST 1.0 (3): struct ofp10_table_stats[]. */
-OFPRAW_OFPST_TABLE_REPLY,
+OFPRAW_OFPST10_TABLE_REPLY,
+/* OFPST 1.1 (3): struct ofp11_table_stats[]. */
+OFPRAW_OFPST11_TABLE_REPLY,
+/* OFPST 1.2 (3): struct ofp12_table_stats[]. */
+OFPRAW_OFPST12_TABLE_REPLY,
/* OFPST 1.0 (4): struct ofp10_port_stats_request. */
OFPRAW_OFPST_PORT_REQUEST,
@@ -375,7 +379,9 @@ enum ofptype {
OFPTYPE_AGGREGATE_STATS_REPLY, /* OFPRAW_OFPST_AGGREGATE_REPLY.
* OFPRAW_NXST_AGGREGATE_REPLY. */
OFPTYPE_TABLE_STATS_REQUEST, /* OFPRAW_OFPST_TABLE_REQUEST. */
-OFPTYPE_TABLE_STATS_REPLY, /* OFPRAW_OFPST_TABLE_REPLY. */
+OFPTYPE_TABLE_STATS_REPLY, /* OFPRAW_OFPST10_TABLE_REPLY.
+ * OFPRAW_OFPST11_TABLE_REPLY.
+ * OFPRAW_OFPST12_TABLE_REPLY. */
OFPTYPE_PORT_STATS_REQUEST, /* OFPRAW_OFPST_PORT_REQUEST. */
OFPTYPE_PORT_STATS_REPLY,/* OFPRAW_OFPST_PORT_REPLY. */
OFPTYPE_QUEUE_STATS_REQUEST, /* OFPRAW_OFPST_QUEUE_REQUEST. */
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 18/41] ofp-util: Allow decoding of Open Flow 1.2 Flow Statistics Request Messages
Allow decoding of Open Flow 1.1 and 1.2 flow and aggregate flow statistics
request messages.
Signed-off-by: Simon Horman
---
v10
* No change
v10
* Manual rebase
v9
* No change
v8
* Manual rebase
* Add ofp-print test
v7
* Omitted
v6
* No change
v5
* Manual rebase
v4
* Use OFPG11_ANY in place of OFPG_ANY.
The value is the same, but it seems to make sense to
use the Open Flow 1.1 constant when working with Open Flow 1.1.
* Do not handle Open Flow 1.1, the parsing is slightly different
and not supplied by this patch.
* Use sizeof(struct ofp11_flow_stats_request) as the minimum size of the
message, previously sizeof(struct ofp10_flow_stats_request) was
incorrectly used.
* Indicate that the message may be larger than the minimum size.
* Handle OFPUTIL_OFPST11_FLOW_REQUEST and OFPUTIL_OFPST11_AGGREGATE_REQUEST in
ofputil_decode_flow_stats_request().
* Add entry for Open Flow 1.2 aggregate request to ofputil_msg_types[]
v3
* Initial post
---
lib/ofp-util.c | 20
tests/ofp-print.at | 14 +-
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d5e8bbf..1afd29c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1420,6 +1420,25 @@ ofputil_decode_ofpst_flow_request(struct
ofputil_flow_stats_request *fsr,
if (!msg->size) {
return EOF;
+} else if (raw == OFPRAW_OFPST11_FLOW_REQUEST) {
+const struct ofp11_flow_stats_request *ofsr;
+enum ofperr error;
+
+ofsr = ofpbuf_pull(msg, sizeof *ofsr);
+fsr->table_id = ofsr->table_id;
+error = ofputil_port_from_ofp11(ofsr->out_port, &fsr->out_port);
+if (error) {
+return error;
+}
+if (ofsr->out_group != htonl(OFPG11_ANY)) {
+return OFPERR_NXFMFC_GROUPS_NOT_SUPPORTED;
+}
+fsr->cookie = ofsr->cookie;
+fsr->cookie_mask = ofsr->cookie_mask;
+error = ofputil_pull_ofp11_match(msg, 0, &fsr->match, NULL);
+if (error) {
+return error;
+}
} else if (raw == OFPRAW_OFPST10_FLOW_REQUEST ||
raw == OFPRAW_OFPST_AGGREGATE_REQUEST) {
const struct ofp10_flow_stats_request *ofsr = msg->data;
@@ -1473,6 +1492,7 @@ ofputil_decode_flow_stats_request(struct
ofputil_flow_stats_request *fsr,
raw = ofpraw_pull_assert(&b);
switch ((int) raw) {
case OFPRAW_OFPST10_FLOW_REQUEST:
+case OFPRAW_OFPST11_FLOW_REQUEST:
return ofputil_decode_ofpst_flow_request(fsr, &b, false);
case OFPRAW_OFPST_AGGREGATE_REQUEST:
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 827d4f3..910d146 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -634,7 +634,7 @@ DP Description: None
])
AT_CLEANUP
-AT_SETUP([OFPST_FLOW request])
+AT_SETUP([OFPST_FLOW request - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
AT_CHECK([ovs-ofctl ofp-print "\
01 10 00 38 00 00 00 04 00 01 00 00 00 38 20 ff \
@@ -646,6 +646,18 @@ OFPST_FLOW request (xid=0x4): @&t@
])
AT_CLEANUP
+AT_SETUP([OFPST_FLOW request - OF1.2])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 12 00 38 00 00 00 02 00 01 00 00 00 00 00 00 \
+ff 00 00 00 ff ff ff ff ff ff ff ff 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 01 00 04 00 00 00 00 \
+"], [0], [dnl
+OFPST_FLOW request (OF1.2) (xid=0x2): @&t@
+])
+AT_CLEANUP
+
AT_SETUP([OFPST_FLOW reply])
AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "\
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 16/41] ofp-msgs: Split OFPRAW_OFPST_FLOW_{REQUEST, REPLY}
Split OFPRAW_OFPST_FLOW_{REQUEST,REPLY} into OpenFlow 1.0 and 1.1+
versions.
This is in preparation for adding encoding and decoding of
Open Flow 1.1 & 1.2 messages.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
lib/ofp-msgs.h | 14 ++
lib/ofp-util.c | 10 +-
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 0683bc5..7e7b21b 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -180,12 +180,16 @@ enum ofpraw {
OFPRAW_OFPST_DESC_REPLY,
/* OFPST 1.0 (1): struct ofp10_flow_stats_request. */
-OFPRAW_OFPST_FLOW_REQUEST,
+OFPRAW_OFPST10_FLOW_REQUEST,
+/* OFPST 1.1+ (1): struct ofp11_flow_stats_request, uint8_t[8][]. */
+OFPRAW_OFPST11_FLOW_REQUEST,
/* NXST 1.0 (0): struct nx_flow_stats_request, uint8_t[8][]. */
OFPRAW_NXST_FLOW_REQUEST,
/* OFPST 1.0 (1): uint8_t[]. */
-OFPRAW_OFPST_FLOW_REPLY,
+OFPRAW_OFPST10_FLOW_REPLY,
+/* OFPST 1.1+ (1): uint8_t[]. */
+OFPRAW_OFPST11_FLOW_REPLY,
/* NXST 1.0 (0): uint8_t[]. */
OFPRAW_NXST_FLOW_REPLY,
@@ -357,9 +361,11 @@ enum ofptype {
/* Statistics. */
OFPTYPE_DESC_STATS_REQUEST, /* OFPRAW_OFPST_DESC_REQUEST. */
OFPTYPE_DESC_STATS_REPLY,/* OFPRAW_OFPST_DESC_REPLY. */
-OFPTYPE_FLOW_STATS_REQUEST, /* OFPRAW_OFPST_FLOW_REQUEST.
+OFPTYPE_FLOW_STATS_REQUEST, /* OFPRAW_OFPST10_FLOW_REQUEST.
+ * OFPRAW_OFPST11_FLOW_REQUEST.
* OFPRAW_NXST_FLOW_REQUEST. */
-OFPTYPE_FLOW_STATS_REPLY,/* OFPRAW_OFPST_FLOW_REPLY.
+OFPTYPE_FLOW_STATS_REPLY,/* OFPRAW_OFPST10_FLOW_REPLY.
+ * OFPRAW_OFPST11_FLOW_REPLY.
* OFPRAW_NXST_FLOW_REPLY. */
OFPTYPE_AGGREGATE_STATS_REQUEST, /* OFPRAW_OFPST_AGGREGATE_REQUEST.
* OFPRAW_NXST_AGGREGATE_REQUEST. */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 716e6bc..878d307 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1453,7 +1453,7 @@ ofputil_decode_flow_stats_request(struct
ofputil_flow_stats_request *fsr,
ofpbuf_use_const(&b, oh, ntohs(oh->length));
raw = ofpraw_pull_assert(&b);
switch ((int) raw) {
-case OFPRAW_OFPST_FLOW_REQUEST:
+case OFPRAW_OFPST10_FLOW_REQUEST:
return ofputil_decode_ofpst_flow_request(fsr, b.data, false);
case OFPRAW_OFPST_AGGREGATE_REQUEST:
@@ -1487,7 +1487,7 @@ ofputil_encode_flow_stats_request(const struct
ofputil_flow_stats_request *fsr,
raw = (fsr->aggregate
? OFPRAW_OFPST_AGGREGATE_REQUEST
- : OFPRAW_OFPST_FLOW_REQUEST);
+ : OFPRAW_OFPST11_FLOW_REQUEST);
msg = ofpraw_alloc(raw, OFP12_VERSION, 0);
ofsr = ofpbuf_put_zeros(msg, sizeof *ofsr);
ofsr->table_id = fsr->table_id;
@@ -1505,7 +1505,7 @@ ofputil_encode_flow_stats_request(const struct
ofputil_flow_stats_request *fsr,
raw = (fsr->aggregate
? OFPRAW_OFPST_AGGREGATE_REQUEST
- : OFPRAW_OFPST_FLOW_REQUEST);
+ : OFPRAW_OFPST10_FLOW_REQUEST);
msg = ofpraw_alloc(raw, OFP10_VERSION, 0);
ofsr = ofpbuf_put_zeros(msg, sizeof *ofsr);
ofputil_cls_rule_to_ofp10_match(&fsr->match, &ofsr->match);
@@ -1596,7 +1596,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats
*fs,
if (!msg->size) {
return EOF;
-} else if (raw == OFPRAW_OFPST_FLOW_REPLY) {
+} else if (raw == OFPRAW_OFPST10_FLOW_REPLY) {
const struct ofp10_flow_stats *ofs;
size_t length;
@@ -1708,7 +1708,7 @@ ofputil_append_flow_stats_reply(const struct
ofputil_flow_stats *fs,
enum ofpraw raw;
ofpraw_decode_partial(&raw, reply->data, reply->size);
-if (raw == OFPRAW_OFPST_FLOW_REPLY) {
+if (raw == OFPRAW_OFPST10_FLOW_REPLY) {
struct ofp10_flow_stats *ofs;
ofpbuf_put_uninit(reply, sizeof *ofs);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 20/41] ofp-util: Allow decoding of Open Flow 1.2 Flow Statistics Response Messages
Signed-off-by: Simon Horman
---
v10
* Manual rebase
v9
* No change
v8
* Manual rebase
* Add ofp-print test
v7
* Omitted
v6
* No change
v5
* Manual rebase
* Use padded_match_len parameter of ofputil_pull_ofp12_match()
to obtain the length of a match. Using the remaining
length of the message only works for the last flow.
v4
* Don't explicitly pill the pad of the header of the message.
This is now handled ofputil_pull_stats_msg()
* Use OFPST11_FLOW as the message code, it already exists and
seems appropriate as struct ofp11_flow_stats is used.
* Log a warning if match parsing fails
* Log a warning if instruction parsing fails
* Correct length passed to ofpacts_pull_openflow11_instructions()
v3
* Initial post
---
lib/ofp-util.c | 41 +
tests/ofp-print.at | 37 -
2 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 2a12d95..309aca3 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1635,6 +1635,47 @@ ofputil_decode_flow_stats_reply(struct
ofputil_flow_stats *fs,
if (!msg->size) {
return EOF;
+} else if (raw == OFPRAW_OFPST11_FLOW_REPLY) {
+const struct ofp11_flow_stats *ofs;
+size_t length;
+uint16_t padded_match_len;
+
+ofs = ofpbuf_try_pull(msg, sizeof *ofs);
+if (!ofs) {
+VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply has %zu leftover "
+ "bytes at end", msg->size);
+return EINVAL;
+}
+
+length = ntohs(ofs->length);
+if (length < sizeof *ofs) {
+VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply claims invalid "
+ "length %zu", length);
+return EINVAL;
+}
+
+if (ofputil_pull_ofp11_match(msg, ntohs(ofs->priority), &fs->rule,
+ &padded_match_len)) {
+VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad match");
+return EINVAL;
+}
+
+if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -
+ padded_match_len, ofpacts)) {
+VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
+return EINVAL;
+}
+
+fs->table_id = ofs->table_id;
+fs->duration_sec = ntohl(ofs->duration_sec);
+fs->duration_nsec = ntohl(ofs->duration_nsec);
+fs->idle_timeout = ntohs(ofs->idle_timeout);
+fs->hard_timeout = ntohs(ofs->hard_timeout);
+fs->idle_age = -1;
+fs->hard_age = -1;
+fs->cookie = ofs->cookie;
+fs->packet_count = ntohll(ofs->packet_count);
+fs->byte_count = ntohll(ofs->byte_count);
} else if (raw == OFPRAW_OFPST10_FLOW_REPLY) {
const struct ofp10_flow_stats *ofs;
size_t length;
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 910d146..c6e55c9 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -658,7 +658,7 @@ OFPST_FLOW request (OF1.2) (xid=0x2): @&t@
])
AT_CLEANUP
-AT_SETUP([OFPST_FLOW reply])
+AT_SETUP([OFPST_FLOW reply - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "\
01 11 01 e4 00 00 00 04 00 01 00 00 00 60 00 00 \
@@ -702,6 +702,41 @@ OFPST_FLOW reply (xid=0x4):
])
AT_CLEANUP
+AT_SETUP([OFPST_FLOW reply - OF1.2])
+AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 13 01 78 00 00 00 02 00 01 00 00 00 00 00 00 \
+00 78 00 00 00 00 00 03 01 5e f3 c0 80 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 62 \
+00 01 00 2d 80 00 00 04 00 00 00 02 80 00 06 06 \
+ca da ad d6 0d 37 80 00 0a 02 08 00 80 00 10 01 \
+00 80 00 04 08 00 00 00 00 00 00 00 00 00 00 00 \
+00 04 00 18 00 00 00 00 00 00 00 10 00 00 00 02 \
+05 dc 00 00 00 00 00 00 00 78 00 00 00 00 00 04 \
+20 7c 0a 40 80 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 \
+00 00 00 00 00 00 00 8c 00 01 00 2d 80 00 00 04 \
+00 00 00 02 80 00 06 06 52 54 00 c3 00 89 80 00 \
+0a 02 08 00 80 00 10 01 00 80 00 04 08 00 00 00 \
+00 00 00 00 00 00 00 00 00 04 00 18 00 00 00 00 \
+00 00 00 10 00 00 00 02 05 dc 00 00 00 00 00 00 \
+00 78 00 00 00 00 00 04 20 a9 d1 00 80 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 2a \
+00 01 00 2d 80 00 00 04 00 00 00 02 80 00 06 06 \
+52 54 00 97 00 69 80 00 0a 02 08 00 80 00 10 01 \
+00 80 00 04 08 00 00 00 00 00 00 00 00 00 00 00 \
+00 04 00 18 00 00 00 00 00 00 00 10 00 00 00 02 \
+05 dc 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPST_FLOW reply (OF1.2) (xid=0x2):
+ cookie=0x0, duration=3.023s, table=0, n_packets=1, n_bytes=98,
ip,metadata=0,in_port=2,dl_dst=ca:da:ad:d6:0d:37,nw_tos=0 actions=output:2
+ cookie=0x0, duration=4.545s, table=0, n_packets=2, n_bytes=140,
ip,metadata=0,in_port=2,dl_dst=5
[ovs-dev] [PATCH 21/41] ofp-msgs: Split OFPRAW_OFPST_AGGREGATE_REQUEST
Split OFPRAW_OFPST_AGGREGATE_REQUEST into OpenFlow 1.0 and 1.1+
versions.
This should be sufficient to allow adding encoding and decoding of
Open Flow 1.1 and 1.2 Aggregate Stats Request messages.
Encoding and decoding of Open Flow 1.1 and 1.2 Aggregate Stats Response
messages works using the existing code without modification.
Signed-off-by: Simon Horman
---
v10
* Remove OpenFlow1.1 ofp-print tests, I do not have data
for these at this time.
v9
* No change
v8
* Initial post
---
lib/ofp-msgs.h | 9 ++---
lib/ofp-util.c | 14 --
tests/ofp-print.at | 27 +--
3 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 7e7b21b..dfd646d 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -194,11 +194,13 @@ enum ofpraw {
OFPRAW_NXST_FLOW_REPLY,
/* OFPST 1.0 (2): struct ofp10_flow_stats_request. */
-OFPRAW_OFPST_AGGREGATE_REQUEST,
+OFPRAW_OFPST10_AGGREGATE_REQUEST,
+/* OFPST 1.1+ (2): struct ofp11_flow_stats_request, uint8_t[8][]. */
+OFPRAW_OFPST11_AGGREGATE_REQUEST,
/* NXST 1.0 (1): struct nx_flow_stats_request, uint8_t[8][]. */
OFPRAW_NXST_AGGREGATE_REQUEST,
-/* OFPST 1.0 (2): struct ofp_aggregate_stats_reply. */
+/* OFPST 1.0+ (2): struct ofp_aggregate_stats_reply. */
OFPRAW_OFPST_AGGREGATE_REPLY,
/* NXST 1.0 (1): struct ofp_aggregate_stats_reply. */
OFPRAW_NXST_AGGREGATE_REPLY,
@@ -367,7 +369,8 @@ enum ofptype {
OFPTYPE_FLOW_STATS_REPLY,/* OFPRAW_OFPST10_FLOW_REPLY.
* OFPRAW_OFPST11_FLOW_REPLY.
* OFPRAW_NXST_FLOW_REPLY. */
-OFPTYPE_AGGREGATE_STATS_REQUEST, /* OFPRAW_OFPST_AGGREGATE_REQUEST.
+OFPTYPE_AGGREGATE_STATS_REQUEST, /* OFPRAW_OFPST10_AGGREGATE_REQUEST.
+ * OFPRAW_OFPST11_AGGREGATE_REQUEST.
* OFPRAW_NXST_AGGREGATE_REQUEST. */
OFPTYPE_AGGREGATE_STATS_REPLY, /* OFPRAW_OFPST_AGGREGATE_REPLY.
* OFPRAW_NXST_AGGREGATE_REPLY. */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 309aca3..504ad3e 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1420,7 +1420,8 @@ ofputil_decode_ofpst_flow_request(struct
ofputil_flow_stats_request *fsr,
if (!msg->size) {
return EOF;
-} else if (raw == OFPRAW_OFPST11_FLOW_REQUEST) {
+} else if (raw == OFPRAW_OFPST11_FLOW_REQUEST ||
+ raw == OFPRAW_OFPST11_AGGREGATE_REQUEST) {
const struct ofp11_flow_stats_request *ofsr;
enum ofperr error;
@@ -1440,7 +1441,7 @@ ofputil_decode_ofpst_flow_request(struct
ofputil_flow_stats_request *fsr,
return error;
}
} else if (raw == OFPRAW_OFPST10_FLOW_REQUEST ||
- raw == OFPRAW_OFPST_AGGREGATE_REQUEST) {
+ raw == OFPRAW_OFPST10_AGGREGATE_REQUEST) {
const struct ofp10_flow_stats_request *ofsr = msg->data;
ofputil_cls_rule_from_ofp10_match(&ofsr->match, 0, &fsr->match);
@@ -1495,7 +1496,8 @@ ofputil_decode_flow_stats_request(struct
ofputil_flow_stats_request *fsr,
case OFPRAW_OFPST11_FLOW_REQUEST:
return ofputil_decode_ofpst_flow_request(fsr, &b, false);
-case OFPRAW_OFPST_AGGREGATE_REQUEST:
+case OFPRAW_OFPST10_AGGREGATE_REQUEST:
+case OFPRAW_OFPST11_AGGREGATE_REQUEST:
return ofputil_decode_ofpst_flow_request(fsr, &b, true);
case OFPRAW_NXST_FLOW_REQUEST:
@@ -1525,7 +1527,7 @@ ofputil_encode_flow_stats_request(const struct
ofputil_flow_stats_request *fsr,
struct ofp11_flow_stats_request *ofsr;
raw = (fsr->aggregate
- ? OFPRAW_OFPST_AGGREGATE_REQUEST
+ ? OFPRAW_OFPST11_AGGREGATE_REQUEST
: OFPRAW_OFPST11_FLOW_REQUEST);
msg = ofpraw_alloc(raw, OFP12_VERSION, 0);
ofsr = ofpbuf_put_zeros(msg, sizeof *ofsr);
@@ -1543,7 +1545,7 @@ ofputil_encode_flow_stats_request(const struct
ofputil_flow_stats_request *fsr,
struct ofp10_flow_stats_request *ofsr;
raw = (fsr->aggregate
- ? OFPRAW_OFPST_AGGREGATE_REQUEST
+ ? OFPRAW_OFPST10_AGGREGATE_REQUEST
: OFPRAW_OFPST10_FLOW_REQUEST);
msg = ofpraw_alloc(raw, OFP10_VERSION, 0);
ofsr = ofpbuf_put_zeros(msg, sizeof *ofsr);
@@ -1879,7 +1881,7 @@ ofputil_encode_aggregate_stats_reply(
enum ofpraw raw;
ofpraw_decode(&raw, request);
-if (raw == OFPRAW_OFPST_AGGREGATE_REQUEST) {
+if (raw == OFPRAW_OFPST10_AGGREGATE_REQUEST) {
packet_count = unknown_to_zero(stats->packet_count);
byte_count = unknown_to_zero(stats->byte_count);
} else {
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index c6e55c9..5920a73 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -737,7 +737,7 @@ OFPST_FLOW reply (OF1.2) (xid=0x2):
])
AT_CLEANUP
-AT_SETUP([OFPST_AGGRE
[ovs-dev] [PATCH 25/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table Statistics Request Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Set wildcards, match, write_setfields and apply_setfields based
on a bitmap of (1 << OFPXMT_*)
v8
* Manual rebase
* Make use of enum ofp_version
* Add ofp-tests
v7
* Omitted
v6
* No change
v5
* Manual rebase
* Add OFPST_TABLE entry for Open Flow 1.1 and 1.2 to ofputil_msg_types,
this wires-up decoding of table statistics messages.
v4
* Initial post
table test
---
include/openflow/openflow-1.1.h | 4 ++
include/openflow/openflow-1.2.h | 5 ++
ofproto/ofproto-dpif.c | 32 --
ofproto/ofproto-provider.h | 71 +++--
ofproto/ofproto.c | 133
tests/ofp-print.at | 16 -
6 files changed, 237 insertions(+), 24 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 696c3ec..5592520 100644
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -281,6 +281,10 @@ enum ofp11_instruction_type {
OFPIT11_EXPERIMENTER = 0x /* Experimenter instruction */
};
+#define OFPIT11_ALL OFPIT11_GOTO_TABLE | OFPIT11_WRITE_METADATA | \
+OFPIT11_WRITE_ACTIONS | OFPIT11_APPLY_ACTIONS | \
+OFPIT11_CLEAR_ACTIONS
+
#define OFP11_INSTRUCTION_ALIGN 8
/* Generic ofp_instruction structure. */
diff --git a/include/openflow/openflow-1.2.h b/include/openflow/openflow-1.2.h
index 0a73ed1..64bc993 100644
--- a/include/openflow/openflow-1.2.h
+++ b/include/openflow/openflow-1.2.h
@@ -106,8 +106,13 @@ enum oxm12_ofb_match_fields {
OFPXMT12_OFB_IPV6_ND_TLL,/* Target link-layer for ND. */
OFPXMT12_OFB_MPLS_LABEL, /* MPLS label. */
OFPXMT12_OFB_MPLS_TC,/* MPLS TC. */
+
+/* End Marker */
+OFPXMT12_OFB_MAX,
};
+#define OFPXMT12_MASK ((1ULL << OFPXMT12_OFB_MAX) - 1)
+
/* OXM implementation makes use of NXM as they are the same format
* with different field definitions
*/
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 95195a3..dee0e0b 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1153,17 +1153,37 @@ get_features(struct ofproto *ofproto_ OVS_UNUSED,
}
static void
-get_tables(struct ofproto *ofproto_, struct ofp10_table_stats *ots)
+get_tables(struct ofproto *ofproto_, struct ofproto_table_stats *ots)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
struct dpif_dp_stats s;
-strcpy(ots->name, "classifier");
-
dpif_get_dp_stats(ofproto->dpif, &s);
-put_32aligned_be64(&ots->lookup_count, htonll(s.n_hit + s.n_missed));
-put_32aligned_be64(&ots->matched_count,
- htonll(s.n_hit + ofproto->n_matches));
+
+switch ((enum ofp_version)ots->ofp_version) {
+case OFP12_VERSION:
+strcpy(ots->o12ts->name, "classifier");
+ots->o12ts->lookup_count = htonll(s.n_hit + s.n_missed);
+ots->o12ts->matched_count = htonll(s.n_hit + ofproto->n_matches);
+break;
+
+case OFP11_VERSION:
+strcpy(ots->o11ts->name, "classifier");
+ots->o11ts->lookup_count = htonll(s.n_hit + s.n_missed);
+ots->o11ts->matched_count = htonll(s.n_hit + ofproto->n_matches);
+break;
+
+case OFP10_VERSION:
+strcpy(ots->o10ts->name, "classifier");
+put_32aligned_be64(&ots->o10ts->lookup_count,
+ htonll(s.n_hit + s.n_missed));
+put_32aligned_be64(&ots->o10ts->matched_count,
+ htonll(s.n_hit + ofproto->n_matches));
+break;
+
+default:
+NOT_REACHED();
+}
}
static struct ofport *
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index f0d57ee..3b7caad 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -194,6 +194,15 @@ struct rule {
uint64_t modify_seqno; /* Sequence number when changed. */
};
+struct ofproto_table_stats {
+enum ofp_version ofp_version;
+union {
+struct ofp10_table_stats *o10ts;
+struct ofp11_table_stats *o11ts;
+struct ofp12_table_stats *o12ts;
+};
+};
+
static inline struct rule *
rule_from_cls_rule(const struct cls_rule *cls_rule)
{
@@ -449,14 +458,48 @@ struct ofproto_class {
/* Helper for the OpenFlow OFPST_TABLE statistics request.
*
- * The 'ots' array contains 'ofproto->n_tables' elements. Each element is
- * initialized as:
+ * The 'ots' structure contains two elements
+ *
+ * - 'ofp_version' the OpenFLow version in use, set to one of:
+ * OFP10_VERSION, OFP11_VERSION, OFP12_VERSION.
+ * These values denote OpenFLow 1.0, 1.1 and 1.2
+ * respectively.
+ *
+ * - A union of 'o10ts', 'o11ts' and 'o12ts'.
+ *
+ *This is an array of OpenFLow version-specific table statistics
+ *elements.
+ *
+ *'o10ts' should be used for
[ovs-dev] [PATCH 27/41] ofp-msgs: Allow encoding and decoding of Open Flow 1.1 & 1.2 Barrier Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
lib/ofp-msgs.h | 4 ++--
tests/ofp-print.at | 32 ++--
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 7ef7c5a..752d12c 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -163,12 +163,12 @@ enum ofpraw {
/* OFPT 1.0 (18): void. */
OFPRAW_OFPT10_BARRIER_REQUEST,
-/* OFPT 1.1 (20): void. */
+/* OFPT 1.1+ (20): void. */
OFPRAW_OFPT11_BARRIER_REQUEST,
/* OFPT 1.0 (19): void. */
OFPRAW_OFPT10_BARRIER_REPLY,
-/* OFPT 1.1 (21): void. */
+/* OFPT 1.1+ (21): void. */
OFPRAW_OFPT11_BARRIER_REPLY,
/* Standard statistics. */
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 5d5873d..fef429c 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -990,20 +990,48 @@ OFPST_PORT_DESC reply (xid=0x0):
])
AT_CLEANUP
-AT_SETUP([OFPT_BARRIER_REQUEST])
+AT_SETUP([OFPT_BARRIER_REQUEST - OF1.0])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print '01 12 00 08 00 00 00 01'], [0], [dnl
OFPT_BARRIER_REQUEST (xid=0x1):
])
AT_CLEANUP
-AT_SETUP([OFPT_BARRIER_REPLY])
+AT_SETUP([OFPT_BARRIER_REQUEST - OF1.1])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print '02 14 00 08 00 00 00 01'], [0], [dnl
+OFPT_BARRIER_REQUEST (OF1.1) (xid=0x1):
+])
+AT_CLEANUP
+
+AT_SETUP([OFPT_BARRIER_REQUEST - OF1.2])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print '03 14 00 08 00 00 00 01'], [0], [dnl
+OFPT_BARRIER_REQUEST (OF1.2) (xid=0x1):
+])
+AT_CLEANUP
+
+AT_SETUP([OFPT_BARRIER_REPLY - OF1.0])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print '01 13 00 08 00 00 00 01'], [0], [dnl
OFPT_BARRIER_REPLY (xid=0x1):
])
AT_CLEANUP
+AT_SETUP([OFPT_BARRIER_REPLY] - OF1.1)
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print '02 15 00 08 00 00 00 01'], [0], [dnl
+OFPT_BARRIER_REPLY (OF1.1) (xid=0x1):
+])
+AT_CLEANUP
+
+AT_SETUP([OFPT_BARRIER_REPLY] - OF1.2)
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print '03 15 00 08 00 00 00 01'], [0], [dnl
+OFPT_BARRIER_REPLY (OF1.2) (xid=0x1):
+])
+AT_CLEANUP
+
AT_SETUP([NXT_ROLE_REQUEST])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print "\
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 30/41] ofp-util: Pass vconn to fetch_port_by_features()
Pass vconn to fetch_port_by_features() and callers.
In some cases this will reduce the number of connections
that ovs-ofputil sets up.
It should not alter the behaviour of ovs-ofputil.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Manual rebase
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial post
---
utilities/ovs-ofctl.c | 46 ++
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 4f4b149..606a3f2 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -591,29 +591,28 @@ ofctl_dump_tables(int argc OVS_UNUSED, char *argv[])
}
static bool
-fetch_port_by_features(const char *vconn_name,
- const char *port_name, unsigned int port_no,
- struct ofputil_phy_port *pp, bool *trunc)
+fetch_port_by_features(struct vconn *vconn, const char *port_name,
+ unsigned int port_no, struct ofputil_phy_port *pp,
+ bool *trunc)
{
struct ofputil_switch_features features;
const struct ofp_header *oh;
struct ofpbuf *request, *reply;
-struct vconn *vconn;
enum ofperr error;
enum ofptype type;
struct ofpbuf b;
bool found = false;
/* Fetch the switch's ofp_switch_features. */
-request = ofpraw_alloc(OFPRAW_OFPT_FEATURES_REQUEST, OFP10_VERSION, 0);
-open_vconn(vconn_name, &vconn);
-run(vconn_transact(vconn, request, &reply), "talking to %s", vconn_name);
-vconn_close(vconn);
+request = ofpraw_alloc(OFPRAW_OFPT_FEATURES_REQUEST,
+ vconn_get_version(vconn), 0);
+run(vconn_transact(vconn, request, &reply), "talking to %s",
+vconn_get_name(vconn));
oh = reply->data;
if (ofptype_decode(&type, reply->data)
|| type != OFPTYPE_FEATURES_REPLY) {
-ovs_fatal(0, "%s: received bad features reply", vconn_name);
+ovs_fatal(0, "%s: received bad features reply", vconn_get_name(vconn));
}
*trunc = false;
@@ -625,7 +624,7 @@ fetch_port_by_features(const char *vconn_name,
error = ofputil_decode_switch_features(oh, &features, &b);
if (error) {
ovs_fatal(0, "%s: failed to decode features reply (%s)",
- vconn_name, ofperr_to_string(error));
+ vconn_get_name(vconn), ofperr_to_string(error));
}
while (!ofputil_pull_phy_port(oh->version, &b, pp)) {
@@ -643,20 +642,18 @@ exit:
}
static bool
-fetch_port_by_stats(const char *vconn_name,
-const char *port_name, unsigned int port_no,
-struct ofputil_phy_port *pp)
+fetch_port_by_stats(struct vconn* vconn, const char *port_name,
+unsigned int port_no, struct ofputil_phy_port *pp)
{
struct ofpbuf *request;
-struct vconn *vconn;
ovs_be32 send_xid;
bool done = false;
bool found = false;
-request = ofpraw_alloc(OFPRAW_OFPST_PORT_DESC_REQUEST, OFP10_VERSION, 0);
+request = ofpraw_alloc(OFPRAW_OFPST_PORT_DESC_REQUEST,
+ vconn_get_version(vconn), 0);
send_xid = ((struct ofp_header *) request->data)->xid;
-open_vconn(vconn_name, &vconn);
send_openflow_buffer(vconn, request);
while (!done) {
ovs_be32 recv_xid;
@@ -700,7 +697,6 @@ fetch_port_by_stats(const char *vconn_name,
}
ofpbuf_delete(reply);
}
-vconn_close(vconn);
return found;
}
@@ -710,7 +706,7 @@ fetch_port_by_stats(const char *vconn_name,
* 'port_name' (which may be a port name or number), and copies it into
* '*pp'. */
static void
-fetch_ofputil_phy_port(const char *vconn_name, const char *port_name,
+fetch_ofputil_phy_port(struct vconn *vconn, const char *port_name,
struct ofputil_phy_port *pp)
{
unsigned int port_no;
@@ -725,14 +721,15 @@ fetch_ofputil_phy_port(const char *vconn_name, const char
*port_name,
/* Try to find the port based on the Features Reply. If it looks
* like the results may be truncated, then use the Port Description
* stats message introduced in OVS 1.7. */
-found = fetch_port_by_features(vconn_name, port_name, port_no, pp,
+found = fetch_port_by_features(vconn, port_name, port_no, pp,
&trunc);
if (trunc) {
-found = fetch_port_by_stats(vconn_name, port_name, port_no, pp);
+found = fetch_port_by_stats(vconn, port_name, port_no, pp);
}
if (!found) {
-ovs_fatal(0, "%s: couldn't find port `%s'", vconn_name, port_name);
+ovs_fatal(0, "%s: couldn't find port `%s'",
+ vconn_get_name(vconn), port_name);
}
}
@@ -769,7 +766,7 @@ str_to_port_no(struct vconn *vconn, const char *port_name)
} else {
struct ofputil_phy_port pp;
-fetch_ofputil_phy_port(vconn_get_name(vconn), port_name, &pp);
+fetch_ofputil_phy_port(vconn, port_
[ovs-dev] [PATCH 26/41] ovs-ofctl: Use vconn as a parameter of dump_stats_transaction()
In order to form a stats message for the prevailing OpenFlow version
of a vconn the vconn needs to be open at the time the request is
encoded. Thus there is no longer a case where it makes sense to
use dump_stats_transaction() without a vconn already being open and
available to pass as a parameter.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* No change
v8
* Initial post
---
tests/ofproto.at | 12 ++--
utilities/ovs-ofctl.c | 37 +++--
2 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 46675bc..23b5023 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -60,12 +60,12 @@ AT_CHECK([STRIP_XIDS stdout], [0], [dnl
OFPST_QUEUE reply: 0 queues
])
AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ALL 5], [0],
- [OFPT_ERROR (xid=0x1): OFPQOFC_BAD_QUEUE
-OFPST_QUEUE request (xid=0x1):port=ALL queue=5
+ [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_QUEUE
+OFPST_QUEUE request (xid=0x2):port=ALL queue=5
])
AT_CHECK([ovs-ofctl -vwarn queue-stats br0 10], [0],
- [OFPT_ERROR (xid=0x1): OFPQOFC_BAD_PORT
-OFPST_QUEUE request (xid=0x1):port=10 queue=ALL
+ [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_PORT
+OFPST_QUEUE request (xid=0x2):port=10 queue=ALL
])
OVS_VSWITCHD_STOP
AT_CLEANUP
@@ -354,7 +354,7 @@ AT_CLEANUP
AT_SETUP([ofproto - flow table configuration])
OVS_VSWITCHD_START
# Check the default configuration.
-(echo "OFPST_TABLE reply (xid=0x1): 255 tables
+(echo "OFPST_TABLE reply (xid=0x2): 255 tables
0: classifier: wild=0x3f, max=100, active=0
lookup=0, matched=0"
x=1
@@ -379,7 +379,7 @@ AT_CHECK(
])
# Check that the configuration was updated.
mv expout orig-expout
-(echo "OFPST_TABLE reply (xid=0x1): 255 tables
+(echo "OFPST_TABLE reply (xid=0x2): 255 tables
0: main: wild=0x3f, max=100, active=0
lookup=0, matched=0
1: table1 : wild=0x3f, max= 1024, active=0
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 4bbd536..0a6ac0e 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -424,7 +424,7 @@ dump_trivial_transaction(const char *vconn_name, enum
ofpraw raw)
}
static void
-dump_stats_transaction__(struct vconn *vconn, struct ofpbuf *request)
+dump_stats_transaction(struct vconn *vconn, struct ofpbuf *request)
{
const struct ofp_header *request_oh = request->data;
ovs_be32 send_xid = request_oh->xid;
@@ -467,24 +467,17 @@ dump_stats_transaction__(struct vconn *vconn, struct
ofpbuf *request)
}
static void
-dump_stats_transaction(const char *vconn_name, struct ofpbuf *request)
+dump_trivial_stats_transaction(const char *vconn_name, enum ofpraw raw)
{
+struct ofpbuf *request;
struct vconn *vconn;
open_vconn(vconn_name, &vconn);
-dump_stats_transaction__(vconn, request);
+request = ofpraw_alloc(raw, vconn_get_version(vconn), 0);
+dump_stats_transaction(vconn, request);
vconn_close(vconn);
}
-static void
-dump_trivial_stats_transaction(const char *vconn_name, enum ofpraw raw)
-{
-struct ofpbuf *request;
-
-request = ofpraw_alloc(raw, OFP10_VERSION, 0);
-dump_stats_transaction(vconn_name, request);
-}
-
/* Sends 'request', which should be a request that only has a reply if an error
* occurs, and waits for it to succeed or fail. If an error does occur, prints
* it and exits with an error.
@@ -839,7 +832,7 @@ ofctl_dump_flows__(int argc, char *argv[], bool aggregate)
struct vconn *vconn;
vconn = prepare_dump_flows(argc, argv, aggregate, &request);
-dump_stats_transaction__(vconn, request);
+dump_stats_transaction(vconn, request);
vconn_close(vconn);
}
@@ -956,8 +949,11 @@ ofctl_queue_stats(int argc, char *argv[])
{
struct ofp10_queue_stats_request *req;
struct ofpbuf *request;
+struct vconn *vconn;
-request = ofpraw_alloc(OFPRAW_OFPST_QUEUE_REQUEST, OFP10_VERSION, 0);
+open_vconn(argv[1], &vconn);
+request = ofpraw_alloc(OFPRAW_OFPST_QUEUE_REQUEST,
+ vconn_get_version(vconn), 0);
req = ofpbuf_put_zeros(request, sizeof *req);
if (argc > 2 && argv[2][0] && strcasecmp(argv[2], "all")) {
@@ -973,7 +969,8 @@ ofctl_queue_stats(int argc, char *argv[])
memset(req->pad, 0, sizeof req->pad);
-dump_stats_transaction(argv[1], request);
+dump_stats_transaction(vconn, request);
+vconn_close(vconn);
}
static enum ofputil_protocol
@@ -1377,7 +1374,7 @@ ofctl_monitor(int argc, char *argv[])
msg = ofpbuf_new(0);
ofputil_append_flow_monitor_request(&fmr, msg);
-dump_stats_transaction__(vconn, msg);
+dump_stats_transaction(vconn, msg);
} else {
ovs_fatal(0, "%s: unsupported \"monitor\" argument", arg);
}
@@ -1418,13 +1415,17 @@ ofctl_dump_ports(int argc, char *argv[])
{
struct ofp10_port_stats_request *req;
struct ofpbuf *request;
+struct vconn *vconn;
[ovs-dev] [PATCH 28/41] ofp-msgs: Split OFPRAW_OFPST_PORT_{REQUEST, REPLY}
Split OFPRAW_OFPST_PORT_{REQUEST,REPLY} into OpenFlow 1.0 and 1.1+
versions.
This prepares for encoding and decoding Open Flow 1.1 and 1.2 Port Stats
Request and Reply messages.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Update description - this only prepares for both encoding and decoding
* Move tests to "ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port
Reply Messages" as this is just a preparatory patch.
v8
* Initial post
---
lib/ofp-msgs.h| 14 ++
utilities/ovs-ofctl.c | 15 +--
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 752d12c..6420c5d 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -216,10 +216,14 @@ enum ofpraw {
OFPRAW_OFPST12_TABLE_REPLY,
/* OFPST 1.0 (4): struct ofp10_port_stats_request. */
-OFPRAW_OFPST_PORT_REQUEST,
+OFPRAW_OFPST10_PORT_REQUEST,
+/* OFPST 1.1+ (4): struct ofp11_port_stats_request. */
+OFPRAW_OFPST11_PORT_REQUEST,
/* OFPST 1.0 (4): struct ofp10_port_stats[]. */
-OFPRAW_OFPST_PORT_REPLY,
+OFPRAW_OFPST10_PORT_REPLY,
+/* OFPST 1.1+ (4): struct ofp11_port_stats[]. */
+OFPRAW_OFPST11_PORT_REPLY,
/* OFPST 1.0 (5): struct ofp10_queue_stats_request. */
OFPRAW_OFPST_QUEUE_REQUEST,
@@ -382,8 +386,10 @@ enum ofptype {
OFPTYPE_TABLE_STATS_REPLY, /* OFPRAW_OFPST10_TABLE_REPLY.
* OFPRAW_OFPST11_TABLE_REPLY.
* OFPRAW_OFPST12_TABLE_REPLY. */
-OFPTYPE_PORT_STATS_REQUEST, /* OFPRAW_OFPST_PORT_REQUEST. */
-OFPTYPE_PORT_STATS_REPLY,/* OFPRAW_OFPST_PORT_REPLY. */
+OFPTYPE_PORT_STATS_REQUEST, /* OFPRAW_OFPST10_PORT_REQUEST.
+ * OFPRAW_OFPST11_PORT_REQUEST. */
+OFPTYPE_PORT_STATS_REPLY,/* OFPRAW_OFPST10_PORT_REPLY.
+ * OFPRAW_OFPST11_PORT_REPLY. */
OFPTYPE_QUEUE_STATS_REQUEST, /* OFPRAW_OFPST_QUEUE_REQUEST. */
OFPTYPE_QUEUE_STATS_REPLY, /* OFPRAW_OFPST_QUEUE_REPLY. */
OFPTYPE_PORT_DESC_STATS_REQUEST, /* OFPRAW_OFPST_PORT_DESC_REQUEST. */
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 0a6ac0e..06f6dd8 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1417,10 +1417,21 @@ ofctl_dump_ports(int argc, char *argv[])
struct ofpbuf *request;
struct vconn *vconn;
uint16_t port;
+enum ofpraw raw;
open_vconn(argv[1], &vconn);
-request = ofpraw_alloc(OFPRAW_OFPST_PORT_REQUEST,
- vconn_get_version(vconn), 0);
+switch (vconn_get_version(vconn)) {
+case OFP10_VERSION:
+raw = OFPRAW_OFPST10_PORT_REQUEST;
+break;
+case OFP11_VERSION:
+case OFP12_VERSION:
+raw = OFPRAW_OFPST11_PORT_REQUEST;
+break;
+default:
+NOT_REACHED();
+}
+request = ofpraw_alloc(raw, vconn_get_version(vconn), 0);
req = ofpbuf_put_zeros(request, sizeof *req);
port = argc > 2 ? str_to_port_no(argv[1], argv[2]) : OFPP_NONE;
req->port_no = htons(port);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 29/41] ovs-ofputil: Make str_to_port_no() aware of invalid ports
Open Flow 1.1 and 1.2 make use of 32 bit ports,
however Open vSwtich maps them to 16 bits. Make
ovs-ofputl aware of this.
Also, only accept ports that fit into 16 bits
for Open Flow 1.0.
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Manual Rebase
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial post
---
utilities/ovs-ofctl.c | 39 ---
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 06f6dd8..4f4b149 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -739,18 +739,42 @@ fetch_ofputil_phy_port(const char *vconn_name, const char
*port_name,
/* Returns the port number corresponding to 'port_name' (which may be a port
* name or number) within the switch 'vconn_name'. */
static uint16_t
-str_to_port_no(const char *vconn_name, const char *port_name)
+str_to_port_no(struct vconn *vconn, const char *port_name)
{
unsigned int port_no;
if (str_to_uint(port_name, 10, &port_no)) {
-return port_no;
+switch (vconn_get_version(vconn)) {
+case OFP12_VERSION:
+case OFP11_VERSION: {
+enum ofperr error;
+uint16_t ofp10_port;
+
+error = ofputil_port_from_ofp11(htonl(port_no), &ofp10_port);
+if (!error) {
+return ofp10_port;
+}
+break;
+}
+
+case OFP10_VERSION:
+if (port_no <= OFPP_NONE) {
+return port_no;
+}
+break;
+
+default:
+NOT_REACHED();
+}
} else {
struct ofputil_phy_port pp;
-fetch_ofputil_phy_port(vconn_name, port_name, &pp);
+fetch_ofputil_phy_port(vconn_get_name(vconn), port_name, &pp);
return pp.port_no;
}
+
+ovs_fatal(0, "%s: port out of range`%s'",
+ vconn_get_name(vconn), port_name);
}
static bool
@@ -957,7 +981,7 @@ ofctl_queue_stats(int argc, char *argv[])
req = ofpbuf_put_zeros(request, sizeof *req);
if (argc > 2 && argv[2][0] && strcasecmp(argv[2], "all")) {
-req->port_no = htons(str_to_port_no(argv[1], argv[2]));
+req->port_no = htons(str_to_port_no(vconn, argv[2]));
} else {
req->port_no = htons(OFPP_ALL);
}
@@ -1433,7 +1457,7 @@ ofctl_dump_ports(int argc, char *argv[])
}
request = ofpraw_alloc(raw, vconn_get_version(vconn), 0);
req = ofpbuf_put_zeros(request, sizeof *req);
-port = argc > 2 ? str_to_port_no(argv[1], argv[2]) : OFPP_NONE;
+port = argc > 2 ? str_to_port_no(vconn, argv[2]) : OFPP_NONE;
req->port_no = htons(port);
dump_stats_transaction(vconn, request);
vconn_close(vconn);
@@ -1471,17 +1495,18 @@ ofctl_packet_out(int argc, char *argv[])
struct vconn *vconn;
int i;
+protocol = open_vconn(argv[1], &vconn);
+
ofpbuf_init(&ofpacts, 64);
parse_ofpacts(argv[3], &ofpacts);
po.buffer_id = UINT32_MAX;
po.in_port = (!strcasecmp(argv[2], "none") ? OFPP_NONE
: !strcasecmp(argv[2], "local") ? OFPP_LOCAL
- : str_to_port_no(argv[1], argv[2]));
+ : str_to_port_no(vconn, argv[2]));
po.ofpacts = ofpacts.data;
po.ofpacts_len = ofpacts.size;
-protocol = open_vconn(argv[1], &vconn);
for (i = 4; i < argc; i++) {
struct ofpbuf *packet, *opo;
const char *error_msg;
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 24/41] ofp-print: Enable display of Open Flow 1.1 & 1.2 Table Stats Reply Messages
Signed-off-by: Simon Horman
---
v10
* Generate OFPST_TABLE reply - OF1.2 test to save
rather a lot of lines in tests/ofp-print.at.
v9
* No change
v8
* Manual rebase
v8
* Manual rebase
* Make use of enum ofp_version
* Add ofp-print test
- Currently there is no Open Flow 1.1 test as I am unsure how
to generate the data at this time
v7
* Omitted
v6
* Copy entire name, previously only the first 8 bytes were copied.
v5
* Initial post
---
lib/ofp-print.c| 162 -
tests/ofp-print.at | 57 ++-
2 files changed, 204 insertions(+), 15 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index b4b8bb6..0a9a5c2 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1135,8 +1135,124 @@ ofp_print_ofpst_port_reply(struct ds *string, const
struct ofp_header *oh,
}
static void
-ofp_print_ofpst_table_reply(struct ds *string, const struct ofp_header *oh,
-int verbosity)
+ofp_print_one_ofpst_table_reply(struct ds *string, enum ofp_version
ofp_version,
+const char *name, struct ofp12_table_stats *ts)
+{
+char name_[OFP_MAX_TABLE_NAME_LEN + 1];
+
+ovs_strlcpy(name_, name, sizeof name_);
+
+ds_put_format(string, " %d: %-8s: ", ts->table_id, name_);
+ds_put_format(string, "wild=0x%05"PRIx64", ", ntohll(ts->wildcards));
+ds_put_format(string, "max=%6"PRIu32", ", ntohl(ts->max_entries));
+ds_put_format(string, "active=%"PRIu32"\n", ntohl(ts->active_count));
+ds_put_cstr(string, " ");
+ds_put_format(string, "lookup=%"PRIu64", ", ntohll(ts->lookup_count));
+ds_put_format(string, "matched=%"PRIu64"\n", ntohll(ts->matched_count));
+
+if (ofp_version < OFP11_VERSION) {
+return;
+}
+
+ds_put_cstr(string, " ");
+ds_put_format(string, "match=%08"PRIx64", ", ntohll(ts->match));
+ds_put_format(string, "instructions=%08"PRIx32", ",
+ ntohl(ts->instructions));
+ds_put_format(string, "config=%08"PRIx32"\n", ntohl(ts->config));
+ds_put_cstr(string, " ");
+ds_put_format(string, "write_actions=%08"PRIx32", ",
+ ntohl(ts->write_actions));
+ds_put_format(string, "apply_actions=%08"PRIx32"\n",
+ ntohl(ts->apply_actions));
+
+if (ofp_version < OFP12_VERSION) {
+return;
+}
+
+ds_put_cstr(string, " ");
+ds_put_format(string, "write_setfields=%016"PRIx64"\n",
+ ntohll(ts->write_setfields));
+ds_put_cstr(string, " ");
+ds_put_format(string, "apply_setfields=%016"PRIx64"\n",
+ ntohll(ts->apply_setfields));
+ds_put_cstr(string, " ");
+ds_put_format(string, "metadata_match=%016"PRIx64"\n",
+ ntohll(ts->metadata_match));
+ds_put_cstr(string, " ");
+ds_put_format(string, "metadata_write=%016"PRIx64"\n",
+ ntohll(ts->metadata_write));
+}
+
+static void
+ofp_print_ofpst_table_reply12(struct ds *string, const struct ofp_header *oh,
+ int verbosity)
+{
+struct ofp12_table_stats *ts;
+struct ofpbuf b;
+size_t n;
+
+ofpbuf_use_const(&b, oh, ntohs(oh->length));
+ofpraw_pull_assert(&b);
+
+n = b.size / sizeof *ts;
+ds_put_format(string, " %zu tables\n", n);
+if (verbosity < 1) {
+return;
+}
+
+for (;;) {
+ts = ofpbuf_try_pull(&b, sizeof *ts);
+if (!ts) {
+return;
+}
+
+ofp_print_one_ofpst_table_reply(string, OFP12_VERSION, ts->name, ts);
+ }
+}
+
+static void
+ofp_print_ofpst_table_reply11(struct ds *string, const struct ofp_header *oh,
+ int verbosity)
+{
+struct ofp11_table_stats *ts;
+struct ofpbuf b;
+size_t n;
+
+ofpbuf_use_const(&b, oh, ntohs(oh->length));
+ofpraw_pull_assert(&b);
+
+n = b.size / sizeof *ts;
+ds_put_format(string, " %zu tables\n", n);
+if (verbosity < 1) {
+return;
+}
+
+for (;;) {
+struct ofp12_table_stats ts12;
+
+ts = ofpbuf_try_pull(&b, sizeof *ts);
+if (!ts) {
+return;
+}
+
+ts12.table_id = ts->table_id;
+ts12.wildcards = htonll(ntohl(ts->wildcards));
+ts12.max_entries = ts->max_entries;
+ts12.active_count = ts->active_count;
+ts12.lookup_count = ts->lookup_count;
+ts12.matched_count = ts->matched_count;
+ts12.match = htonll(ntohl(ts->match));
+ts12.instructions = ts->instructions;
+ts12.config = ts->config;
+ts12.write_actions = ts->write_actions;
+ts12.apply_actions = ts->apply_actions;
+ofp_print_one_ofpst_table_reply(string, OFP11_VERSION, ts->name,
&ts12);
+ }
+}
+
+static void
+ofp_print_ofpst_table_reply10(struct ds *string, const struct ofp_header *oh,
+ int verbosi
[ovs-dev] [PATCH 31/41] ofp-util: Allow encoding of Open Flow 1.1 & 1.2 Port Statistics Reply Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Move print test to "ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port
Reply Messages"
v8
* Manual rebase
* Make use of enum ofp_version
* Add ofp-print test
v7
* Omitted
v6
* No change
v5
* No change
v4
* Initial post
fix port test
---
ofproto/ofproto.c | 62 +++
1 file changed, 53 insertions(+), 9 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index fbb6448..cbf90c9 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2352,16 +2352,11 @@ handle_table_stats_request(struct ofconn *ofconn,
}
static void
-append_port_stat(struct ofport *port, struct list *replies)
+append_port_stat10(struct ofport *port, struct netdev_stats stats,
+ struct list *replies)
{
-struct netdev_stats stats;
struct ofp10_port_stats *ops;
-/* Intentionally ignore return value, since errors will set
- * 'stats' to all-1s, which is correct for OpenFlow, and
- * netdev_get_stats() will log errors. */
-ofproto_port_get_stats(port, &stats);
-
ops = ofpmp_append(replies, sizeof *ops);
ops->port_no = htons(port->pp.port_no);
memset(ops->pad, 0, sizeof ops->pad);
@@ -2379,6 +2374,55 @@ append_port_stat(struct ofport *port, struct list
*replies)
put_32aligned_be64(&ops->collisions, htonll(stats.collisions));
}
+static void
+append_port_stat11(struct ofport *port, struct netdev_stats stats,
+ struct list *replies)
+{
+struct ofp11_port_stats *ops;
+
+ops = ofpmp_append(replies, sizeof *ops);
+ops->port_no = ofputil_port_to_ofp11(port->pp.port_no);
+memset(ops->pad, 0, sizeof ops->pad);
+ops->rx_packets = htonll(stats.rx_packets);
+ops->tx_packets = htonll(stats.tx_packets);
+ops->rx_bytes = htonll(stats.rx_bytes);
+ops->tx_bytes = htonll(stats.tx_bytes);
+ops->rx_dropped = htonll(stats.rx_dropped);
+ops->tx_dropped = htonll(stats.tx_dropped);
+ops->rx_errors = htonll(stats.rx_errors);
+ops->tx_errors = htonll(stats.tx_errors);
+ops->rx_frame_err = htonll(stats.rx_frame_errors);
+ops->rx_over_err = htonll(stats.rx_over_errors);
+ops->rx_crc_err = htonll(stats.rx_crc_errors);
+ops->collisions = htonll(stats.collisions);
+}
+
+static void
+append_port_stat(struct ofport *port, enum ofp_version ofp_version,
+ struct list *replies)
+{
+struct netdev_stats stats;
+
+/* Intentionally ignore return value, since errors will set
+ * 'stats' to all-1s, which is correct for OpenFlow, and
+ * netdev_get_stats() will log errors. */
+ofproto_port_get_stats(port, &stats);
+
+switch (ofp_version) {
+case OFP12_VERSION:
+case OFP11_VERSION:
+append_port_stat11(port, stats, replies);
+break;
+
+case OFP10_VERSION:
+append_port_stat10(port, stats, replies);
+break;
+
+default:
+NOT_REACHED();
+}
+}
+
static enum ofperr
handle_port_stats_request(struct ofconn *ofconn,
const struct ofp_header *request)
@@ -2392,11 +2436,11 @@ handle_port_stats_request(struct ofconn *ofconn,
if (psr->port_no != htons(OFPP_NONE)) {
port = ofproto_get_port(p, ntohs(psr->port_no));
if (port) {
-append_port_stat(port, &replies);
+append_port_stat(port, request->version, &replies);
}
} else {
HMAP_FOR_EACH (port, hmap_node, &p->ports) {
-append_port_stat(port, &replies);
+append_port_stat(port, request->version, &replies);
}
}
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 33/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Port Status Request Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Manual Rebase
* Make use of enum ofp_version
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial Post
---
ofproto/ofproto.c | 27 ---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index cbf90c9..00b9bc4 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2428,13 +2428,34 @@ handle_port_stats_request(struct ofconn *ofconn,
const struct ofp_header *request)
{
struct ofproto *p = ofconn_get_ofproto(ofconn);
-const struct ofp10_port_stats_request *psr = ofpmsg_body(request);
struct ofport *port;
struct list replies;
+uint16_t port_no;
+
+switch ((enum ofp_version)request->version) {
+case OFP12_VERSION:
+case OFP11_VERSION: {
+const struct ofp11_port_stats_request *psr = ofpmsg_body(request);
+enum ofperr error = ofputil_port_from_ofp11(psr->port_no, &port_no);
+if (error) {
+return error;
+}
+break;
+}
+
+case OFP10_VERSION: {
+const struct ofp10_port_stats_request *psr = ofpmsg_body(request);
+port_no = ntohs(psr->port_no);
+break;
+}
+
+default:
+NOT_REACHED();
+}
ofpmp_init(&replies, request);
-if (psr->port_no != htons(OFPP_NONE)) {
-port = ofproto_get_port(p, ntohs(psr->port_no));
+if (port_no != OFPP_NONE) {
+port = ofproto_get_port(p, port_no);
if (port) {
append_port_stat(port, request->version, &replies);
}
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 32/41] ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port Reply Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Make use of enum ofp_version
* Add ofp-print test
* Correct printing of port number for Open Flow 1.0
* Replace UINT64_MAX with htonll(UINT64_MAX) in ofp_print_port_stat()
to correct byte-order problem detected by sparse
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial Post
---
lib/ofp-print.c| 87 +++---
tests/ofp-print.at | 37 ++-
2 files changed, 99 insertions(+), 25 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 0a9a5c2..c1bcbcf 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1066,14 +1066,12 @@ ofp_print_aggregate_stats_reply(struct ds *string,
const struct ofp_header *oh)
ds_put_format(string, " flow_count=%"PRIu32, as.flow_count);
}
-static void print_port_stat(struct ds *string, const char *leader,
-const ovs_32aligned_be64 *statp, int more)
+static void ofp_print_port_stat(struct ds *string, const char *leader,
+ovs_be64 stat, int more)
{
-uint64_t stat = ntohll(get_32aligned_be64(statp));
-
ds_put_cstr(string, leader);
-if (stat != UINT64_MAX) {
-ds_put_format(string, "%"PRIu64, stat);
+if (stat != htonll(UINT64_MAX)) {
+ds_put_format(string, "%"PRIu64, ntohll(stat));
} else {
ds_put_char(string, '?');
}
@@ -1084,6 +1082,12 @@ static void print_port_stat(struct ds *string, const
char *leader,
}
}
+static void print_port_stat(struct ds *string, const char *leader,
+const ovs_32aligned_be64 *statp, int more)
+{
+ofp_print_port_stat(string, leader, get_32aligned_be64(statp), more);
+}
+
static void
ofp_print_ofpst_port_request(struct ds *string, const struct ofp_header *oh)
{
@@ -1095,42 +1099,77 @@ static void
ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh,
int verbosity)
{
-struct ofp10_port_stats *ps;
struct ofpbuf b;
size_t n;
+struct ofp11_port_stats ps;
ofpbuf_use_const(&b, oh, ntohs(oh->length));
ofpraw_pull_assert(&b);
-n = b.size / sizeof *ps;
+/* struct ofp10_port_stats and struct ofp11_port_stats are the
+ * same size.
+ */
+n = b.size / sizeof ps;
ds_put_format(string, " %zu ports\n", n);
if (verbosity < 1) {
return;
}
for (;;) {
-ps = ofpbuf_try_pull(&b, sizeof *ps);
-if (!ps) {
-return;
+
+switch ((enum ofp_version)oh->version) {
+case OFP12_VERSION:
+case OFP11_VERSION: {
+struct ofp11_port_stats *ps11 = ofpbuf_try_pull(&b, sizeof *ps11);
+if (!ps11) {
+return;
+}
+ds_put_format(string, " port %2"PRIu32": ", ntohl(ps11->port_no));
+ps = *ps11;
+break;
}
-ds_put_format(string, " port %2"PRIu16": ", ntohs(ps->port_no));
+case OFP10_VERSION: {
+struct ofp10_port_stats *ps10 = ofpbuf_try_pull(&b, sizeof *ps10);
+if (!ps10) {
+return;
+}
+
+ds_put_format(string, " port %2"PRIu16": ", ntohs(ps10->port_no));
+ps.rx_packets = get_32aligned_be64(&ps10->rx_packets);
+ps.rx_bytes = get_32aligned_be64(&ps10->rx_bytes);
+ps.rx_dropped = get_32aligned_be64(&ps10->rx_dropped);
+ps.rx_errors = get_32aligned_be64(&ps10->rx_errors);
+ps.rx_frame_err = get_32aligned_be64(&ps10->rx_frame_err);
+ps.rx_over_err = get_32aligned_be64(&ps10->rx_over_err);
+ps.rx_crc_err = get_32aligned_be64(&ps10->rx_crc_err);
+ps.tx_packets = get_32aligned_be64(&ps10->tx_packets);
+ps.tx_bytes = get_32aligned_be64(&ps10->tx_bytes);
+ps.tx_dropped = get_32aligned_be64(&ps10->tx_dropped);
+ps.tx_errors = get_32aligned_be64(&ps10->tx_errors);
+ps.collisions = get_32aligned_be64(&ps10->collisions);
+break;
+}
+
+default:
+NOT_REACHED();
+}
ds_put_cstr(string, "rx ");
-print_port_stat(string, "pkts=", &ps->rx_packets, 1);
-print_port_stat(string, "bytes=", &ps->rx_bytes, 1);
-print_port_stat(string, "drop=", &ps->rx_dropped, 1);
-print_port_stat(string, "errs=", &ps->rx_errors, 1);
-print_port_stat(string, "frame=", &ps->rx_frame_err, 1);
-print_port_stat(string, "over=", &ps->rx_over_err, 1);
-print_port_stat(string, "crc=", &ps->rx_crc_err, 0);
+ofp_print_port_stat(string, "pkts=", ps.rx_packets, 1);
+ofp_print_port_stat(string, "bytes=", ps.rx_bytes, 1);
+ofp_print_port_stat(string, "drop=", ps.rx_dropped, 1);
+ofp_print_port_stat(string, "errs=", ps.rx_errors, 1);
+ofp_print_port_stat(string, "frame=", ps.rx_frame_err, 1);
+ofp_p
[ovs-dev] [PATCH 35/41] ovs-ofctl: Teach dump-ports about Open Flow 1.1 & 1.2
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Initial post
---
utilities/ovs-ofctl.c | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 606a3f2..81df40c 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1434,7 +1434,6 @@ ofctl_snoop(int argc OVS_UNUSED, char *argv[])
static void
ofctl_dump_ports(int argc, char *argv[])
{
-struct ofp10_port_stats_request *req;
struct ofpbuf *request;
struct vconn *vconn;
uint16_t port;
@@ -1453,9 +1452,24 @@ ofctl_dump_ports(int argc, char *argv[])
NOT_REACHED();
}
request = ofpraw_alloc(raw, vconn_get_version(vconn), 0);
-req = ofpbuf_put_zeros(request, sizeof *req);
port = argc > 2 ? str_to_port_no(vconn, argv[2]) : OFPP_NONE;
-req->port_no = htons(port);
+switch (vconn_get_version(vconn)) {
+case OFP10_VERSION: {
+struct ofp10_port_stats_request *req;
+req = ofpbuf_put_zeros(request, sizeof *req);
+req->port_no = htons(port);
+break;
+}
+case OFP11_VERSION:
+case OFP12_VERSION: {
+struct ofp11_port_stats_request *req;
+req = ofpbuf_put_zeros(request, sizeof *req);
+req->port_no = ofputil_port_to_ofp11(port);
+break;
+}
+default:
+NOT_REACHED();
+}
dump_stats_transaction(vconn, request);
vconn_close(vconn);
}
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 34/41] ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port Status Request Messages
Signed-off-by: Simon Horman
---
v10
* No change
v9
* Manual Rebase
* Make use of enum ofp_version
* Change title from "ofp-print: Allow display of Open Flow 1.1 & 1.2 Port Status
Request Messages" to "ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port
Status
Request Messages"
* Add ofp-print test
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial Post
---
lib/ofp-print.c| 19 +--
tests/ofp-print.at | 22 +-
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index c1bcbcf..20965a2 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1091,8 +1091,23 @@ static void print_port_stat(struct ds *string, const
char *leader,
static void
ofp_print_ofpst_port_request(struct ds *string, const struct ofp_header *oh)
{
-const struct ofp10_port_stats_request *psr = ofpmsg_body(oh);
-ds_put_format(string, " port_no=%"PRIu16, ntohs(psr->port_no));
+switch ((enum ofp_version)oh->version) {
+case OFP12_VERSION:
+case OFP11_VERSION: {
+const struct ofp11_port_stats_request *psr = ofpmsg_body(oh);
+ds_put_format(string, " port_no=%"PRIu32, ntohl(psr->port_no));
+break;
+}
+
+case OFP10_VERSION: {
+const struct ofp10_port_stats_request *psr = ofpmsg_body(oh);
+ds_put_format(string, " port_no=%"PRIu16, ntohs(psr->port_no));
+break;
+}
+
+default:
+NOT_REACHED();
+}
}
static void
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 732a771..0871526 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -874,7 +874,7 @@ AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "$(cat in)"], [0], [expout])
AT_CLEANUP
-AT_SETUP([OFPST_PORT request])
+AT_SETUP([OFPST_PORT request - 1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
AT_CHECK([ovs-ofctl ofp-print "\
01 10 00 14 00 00 00 01 00 04 00 00 ff ff 00 00 \
@@ -884,6 +884,26 @@ OFPST_PORT request (xid=0x1): port_no=65535
])
AT_CLEANUP
+AT_SETUP([OFPST_PORT request - 1.1])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+02 12 00 18 00 00 00 02 00 04 00 00 00 00 00 00 \
+ff ff 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPST_PORT request (OF1.1) (xid=0x2): port_no=4294901760
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_PORT request - 1.2])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 12 00 18 00 00 00 02 00 04 00 00 00 00 00 00 \
+ff ff 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPST_PORT request (OF1.2) (xid=0x2): port_no=4294901760
+])
+AT_CLEANUP
+
AT_SETUP([OFPST_PORT reply - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "\
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 38/41] ofp-util: Allow encoding of Open Flow 1.1 & 1.2 Queue Stats Reply Messages
Signed-off-by: Simon Horman
---
v10
* Manual rebase
* Make use of enum ofp_version
v9
* Omitted
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial Post
---
ofproto/ofproto.c | 40 ++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 00b9bc4..51937c2 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2879,8 +2879,22 @@ struct queue_stats_cbdata {
};
static void
-put_queue_stats(struct queue_stats_cbdata *cbdata, uint32_t queue_id,
-const struct netdev_queue_stats *stats)
+put_queue_stats11(struct queue_stats_cbdata *cbdata, uint32_t queue_id,
+ const struct netdev_queue_stats *stats)
+{
+struct ofp11_queue_stats *reply;
+
+reply = ofpmp_append(&cbdata->replies, sizeof *reply);
+reply->port_no = ofputil_port_to_ofp11(cbdata->ofport->pp.port_no);
+reply->queue_id = htonl(queue_id);
+reply->tx_bytes = htonll(stats->tx_bytes);
+reply->tx_packets = htonll(stats->tx_packets);
+reply->tx_errors = htonll(stats->tx_errors);
+}
+
+static void
+put_queue_stats10(struct queue_stats_cbdata *cbdata, uint32_t queue_id,
+ const struct netdev_queue_stats *stats)
{
struct ofp10_queue_stats *reply;
@@ -2894,6 +2908,28 @@ put_queue_stats(struct queue_stats_cbdata *cbdata,
uint32_t queue_id,
}
static void
+put_queue_stats(struct queue_stats_cbdata *cbdata, uint32_t queue_id,
+const struct netdev_queue_stats *stats)
+{
+struct ofpbuf *msg = ofpbuf_from_list(list_back(&cbdata->replies));
+struct ofp_header *oh = msg->data;
+
+switch ((enum ofp_version)oh->version) {
+case OFP12_VERSION:
+case OFP11_VERSION:
+put_queue_stats11(cbdata, queue_id, stats);
+break;
+
+case OFP10_VERSION:
+put_queue_stats10(cbdata, queue_id, stats);
+break;
+
+default:
+NOT_REACHED();
+}
+}
+
+static void
handle_queue_stats_dump_cb(uint32_t queue_id,
struct netdev_queue_stats *stats,
void *cbdata_)
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 36/41] ofp-msgs: Split OFPRAW_OFPST_QUEUE_{REQUEST, REPLY}
Split OFPRAW_OFPST_QUEUE_{REQUEST,REPLY} into OpenFlow 1.0 and 1.1+
versions.
This prepares for encoding and decoding Open Flow 1.1 and 1.2 Queue Stats
Request and Reply messages.
Signed-off-by: Simon Horman
---
v10
* No change
v10
* Initial Post
---
lib/ofp-msgs.h| 14 ++
utilities/ovs-ofctl.c | 2 +-
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 6420c5d..ceb3fc3 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -226,10 +226,14 @@ enum ofpraw {
OFPRAW_OFPST11_PORT_REPLY,
/* OFPST 1.0 (5): struct ofp10_queue_stats_request. */
-OFPRAW_OFPST_QUEUE_REQUEST,
+OFPRAW_OFPST10_QUEUE_REQUEST,
+/* OFPST 1.1+ (5): struct ofp11_queue_stats_request. */
+OFPRAW_OFPST11_QUEUE_REQUEST,
/* OFPST 1.0 (5): struct ofp10_queue_stats[]. */
-OFPRAW_OFPST_QUEUE_REPLY,
+OFPRAW_OFPST10_QUEUE_REPLY,
+/* OFPST 1.1+ (5): struct ofp11_queue_stats[]. */
+OFPRAW_OFPST11_QUEUE_REPLY,
/* OFPST 1.0 (13): void. */
OFPRAW_OFPST_PORT_DESC_REQUEST,
@@ -390,8 +394,10 @@ enum ofptype {
* OFPRAW_OFPST11_PORT_REQUEST. */
OFPTYPE_PORT_STATS_REPLY,/* OFPRAW_OFPST10_PORT_REPLY.
* OFPRAW_OFPST11_PORT_REPLY. */
-OFPTYPE_QUEUE_STATS_REQUEST, /* OFPRAW_OFPST_QUEUE_REQUEST. */
-OFPTYPE_QUEUE_STATS_REPLY, /* OFPRAW_OFPST_QUEUE_REPLY. */
+OFPTYPE_QUEUE_STATS_REQUEST, /* OFPRAW_OFPST10_QUEUE_REQUEST.
+ * OFPRAW_OFPST11_QUEUE_REQUEST. */
+OFPTYPE_QUEUE_STATS_REPLY, /* OFPRAW_OFPST10_QUEUE_REPLY.
+ * OFPRAW_OFPST11_QUEUE_REPLY. */
OFPTYPE_PORT_DESC_STATS_REQUEST, /* OFPRAW_OFPST_PORT_DESC_REQUEST. */
OFPTYPE_PORT_DESC_STATS_REPLY, /* OFPRAW_OFPST_PORT_DESC_REPLY. */
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 81df40c..ab3ace2 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -973,7 +973,7 @@ ofctl_queue_stats(int argc, char *argv[])
struct vconn *vconn;
open_vconn(argv[1], &vconn);
-request = ofpraw_alloc(OFPRAW_OFPST_QUEUE_REQUEST,
+request = ofpraw_alloc(OFPRAW_OFPST10_QUEUE_REQUEST,
vconn_get_version(vconn), 0);
req = ofpbuf_put_zeros(request, sizeof *req);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 39/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Queue Stats Request Messages
Signed-off-by: Simon Horman
---
v10
* Make use of enum ofp_version
v9
* Omitted
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial Post
---
ofproto/ofproto.c | 33 +
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 51937c2..ed1617e 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2964,9 +2964,8 @@ handle_queue_stats_request(struct ofconn *ofconn,
const struct ofp_header *rq)
{
struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-const struct ofp10_queue_stats_request *qsr = ofpmsg_body(rq);
struct queue_stats_cbdata cbdata;
-unsigned int port_no;
+uint16_t port_no;
struct ofport *port;
uint32_t queue_id;
enum ofperr error;
@@ -2975,8 +2974,34 @@ handle_queue_stats_request(struct ofconn *ofconn,
ofpmp_init(&cbdata.replies, rq);
-port_no = ntohs(qsr->port_no);
-queue_id = ntohl(qsr->queue_id);
+switch ((enum ofp_version)rq->version) {
+case OFP12_VERSION:
+case OFP11_VERSION: {
+const struct ofp11_queue_stats_request *qsr;
+enum ofperr error;
+
+qsr = ofpmsg_body(rq);
+error = ofputil_port_from_ofp11(qsr->port_no, &port_no);
+if (error) {
+return error;
+}
+queue_id = ntohl(qsr->queue_id);
+break;
+}
+
+case OFP10_VERSION: {
+const struct ofp10_queue_stats_request *qsr;
+
+qsr = ofpmsg_body(rq);
+port_no = ntohs(qsr->port_no);
+queue_id = ntohl(qsr->queue_id);
+break;
+}
+
+default:
+NOT_REACHED();
+}
+
if (port_no == OFPP_ALL) {
error = OFPERR_OFPQOFC_BAD_QUEUE;
HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 41/41] ovs-ofpctl: Enable queue-stats for Open Flow 1.1 & 1.2
Signed-off-by: Simon Horman
---
v10
* Manual rebase
* Make use of enum ofp_version
v9
* Omitted
v8
* Omitted
v7
* Omitted
v6
* The queue_id variable in do_dump_aggregate() should be uint32_t not
uint16_t.
v5
* Initial Post
---
utilities/ovs-ofctl.c | 43 ++-
1 file changed, 34 insertions(+), 9 deletions(-)
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index ab3ace2..b7f1cc8 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -968,27 +968,52 @@ ofctl_dump_aggregate(int argc, char *argv[])
static void
ofctl_queue_stats(int argc, char *argv[])
{
-struct ofp10_queue_stats_request *req;
struct ofpbuf *request;
struct vconn *vconn;
+uint16_t port_no;
+uint32_t queue_id;
open_vconn(argv[1], &vconn);
-request = ofpraw_alloc(OFPRAW_OFPST10_QUEUE_REQUEST,
- vconn_get_version(vconn), 0);
-req = ofpbuf_put_zeros(request, sizeof *req);
if (argc > 2 && argv[2][0] && strcasecmp(argv[2], "all")) {
-req->port_no = htons(str_to_port_no(vconn, argv[2]));
+port_no = str_to_port_no(vconn, argv[2]);
} else {
-req->port_no = htons(OFPP_ALL);
+port_no = OFPP_ALL;
}
if (argc > 3 && argv[3][0] && strcasecmp(argv[3], "all")) {
-req->queue_id = htonl(atoi(argv[3]));
+queue_id = atoi(argv[3]);
} else {
-req->queue_id = htonl(OFPQ_ALL);
+queue_id = OFPQ_ALL;
}
-memset(req->pad, 0, sizeof req->pad);
+switch (vconn_get_version(vconn)) {
+case OFP12_VERSION:
+case OFP11_VERSION: {
+struct ofp11_queue_stats_request *req;
+
+request = ofpraw_alloc(OFPRAW_OFPST11_QUEUE_REQUEST,
+ vconn_get_version(vconn), 0);
+req = ofpbuf_put_zeros(request, sizeof *req);
+req->port_no = ofputil_port_to_ofp11(port_no);
+req->queue_id = htonl(queue_id);
+break;
+}
+
+case OFP10_VERSION: {
+struct ofp10_queue_stats_request *req;
+
+request = ofpraw_alloc(OFPRAW_OFPST10_QUEUE_REQUEST,
+ vconn_get_version(vconn), 0);
+req = ofpbuf_put_zeros(request, sizeof *req);
+req->port_no = htons(port_no);
+memset(req->pad, 0, sizeof req->pad);
+req->queue_id = htonl(queue_id);
+break;
+}
+
+default:
+NOT_REACHED();
+}
dump_stats_transaction(vconn, request);
vconn_close(vconn);
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 37/41] ofp-print: Enable display of Open Flow 1.1 & 1.2 Queue Status Reply Messages
Signed-off-by: Simon Horman
---
v10
* Make use of enum ofp_version
* Add ofp-print test
v9
* Omitted
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial Post
---
lib/ofp-print.c| 63 --
tests/ofp-print.at | 56 +++-
2 files changed, 102 insertions(+), 17 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 20965a2..ae0e140 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1082,12 +1082,6 @@ static void ofp_print_port_stat(struct ds *string, const
char *leader,
}
}
-static void print_port_stat(struct ds *string, const char *leader,
-const ovs_32aligned_be64 *statp, int more)
-{
-ofp_print_port_stat(string, leader, get_32aligned_be64(statp), more);
-}
-
static void
ofp_print_ofpst_port_request(struct ds *string, const struct ofp_header *oh)
{
@@ -1387,34 +1381,71 @@ static void
ofp_print_ofpst_queue_reply(struct ds *string, const struct ofp_header *oh,
int verbosity)
{
-struct ofp10_queue_stats *qs;
struct ofpbuf b;
size_t n;
+struct ofp11_queue_stats qs;
ofpbuf_use_const(&b, oh, ntohs(oh->length));
ofpraw_pull_assert(&b);
-n = b.size / sizeof *qs;
+/* This calculation is correct because
+ * struct ofp10_queue_stats and struct ofp11_queue_stats
+ * are the same size.
+ */
+n = b.size / sizeof qs;
ds_put_format(string, " %zu queues\n", n);
if (verbosity < 1) {
return;
}
for (;;) {
-qs = ofpbuf_try_pull(&b, sizeof *qs);
-if (!qs) {
-return;
+uint16_t port_no;
+
+switch ((enum ofp_version)oh->version) {
+case OFP11_VERSION:
+case OFP12_VERSION: {
+const struct ofp11_queue_stats *qs11;
+
+qs11 = ofpbuf_try_pull(&b, sizeof *qs11);
+if (!qs11) {
+return;
+}
+if (ofputil_port_from_ofp11(qs11->port_no, &port_no)) {
+ds_put_cstr(string, "*** parse error: invalid port ***\n");
+return;
+}
+qs = *qs11;
+break;
+}
+
+case OFP10_VERSION: {
+const struct ofp10_queue_stats *qs10;
+
+qs10 = ofpbuf_try_pull(&b, sizeof *qs10);
+if (!qs10) {
+return;
+}
+port_no = ntohs(qs10->port_no);
+qs.queue_id = qs10->queue_id;
+qs.tx_bytes = get_32aligned_be64(&qs10->tx_bytes);
+qs.tx_packets = get_32aligned_be64(&qs10->tx_packets);
+qs.tx_errors = get_32aligned_be64(&qs10->tx_errors);
+break;
+}
+
+default:
+NOT_REACHED();
}
ds_put_cstr(string, " port ");
-ofputil_format_port(ntohs(qs->port_no), string);
+ofputil_format_port(port_no, string);
ds_put_cstr(string, " queue ");
-ofp_print_queue_name(string, ntohl(qs->queue_id));
+ofp_print_queue_name(string, ntohl(qs.queue_id));
ds_put_cstr(string, ": ");
-print_port_stat(string, "bytes=", &qs->tx_bytes, 1);
-print_port_stat(string, "pkts=", &qs->tx_packets, 1);
-print_port_stat(string, "errors=", &qs->tx_errors, 0);
+ofp_print_port_stat(string, "bytes=", qs.tx_bytes, 1);
+ofp_print_port_stat(string, "pkts=", qs.tx_packets, 1);
+ofp_print_port_stat(string, "errors=", qs.tx_errors, 0);
}
}
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 0871526..cd72cff 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -992,7 +992,7 @@ OFPST_QUEUE request (xid=0x1):port=ALL queue=ALL
])
AT_CLEANUP
-AT_SETUP([OFPST_QUEUE reply])
+AT_SETUP([OFPST_QUEUE reply - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "\
01 11 00 cc 00 00 00 01 00 05 00 00 00 03 00 00 \
@@ -1026,6 +1026,60 @@ OFPST_PORT_DESC request (xid=0x1):
])
AT_CLEANUP
+AT_SETUP([OFPST_QUEUE reply - OF1.1])
+AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
+AT_CHECK([ovs-ofctl ofp-print "\
+02 13 00 d0 00 00 00 01 00 05 00 00 00 00 00 00 \
+00 00 00 03 00 00 00 01 00 00 00 00 00 00 01 2e \
+00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 \
+00 00 00 03 00 00 00 02 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 02 00 00 00 01 00 00 00 00 00 00 08 34 \
+00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 00 \
+00 00 00 02 00 00 00 02 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 01 00 00 00 02 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPST_QUEUE reply (OF1.1) (xid=0x1): 6 queues
+ port 3 queue 1: bytes=302, pkts=1, errors=0
+ port 3 queue 2: bytes=0, pkts=0, errors=0
+ port 2 que
[ovs-dev] [PATCH 40/41] ovs-print: Enable display of Open Flow 1.1 & 1.2 Queue Stats Request
Signed-off-by: Simon Horman
---
v10
* Manual rebase
* Make use of enum ofp_version
* Change subject name from "ovs-print: Enable display of Open Flow 1.1 &
1.2 Queue Stats Response" to "ovs-print: Enable display of Open Flow 1.1
& 1.2 Queue Stats Request"
* Add ofp-print test
* Use ofputil_port_from_ofp11 to decode port number
v9
* Omitted
v8
* Omitted
v7
* Omitted
v6
* No change
v5
* Initial Post
---
lib/ofp-print.c| 38 +++---
tests/ofp-print.at | 22 +-
2 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index ae0e140..81c803b 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1368,13 +1368,45 @@ ofp_print_queue_name(struct ds *string, uint32_t
queue_id)
static void
ofp_print_ofpst_queue_request(struct ds *string, const struct ofp_header *oh)
{
-const struct ofp10_queue_stats_request *qsr = ofpmsg_body(oh);
+uint16_t port_no;
+uint32_t queue_id;
+
+switch ((enum ofp_version)oh->version) {
+case OFP11_VERSION:
+case OFP12_VERSION: {
+const struct ofp11_queue_stats_request *qsr;
+
+qsr = ofpmsg_body(oh);
+if (ofputil_port_from_ofp11(qsr->port_no, &port_no)) {
+ds_put_cstr(string, "*** parse error: invalid port ***\n");
+return;
+}
+if (ofputil_port_from_ofp11(qsr->port_no, &port_no)) {
+ds_put_cstr(string, "*** parse error: invalid port ***\n");
+return;
+}
+queue_id = ntohl(qsr->queue_id);
+break;
+}
+
+case OFP10_VERSION: {
+const struct ofp10_queue_stats_request *qsr;
+
+qsr = ofpmsg_body(oh);
+port_no = ntohs(qsr->port_no);
+queue_id = ntohl(qsr->queue_id);
+break;
+}
+
+default:
+ NOT_REACHED();
+}
ds_put_cstr(string, "port=");
-ofputil_format_port(ntohs(qsr->port_no), string);
+ofputil_format_port(port_no, string);
ds_put_cstr(string, " queue=");
-ofp_print_queue_name(string, ntohl(qsr->queue_id));
+ofp_print_queue_name(string, queue_id);
}
static void
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index cd72cff..727bcac 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -982,7 +982,7 @@ OFPST_PORT reply (OF1.2) (xid=0x2): 3 ports
])
AT_CLEANUP
-AT_SETUP([OFPST_QUEUE request])
+AT_SETUP([OFPST_QUEUE request - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
AT_CHECK([ovs-ofctl ofp-print "\
01 10 00 14 00 00 00 01 00 05 00 00 ff fc 00 00 \
@@ -992,6 +992,26 @@ OFPST_QUEUE request (xid=0x1):port=ALL queue=ALL
])
AT_CLEANUP
+AT_SETUP([OFPST_QUEUE request - OF1.1])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+02 12 00 18 00 00 00 02 00 05 00 00 00 00 00 00 \
+ff ff ff fc ff ff ff ff \
+"], [0], [dnl
+OFPST_QUEUE request (OF1.1) (xid=0x2):port=ALL queue=ALL
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_QUEUE request - OF1.2])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 12 00 18 00 00 00 02 00 05 00 00 00 00 00 00 \
+ff ff ff fc ff ff ff ff \
+"], [0], [dnl
+OFPST_QUEUE request (OF1.2) (xid=0x2):port=ALL queue=ALL
+])
+AT_CLEANUP
+
AT_SETUP([OFPST_QUEUE reply - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "\
--
1.7.10.2.484.gcd07cc5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 25/28] classifier: Break cls_rule 'flow' and 'wc' members into new "struct match".
On Tue, Aug 07, 2012 at 02:49:19PM -0700, Ethan Jackson wrote: > > I dropped the parameter and return value from > > ofputil_match_from_ofp10_match(). After some thought, it seemed that > > I only needed the priority calculation in one place (in > > ofputil_decode_flow_mod()) so I inlined it there. > > The other caller didn't need to calculate it? It was in decoding a flow stats reply, IIRC. I think that there it's actually deceptive to change something else to 65535 because it misrepresents what the switch actually reported. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 25/28] classifier: Break cls_rule 'flow' and 'wc' members into new "struct match".
Makes sense. Thanks. Ethan (iPhone) On Aug 7, 2012, at 14:53, Ben Pfaff wrote: > On Tue, Aug 07, 2012 at 02:49:19PM -0700, Ethan Jackson wrote: >>> I dropped the parameter and return value from >>> ofputil_match_from_ofp10_match(). After some thought, it seemed that >>> I only needed the priority calculation in one place (in >>> ofputil_decode_flow_mod()) so I inlined it there. >> >> The other caller didn't need to calculate it? > > It was in decoding a flow stats reply, IIRC. I think that there it's > actually deceptive to change something else to 65535 because it > misrepresents what the switch actually reported. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] vconn: Properly line up description for "tcp:" and "ssl:" usage.
Signed-off-by: Justin Pettit
---
lib/vconn.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/vconn.c b/lib/vconn.c
index cbe3c77..da4d9ca 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -138,10 +138,10 @@ vconn_usage(bool active, bool passive, bool bootstrap
OVS_UNUSED)
printf("\n");
if (active) {
printf("Active OpenFlow connection methods:\n");
-printf(" tcp:IP[:PORT] "
+printf(" tcp:IP[:PORT] "
"PORT (default: %d) at remote IP\n", OFP_TCP_PORT);
#ifdef HAVE_OPENSSL
-printf(" ssl:IP[:PORT] "
+printf(" ssl:IP[:PORT] "
"SSL PORT (default: %d) at remote IP\n", OFP_SSL_PORT);
#endif
printf(" unix:FILE Unix domain socket named FILE\n");
--
1.7.5.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vconn: Properly line up description for "tcp:" and "ssl:" usage.
On Tue, Aug 07, 2012 at 03:19:33PM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit Looks good. I didn't compare the output, but I'll assume you did. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Video cámara de 110gr por 33,28¤ / Escápate a Madrid por sólo 36¤ / Crema Reductora por 26¤
Cabestan. Layout Doble Asegúrate de no perderte ninguna oferta, añade ofer...@globalbono.com a tu lista de contactos. Si no ves correctamente las imágenes, pulsa [ http://email.globalbono.com/E07082012110854.cfm?WL=955&WS=208833_8707685&WA=399 ] aquí [ http://email.globalbono.com/Go/index.cfm?WL=564&WS=208833_8707685&WA=399 ] Tus Ofertas de hoy Síguenos en: [ http://email.globalbono.com/Go/index.cfm?WL=50&WS=208833_8707685&WA=399 ] [ http://email.globalbono.com/Go/index.cfm?WL=51&WS=208833_8707685&WA=399 ] [ http://email.globalbono.com/Go/index.cfm?WL=507&WS=208833_8707685&WA=399 ] 63% Dto. Afilador de Cuchillos Afila tus cuchillos de una forma cómoda y segura con éste afilador. Como bien sabemos qu... 9,35¤ [ http://email.globalbono.com/Go/index.cfm?WL=507&WS=208833_8707685&WA=399 ] Antes 25,00¤Dto: 63% Ahorro: 15,65¤ [ http://email.globalbono.com/Go/index.cfm?WL=56&WS=208833_8707685&WA=399 ] 59% Dto. Clip para Sujetador Seis clips para sujetador con los que podrás convertir cualquiera de tus sujetadores en ... 8,17¤ [ http://email.globalbono.com/Go/index.cfm?WL=56&WS=208833_8707685&WA=399 ] Antes 19,95¤Dto: 59% Ahorro: 11,78¤ [ http://email.globalbono.com/Go/index.cfm?WL=948&WS=208833_8707685&WA=399 ] 54% Dto. Noche en Santiago, Hártate de Mariscada, Duerme en Hotel Tres Estrellas Disfruta de una fantástica noche en Santiago, y de una maravillosa mariscada. Porque con... 60,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=948&WS=208833_8707685&WA=399 ] Antes 130,00¤Dto: 54% Ahorro: 70,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=932&WS=208833_8707685&WA=399 ] 45% Dto. Disfruta de Madrid con tu Pareja, Descansa en Hotel Cuatro Estrellas Disfruta de Madrid con tu pareja, recórrela, y ven a descansar al hotel Nuevo Boston, 4*... 36,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=932&WS=208833_8707685&WA=399 ] Antes 65,00¤Dto: 45% Ahorro: 29,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=866&WS=208833_8707685&WA=399 ] 66% Dto. Crema Reductora Crema reductora para perder peso y volumen mientras duermes. Dos botes de 200 ml. de tra... 26,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=866&WS=208833_8707685&WA=399 ] Antes 76,00¤Dto: 66% Ahorro: 50,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=942&WS=208833_8707685&WA=399 ] 44% Dto. Video Cámara 110 Gramos Videocámara perfecta para cualquier situación. Es muy reducida y ligera, sin que por ell... 33,28¤ [ http://email.globalbono.com/Go/index.cfm?WL=942&WS=208833_8707685&WA=399 ] Antes 59,90¤Dto: 44% Ahorro: 26,62¤ [ http://email.globalbono.com/Go/index.cfm?WL=706&WS=208833_8707685&WA=399 ] De conformidad con lo que establece la Ley Orgánica 15/1999 de Protección de Datos de Carácter Personal, le informamos que sus datos personales serán incorporados a un fichero bajo la responsabilidad de GlobalBono, con la finalidad de poder atender los compromisos derivados de la relación que mantenemos con usted. Puede ejercer sus derechos de acceso, cancelación, rectificación y oposición mediante un escrito a la dirección email i...@globalbono.com. Si en el plazo de 30 días no nos comunica lo contrario, entenderemos que los datos no han sido modificados, que se compromete a notificarnos cualquier variación y que tenemos el consentimiento para utilizarlos a fin de poder fidelizar la relación entre las partes. ¿Quieres dejar de recibir este email? Haz click [ http://email.globalbono.com/baja/form_baja_GB.cfm?WL=182&WS=208833_8707685&WA=399 ] aquí. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vconn: Properly line up description for "tcp:" and "ssl:" usage.
On Aug 7, 2012, at 3:21 PM, Ben Pfaff wrote: > On Tue, Aug 07, 2012 at 03:19:33PM -0700, Justin Pettit wrote: >> Signed-off-by: Justin Pettit > > Looks good. > > I didn't compare the output, but I'll assume you did. Yep. Thanks. I pushed this. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [classifier-opt 26/28] classifier: Prepare for "struct cls_rule" needing to be destroyed.
It's a good idea.
There's one case, in collect_rules_strict(), where it's nice to avoid
the repeated overhead of converting the same match to a cls_rule
repeatedly, so I just added a wrapper classifier_find_match_exactly()
and used that in the other cases.
Thanks,
Ben.
On Mon, Aug 06, 2012 at 03:14:19PM -0700, Ethan Jackson wrote:
> Some of this would be a bit simpler if classifier_find_rule_exactly()
> took a match and a priority instead of a cls_rule. May make sense to
> insert that change into a new patch preceding this one.
>
> Looks good, thanks.
> Ethan
>
> On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff wrote:
> > Until now, "struct cls_rule" didn't own any data outside its own memory
> > block. An upcoming commit will make "struct cls_rule" sometimes own blocks
> > of memory, so it needs "destroy" and to a lesser extent "clone" functions.
> > This commit adds these in advance, even though they are mostly no-ops, to
> > make it possible to separately review the memory management.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/classifier.c| 17 +
> > lib/classifier.h|2 ++
> > ofproto/ofproto.c | 22 +++---
> > tests/test-classifier.c | 46
> > ++
> > utilities/ovs-ofctl.c |2 ++
> > 5 files changed, 74 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/classifier.c b/lib/classifier.c
> > index cb66295..a482666 100644
> > --- a/lib/classifier.c
> > +++ b/lib/classifier.c
> > @@ -69,6 +69,23 @@ cls_rule_init(struct cls_rule *rule,
> > rule->priority = priority;
> > }
> >
> > +/* Initializes 'dst' as a copy of 'src'. */
> > +void
> > +cls_rule_clone(struct cls_rule *dst, const struct cls_rule *src)
> > +{
> > +*dst = *src;
> > +}
> > +
> > +/* Frees memory referenced by 'rule'. Doesn't free 'rule' itself (it's
> > + * normally embedded into a larger structure).
> > + *
> > + * ('rule' must not currently be in a classifier.) */
> > +void
> > +cls_rule_destroy(struct cls_rule *rule OVS_UNUSED)
> > +{
> > +/* Nothing to do yet. */
> > +}
> > +
> > /* Returns true if 'a' and 'b' match the same packets at the same priority,
> > * false if they differ in some way. */
> > bool
> > diff --git a/lib/classifier.h b/lib/classifier.h
> > index 8a11f05..74f9211 100644
> > --- a/lib/classifier.h
> > +++ b/lib/classifier.h
> > @@ -70,6 +70,8 @@ struct cls_rule {
> >
> > void cls_rule_init(struct cls_rule *,
> > const struct match *, unsigned int priority);
> > +void cls_rule_clone(struct cls_rule *, const struct cls_rule *);
> > +void cls_rule_destroy(struct cls_rule *);
> >
> > bool cls_rule_equal(const struct cls_rule *, const struct cls_rule *);
> > uint32_t cls_rule_hash(const struct cls_rule *, uint32_t basis);
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index e55e89f..4cb56e2 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -1428,6 +1428,8 @@ ofproto_add_flow(struct ofproto *ofproto, const
> > struct match *match,
> > cls_rule_init(&cr, match, priority);
> > rule = rule_from_cls_rule(classifier_find_rule_exactly(
> > &ofproto->tables[0].cls, &cr));
> > +cls_rule_destroy(&cr);
> > +
> > if (!rule || !ofpacts_equal(rule->ofpacts, rule->ofpacts_len,
> > ofpacts, ofpacts_len)) {
> > struct ofputil_flow_mod fm;
> > @@ -1468,6 +1470,8 @@ ofproto_delete_flow(struct ofproto *ofproto,
> > cls_rule_init(&cr, target, priority);
> > rule = rule_from_cls_rule(classifier_find_rule_exactly(
> >&ofproto->tables[0].cls, &cr));
> > +cls_rule_destroy(&cr);
> > +
> > if (!rule) {
> > /* No such rule -> success. */
> > return true;
> > @@ -1904,6 +1908,7 @@ static void
> > ofproto_rule_destroy__(struct rule *rule)
> > {
> > if (rule) {
> > +cls_rule_destroy(&rule->cr);
> > free(rule->ofpacts);
> > rule->ofproto->ofproto_class->rule_dealloc(rule);
> > }
> > @@ -2456,7 +2461,8 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t
> > table_id,
> > cls_cursor_init(&cursor, &table->cls, &cr);
> > CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
> > if (rule->pending) {
> > -return OFPROTO_POSTPONE;
> > +error = OFPROTO_POSTPONE;
> > +goto exit;
> > }
> > if (!ofproto_rule_is_hidden(rule)
> > && ofproto_rule_has_out_port(rule, out_port)
> > @@ -2465,7 +2471,10 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t
> > table_id,
> > }
> > }
> > }
> > -return 0;
> > +
> > +exit:
> > +cls_rule_destroy(&cr);
> > +return error;
> > }
> >
> > /* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if
> > @@ -2503,7 +2512,8 @@ collect_rules_strict(struct ofproto
Re: [ovs-dev] [PATCH] tests: Test that ofp10_match bytes that should be ignored really are.
Thank you. I applied this to master. On Mon, Jul 30, 2012 at 02:16:37PM -0700, Mehak Mahajan wrote: > Hey Ben, > > Looks good to me. > > thanx! > mehak > > On Fri, Jul 27, 2012 at 1:23 PM, Ben Pfaff wrote: > > > Would someone review this please? It should not be hard. > > > > Thanks, > > > > Ben. > > > > On Sat, Jul 21, 2012 at 09:56:28AM -0700, Ben Pfaff wrote: > > > Rob Sherwood reported a bug in OVS treatment of ofp10_match bytes that > > > should be ignored some time ago: > > > > > > > In any case, the pktact.SingleWildcardMatch and > > > > pktact.AllExceptOneWildcardMatch tests were failing because it looks > > > > like OVS (v1.4 release) was not matching vlan tagged packets when the > > > > match wildcarded vlan but the dl_vlan value (which should be ignored, > > > > because it is wildcarded) was non-zero. We've worked around this in > > > > OFTest by making sure that the dl_vlan value is zero when vlan is > > > > wildcarded and now the test passes. > > > > > > > > In other words: > > > > > > > > if (ofp_match->wildcards&OFPFW_DL_VLAN) is true, then the match should > > > > match both tagged and untagged packets, independent of the value of > > > > ofp_match->dl_vlan. OVS (seemingly) only matches tagged packets if > > > > ofp_match->dl_vlan == 0. > > > > > > I wasn't able to spot the problem at the time, and I still don't see a > > > problem (perhaps it has been fixed since then), but this commit should > > > prevent any regression for this specific problem and for anything like > > it. > > > > > > It would be natural to modify the parse-ofp11-match test in the same way, > > > but this commit doesn't do it. > > > > > > Rob's original bug report is at: > > > > > https://mailman.stanford.edu/pipermail/openflow-discuss/2012-March/003107.html > > > > > > Reported-by: Rob Sherwood > > > Signed-off-by: Ben Pfaff > > > --- > > > tests/ovs-ofctl.at| 136 > > > > > utilities/ovs-ofctl.c | 40 +-- > > > 2 files changed, 104 insertions(+), 72 deletions(-) > > > > > > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at > > > index 08026ec..9f47f78 100644 > > > --- a/tests/ovs-ofctl.at > > > +++ b/tests/ovs-ofctl.at > > > @@ -730,55 +730,55 @@ AT_SETUP([ovs-ofctl parse-ofp10-match]) > > > AT_KEYWORDS([OF1.0]) > > > AT_DATA([test-data], [dnl > > > # in_port=65534 > > > -003820fe fffe 00 00 00 00 dnl > > > - > > > +003820fe fffe xx xx xx xx dnl > > > + > > > > > > # dl_src=00:01:02:03:04:05 > > > -003820fb 000102030405 00 00 00 00 dnl > > > - > > > +003820fb 000102030405 xx xx xx xx dnl > > > + > > > > > > # dl_dst=10:20:30:40:50:60 > > > -003820f7 102030405060 00 00 00 00 dnl > > > - > > > +003820f7 102030405060 xx xx xx xx dnl > > > + > > > > > > # dl_vlan=291 > > > -003820fd 0123 00 00 00 00 dnl > > > - > > > +003820fd 0123 xx xx xx xx dnl > > > + > > > > > > # dl_vlan_pcp=5 > > > -002820ff 05 00 00 00 dnl > > > - > > > +002820ff 05 xx xx xx dnl > > > + > > > > > > # dl_vlan=291,dl_vlan_pcp=4 > > > -002820fd 0123 04 00 00 00 dnl > > > - > > > +002820fd 0123 04 xx xx xx dnl > > > + > > > > > > # vlan_tci=0x > > > -003820fd 00 00 00 00 dnl > > > - > > > +003820fd xx xx xx xx dnl > > > + > > > > > > dnl dl_vlan_pcp doesn't make sense when dl_vlan is "none", so > > > dnl OVS ignores it and drops it on output. > > > # vlan_tci=0x > > > # 1: 28 -> 38 > > > # 20: 05 -> 00 > > > -002820fd 05 00 00 00 dnl > > > - > > > +002820fd 05 xx xx xx dnl > > > + > > > > > > dnl Invalid VID and PCP discards out-of-range bits: > > > # dl_vlan=256,dl_vlan_pcp=7 > > > # 18: f1 -> 01 > > > # 20: ff -> 07 > > > -002820fd f100 ff 00 00 00 dnl > > > - > > > +002820fd f100 ff xx xx xx dnl > > > + > > > > > > # dl_type=0x1234 > > > -
[ovs-dev] [PATCH] Bump up the supported kernel versions to include 3.5.x.
Signed-off-by: Kyle Mestery --- datapath/datapath.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index 7a7dc4c..e98c84b 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -61,8 +61,8 @@ #include "vport-internal_dev.h" #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,18) || \ -LINUX_VERSION_CODE >= KERNEL_VERSION(3,5,0) -#error Kernels before 2.6.18 or after 3.4 are not supported by this version of Open vSwitch. +LINUX_VERSION_CODE >= KERNEL_VERSION(3,6,0) +#error Kernels before 2.6.18 or after 3.5 are not supported by this version of Open vSwitch. #endif #define REHASH_FLOW_INTERVAL (10 * 60 * HZ) -- 1.7.11.2 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] vlog: Ignore return value of some write() calls.
A couple of calls to write() would generate warnings when the
"-Wunused-result" compiler option is enabled. This change ignores the
return value, since we can't do anything about it in logging code.
Signed-off-by: Justin Pettit
---
lib/vlog.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/vlog.c b/lib/vlog.c
index 683cdfc..0bd9bd1 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -892,7 +892,7 @@ vlog_write_file(struct ds *s)
vlog_async_write_request_cb, NULL, NULL);
vlog_async_inited = true;
} else {
-write(log_fd, s->string, s->length);
+ignore(write(log_fd, s->string, s->length));
}
}
@@ -918,6 +918,6 @@ vlog_async_write_request_cb(struct ofpbuf *request,
}
if (request->size > 0) {
-write(log_fd, request->data, request->size);
+ignore(write(log_fd, request->data, request->size));
}
}
--
1.7.5.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vlog: Ignore return value of some write() calls.
On Tue, Aug 07, 2012 at 03:49:00PM -0700, Justin Pettit wrote: > A couple of calls to write() would generate warnings when the > "-Wunused-result" compiler option is enabled. This change ignores the > return value, since we can't do anything about it in logging code. > > Signed-off-by: Justin Pettit Looks good, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vlog: Ignore return value of some write() calls.
On Aug 7, 2012, at 4:08 PM, Ben Pfaff wrote: > On Tue, Aug 07, 2012 at 03:49:00PM -0700, Justin Pettit wrote: >> A couple of calls to write() would generate warnings when the >> "-Wunused-result" compiler option is enabled. This change ignores the >> return value, since we can't do anything about it in logging code. >> >> Signed-off-by: Justin Pettit > > Looks good, thanks. Thanks. I pushed it. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] timeval: On 64-bit systems refresh time whenever it is requested.
64-bit glibc appears to avoid syscalls for clock_gettime(), so we can get
higher resolution timing and avoid having a timer firing off SIGALRM
without introducing extra overhead.
Signed-off-by: Leo Alterman
---
lib/timeval.c | 46 --
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/lib/timeval.c b/lib/timeval.c
index d29b661..a5068de 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -33,6 +33,19 @@
#include "util.h"
#include "vlog.h"
+/* On Linux IA64 systems, glibc avoids using syscalls for clock_gettime().
+ *
+ * For systems which do invoke a system call we wait at least
+ * TIME_UPDATE_INTERVAL ms between clock_gettime() calls and cache the time for
+ * the interim.
+ *
+ * For systems which do not invoke a system call, we just call clock_gettime()
+ * whenever the time is requested. As a result we don't start the background
+ * SIGALRM timer unless explicitly needed by time_alarm() */
+#ifndef __LP64__
+#define CACHE_TIME 1
+#endif
+
VLOG_DEFINE_THIS_MODULE(timeval);
/* The clock to use for measuring time intervals. This is CLOCK_MONOTONIC by
@@ -40,7 +53,7 @@ VLOG_DEFINE_THIS_MODULE(timeval);
* to CLOCK_REALTIME. */
static clockid_t monotonic_clock;
-/* Has a timer tick occurred?
+/* Has a timer tick occurred? Only relevant if CACHE_TIME is defined.
*
* We initialize these to true to force time_init() to get called on the first
* call to time_msec() or another function that queries the current time. */
@@ -94,8 +107,11 @@ time_init(void)
VLOG_DBG("monotonic timer not available");
}
+#ifdef CACHE_TIME
set_up_signal(SA_RESTART);
set_up_timer();
+#endif
+
boot_time = time_msec();
}
@@ -168,7 +184,16 @@ void
time_postfork(void)
{
time_init();
+
+#ifdef CACHE_TIME
set_up_timer();
+#else
+/* If we are not caching kernel time, the only reason the timer should
+ * exist is if time_alarm() was called and deadline is set */
+if (deadline != TIME_MIN) {
+set_up_timer();
+}
+#endif
}
static void
@@ -199,7 +224,9 @@ refresh_monotonic(void)
/* Forces a refresh of the current time from the kernel. It is not usually
* necessary to call this function, since the time will be refreshed
- * automatically at least every TIME_UPDATE_INTERVAL milliseconds. */
+ * automatically at least every TIME_UPDATE_INTERVAL milliseconds. If
+ * CACHE_TIME is undefined, we will always refresh the current time so this
+ * function has no effect. */
void
time_refresh(void)
{
@@ -275,9 +302,16 @@ time_alarm(unsigned int secs)
sigset_t oldsigs;
time_init();
+
block_sigalrm(&oldsigs);
deadline = secs ? time_add(time_now(), secs) : TIME_MIN;
unblock_sigalrm(&oldsigs);
+
+/* If we aren't timing the gaps between kernel time refreshes we need to
+ * to start the timer up now */
+#ifndef CACHE_TIME
+set_up_timer();
+#endif
}
/* Like poll(), except:
@@ -366,17 +400,25 @@ sigalrm_handler(int sig_nr)
static void
refresh_wall_if_ticked(void)
{
+#ifdef CACHE_TIME
if (wall_tick) {
refresh_wall();
}
+#else
+refresh_wall();
+#endif
}
static void
refresh_monotonic_if_ticked(void)
{
+#ifdef CACHE_TIME
if (monotonic_tick) {
refresh_monotonic();
}
+#else
+refresh_monotonic();
+#endif
}
static void
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] timeval: On 64-bit systems refresh time whenever it is requested.
> > +/* On Linux IA64 systems, glibc avoids using syscalls for clock_gettime(). > I assume you mean x86-64 or one of the equivalent names here, not IA64. > +#ifndef __LP64__ > +#define CACHE_TIME 1 > +#endif > This will disable the syscall mitigation for 64-bit FreeBSD as well, which is not (yet) desirable; I think this test needs a Linux check as well. -Ed ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 11/24] ovs-controller: Make sure vconn is connected before making messages
On Tue, Aug 07, 2012 at 12:46:12PM -0700, Ben Pfaff wrote: > On Wed, Jul 25, 2012 at 09:05:58AM +0900, Simon Horman wrote: > > On Tue, Jul 24, 2012 at 04:03:59PM -0700, Ben Pfaff wrote: > > > On Mon, Jul 23, 2012 at 03:16:40PM +0900, Simon Horman wrote: > > > > This is a rather unpleasant hack but I am unsure of how to rework things > > > > such that ovs-controller will work with a version of make_openflow() > > > > which > > > > uses prevailing OpenFlow version in the header of OpenFlow messages. > > > > > > > > Signed-off-by: Simon Horman > > > > > > Oh, OK, I see the problem that the previous patch pointed out. > > > > > > I'll send out some proper fixes in a while. > > > > Thanks. > > The fixes were in the series starting here: > http://openvswitch.org/pipermail/dev/2012-August/020071.html > which I've now applied to master. > > If you still see problems with ovs-controller then please raise the > issue again. Thanks, will do. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 00/41 v10] Preliminary Open Flow 1.2 Message Support
On Wed, Aug 08, 2012 at 06:49:35AM +0900, Simon Horman wrote:
> Hi Ben, Hi All,
>
> this series adds infrastructure and message encoding and decoding
> to allow OpenFlow 1.2 to be used to the extent that ovs-controller
> works as a learning-switch and similar logic implemented using Ryu[1].
> Note that some previously posted hacks are required in order for Open
> vSwtich to actually use OpenFlow 1.2.
>
> [1] Ryu is an OpenFlow controller implemented in Python
> http://osrg.github.com/ryu/
>
> Differing from the previous posting, this series supports encoding and
> decoding of Queue Status request and reply messages.
>
> This series also allows OXM for OpenFlow 1.1, as suggested by
> Ben Pfaff when reviewing v9 of this series.
>
>
> Base
>
>
> This series is based the current master branch
That was true yesterday afternoon when I prepared the series,
but it was no longer true by the time that I posted it this morning.
I have rebased again. And resolved a minor conflict in
patch 7, which I will re-post.
> Availability
>
>
> This series is available in the devel/of1.2 (n.b: not devel/of12) branch
> of git://github.com/horms/openvswitch.git
> (ef8848df4aca375913d0829554effa260064d74b "ovs-ofpctl: Enable queue-stats
> for Open Flow 1.1 & 1.2")
The series is now available in the devel/of1.2 branch
of git://github.com/horms/openvswitch.git, revision
eea3e4b58c8e20c2747879b8bed46e3263e480f0 ("ovs-ofpctl: Enable
queue-stats for Open Flow 1.1 & 1.2").
The only change is the merge conflict resolution of patch 7.
> Patches
> ---
>
> [PATCH 01/41] ofp-util: ofputil_pull_ofp11_match: Allow OXM match
> [PATCH 02/41] ofp-print: Open Flow 1.2 Flow Mod message tests
> [PATCH 03/41] ofp-util: Allow encoding of Open Flow 1.2 Packet In
> [PATCH 04/41] ofp-util: Allow decoding of Open Flow 1.2 Packet In
> [PATCH 05/41] ofp-msgs: Update OFPRAW_OFPT_SET_CONFIG for OpenFlow
> [PATCH 06/41] ofp-actions: Return action size
> [PATCH 07/41] ofp-util: Prepare Packet Out encoder for other Open
> [PATCH 08/41] ofp-util: Allow encoding of Open Flow 1.1 and 1.2
> [PATCH 09/41] ofp-util: Prepare Packet Out decoder for other Open
> [PATCH 10/41] ofp-util: Allow decoding of Open Flow 1.1 and 1.2
> [PATCH 11/41] ofp-util: Allow encoding of Open Flow 1.2 Flow Removed
> [PATCH 12/41] ofp-util: Allow encoding Open Flow 1.2 Flow Stats
> [PATCH 13/41] ofp-util: Allow use of OF12 flow format
> [PATCH 14/41] ofp-util: Allow encoding of Open Flow 1.2 Port Mod
> [PATCH 15/41] ofp-util: Allow decoding of Open Flow 1.2 Port Mod
> [PATCH 16/41] ofp-msgs: Split OFPRAW_OFPST_FLOW_{REQUEST,REPLY}
> [PATCH 17/41] ofp-util: Prepare Flow Statistics Request Decoder for
> [PATCH 18/41] ofp-util: Allow decoding of Open Flow 1.2 Flow
> [PATCH 19/41] ofp-util: Allow encoding of Open Flow 1.2 Flow
> [PATCH 20/41] ofp-util: Allow decoding of Open Flow 1.2 Flow
> [PATCH 21/41] ofp-msgs: Split OFPRAW_OFPST_AGGREGATE_REQUEST
> [PATCH 22/41] ofp-msgs: Allow 1.0-1.2 range
> [PATCH 23/41] ofp-msgs: Split OFPRAW_OFPST_TABLE_REPLY
> [PATCH 24/41] ofp-print: Enable display of Open Flow 1.1 & 1.2 Table
> [PATCH 25/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table
> [PATCH 26/41] ovs-ofctl: Use vconn as a parameter of
> [PATCH 27/41] ofp-msgs: Allow encoding and decoding of Open Flow 1.1
> [PATCH 28/41] ofp-msgs: Split OFPRAW_OFPST_PORT_{REQUEST,REPLY}
> [PATCH 29/41] ovs-ofputil: Make str_to_port_no() aware of invalid
> [PATCH 30/41] ofp-util: Pass vconn to fetch_port_by_features()
> [PATCH 31/41] ofp-util: Allow encoding of Open Flow 1.1 & 1.2 Port
> [PATCH 32/41] ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port
> [PATCH 33/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Port
> [PATCH 34/41] ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port
> [PATCH 35/41] ovs-ofctl: Teach dump-ports about Open Flow 1.1 & 1.2
> [PATCH 36/41] ofp-msgs: Split OFPRAW_OFPST_QUEUE_{REQUEST,REPLY}
> [PATCH 37/41] ofp-print: Enable display of Open Flow 1.1 & 1.2 Queue
> [PATCH 38/41] ofp-util: Allow encoding of Open Flow 1.1 & 1.2 Queue
> [PATCH 39/41] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Queue
> [PATCH 40/41] ovs-print: Enable display of Open Flow 1.1 & 1.2 Queue
> [PATCH 41/41] ovs-ofpctl: Enable queue-stats for Open Flow 1.1 & 1.2
>
> Overall Diffstat
>
>
> build-aux/extract-ofp-msgs |3
> include/openflow/openflow-1.1.h |4
> include/openflow/openflow-1.2.h |5
> lib/learning-switch.c |4
> lib/ofp-actions.c |5
> lib/ofp-actions.h |4
> lib/ofp-msgs.h | 77 +++--
> lib/ofp-print.c | 357 +
> lib/ofp-util.c | 426 --
> lib/ofp-util.h | 11
> ofproto/connmgr.c |3
> ofproto/ofproto-dpif.c | 32 +-
> ofproto/ofproto-provider.h | 71 -
> ofproto/ofproto.c |
[ovs-dev] [PATCH 07/41 v11] ofp-util: Prepare Packet Out encoder for other Open Flow versions
Signed-off-by: Simon Horman
---
v11
* Manual rebase
v10
* No change
v9
* Resolve breakage caused by reordering patches in v8
v8
* Make use of enum ofp_version
v7
* Manual Rebase
v6
* No change
v5
* No change
v4
* Manual rebase
v3
* Add protocol variable to do_probe().
Previously this was added by a separate patch.
v2
* No change
---
lib/learning-switch.c | 4 ++--
lib/ofp-util.c| 39 ++-
lib/ofp-util.h| 3 ++-
utilities/ovs-ofctl.c | 5 +++--
4 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index c1cd909..235e9d4 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -590,13 +590,13 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
/* If the switch didn't buffer the packet, we need to send a copy. */
if (pi.buffer_id == UINT32_MAX && out_port != OFPP_NONE) {
-queue_tx(sw, ofputil_encode_packet_out(&po));
+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) {
-queue_tx(sw, ofputil_encode_packet_out(&po));
+queue_tx(sw, ofputil_encode_packet_out(&po, sw->protocol));
}
}
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 58999d1..fbfd17e 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3044,27 +3044,40 @@ ofputil_append_flow_update(const struct
ofputil_flow_update *update,
}
struct ofpbuf *
-ofputil_encode_packet_out(const struct ofputil_packet_out *po)
+ofputil_encode_packet_out(const struct ofputil_packet_out *po,
+ enum ofputil_protocol protocol)
{
-struct ofp_packet_out *opo;
-size_t actions_ofs;
+enum ofp_version ofp_version = ofputil_protocol_to_ofp_version(protocol);
struct ofpbuf *msg;
-size_t size;
+size_t size = 0;
size = po->ofpacts_len;
if (po->buffer_id == UINT32_MAX) {
-size += po->packet_len;
+size = po->packet_len;
}
-msg = ofpraw_alloc(OFPRAW_OFPT10_PACKET_OUT, OFP10_VERSION, size);
-ofpbuf_put_zeros(msg, sizeof *opo);
-actions_ofs = msg->size;
-ofpacts_put_openflow10(po->ofpacts, po->ofpacts_len, msg);
+switch (ofp_version) {
+case OFP10_VERSION: {
+struct ofp_packet_out *opo;
+size_t actions_ofs;
+
+msg = ofpraw_alloc(OFPRAW_OFPT10_PACKET_OUT, OFP10_VERSION, size);
+ofpbuf_put_zeros(msg, sizeof *opo);
+actions_ofs = msg->size;
+ofpacts_put_openflow10(po->ofpacts, po->ofpacts_len, msg);
+
+opo = msg->l3;
+opo->buffer_id = htonl(po->buffer_id);
+opo->in_port = htons(po->in_port);
+opo->actions_len = htons(msg->size - actions_ofs);
+break;
+}
-opo = msg->l3;
-opo->buffer_id = htonl(po->buffer_id);
-opo->in_port = htons(po->in_port);
-opo->actions_len = htons(msg->size - actions_ofs);
+case OFP11_VERSION:
+case OFP12_VERSION:
+default:
+NOT_REACHED();
+}
if (po->buffer_id == UINT32_MAX) {
ofpbuf_put(msg, po->packet, po->packet_len);
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index bc57226..51f0f5a 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -300,7 +300,8 @@ struct ofputil_packet_out {
enum ofperr ofputil_decode_packet_out(struct ofputil_packet_out *,
const struct ofp_header *,
struct ofpbuf *ofpacts);
-struct ofpbuf *ofputil_encode_packet_out(const struct ofputil_packet_out *);
+struct ofpbuf *ofputil_encode_packet_out(const struct ofputil_packet_out *,
+ enum ofputil_protocol protocol);
enum ofputil_port_config {
/* OpenFlow 1.0 and 1.1 share these values for these port config bits. */
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index cd42b96..dd5f4ba 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1453,6 +1453,7 @@ ofctl_probe(int argc OVS_UNUSED, char *argv[])
static void
ofctl_packet_out(int argc, char *argv[])
{
+enum ofputil_protocol protocol;
struct ofputil_packet_out po;
struct ofpbuf ofpacts;
struct vconn *vconn;
@@ -1468,7 +1469,7 @@ ofctl_packet_out(int argc, char *argv[])
po.ofpacts = ofpacts.data;
po.ofpacts_len = ofpacts.size;
-open_vconn(argv[1], &vconn);
+protocol = open_vconn(argv[1], &vconn);
for (i = 4; i < argc; i++) {
struct ofpbuf *packet, *opo;
const char *error_msg;
@@ -1480,7 +1481,7 @@ ofctl_packet_out(int argc, char *argv[])
po.packet = packet->data;
po.packet_len = packet->size;
-opo = ofputil_encode_packet_out(&po);
+opo = ofputil_encode_packet_out(&po, protocol);
[ovs-dev] Threaded userspace datapath
The original developer of the BSD port (Gaetano Catalli) also implemented a threaded version of the userspace datapath to increase performance. As with the original port, the threading work was done against Open vSwitch 1.1.0pre2. Giuseppe Lettieri and I have ported it forward, and it's functional and generally passes unit tests (with a few caveats). The threaded datapath has shown about 10x improvement in throughput in some tests, when compared to the current userspace datapath. There are still a few issues to work through, but I wanted to present the patch now for discussion of the general approach. The threaded patches are available in the bsd-port-threaded branch in this repo: https://github.com/emaste/openvswitch/tree/bsd-port-threaded Or as a diff: http://people.freebsd.org/~emaste/openvswitch/threaded.diff The patch introduces a configure option to enable threaded mode: --enable-threaded, and the changes are largely confined to #ifdef THREADED blocks. Two mutexes are added to dp_netdev: one for the flow table, and one for the port list. (I've also added a mutex over most of lib/vlog.c, although a less conservative approach may be possible.) Significant outstanding issues are: 1. Buffer ownership and management isn't really correct right now, apparently done due to a desire to avoid extra copies. This seems to be a consequence of the standard BPF / libpcap interface. 2. The patch needs a better method of ensuring flows aren't deleted while there may be a reference in the datapath thread. 3. The datapath thread waits in poll() with each port's file descriptor in the fd array. However there's currently no fd to signal a bridge reconfiguration, so the poll() has to timeout before the thread will pick up the new config on the next time through its loop. In any case, I wanted to post the current patch for discussion and review while working on the remaining issues. I'm very interested in your thoughts and comments. -Ed ** ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
