Eliminate ODPL_* from userspace-facing interface.

Reviewed by Justin Pettit.

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 95449f6..94f908a 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -672,10 +672,10 @@ static const struct nla_policy execute_policy[ODP_PACKET_ATTR_MAX + 1] = {
        [ODP_PACKET_ATTR_ACTIONS] = { .type = NLA_NESTED },
 };
 
-static int execute_packet(const struct odp_upcall __user *uodp_upcall)
+static int execute_packet(const struct odp_packet __user *uodp_packet)
 {
        struct nlattr *a[ODP_PACKET_ATTR_MAX + 1];
-       struct odp_upcall *odp_upcall;
+       struct odp_packet *odp_packet;
        struct sk_buff *skb, *packet;
        unsigned int actions_len;
        struct nlattr *actions;
@@ -686,9 +686,9 @@ static int execute_packet(const struct odp_upcall __user *uodp_upcall)
        u32 len;
        int err;
 
-       if (get_user(len, &uodp_upcall->len))
+       if (get_user(len, &uodp_packet->len))
                return -EFAULT;
-       if (len < sizeof(struct odp_upcall))
+       if (len < sizeof(struct odp_packet))
                return -EINVAL;
 
        skb = alloc_skb(len, GFP_KERNEL);
@@ -696,15 +696,15 @@ static int execute_packet(const struct odp_upcall __user *uodp_upcall)
                return -ENOMEM;
 
        err = -EFAULT;
-       if (copy_from_user(__skb_put(skb, len), uodp_upcall, len))
+       if (copy_from_user(__skb_put(skb, len), uodp_packet, len))
                goto exit_free_skb;
 
-       odp_upcall = (struct odp_upcall *)skb->data;
+       odp_packet = (struct odp_packet *)skb->data;
        err = -EINVAL;
-       if (odp_upcall->len != len)
+       if (odp_packet->len != len)
                goto exit_free_skb;
 
-       __skb_pull(skb, sizeof(struct odp_upcall));
+       __skb_pull(skb, sizeof(struct odp_packet));
        err = nla_parse(a, ODP_PACKET_ATTR_MAX, (struct nlattr *)skb->data,
                        skb->len, execute_policy);
        if (err)
@@ -744,7 +744,7 @@ static int execute_packet(const struct odp_upcall __user *uodp_upcall)
                goto exit_free_skb;
 
        rcu_read_lock();
-       dp = get_dp(odp_upcall->dp_idx);
+       dp = get_dp(odp_packet->dp_idx);
        err = -ENODEV;
        if (dp)
                err = execute_actions(dp, packet, &key, actions, actions_len);
@@ -2036,7 +2036,7 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd,
                goto exit;
 
        case ODP_EXECUTE:
-               err = execute_packet((struct odp_upcall __user *)argp);
+               err = execute_packet((struct odp_packet __user *)argp);
                goto exit;
        }
 

diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index d76b547..84c8ee2 100644 (file)
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -92,7 +92,7 @@
 #define ODP_FLOW_DUMP           _IOWR('O', 17, struct odp_flow)
 #define ODP_FLOW_FLUSH          _IO('O', 19)
 
