From 758c456df570a1af1d9e913d50a3478785663e66 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 30 Dec 2013 15:58:58 -0800 Subject: [PATCH 1/1] dpif: Use explicit packet metadata. This helps reduce confusion about when a flow is a flow and when it is just metadata. Signed-off-by: Jarno Rajahalme Acked-by: Ben Pfaff --- lib/dpif-linux.c | 11 +++- lib/dpif-netdev.c | 58 ++++++++--------- lib/dpif-provider.h | 10 ++- lib/dpif.c | 117 +++++++++++----------------------- lib/dpif.h | 15 ++--- lib/odp-execute.c | 11 ++-- lib/odp-execute.h | 6 +- lib/odp-util.c | 71 ++++++++++++++++++++- lib/odp-util.h | 7 ++ lib/packets.h | 11 ++++ ofproto/ofproto-dpif-upcall.c | 4 +- ofproto/ofproto-dpif-xlate.c | 8 +-- ofproto/ofproto-dpif.c | 42 ++++++------ 13 files changed, 200 insertions(+), 171 deletions(-) diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index 482ba7730..3587bbd75 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -1090,10 +1090,11 @@ dpif_linux_encode_execute(int dp_ifindex, const struct dpif_execute *d_exec, struct ofpbuf *buf) { struct ovs_header *k_exec; + size_t key_ofs; ofpbuf_prealloc_tailroom(buf, (64 + d_exec->packet->size - + d_exec->key_len + + ODP_KEY_METADATA_SIZE + d_exec->actions_len)); nl_msg_put_genlmsghdr(buf, 0, ovs_packet_family, NLM_F_REQUEST, @@ -1104,7 +1105,11 @@ dpif_linux_encode_execute(int dp_ifindex, const struct dpif_execute *d_exec, nl_msg_put_unspec(buf, OVS_PACKET_ATTR_PACKET, d_exec->packet->data, d_exec->packet->size); - nl_msg_put_unspec(buf, OVS_PACKET_ATTR_KEY, d_exec->key, d_exec->key_len); + + key_ofs = nl_msg_start_nested(buf, OVS_PACKET_ATTR_KEY); + odp_key_from_pkt_metadata(buf, &d_exec->md); + nl_msg_end_nested(buf, key_ofs); + nl_msg_put_unspec(buf, OVS_PACKET_ATTR_ACTIONS, d_exec->actions, d_exec->actions_len); } @@ -1125,7 +1130,7 @@ dpif_linux_execute__(int dp_ifindex, const struct dpif_execute *execute) } static int -dpif_linux_execute(struct dpif *dpif_, const struct dpif_execute *execute) +dpif_linux_execute(struct dpif *dpif_, struct dpif_execute *execute) { const struct dpif_linux *dpif = dpif_linux_cast(dpif_); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 076369745..6ef749566 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -170,12 +170,11 @@ static int dp_netdev_output_userspace(struct dp_netdev *, struct ofpbuf *, int queue_no, const struct flow *, const struct nlattr *userdata); static void dp_netdev_execute_actions(struct dp_netdev *, const struct flow *, - struct ofpbuf *, + struct ofpbuf *, struct pkt_metadata *, const struct nlattr *actions, size_t actions_len); -static void dp_netdev_port_input(struct dp_netdev *dp, - struct dp_netdev_port *port, - struct ofpbuf *packet); +static void dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf *packet, + struct pkt_metadata *md); static struct dpif_netdev * dpif_netdev_cast(const struct dpif *dpif) @@ -1140,31 +1139,25 @@ dpif_netdev_flow_dump_done(const struct dpif *dpif OVS_UNUSED, void *state_) } static int -dpif_netdev_execute(struct dpif *dpif, const struct dpif_execute *execute) +dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) { struct dp_netdev *dp = get_dp_netdev(dpif); - struct flow md; - int error; + struct pkt_metadata *md = &execute->md; + struct flow key; if (execute->packet->size < ETH_HEADER_LEN || execute->packet->size > UINT16_MAX) { return EINVAL; } - /* Get packet metadata. */ - error = dpif_netdev_flow_from_nlattrs(execute->key, execute->key_len, &md); - if (!error) { - struct flow key; - - /* Extract flow key. */ - flow_extract(execute->packet, md.skb_priority, md.pkt_mark, &md.tunnel, - &md.in_port, &key); - ovs_mutex_lock(&dp_netdev_mutex); - dp_netdev_execute_actions(dp, &key, execute->packet, - execute->actions, execute->actions_len); - ovs_mutex_unlock(&dp_netdev_mutex); - } - return error; + /* Extract flow key. */ + flow_extract(execute->packet, md->skb_priority, md->pkt_mark, &md->tunnel, + (union flow_in_port *)&md->in_port, &key); + ovs_mutex_lock(&dp_netdev_mutex); + dp_netdev_execute_actions(dp, &key, execute->packet, md, execute->actions, + execute->actions_len); + ovs_mutex_unlock(&dp_netdev_mutex); + return 0; } static int @@ -1258,22 +1251,21 @@ dp_netdev_flow_used(struct dp_netdev_flow *netdev_flow, } static void -dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port, - struct ofpbuf *packet) +dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf *packet, + struct pkt_metadata *md) { struct dp_netdev_flow *netdev_flow; struct flow key; - union flow_in_port in_port_; if (packet->size < ETH_HEADER_LEN) { return; } - in_port_.odp_port = port->port_no; - flow_extract(packet, 0, 0, NULL, &in_port_, &key); + flow_extract(packet, md->skb_priority, md->pkt_mark, &md->tunnel, + (union flow_in_port *)&md->in_port, &key); netdev_flow = dp_netdev_lookup_flow(dp, &key); if (netdev_flow) { dp_netdev_flow_used(netdev_flow, packet); - dp_netdev_execute_actions(dp, &key, packet, + dp_netdev_execute_actions(dp, &key, packet, md, netdev_flow->actions, netdev_flow->actions_len); dp->n_hit++; @@ -1306,7 +1298,8 @@ dpif_netdev_run(struct dpif *dpif) error = port->rx ? netdev_rx_recv(port->rx, &packet) : EOPNOTSUPP; if (!error) { - dp_netdev_port_input(dp, port, &packet); + struct pkt_metadata md = PKT_METADATA_INITIALIZER(port->port_no); + dp_netdev_port_input(dp, &packet, &md); } else if (error != EAGAIN && error != EOPNOTSUPP) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); @@ -1407,7 +1400,8 @@ struct dp_netdev_execute_aux { }; static void -dp_execute_cb(void *aux_, struct ofpbuf *packet, struct flow *flow OVS_UNUSED, +dp_execute_cb(void *aux_, struct ofpbuf *packet, + const struct pkt_metadata *md OVS_UNUSED, const struct nlattr *a, bool may_steal) { struct dp_netdev_execute_aux *aux = aux_; @@ -1448,14 +1442,12 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet, struct flow *flow OVS_UNUSED, static void dp_netdev_execute_actions(struct dp_netdev *dp, const struct flow *key, - struct ofpbuf *packet, + struct ofpbuf *packet, struct pkt_metadata *md, const struct nlattr *actions, size_t actions_len) { struct dp_netdev_execute_aux aux = {dp, key}; - struct flow md = *key; /* Packet metadata, may be modified by actions. */ - odp_execute_actions(&aux, packet, &md, actions, actions_len, - dp_execute_cb); + odp_execute_actions(&aux, packet, md, actions, actions_len, dp_execute_cb); } const struct dpif_class dpif_netdev_class = { diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 90a02b436..e16a4263f 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -311,12 +311,10 @@ struct dpif_class { int (*flow_dump_done)(const struct dpif *dpif, void *state); /* Performs the 'execute->actions_len' bytes of actions in - * 'execute->actions' on the Ethernet frame specified in 'execute->packet' - * taken from the flow specified in the 'execute->key_len' bytes of - * 'execute->key'. ('execute->key' is mostly redundant with - * 'execute->packet', but it contains some metadata that cannot be - * recovered from 'execute->packet', such as tunnel and in_port.) */ - int (*execute)(struct dpif *dpif, const struct dpif_execute *execute); + * 'execute->actions' on the Ethernet frame in 'execute->packet' + * and on the packet metadata in 'execute->md'. + * May modify both packet and metadata. */ + int (*execute)(struct dpif *dpif, struct dpif_execute *execute); /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order * in which they are specified, placing each operation's results in the diff --git a/lib/dpif.c b/lib/dpif.c index e25655875..2b79a6e34 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1064,46 +1064,28 @@ struct dpif_execute_helper_aux { int error; }; -static void -dpif_execute_helper_execute__(void *aux_, struct ofpbuf *packet, - const struct flow *flow, - const struct nlattr *actions, size_t actions_len) -{ - struct dpif_execute_helper_aux *aux = aux_; - struct dpif_execute execute; - struct odputil_keybuf key_stub; - struct ofpbuf key; - int error; - - ofpbuf_use_stub(&key, &key_stub, sizeof key_stub); - odp_flow_key_from_flow(&key, flow, flow->in_port.odp_port); - - execute.key = key.data; - execute.key_len = key.size; - execute.actions = actions; - execute.actions_len = actions_len; - execute.packet = packet; - execute.needs_help = false; - - error = aux->dpif->dpif_class->execute(aux->dpif, &execute); - if (error) { - aux->error = error; - } -} - /* This is called for actions that need the context of the datapath to be * meaningful. */ static void -dpif_execute_helper_cb(void *aux, struct ofpbuf *packet, struct flow *flow, +dpif_execute_helper_cb(void *aux_, struct ofpbuf *packet, + const struct pkt_metadata *md, const struct nlattr *action, bool may_steal OVS_UNUSED) { + struct dpif_execute_helper_aux *aux = aux_; + struct dpif_execute execute; int type = nl_attr_type(action); + switch ((enum ovs_action_attr)type) { case OVS_ACTION_ATTR_OUTPUT: case OVS_ACTION_ATTR_USERSPACE: - dpif_execute_helper_execute__(aux, packet, flow, - action, NLA_ALIGN(action->nla_len)); + execute.actions = action; + execute.actions_len = NLA_ALIGN(action->nla_len); + execute.packet = packet; + execute.md = *md; + execute.needs_help = false; + aux->error = aux->dpif->dpif_class->execute(aux->dpif, &execute); break; + case OVS_ACTION_ATTR_PUSH_VLAN: case OVS_ACTION_ATTR_POP_VLAN: case OVS_ACTION_ATTR_PUSH_MPLS: @@ -1122,36 +1104,42 @@ dpif_execute_helper_cb(void *aux, struct ofpbuf *packet, struct flow *flow, * * This helps with actions that a given 'dpif' doesn't implement directly. */ static int -dpif_execute_with_help(struct dpif *dpif, const struct dpif_execute *execute) +dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute) { - struct dpif_execute_helper_aux aux; - enum odp_key_fitness fit; - struct flow flow; + struct dpif_execute_helper_aux aux = {dpif, 0}; COVERAGE_INC(dpif_execute_with_help); - fit = odp_flow_key_to_flow(execute->key, execute->key_len, &flow); - if (fit == ODP_FIT_ERROR) { - return EINVAL; - } - - aux.dpif = dpif; - aux.error = 0; - - odp_execute_actions(&aux, execute->packet, &flow, + odp_execute_actions(&aux, execute->packet, &execute->md, execute->actions, execute->actions_len, dpif_execute_helper_cb); return aux.error; } -static int -dpif_execute__(struct dpif *dpif, const struct dpif_execute *execute) +/* Causes 'dpif' to perform the 'execute->actions_len' bytes of actions in + * 'execute->actions' on the Ethernet frame in 'execute->packet' and on packet + * metadata in 'execute->md'. The implementation is allowed to modify both the + * '*execute->packet' and 'execute->md'. + * + * Some dpif providers do not implement every action. The Linux kernel + * datapath, in particular, does not implement ARP field modification. If + * 'needs_help' is true, the dpif layer executes in userspace all of the + * actions that it can, and for OVS_ACTION_ATTR_OUTPUT and + * OVS_ACTION_ATTR_USERSPACE actions it passes the packet through to the dpif + * implementation. + * + * This works even if 'execute->actions_len' is too long for a Netlink + * attribute. + * + * Returns 0 if successful, otherwise a positive errno value. */ +int +dpif_execute(struct dpif *dpif, struct dpif_execute *execute) { int error; COVERAGE_INC(dpif_execute); if (execute->actions_len > 0) { - error = (execute->needs_help + error = (execute->needs_help || nl_attr_oversized(execute->actions_len) ? dpif_execute_with_help(dpif, execute) : dpif->dpif_class->execute(dpif, execute)); } else { @@ -1163,39 +1151,6 @@ dpif_execute__(struct dpif *dpif, const struct dpif_execute *execute) return error; } -/* Causes 'dpif' to perform the 'actions_len' bytes of actions in 'actions' on - * the Ethernet frame specified in 'packet' taken from the flow specified in - * the 'key_len' bytes of 'key'. ('key' is mostly redundant with 'packet', but - * it contains some metadata that cannot be recovered from 'packet', such as - * tunnel and in_port.) - * - * Some dpif providers do not implement every action. The Linux kernel - * datapath, in particular, does not implement ARP field modification. If - * 'needs_help' is true, the dpif layer executes in userspace all of the - * actions that it can, and for OVS_ACTION_ATTR_OUTPUT and - * OVS_ACTION_ATTR_USERSPACE actions it passes the packet through to the dpif - * implementation. - * - * This works even if 'actions_len' is too long for a Netlink attribute. - * - * Returns 0 if successful, otherwise a positive errno value. */ -int -dpif_execute(struct dpif *dpif, - const struct nlattr *key, size_t key_len, - const struct nlattr *actions, size_t actions_len, - struct ofpbuf *buf, bool needs_help) -{ - struct dpif_execute execute; - - execute.key = key; - execute.key_len = key_len; - execute.actions = actions; - execute.actions_len = actions_len; - execute.packet = buf; - execute.needs_help = needs_help || nl_attr_oversized(actions_len); - return dpif_execute__(dpif, &execute); -} - /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order in * which they are specified, placing each operation's results in the "output" * members documented in comments. @@ -1251,7 +1206,7 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops) /* Help the dpif provider to execute one op. */ struct dpif_op *op = ops[0]; - op->error = dpif_execute__(dpif, &op->u.execute); + op->error = dpif_execute(dpif, &op->u.execute); ops++; n_ops--; } @@ -1272,7 +1227,7 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops) break; case DPIF_OP_EXECUTE: - op->error = dpif_execute__(dpif, &op->u.execute); + op->error = dpif_execute(dpif, &op->u.execute); break; default: diff --git a/lib/dpif.h b/lib/dpif.h index 974fbbfe7..7f986f957 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -368,9 +368,10 @@ #include #include #include +#include "netdev.h" #include "ofpbuf.h" #include "openflow/openflow.h" -#include "netdev.h" +#include "packets.h" #include "util.h" #ifdef __cplusplus @@ -516,13 +517,6 @@ bool dpif_flow_dump_next(struct dpif_flow_dump *, const struct dpif_flow_stats **); int dpif_flow_dump_done(struct dpif_flow_dump *); -/* Packet operations. */ - -int dpif_execute(struct dpif *, - const struct nlattr *key, size_t key_len, - const struct nlattr *actions, size_t actions_len, - struct ofpbuf *, bool needs_help); - /* Operation batching interface. * * Some datapaths are faster at performing N operations together than the same @@ -560,11 +554,10 @@ struct dpif_flow_del { struct dpif_execute { /* Raw support for execute passed along to the provider. */ - const struct nlattr *key; /* Partial flow key (only for metadata). */ - size_t key_len; /* Length of 'key' in bytes. */ const struct nlattr *actions; /* Actions to execute on packet. */ size_t actions_len; /* Length of 'actions' in bytes. */ struct ofpbuf *packet; /* Packet to execute. */ + struct pkt_metadata md; /* Packet metadata. */ /* Some dpif providers do not implement every action. The Linux kernel * datapath, in particular, does not implement ARP field modification. @@ -576,6 +569,8 @@ struct dpif_execute { bool needs_help; }; +int dpif_execute(struct dpif *, struct dpif_execute *); + struct dpif_op { enum dpif_op_type type; int error; diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 7a96ca0ad..5b77fa9a9 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -21,6 +21,7 @@ #include #include +#include "dpif.h" #include "netlink.h" #include "ofpbuf.h" #include "odp-util.h" @@ -61,7 +62,7 @@ set_arp(struct ofpbuf *packet, const struct ovs_key_arp *arp_key) static void odp_execute_set_action(struct ofpbuf *packet, const struct nlattr *a, - struct flow *md) + struct pkt_metadata *md) { enum ovs_key_attr type = nl_attr_type(a); const struct ovs_key_ipv4 *ipv4_key; @@ -140,12 +141,12 @@ odp_execute_set_action(struct ofpbuf *packet, const struct nlattr *a, } static void -odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key, +odp_execute_actions__(void *dp, struct ofpbuf *packet, struct pkt_metadata *, const struct nlattr *actions, size_t actions_len, odp_execute_cb dp_execute_action, bool more_actions); static void -odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *md, +odp_execute_sample(void *dp, struct ofpbuf *packet, struct pkt_metadata *md, const struct nlattr *action, odp_execute_cb dp_execute_action, bool more_actions) { @@ -180,7 +181,7 @@ odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *md, } static void -odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *md, +odp_execute_actions__(void *dp, struct ofpbuf *packet, struct pkt_metadata *md, const struct nlattr *actions, size_t actions_len, odp_execute_cb dp_execute_action, bool more_actions) { @@ -239,7 +240,7 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *md, } void -odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *md, +odp_execute_actions(void *dp, struct ofpbuf *packet, struct pkt_metadata *md, const struct nlattr *actions, size_t actions_len, odp_execute_cb dp_execute_action) { diff --git a/lib/odp-execute.h b/lib/odp-execute.h index 993ff471f..670e8ea51 100644 --- a/lib/odp-execute.h +++ b/lib/odp-execute.h @@ -23,12 +23,12 @@ #include #include "openvswitch/types.h" -struct flow; struct nlattr; struct ofpbuf; +struct pkt_metadata; typedef void (*odp_execute_cb)(void *dp, struct ofpbuf *packet, - struct flow *metadata, + const struct pkt_metadata *, const struct nlattr *action, bool may_steal); /* Actions that need to be executed in the context of a datapath are handed @@ -36,7 +36,7 @@ typedef void (*odp_execute_cb)(void *dp, struct ofpbuf *packet, * actions OVS_ACTION_ATTR_OUTPUT and OVS_ACTION_ATTR_USERSPACE so * 'dp_execute_action' needs to handle only these. */ void -odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *metadata, +odp_execute_actions(void *dp, struct ofpbuf *packet, struct pkt_metadata *, const struct nlattr *actions, size_t actions_len, odp_execute_cb dp_execute_action); #endif diff --git a/lib/odp-util.c b/lib/odp-util.c index f44c7d49d..247e5f6ff 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -26,6 +26,7 @@ #include #include "byte-order.h" #include "coverage.h" +#include "dpif.h" #include "dynamic-string.h" #include "flow.h" #include "netlink.h" @@ -845,7 +846,7 @@ odp_tun_key_from_attr(const struct nlattr *attr, struct flow_tnl *tun) return ODP_FIT_ERROR; } if (unknown) { - return ODP_FIT_TOO_MUCH; + return ODP_FIT_TOO_MUCH; } return ODP_FIT_PERFECT; } @@ -2596,6 +2597,74 @@ odp_flow_key_from_mask(struct ofpbuf *buf, const struct flow *mask, odp_flow_key_from_flow__(buf, mask, flow, u32_to_odp(odp_in_port_mask)); } +/* Generate ODP flow key from the given packet metadata */ +void +odp_key_from_pkt_metadata(struct ofpbuf *buf, const struct pkt_metadata *md) +{ + nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, md->skb_priority); + + if (md->tunnel.ip_dst) { + tun_key_to_attr(buf, &md->tunnel); + } + + nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, md->pkt_mark); + + /* Add an ingress port attribute if 'odp_in_port' is not the magical + * value "ODPP_NONE". */ + if (md->in_port != ODPP_NONE) { + nl_msg_put_odp_port(buf, OVS_KEY_ATTR_IN_PORT, md->in_port); + } +} + +/* Generate packet metadata from the given ODP flow key. */ +void +odp_key_to_pkt_metadata(const struct nlattr *key, size_t key_len, + struct pkt_metadata *md) +{ + const struct nlattr *nla; + size_t left; + uint32_t wanted_attrs = 1u << OVS_KEY_ATTR_PRIORITY | + 1u << OVS_KEY_ATTR_SKB_MARK | 1u << OVS_KEY_ATTR_TUNNEL | + 1u << OVS_KEY_ATTR_IN_PORT; + + memset(md, 0, sizeof *md); + md->in_port = ODPP_NONE; + + NL_ATTR_FOR_EACH (nla, left, key, key_len) { + uint16_t type = nl_attr_type(nla); + size_t len = nl_attr_get_size(nla); + int expected_len = odp_flow_key_attr_len(type); + + if (len != expected_len && expected_len >= 0) { + continue; + } + + if (type == OVS_KEY_ATTR_PRIORITY) { + md->skb_priority = nl_attr_get_u32(nla); + wanted_attrs &= ~(1u << OVS_KEY_ATTR_PRIORITY); + } else if (type == OVS_KEY_ATTR_SKB_MARK) { + md->pkt_mark = nl_attr_get_u32(nla); + wanted_attrs &= ~(1u << OVS_KEY_ATTR_SKB_MARK); + } else if (type == OVS_KEY_ATTR_TUNNEL) { + enum odp_key_fitness res; + + res = odp_tun_key_from_attr(nla, &md->tunnel); + if (res == ODP_FIT_ERROR) { + memset(&md->tunnel, 0, sizeof md->tunnel); + } else if (res == ODP_FIT_PERFECT) { + wanted_attrs &= ~(1u << OVS_KEY_ATTR_TUNNEL); + } + } else if (type == OVS_KEY_ATTR_IN_PORT) { + md->in_port = nl_attr_get_odp_port(nla); + wanted_attrs &= ~(1u << OVS_KEY_ATTR_IN_PORT); + } + + if (!wanted_attrs) { + return; /* Have everything. */ + } + } +} + uint32_t odp_flow_key_hash(const struct nlattr *key, size_t key_len) { diff --git a/lib/odp-util.h b/lib/odp-util.h index 821b2c412..2b6261486 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -34,6 +34,7 @@ struct flow_wildcards; struct nlattr; struct ofpbuf; struct simap; +struct pkt_metadata; #define SLOW_PATH_REASONS \ /* These reasons are mutually exclusive. */ \ @@ -149,6 +150,12 @@ void odp_flow_key_from_mask(struct ofpbuf *, const struct flow *mask, uint32_t odp_flow_key_hash(const struct nlattr *, size_t); +/* Estimated space needed for metadata. */ +enum { ODP_KEY_METADATA_SIZE = 9 * 8 }; +void odp_key_from_pkt_metadata(struct ofpbuf *, const struct pkt_metadata *); +void odp_key_to_pkt_metadata(const struct nlattr *key, size_t key_len, + struct pkt_metadata *md); + /* How well a kernel-provided flow key (a sequence of OVS_KEY_ATTR_* * attributes) matches OVS userspace expectations. * diff --git a/lib/packets.h b/lib/packets.h index d291c1445..63873b036 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -31,6 +31,17 @@ struct ofpbuf; struct ds; +/* Datapath packet metadata */ +struct pkt_metadata { + struct flow_tnl tunnel; /* Encapsulating tunnel parameters. */ + uint32_t skb_priority; /* Packet priority for QoS. */ + uint32_t pkt_mark; /* Packet mark. */ + odp_port_t in_port; /* Input port. */ +}; + +#define PKT_METADATA_INITIALIZER(PORT) \ + (struct pkt_metadata){ { 0, 0, 0, 0, 0, 0}, 0, 0, (PORT) } + bool dpid_from_string(const char *s, uint64_t *dpidp); #define ETH_ADDR_LEN 6 diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 836995968..b57afdc54 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1134,9 +1134,9 @@ handle_upcalls(struct handler *handler, struct list *upcalls) op = &ops[n_ops++]; op->type = DPIF_OP_EXECUTE; - op->u.execute.key = miss->key; - op->u.execute.key_len = miss->key_len; op->u.execute.packet = packet; + odp_key_to_pkt_metadata(miss->key, miss->key_len, + &op->u.execute.md); op->u.execute.actions = miss->xout.odp_actions.data; op->u.execute.actions_len = miss->xout.odp_actions.size; op->u.execute.needs_help = (miss->xout.slow & SLOW_ACTION) != 0; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 11e0855dc..13a5d07cc 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2074,7 +2074,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len, { struct ofproto_packet_in *pin; struct ofpbuf *packet; - struct flow key; + struct pkt_metadata md = PKT_METADATA_INITIALIZER(0); ctx->xout->slow |= SLOW_CONTROLLER; if (!ctx->xin->packet) { @@ -2083,16 +2083,12 @@ execute_controller_action(struct xlate_ctx *ctx, int len, packet = ofpbuf_clone(ctx->xin->packet); - key.skb_priority = 0; - key.pkt_mark = 0; - memset(&key.tunnel, 0, sizeof key.tunnel); - ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow, &ctx->xout->odp_actions, &ctx->xout->wc, &ctx->mpls_depth_delta); - odp_execute_actions(NULL, packet, &key, ctx->xout->odp_actions.data, + odp_execute_actions(NULL, packet, &md, ctx->xout->odp_actions.data, ctx->xout->odp_actions.size, NULL); pin = xmalloc(sizeof *pin); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index e48cc00b3..91ffe2344 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -903,7 +903,7 @@ check_variable_length_userdata(struct dpif_backer *backer) { struct eth_header *eth; struct ofpbuf actions; - struct ofpbuf key; + struct dpif_execute execute; struct ofpbuf packet; size_t start; int error; @@ -922,26 +922,22 @@ check_variable_length_userdata(struct dpif_backer *backer) nl_msg_put_unspec_zero(&actions, OVS_USERSPACE_ATTR_USERDATA, 4); nl_msg_end_nested(&actions, start); - /* Compose an ODP flow key. The key is arbitrary but it must match the - * packet that we compose later. */ - ofpbuf_init(&key, 64); - nl_msg_put_u32(&key, OVS_KEY_ATTR_IN_PORT, 0); - nl_msg_put_unspec_zero(&key, OVS_KEY_ATTR_ETHERNET, - sizeof(struct ovs_key_ethernet)); - nl_msg_put_be16(&key, OVS_KEY_ATTR_ETHERTYPE, htons(0x1234)); - - /* Compose a packet that matches the key. */ + /* Compose a dummy ethernet packet. */ ofpbuf_init(&packet, ETH_HEADER_LEN); eth = ofpbuf_put_zeros(&packet, ETH_HEADER_LEN); eth->eth_type = htons(0x1234); - /* Execute the actions. On older datapaths this fails with -ERANGE, on + /* Execute the actions. On older datapaths this fails with ERANGE, on * newer datapaths it succeeds. */ - error = dpif_execute(backer->dpif, key.data, key.size, - actions.data, actions.size, &packet, false); + execute.actions = actions.data; + execute.actions_len = actions.size; + execute.packet = &packet; + execute.md = PKT_METADATA_INITIALIZER(0); + execute.needs_help = false; + + error = dpif_execute(backer->dpif, &execute); ofpbuf_uninit(&packet); - ofpbuf_uninit(&key); ofpbuf_uninit(&actions); switch (error) { @@ -2890,12 +2886,11 @@ ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto, const struct ofpact *ofpacts, size_t ofpacts_len, struct ofpbuf *packet) { - struct odputil_keybuf keybuf; struct dpif_flow_stats stats; struct xlate_out xout; struct xlate_in xin; ofp_port_t in_port; - struct ofpbuf key; + struct dpif_execute execute; int error; ovs_assert((rule != NULL) != (ofpacts != NULL)); @@ -2911,16 +2906,21 @@ ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto, xin.resubmit_stats = &stats; xlate_actions(&xin, &xout); - ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); in_port = flow->in_port.ofp_port; if (in_port == OFPP_NONE) { in_port = OFPP_LOCAL; } - odp_flow_key_from_flow(&key, flow, ofp_port_to_odp_port(ofproto, in_port)); + execute.actions = xout.odp_actions.data; + execute.actions_len = xout.odp_actions.size; + execute.packet = packet; + execute.md.tunnel = flow->tunnel; + execute.md.skb_priority = flow->skb_priority; + execute.md.pkt_mark = flow->pkt_mark; + execute.md.in_port = ofp_port_to_odp_port(ofproto, in_port); + execute.needs_help = (xout.slow & SLOW_ACTION) != 0; + + error = dpif_execute(ofproto->backer->dpif, &execute); - error = dpif_execute(ofproto->backer->dpif, key.data, key.size, - xout.odp_actions.data, xout.odp_actions.size, packet, - (xout.slow & SLOW_ACTION) != 0); xlate_out_uninit(&xout); return error; -- 2.43.0