From patchwork Fri Feb 16 22:54:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 874692 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zjpQm6S45z9t1t for ; Sat, 17 Feb 2018 09:56:00 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id A3FDF10AD; Fri, 16 Feb 2018 22:54:58 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id E3EB8FE0 for ; Fri, 16 Feb 2018 22:54:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 9A227124 for ; Fri, 16 Feb 2018 22:54:55 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 5F9281720A3; Fri, 16 Feb 2018 23:54:53 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Fri, 16 Feb 2018 14:54:42 -0800 Message-Id: <20180216225445.28688-3-blp@ovn.org> X-Mailer: git-send-email 2.16.1 In-Reply-To: <20180216225445.28688-1-blp@ovn.org> References: <20180216225445.28688-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 3/6] ofp-packet: Better abstract packet-in format. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This commit relieves the caller of code that deals with the format of packet-in messages from some of the burden of understanding the packet format. It also renames the constants to appear to be at a higher level of abstraction. Signed-off-by: Ben Pfaff Signed-off-by: Justin Pettit Acked-by: Justin Pettit --- include/openflow/nicira-ext.h | 25 ------------- include/openvswitch/ofp-msgs.h | 2 +- include/openvswitch/ofp-packet.h | 31 +++++++++++++--- lib/ofp-packet.c | 78 ++++++++++++++++++++++------------------ lib/ofp-print.c | 19 +++++----- ofproto/connmgr.c | 8 ++--- ofproto/connmgr.h | 5 +-- ofproto/ofproto.c | 14 +++----- ovn/controller/pinctrl.c | 4 +-- utilities/ovs-ofctl.c | 10 +++--- 10 files changed, 97 insertions(+), 99 deletions(-) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index ddb68aa25e79..1f5cbbf67b29 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -113,31 +113,6 @@ enum nx_hash_fields { }; -enum nx_packet_in_format { - NXPIF_STANDARD = 0, /* OFPT_PACKET_IN for this OpenFlow version. */ - NXPIF_NXT_PACKET_IN = 1, /* NXT_PACKET_IN (since OVS v1.1). */ - NXPIF_NXT_PACKET_IN2 = 2, /* NXT_PACKET_IN2 (since OVS v2.6). */ -}; - -/* NXT_SET_PACKET_IN_FORMAT request. - * - * For any given OpenFlow version, Open vSwitch supports multiple formats for - * "packet-in" messages. The default is always the standard format for the - * OpenFlow version in question, but NXT_SET_PACKET_IN_FORMAT can be used to - * set an alternative format. - * - * From OVS v1.1 to OVS v2.5, this request was only honored for OpenFlow 1.0. - * Requests to set format NXPIF_NXT_PACKET_IN were accepted for OF1.1+ but they - * had no effect. (Requests to set formats other than NXPIF_STANDARD or - * NXPIF_NXT_PACKET_IN were rejected with OFPBRC_EPERM.) - * - * From OVS v2.6 onward, this request is honored for all OpenFlow versions. - */ -struct nx_set_packet_in_format { - ovs_be32 format; /* One of NXPIF_*. */ -}; -OFP_ASSERT(sizeof(struct nx_set_packet_in_format) == 4); - /* NXT_PACKET_IN (analogous to OFPT_PACKET_IN). * * NXT_PACKET_IN is similar to the OpenFlow 1.2 OFPT_PACKET_IN. The diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h index 23f3015aee20..5998c4c2298d 100644 --- a/include/openvswitch/ofp-msgs.h +++ b/include/openvswitch/ofp-msgs.h @@ -449,7 +449,7 @@ enum ofpraw { /* NXT 1.0+ (15): uint8_t[8]. */ OFPRAW_NXT_FLOW_MOD_TABLE_ID, - /* NXT 1.0+ (16): struct nx_set_packet_in_format. */ + /* NXT 1.0+ (16): ovs_be32. */ OFPRAW_NXT_SET_PACKET_IN_FORMAT, /* NXT 1.0+ (18): void. */ diff --git a/include/openvswitch/ofp-packet.h b/include/openvswitch/ofp-packet.h index d6e78fb9ef2c..f96e36613ac6 100644 --- a/include/openvswitch/ofp-packet.h +++ b/include/openvswitch/ofp-packet.h @@ -30,12 +30,33 @@ extern "C" { struct vl_mff_map; struct ofputil_table_map; + +/* Packet-in format. + * + * For any given OpenFlow version, Open vSwitch supports multiple formats for + * "packet-in" messages. The default is always the standard format for the + * OpenFlow version in question, but the Open vSwitch extension request + * NXT_SET_PACKET_IN_FORMAT can be used to set an alternative format. + * + * From OVS v1.1 to OVS v2.5, this request was only honored for OpenFlow 1.0. + * Requests to set format NXPIF_NXT_PACKET_IN were accepted for OF1.1+ but they + * had no effect. (Requests to set formats other than NXPIF_STANDARD or + * NXPIF_NXT_PACKET_IN were rejected with OFPBRC_EPERM.) + * + * From OVS v2.6 onward, this request is honored for all OpenFlow versions. + */ +enum ofputil_packet_in_format { + OFPUTIL_PACKET_IN_STD = 0, /* OFPT_PACKET_IN for this OpenFlow version. */ + OFPUTIL_PACKET_IN_NXT = 1, /* NXT_PACKET_IN (since OVS v1.1). */ + OFPUTIL_PACKET_IN_NXT2 = 2, /* NXT_PACKET_IN2 (since OVS v2.6). */ +}; -bool ofputil_packet_in_format_is_valid(enum nx_packet_in_format); int ofputil_packet_in_format_from_string(const char *); -const char *ofputil_packet_in_format_to_string(enum nx_packet_in_format); -struct ofpbuf *ofputil_make_set_packet_in_format(enum ofp_version, - enum nx_packet_in_format); +const char *ofputil_packet_in_format_to_string(enum ofputil_packet_in_format); +struct ofpbuf *ofputil_encode_set_packet_in_format( + enum ofp_version, enum ofputil_packet_in_format); +enum ofperr ofputil_decode_set_packet_in_format( + const struct ofp_header *, enum ofputil_packet_in_format *); /* Abstract packet-in message. * @@ -124,7 +145,7 @@ struct ofputil_packet_in_private { struct ofpbuf *ofputil_encode_packet_in_private( const struct ofputil_packet_in_private *, enum ofputil_protocol protocol, - enum nx_packet_in_format); + enum ofputil_packet_in_format); enum ofperr ofputil_decode_packet_in_private( const struct ofp_header *, bool loose, diff --git a/lib/ofp-packet.c b/lib/ofp-packet.c index 05cbea15b7e2..100f7c5693d9 100644 --- a/lib/ofp-packet.c +++ b/lib/ofp-packet.c @@ -34,28 +34,15 @@ VLOG_DEFINE_THIS_MODULE(ofp_packet); static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); -bool -ofputil_packet_in_format_is_valid(enum nx_packet_in_format packet_in_format) -{ - switch (packet_in_format) { - case NXPIF_STANDARD: - case NXPIF_NXT_PACKET_IN: - case NXPIF_NXT_PACKET_IN2: - return true; - } - - return false; -} - const char * -ofputil_packet_in_format_to_string(enum nx_packet_in_format packet_in_format) +ofputil_packet_in_format_to_string(enum ofputil_packet_in_format format) { - switch (packet_in_format) { - case NXPIF_STANDARD: + switch (format) { + case OFPUTIL_PACKET_IN_STD: return "standard"; - case NXPIF_NXT_PACKET_IN: + case OFPUTIL_PACKET_IN_NXT: return "nxt_packet_in"; - case NXPIF_NXT_PACKET_IN2: + case OFPUTIL_PACKET_IN_NXT2: return "nxt_packet_in2"; default: OVS_NOT_REACHED(); @@ -66,27 +53,48 @@ int ofputil_packet_in_format_from_string(const char *s) { return (!strcmp(s, "standard") || !strcmp(s, "openflow10") - ? NXPIF_STANDARD + ? OFPUTIL_PACKET_IN_STD : !strcmp(s, "nxt_packet_in") || !strcmp(s, "nxm") - ? NXPIF_NXT_PACKET_IN + ? OFPUTIL_PACKET_IN_NXT : !strcmp(s, "nxt_packet_in2") - ? NXPIF_NXT_PACKET_IN2 + ? OFPUTIL_PACKET_IN_NXT2 : -1); } struct ofpbuf * -ofputil_make_set_packet_in_format(enum ofp_version ofp_version, - enum nx_packet_in_format packet_in_format) +ofputil_encode_set_packet_in_format(enum ofp_version ofp_version, + enum ofputil_packet_in_format format) { - struct nx_set_packet_in_format *spif; - struct ofpbuf *msg; - - msg = ofpraw_alloc(OFPRAW_NXT_SET_PACKET_IN_FORMAT, ofp_version, 0); - spif = ofpbuf_put_zeros(msg, sizeof *spif); - spif->format = htonl(packet_in_format); + struct ofpbuf *msg = ofpraw_alloc(OFPRAW_NXT_SET_PACKET_IN_FORMAT, + ofp_version, 0); + ovs_be32 *spif = ofpbuf_put_uninit(msg, sizeof *spif); + *spif = htonl(format); return msg; } + +enum ofperr +ofputil_decode_set_packet_in_format(const struct ofp_header *oh, + enum ofputil_packet_in_format *format) +{ + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_NXT_SET_PACKET_IN_FORMAT); + ovs_be32 *spifp = ofpbuf_pull(&b, sizeof *spifp); + uint32_t spif = ntohl(*spifp); + + switch (spif) { + case OFPUTIL_PACKET_IN_STD: + case OFPUTIL_PACKET_IN_NXT: + case OFPUTIL_PACKET_IN_NXT2: + *format = spif; + return 0; + + default: + VLOG_WARN_RL(&rl, "NXT_SET_PACKET_IN_FORMAT message specified invalid " + "packet-in format %"PRIu32, spif); + return OFPERR_OFPBRC_EPERM; + } +} /* The caller has done basic initialization of '*pin'; the other output * arguments needs to be initialized. */ @@ -619,7 +627,7 @@ ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin, } /* Converts abstract ofputil_packet_in_private 'pin' into a PACKET_IN message - * for 'protocol', using the packet-in format specified by 'packet_in_format'. + * for 'protocol', using the packet-in format specified by 'format'. * * This function is really meant only for use by ovs-vswitchd. To any other * code, the "continuation" data, i.e. the data that is in struct @@ -632,13 +640,13 @@ ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin, struct ofpbuf * ofputil_encode_packet_in_private(const struct ofputil_packet_in_private *pin, enum ofputil_protocol protocol, - enum nx_packet_in_format packet_in_format) + enum ofputil_packet_in_format format) { enum ofp_version version = ofputil_protocol_to_ofp_version(protocol); struct ofpbuf *msg; - switch (packet_in_format) { - case NXPIF_STANDARD: + switch (format) { + case OFPUTIL_PACKET_IN_STD: switch (protocol) { case OFPUTIL_P_OF10_STD: case OFPUTIL_P_OF10_STD_TID: @@ -664,11 +672,11 @@ ofputil_encode_packet_in_private(const struct ofputil_packet_in_private *pin, } break; - case NXPIF_NXT_PACKET_IN: + case OFPUTIL_PACKET_IN_NXT: msg = ofputil_encode_nx_packet_in(&pin->base, version); break; - case NXPIF_NXT_PACKET_IN2: + case OFPUTIL_PACKET_IN_NXT2: return ofputil_encode_nx_packet_in2(pin, version, pin->base.packet_len); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index b13bc380386a..c0bfa92843c6 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2287,18 +2287,15 @@ ofp_print_nxt_set_flow_format(struct ds *string, const struct ofp_header *oh) static enum ofperr ofp_print_nxt_set_packet_in_format(struct ds *string, - const struct nx_set_packet_in_format *nspf) + const struct ofp_header *oh) { - uint32_t format = ntohl(nspf->format); - - ds_put_cstr(string, " format="); - if (ofputil_packet_in_format_is_valid(format)) { - ds_put_cstr(string, ofputil_packet_in_format_to_string(format)); - } else { - ds_put_format(string, "%"PRIu32, format); + enum ofputil_packet_in_format format; + enum ofperr error = ofputil_decode_set_packet_in_format(oh, &format); + if (!error) { + ds_put_format(string, " format=%s", + ofputil_packet_in_format_to_string(format)); } - - return 0; + return error; } /* Returns a string form of 'reason'. The return value is either a statically @@ -3733,7 +3730,7 @@ ofp_to_string__(const struct ofp_header *oh, return ofp_print_nxt_set_flow_format(string, oh); case OFPTYPE_SET_PACKET_IN_FORMAT: - return ofp_print_nxt_set_packet_in_format(string, ofpmsg_body(oh)); + return ofp_print_nxt_set_packet_in_format(string, oh); case OFPTYPE_FLOW_AGE: break; diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index e0e78a1e1a78..3ea558b783b4 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -73,7 +73,7 @@ struct ofconn { /* OpenFlow state. */ enum ofp12_controller_role role; /* Role. */ enum ofputil_protocol protocol; /* Current protocol variant. */ - enum nx_packet_in_format packet_in_format; /* OFPT_PACKET_IN format. */ + enum ofputil_packet_in_format packet_in_format; /* OFPT_PACKET_IN related data. */ struct rconn_packet_counter *packet_in_counter; /* # queued on 'rconn'. */ @@ -1044,7 +1044,7 @@ ofconn_set_protocol(struct ofconn *ofconn, enum ofputil_protocol protocol) * NXPIF_*. * * The default, if no other format has been set, is NXPIF_STANDARD. */ -enum nx_packet_in_format +enum ofputil_packet_in_format ofconn_get_packet_in_format(struct ofconn *ofconn) { return ofconn->packet_in_format; @@ -1054,7 +1054,7 @@ ofconn_get_packet_in_format(struct ofconn *ofconn) * NXPIF_*). */ void ofconn_set_packet_in_format(struct ofconn *ofconn, - enum nx_packet_in_format packet_in_format) + enum ofputil_packet_in_format packet_in_format) { ofconn->packet_in_format = packet_in_format; } @@ -1303,7 +1303,7 @@ ofconn_flush(struct ofconn *ofconn) ofconn->role = OFPCR12_ROLE_EQUAL; ofconn_set_protocol(ofconn, OFPUTIL_P_NONE); - ofconn->packet_in_format = NXPIF_STANDARD; + ofconn->packet_in_format = OFPUTIL_PACKET_IN_STD; rconn_packet_counter_destroy(ofconn->packet_in_counter); ofconn->packet_in_counter = rconn_packet_counter_create(); diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index c41189216b37..2405fbd7978e 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -112,8 +112,9 @@ void ofconn_set_role(struct ofconn *, enum ofp12_controller_role); enum ofputil_protocol ofconn_get_protocol(const struct ofconn *); void ofconn_set_protocol(struct ofconn *, enum ofputil_protocol); -enum nx_packet_in_format ofconn_get_packet_in_format(struct ofconn *); -void ofconn_set_packet_in_format(struct ofconn *, enum nx_packet_in_format); +enum ofputil_packet_in_format ofconn_get_packet_in_format(struct ofconn *); +void ofconn_set_packet_in_format(struct ofconn *, + enum ofputil_packet_in_format); void ofconn_set_controller_id(struct ofconn *, uint16_t controller_id); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 09e5cac6a0c7..0defe52d16af 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -5900,16 +5900,12 @@ static enum ofperr handle_nxt_set_packet_in_format(struct ofconn *ofconn, const struct ofp_header *oh) { - const struct nx_set_packet_in_format *msg = ofpmsg_body(oh); - uint32_t format; - - format = ntohl(msg->format); - if (!ofputil_packet_in_format_is_valid(format)) { - return OFPERR_OFPBRC_EPERM; + enum ofputil_packet_in_format format; + enum ofperr error = ofputil_decode_set_packet_in_format(oh, &format); + if (!error) { + ofconn_set_packet_in_format(ofconn, format); } - - ofconn_set_packet_in_format(ofconn, format); - return 0; + return error; } static enum ofperr diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index 5b936754c55a..f116a2e70da7 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -130,8 +130,8 @@ pinctrl_setup(struct rconn *swconn) rconn_get_version(swconn), 0)); /* Set a packet-in format that supports userdata. */ - queue_msg(ofputil_make_set_packet_in_format(rconn_get_version(swconn), - NXPIF_NXT_PACKET_IN2)); + queue_msg(ofputil_encode_set_packet_in_format(rconn_get_version(swconn), + OFPUTIL_PACKET_IN_NXT2)); } static void diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index e82bb12cae10..8b3817c03f90 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -1816,13 +1816,13 @@ ofctl_del_flows(struct ovs_cmdl_context *ctx) static bool set_packet_in_format(struct vconn *vconn, - enum nx_packet_in_format packet_in_format, + enum ofputil_packet_in_format packet_in_format, bool must_succeed) { struct ofpbuf *spif; - spif = ofputil_make_set_packet_in_format(vconn_get_version(vconn), - packet_in_format); + spif = ofputil_encode_set_packet_in_format(vconn_get_version(vconn), + packet_in_format); if (must_succeed) { transact_noreply(vconn, spif); } else { @@ -2299,13 +2299,13 @@ ofctl_monitor(struct ovs_cmdl_context *ctx) set_packet_in_format(vconn, preferred_packet_in_format, true); } else { /* Otherwise, we always prefer NXT_PACKET_IN2. */ - if (!set_packet_in_format(vconn, NXPIF_NXT_PACKET_IN2, false)) { + if (!set_packet_in_format(vconn, OFPUTIL_PACKET_IN_NXT2, false)) { /* We can't get NXT_PACKET_IN2. For OpenFlow 1.0 only, request * NXT_PACKET_IN. (Before 2.6, Open vSwitch will accept a request * for NXT_PACKET_IN with OF1.1+, but even after that it still * sends packet-ins in the OpenFlow native format.) */ if (vconn_get_version(vconn) == OFP10_VERSION) { - set_packet_in_format(vconn, NXPIF_NXT_PACKET_IN, false); + set_packet_in_format(vconn, OFPUTIL_PACKET_IN_NXT, false); } } }