-#define ODP_EXECUTE             _IOR('O', 18, struct odp_upcall)
+#define ODP_EXECUTE             _IOR('O', 18, struct odp_packet)
 
 /**
  * struct odp_datapath - header with basic information about a datapath.

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index c323494..cbdbbab 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -686,7 +686,7 @@ dpif_linux_execute(struct dpif *dpif_,
                    const struct ofpbuf *packet)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
-    struct odp_upcall *execute;
+    struct odp_packet *execute;
     struct ofpbuf *buf;
     int error;
 
@@ -779,6 +779,7 @@ parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall *upcall)
 
     struct odp_packet *odp_packet = buf->data;
     struct nlattr *a[ARRAY_SIZE(odp_packet_policy)];
+    uint32_t type;
 
     if (!nl_policy_parse(buf, sizeof *odp_packet, odp_packet_policy,
                          a, ARRAY_SIZE(odp_packet_policy))) {
@@ -786,7 +787,13 @@ parse_odp_packet(struct ofpbuf *buf, struct dpif_upcall *upcall)
     }
 
     memset(upcall, 0, sizeof *upcall);
-    upcall->type = nl_attr_get_u32(a[ODP_PACKET_ATTR_TYPE]);
+
+    type = nl_attr_get_u32(a[ODP_PACKET_ATTR_TYPE]);
+    upcall->type = (type == _ODPL_MISS_NR ? DPIF_UC_MISS
+                    : type == _ODPL_ACTION_NR ? DPIF_UC_ACTION
+                    : type == _ODPL_SFLOW_NR ? DPIF_UC_SAMPLE
+                    : -1);
+
     upcall->packet = buf;
     upcall->packet->data = (void *) nl_attr_get(a[ODP_PACKET_ATTR_PACKET]);
     upcall->packet->size = nl_attr_get_size(a[ODP_PACKET_ATTR_PACKET]);

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b1e5481..6b2df7d 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -996,12 +996,8 @@ static int
 dpif_netdev_recv_set_mask(struct dpif *dpif, int listen_mask)
 {
     struct dpif_netdev *dpif_netdev = dpif_netdev_cast(dpif);
-    if (!(listen_mask & ~ODPL_ALL)) {
-        dpif_netdev->listen_mask = listen_mask;
-        return 0;
-    } else {
-        return EINVAL;
-    }
+    dpif_netdev->listen_mask = listen_mask;
+    return 0;
 }
 
 static struct dp_netdev_queue *
@@ -1090,7 +1086,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port,
         dp->n_hit++;
     } else {
         dp->n_missed++;
-        dp_netdev_output_control(dp, packet, _ODPL_MISS_NR, &key, 0);
+        dp_netdev_output_control(dp, packet, DPIF_UC_MISS, &key, 0);
     }
 }
 
@@ -1366,7 +1362,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
             break;
 
         case ODPAT_CONTROLLER:
-            dp_netdev_output_control(dp, packet, _ODPL_ACTION_NR,
+            dp_netdev_output_control(dp, packet, DPIF_UC_ACTION,
                                      key, nl_attr_get_u64(a));
             break;
 

diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index d795382..98a890a 100644 (file)
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -303,14 +303,16 @@ struct dpif_class {
     int (*execute)(struct dpif *dpif, const struct nlattr *actions,
                    size_t actions_len, const struct ofpbuf *packet);
 
-    /* Retrieves 'dpif''s "listen mask" into '*listen_mask'.  Each ODPL_* bit
-     * set in '*listen_mask' indicates the 'dpif' will receive messages of the
-     * corresponding type when it calls the recv member function. */
+    /* Retrieves 'dpif''s "listen mask" into '*listen_mask'.  A 1-bit of value
+     * 2**X set in '*listen_mask' indicates that 'dpif' will receive messages
+     * of the type (from "enum dpif_upcall_type") with value X when its 'recv'
+     * function is called. */
     int (*recv_get_mask)(const struct dpif *dpif, int *listen_mask);
 
