From: Ben Pfaff Date: Wed, 26 Jan 2011 15:14:04 +0000 (-0800) Subject: Eliminate ODPL_* from userspace-facing interface. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=82272eded1ede569bcec3ba4ab212e5e3fb632ff;p=openvswitch Eliminate ODPL_* from userspace-facing interface. Reviewed by Justin Pettit. --- diff --git a/datapath/datapath.c b/datapath/datapath.c index 95449f64..94f908a0 100644 --- 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 d76b5478..84c8ee23 100644 --- 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 c323494b..cbdbbabc 100644 --- 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 b1e54816..6b2df7d1 100644 --- 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 d7953823..98a890a4 100644 --- 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 b4a1d93c..3afc9924 100644 --- 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" : ""), flow.in_port, s); free(s); diff --git a/lib/dpif.h b/lib/dpif.h index e401168e..3534df7d 100644 --- 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 08d80d89..5c7a497e 100644 --- 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. *