[ovs-dev,3/6] ofp-packet: Better abstract packet-in format.

Message ID 20180216225445.28688-3-blp@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev,1/6] ofp-util: Use consistent naming convention.
Related show

Commit Message

Ben Pfaff Feb. 16, 2018, 10:54 p.m.
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 <blp@ovn.org>
---
 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(-)

Comments

Justin Pettit March 13, 2018, 1:22 a.m. | #1
> On Feb 16, 2018, at 2:54 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> 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 error;
> }

Do you think it's worth printing some sort of error message if it's not a known protocol?  The previous version just printed the raw number.

Signed-off-by: Justin Pettit <jpettit@ovn.org>

--Justin
Justin Pettit March 13, 2018, 1:26 a.m. | #2
> On Mar 12, 2018, at 6:22 PM, Justin Pettit <jpettit@ovn.org> wrote:
> 
> 
>> On Feb 16, 2018, at 2:54 PM, Ben Pfaff <blp@ovn.org> wrote:
>> 
>> 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 error;
>> }
> 
> Do you think it's worth printing some sort of error message if it's not a known protocol?  The previous version just printed the raw number.
> 
> Signed-off-by: Justin Pettit <jpettit@ovn.org>

Whoops.  Should have been:

Acked-by: Justin Pettit <jpettit@ovn.org>

--Justin
Ben Pfaff March 14, 2018, 6:34 p.m. | #3
On Mon, Mar 12, 2018 at 06:22:06PM -0700, Justin Pettit wrote:
> 
> > On Feb 16, 2018, at 2:54 PM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > 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 error;
> > }
> 
> Do you think it's worth printing some sort of error message if it's
> not a known protocol?  The previous version just printed the raw
> number.

We'll get two error messages: first, the log warn message from
ofputil_decode_set_packet_in_format(), second, the ofperr printed by the
caller when ofp_print_nxt_set_packet_in_format() returns an error.  I
don't think it's worth worrying otherwise about this corner case.

> Signed-off-by: Justin Pettit <jpettit@ovn.org>

Thanks, I'll take that as an "ack".

Patch

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);
             }
         }
     }