-    /* Sets 'dpif''s "listen mask" to 'listen_mask'.  Each ODPL_* bit set in
-     * 'listen_mask' indicates the 'dpif' will receive messages of the
-     * corresponding type when it calls the recv member function. */
+    /* Sets 'dpif''s "listen mask" to 'listen_mask'.  A 1-bit of value 2**X set
+     * in '*listen_mask' requests that 'dpif' will receive messages of the type
+     * (from "enum dpif_upcall_type") with value X when its 'recv' function is
+     * called. */
     int (*recv_set_mask)(struct dpif *dpif, int listen_mask);
 
     /* Retrieves 'dpif''s sFlow sampling probability into '*probability'.

diff --git a/lib/dpif.c b/lib/dpif.c
index b4a1d93..3afc992 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -973,9 +973,18 @@ dpif_execute(struct dpif *dpif,
     return error;
 }
 
-/* Retrieves 'dpif''s "listen mask" into '*listen_mask'.  Each ODPL_* bit set
- * in '*listen_mask' indicates that dpif_recv() will receive messages of that
- * type.  Returns 0 if successful, otherwise a positive errno value. */
+static bool OVS_UNUSED
+is_valid_listen_mask(int listen_mask)
+{
+    return !(listen_mask & ~((1u << DPIF_UC_MISS) |
+                             (1u << DPIF_UC_ACTION) |
+                             (1u << DPIF_UC_SAMPLE)));
+}
+
+/* Retrieves 'dpif''s "listen mask" into '*listen_mask'.  A 1-bit of value 2**X
+ * set in '*listen_mask' indicates that dpif_recv() will receive messages of
+ * the type (from "enum dpif_upcall_type") with value X.  Returns 0 if
+ * successful, otherwise a positive errno value. */
 int
 dpif_recv_get_mask(const struct dpif *dpif, int *listen_mask)
 {
@@ -983,17 +992,23 @@ dpif_recv_get_mask(const struct dpif *dpif, int *listen_mask)
     if (error) {
         *listen_mask = 0;
     }
+    assert(is_valid_listen_mask(*listen_mask));
     log_operation(dpif, "recv_get_mask", error);
     return error;
 }
 
-/* Sets 'dpif''s "listen mask" to 'listen_mask'.  Each ODPL_* bit set in
- * '*listen_mask' requests that dpif_recv() receive messages of that type.
- * Returns 0 if successful, otherwise a positive errno value. */
+/* Sets 'dpif''s "listen mask" to 'listen_mask'.  A 1-bit of value 2**X set in
+ * '*listen_mask' requests that dpif_recv() will receive messages of the type
+ * (from "enum dpif_upcall_type") with value X.  Returns 0 if successful,
+ * otherwise a positive errno value. */
 int
 dpif_recv_set_mask(struct dpif *dpif, int listen_mask)
 {
-    int error = dpif->dpif_class->recv_set_mask(dpif, listen_mask);
+    int error;
+
+    assert(is_valid_listen_mask(listen_mask));
+
+    error = dpif->dpif_class->recv_set_mask(dpif, listen_mask);
     log_operation(dpif, "recv_set_mask", error);
     return error;
 }
