On Tue, Jun 12, 2012 at 12:32:16AM -0700, Ben Pfaff wrote:
> OpenFlow actions have always been somewhat awkward to handle.
> Moreover, over time we've started creating actions that require more
> complicated parsing. When we maintain those actions internally in
> their wire format, we end up parsing them multiple times, whenever
> we have to look at the set of actions.
>
> When we add support for OpenFlow 1.1 or later protocols, the situation
> will get worse, because these newer protocols support many of the same
> actions but with different representations. It becomes unrealistic to
> handle each protocol in its wire format.
>
> This commit adopts a new strategy, by converting OpenFlow actions into
> an internal form from the wire format when they are read, and converting
> them back to the wire format when flows are dumped. I believe that this
> will be more maintainable over time.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
This is a pretty big patch and I'm not going to pretend to have reviewed it
completely. But I did notice one thing.
[snip]
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 7413455..630e457 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
[snip]
> @@ -1232,22 +1232,23 @@ do_probe(int argc OVS_UNUSED, char *argv[])
> static void
> do_packet_out(int argc, char *argv[])
> {
> + enum ofputil_protocol protocol;
> struct ofputil_packet_out po;
> - struct ofpbuf actions;
> + struct ofpbuf ofpacts;
> struct vconn *vconn;
> int i;
>
> - ofpbuf_init(&actions, sizeof(union ofp_action));
> - parse_ofp_actions(argv[3], &actions);
> + 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]));
> - po.actions = actions.data;
> - po.n_actions = actions.size / sizeof(union ofp_action);
> + 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;
GCC tells me that protocol is set by not used in do_packet_out().
Perhaps it could be removed?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