@@ -1034,7 +1049,7 @@ dpif_set_sflow_probability(struct dpif *dpif, uint32_t probability)
 }
 
 /* Polls for an upcall from 'dpif'.  If successful, stores the upcall into
- * '*upcall'.  Only upcalls of the types selected with the set_listen_mask
+ * '*upcall'.  Only upcalls of the types selected with dpif_recv_set_mask()
  * member function will ordinarily be received (but if a message type is
  * enabled and then later disabled, some stragglers might pop up).
  *
@@ -1059,9 +1074,9 @@ dpif_recv(struct dpif *dpif, struct dpif_upcall *upcall)
         odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow);
 
         VLOG_DBG("%s: %s upcall on port %"PRIu16": %s", dpif_name(dpif),
-                 (upcall->type == _ODPL_MISS_NR ? "miss"
-                  : upcall->type == _ODPL_ACTION_NR ? "action"
-                  : upcall->type == _ODPL_SFLOW_NR ? "sFlow"
+                 (upcall->type == DPIF_UC_MISS ? "miss"
+                  : upcall->type == DPIF_UC_ACTION ? "action"
+                  : upcall->type == DPIF_UC_SAMPLE ? "sample"
                   : "<unknown>"),
                  flow.in_port, s);
         free(s);

diff --git a/lib/dpif.h b/lib/dpif.h
index e401168..3534df7 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -150,6 +150,12 @@ int dpif_flow_dump_done(struct dpif_flow_dump *);
 int dpif_execute(struct dpif *, const struct nlattr *actions,
                  size_t actions_len, const struct ofpbuf *);
 
+enum dpif_upcall_type {
+    DPIF_UC_MISS,               /* Miss in flow table. */
+    DPIF_UC_ACTION,             /* ODPAT_CONTROLLER action. */
+    DPIF_UC_SAMPLE              /* Packet sampling. */
+};
+
 /* A packet passed up from the datapath to userspace.
  *
  * If 'key' or 'actions' is nonnull, then it points into data owned by
@@ -158,17 +164,16 @@ int dpif_execute(struct dpif *, const struct nlattr *actions,
  * clients that exist so far.)
  */
 struct dpif_upcall {
-    uint32_t type;              /* One of _ODPL_*_NR. */
-
     /* All types. */
+    enum dpif_upcall_type type;
     struct ofpbuf *packet;      /* Packet data. */
     struct nlattr *key;         /* Flow key. */
     size_t key_len;             /* Length of 'key' in bytes. */
 
-    /* _ODPL_ACTION_NR only. */
+    /* DPIF_UC_ACTION only. */
     uint64_t userdata;          /* Argument to ODPAT_CONTROLLER. */
 
-    /* _ODPL_SFLOW_NR only. */
+    /* DPIF_UC_SAMPLE only. */
     uint32_t sample_pool;       /* # of sampling candidate packets so far. */
     struct nlattr *actions;     /* Associated flow actions. */
     size_t actions_len;

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 08d80d8..5c7a497 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -301,7 +301,8 @@ struct ofconn {
 
     /* OFPT_PACKET_IN related data. */
     struct rconn_packet_counter *packet_in_counter; /* # queued on 'rconn'. */
-    struct pinsched *schedulers[2]; /* Indexed by reason code; see below. */
+#define N_SCHEDULERS 2
+    struct pinsched *schedulers[N_SCHEDULERS];
     struct pktbuf *pktbuf;         /* OpenFlow packet buffers. */
     int miss_send_len;             /* Bytes to send of buffered packets. */
 
@@ -319,15 +320,6 @@ struct ofconn {
     enum ofproto_band band;      /* In-band or out-of-band? */
 };
 
-/* We use OFPR_NO_MATCH and OFPR_ACTION as indexes into struct ofconn's
- * "schedulers" array.  Their values are 0 and 1, and their meanings and values
- * coincide with _ODPL_MISS_NR and _ODPL_ACTION_NR, so this is convenient.  In
- * case anything ever changes, check their values here.  */
-#define N_SCHEDULERS 2
-BUILD_ASSERT_DECL(OFPR_NO_MATCH == 0);
-BUILD_ASSERT_DECL(OFPR_NO_MATCH == _ODPL_MISS_NR);
-BUILD_ASSERT_DECL(OFPR_ACTION == 1);
-BUILD_ASSERT_DECL(OFPR_ACTION == _ODPL_ACTION_NR);
 
 static struct ofconn *ofconn_create(struct ofproto *, struct rconn *,
                                     enum ofconn_type);
@@ -444,7 +436,10 @@ ofproto_create(const char *datapath, const char *datapath_type,
         VLOG_ERR("failed to open datapath %s: %s", datapath, strerror(error));
         return error;
     }
-    error = dpif_recv_set_mask(dpif, ODPL_MISS | ODPL_ACTION | ODPL_SFLOW);
+    error = dpif_recv_set_mask(dpif,
+                               ((1u << DPIF_UC_MISS) |
+                                (1u << DPIF_UC_ACTION) |
+                                (1u << DPIF_UC_SAMPLE)));
     if (error) {
         VLOG_ERR("failed to listen on datapath %s: %s",
                  datapath, strerror(error));
@@ -2099,7 +2094,7 @@ execute_odp_actions(struct ofproto *ofproto, const struct flow *flow,
          * buffers along the way. */
         struct dpif_upcall upcall;
 
-        upcall.type = _ODPL_ACTION_NR;
+        upcall.type = DPIF_UC_ACTION;
         upcall.packet = packet;
         upcall.key = NULL;
         upcall.key_len = 0;
@@ -4439,13 +4434,13 @@ handle_upcall(struct ofproto *p, struct dpif_upcall *upcall)
     struct flow flow;
 
     switch (upcall->type) {
-    case _ODPL_ACTION_NR:
+    case DPIF_UC_ACTION:
         COVERAGE_INC(ofproto_ctlr_action);
         odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow);
         send_packet_in(p, upcall, &flow, false);
         break;
 
-    case _ODPL_SFLOW_NR:
+    case DPIF_UC_SAMPLE:
         if (p->sflow) {
             odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow);
             ofproto_sflow_received(p->sflow, upcall, &flow);
@@ -4453,7 +4448,7 @@ handle_upcall(struct ofproto *p, struct dpif_upcall *upcall)
         ofpbuf_delete(upcall->packet);
         break;
 
-    case _ODPL_MISS_NR:
+    case DPIF_UC_MISS:
         handle_miss_upcall(p, upcall);
         break;
 
@@ -4809,9 +4804,10 @@ schedule_packet_in(struct ofconn *ofconn, struct dpif_upcall *upcall,
     int total_len, send_len;
     struct ofpbuf *packet;
     uint32_t buffer_id;
+    int idx;
 
     /* Get OpenFlow buffer_id. */
-    if (upcall->type == _ODPL_ACTION_NR) {
+    if (upcall->type == DPIF_UC_ACTION) {
         buffer_id = UINT32_MAX;
     } else if (ofproto->fail_open && fail_open_is_active(ofproto->fail_open)) {
         buffer_id = pktbuf_get_null();
@@ -4826,7 +4822,7 @@ schedule_packet_in(struct ofconn *ofconn, struct dpif_upcall *upcall,
     if (buffer_id != UINT32_MAX) {
         send_len = MIN(send_len, ofconn->miss_send_len);
     }
-    if (upcall->type == _ODPL_ACTION_NR) {
+    if (upcall->type == DPIF_UC_ACTION) {
         send_len = MIN(send_len, upcall->userdata);
     }
 
@@ -4845,18 +4841,19 @@ schedule_packet_in(struct ofconn *ofconn, struct dpif_upcall *upcall,
     opi->header.type = OFPT_PACKET_IN;
     opi->total_len = htons(total_len);
     opi->in_port = htons(odp_port_to_ofp_port(flow->in_port));
-    opi->reason = upcall->type == _ODPL_MISS_NR ? OFPR_NO_MATCH : OFPR_ACTION;
+    opi->reason = upcall->type == DPIF_UC_MISS ? OFPR_NO_MATCH : OFPR_ACTION;
     opi->buffer_id = htonl(buffer_id);
     update_openflow_length(packet);
 
     /* Hand over to packet scheduler.  It might immediately call into
      * do_send_packet_in() or it might buffer it for a while (until a later
      * call to pinsched_run()). */
-    pinsched_send(ofconn->schedulers[opi->reason], flow->in_port,
+    idx = upcall->type == DPIF_UC_MISS ? 0 : 1;
+    pinsched_send(ofconn->schedulers[idx], flow->in_port,
                   packet, do_send_packet_in, ofconn);
 }
 
-/* Given 'upcall', of type _ODPL_ACTION_NR or _ODPL_MISS_NR, sends an
+/* Given 'upcall', of type DPIF_UC_ACTION or DPIF_UC_MISS, sends an
  * OFPT_PACKET_IN message to each OpenFlow controller as necessary according to
  * their individual configurations.
  *