From 09f9da0bcaf0f6fd4221b28ecedc3d4db6235fe5 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 30 Dec 2013 15:58:58 -0800 Subject: [PATCH] odp-execute: Consolidate callbacks. Use one callback instead of many, helps in adding new functionality later on. Signed-off-by: Jarno Rajahalme Acked-by: Ben Pfaff --- lib/dpif-netdev.c | 65 +++++++++++++++++++++++------------- lib/dpif.c | 46 ++++++++++++------------- lib/odp-execute.c | 58 +++++++++++++++----------------- lib/odp-execute.h | 16 +++++---- ofproto/ofproto-dpif-xlate.c | 2 +- 5 files changed, 100 insertions(+), 87 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 19fbdcf40..076369745 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1345,6 +1345,16 @@ dpif_netdev_wait(struct dpif *dpif) ovs_mutex_unlock(&dp_netdev_mutex); } +static void +dp_netdev_output_port(struct dp_netdev *dp, struct ofpbuf *packet, + odp_port_t out_port) +{ + struct dp_netdev_port *p = dp->ports[odp_to_u32(out_port)]; + if (p) { + netdev_send(p->netdev, packet); + } +} + static int dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *packet, int queue_no, const struct flow *flow, @@ -1397,35 +1407,42 @@ struct dp_netdev_execute_aux { }; static void -dp_netdev_action_output(void *aux_, struct ofpbuf *packet, - const struct flow *flow OVS_UNUSED, - odp_port_t out_port) +dp_execute_cb(void *aux_, struct ofpbuf *packet, struct flow *flow OVS_UNUSED, + const struct nlattr *a, bool may_steal) { struct dp_netdev_execute_aux *aux = aux_; - struct dp_netdev_port *p = aux->dp->ports[odp_to_u32(out_port)]; - if (p) { - netdev_send(p->netdev, packet); - } -} + int type = nl_attr_type(a); -static void -dp_netdev_action_userspace(void *aux_, struct ofpbuf *packet, - const struct flow *flow OVS_UNUSED, - const struct nlattr *a, bool may_steal) -{ - struct dp_netdev_execute_aux *aux = aux_; - const struct nlattr *userdata; + switch ((enum ovs_action_attr)type) { + case OVS_ACTION_ATTR_OUTPUT: + dp_netdev_output_port(aux->dp, packet, u32_to_odp(nl_attr_get_u32(a))); + break; + + case OVS_ACTION_ATTR_USERSPACE: { + const struct nlattr *userdata; - userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA); + userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA); - /* Make a copy if we are not allowed to steal the packet's data. */ - if (!may_steal) { - packet = ofpbuf_clone_with_headroom(packet, DP_NETDEV_HEADROOM); + /* Make a copy if we are not allowed to steal the packet's data. */ + if (!may_steal) { + packet = ofpbuf_clone_with_headroom(packet, DP_NETDEV_HEADROOM); + } + dp_netdev_output_userspace(aux->dp, packet, DPIF_UC_ACTION, aux->key, + userdata); + if (!may_steal) { + ofpbuf_uninit(packet); + } + break; } - dp_netdev_output_userspace(aux->dp, packet, DPIF_UC_ACTION, aux->key, - userdata); - if (!may_steal) { - ofpbuf_uninit(packet); + case OVS_ACTION_ATTR_PUSH_VLAN: + case OVS_ACTION_ATTR_POP_VLAN: + case OVS_ACTION_ATTR_PUSH_MPLS: + case OVS_ACTION_ATTR_POP_MPLS: + case OVS_ACTION_ATTR_SET: + case OVS_ACTION_ATTR_SAMPLE: + case OVS_ACTION_ATTR_UNSPEC: + case __OVS_ACTION_ATTR_MAX: + OVS_NOT_REACHED(); } } @@ -1438,7 +1455,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp, const struct flow *key, struct flow md = *key; /* Packet metadata, may be modified by actions. */ odp_execute_actions(&aux, packet, &md, actions, actions_len, - dp_netdev_action_output, dp_netdev_action_userspace); + dp_execute_cb); } const struct dpif_class dpif_netdev_class = { diff --git a/lib/dpif.c b/lib/dpif.c index 6b519b8bd..e25655875 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1091,28 +1091,29 @@ dpif_execute_helper_execute__(void *aux_, struct ofpbuf *packet, } } +/* This is called for actions that need the context of the datapath to be + * meaningful. */ static void -dpif_execute_helper_output_cb(void *aux, struct ofpbuf *packet, - const struct flow *flow, odp_port_t out_port) -{ - uint64_t actions_stub[DIV_ROUND_UP(NL_A_U32_SIZE, 8)]; - struct ofpbuf actions; - - ofpbuf_use_stack(&actions, actions_stub, sizeof actions_stub); - nl_msg_put_u32(&actions, OVS_ACTION_ATTR_OUTPUT, odp_to_u32(out_port)); - - dpif_execute_helper_execute__(aux, packet, flow, - actions.data, actions.size); -} - -static void -dpif_execute_helper_userspace_cb(void *aux, struct ofpbuf *packet, - const struct flow *flow, - const struct nlattr *action, - bool may_steal OVS_UNUSED) -{ - dpif_execute_helper_execute__(aux, packet, flow, - action, NLA_ALIGN(action->nla_len)); +dpif_execute_helper_cb(void *aux, struct ofpbuf *packet, struct flow *flow, + const struct nlattr *action, bool may_steal OVS_UNUSED) +{ + 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)); + break; + case OVS_ACTION_ATTR_PUSH_VLAN: + case OVS_ACTION_ATTR_POP_VLAN: + case OVS_ACTION_ATTR_PUSH_MPLS: + case OVS_ACTION_ATTR_POP_MPLS: + case OVS_ACTION_ATTR_SET: + case OVS_ACTION_ATTR_SAMPLE: + case OVS_ACTION_ATTR_UNSPEC: + case __OVS_ACTION_ATTR_MAX: + OVS_NOT_REACHED(); + } } /* Executes 'execute' by performing most of the actions in userspace and @@ -1139,8 +1140,7 @@ dpif_execute_with_help(struct dpif *dpif, const struct dpif_execute *execute) odp_execute_actions(&aux, execute->packet, &flow, execute->actions, execute->actions_len, - dpif_execute_helper_output_cb, - dpif_execute_helper_userspace_cb); + dpif_execute_helper_cb); return aux.error; } diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 24871184e..7a96ca0ad 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -29,7 +29,8 @@ #include "util.h" static void -odp_eth_set_addrs(struct ofpbuf *packet, const struct ovs_key_ethernet *eth_key) +odp_eth_set_addrs(struct ofpbuf *packet, + const struct ovs_key_ethernet *eth_key) { struct eth_header *eh = packet->l2; @@ -60,7 +61,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 *flow) + struct flow *md) { enum ovs_key_attr type = nl_attr_type(a); const struct ovs_key_ipv4 *ipv4_key; @@ -71,15 +72,15 @@ odp_execute_set_action(struct ofpbuf *packet, const struct nlattr *a, switch (type) { case OVS_KEY_ATTR_PRIORITY: - flow->skb_priority = nl_attr_get_u32(a); + md->skb_priority = nl_attr_get_u32(a); break; case OVS_KEY_ATTR_TUNNEL: - odp_set_tunnel_action(a, &flow->tunnel); + odp_set_tunnel_action(a, &md->tunnel); break; case OVS_KEY_ATTR_SKB_MARK: - flow->pkt_mark = nl_attr_get_u32(a); + md->pkt_mark = nl_attr_get_u32(a); break; case OVS_KEY_ATTR_ETHERNET: @@ -141,13 +142,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, const struct nlattr *actions, size_t actions_len, - odp_output_cb output, odp_userspace_cb userspace, - bool more_actions); + odp_execute_cb dp_execute_action, bool more_actions); static void -odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *key, - const struct nlattr *action, odp_output_cb output, - odp_userspace_cb userspace, bool more_actions) +odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *md, + const struct nlattr *action, + odp_execute_cb dp_execute_action, bool more_actions) { const struct nlattr *subactions = NULL; const struct nlattr *a; @@ -174,16 +174,15 @@ odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *key, } } - odp_execute_actions__(dp, packet, key, nl_attr_get(subactions), - nl_attr_get_size(subactions), output, userspace, + odp_execute_actions__(dp, packet, md, nl_attr_get(subactions), + nl_attr_get_size(subactions), dp_execute_action, more_actions); } static void -odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key, +odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *md, const struct nlattr *actions, size_t actions_len, - odp_output_cb output, odp_userspace_cb userspace, - bool more_actions) + odp_execute_cb dp_execute_action, bool more_actions) { const struct nlattr *a; unsigned int left; @@ -192,21 +191,16 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key, int type = nl_attr_type(a); switch ((enum ovs_action_attr) type) { + /* These only make sense in the context of a datapath. */ case OVS_ACTION_ATTR_OUTPUT: - if (output) { - output(dp, packet, key, u32_to_odp(nl_attr_get_u32(a))); - } - break; - - case OVS_ACTION_ATTR_USERSPACE: { - if (userspace) { - /* Allow 'userspace' to steal the packet data if we do not - * need it any more. */ + case OVS_ACTION_ATTR_USERSPACE: + if (dp_execute_action) { + /* Allow 'dp_execute_action' to steal the packet data if we do + * not need it any more. */ bool steal = !more_actions && left <= NLA_ALIGN(a->nla_len); - userspace(dp, packet, key, a, steal); + dp_execute_action(dp, packet, md, a, steal); } break; - } case OVS_ACTION_ATTR_PUSH_VLAN: { const struct ovs_action_push_vlan *vlan = nl_attr_get(a); @@ -229,11 +223,11 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key, break; case OVS_ACTION_ATTR_SET: - odp_execute_set_action(packet, nl_attr_get(a), key); + odp_execute_set_action(packet, nl_attr_get(a), md); break; case OVS_ACTION_ATTR_SAMPLE: - odp_execute_sample(dp, packet, key, a, output, userspace, + odp_execute_sample(dp, packet, md, a, dp_execute_action, more_actions || left > NLA_ALIGN(a->nla_len)); break; @@ -245,10 +239,10 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key, } void -odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *key, +odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *md, const struct nlattr *actions, size_t actions_len, - odp_output_cb output, odp_userspace_cb userspace) + odp_execute_cb dp_execute_action) { - odp_execute_actions__(dp, packet, key, actions, actions_len, output, - userspace, false); + odp_execute_actions__(dp, packet, md, actions, actions_len, + dp_execute_action, false); } diff --git a/lib/odp-execute.h b/lib/odp-execute.h index 90a6788ce..993ff471f 100644 --- a/lib/odp-execute.h +++ b/lib/odp-execute.h @@ -27,14 +27,16 @@ struct flow; struct nlattr; struct ofpbuf; -typedef void (*odp_output_cb)(void *dp, struct ofpbuf *packet, - const struct flow *key, odp_port_t out_port); -typedef void (*odp_userspace_cb)(void *dp, struct ofpbuf *packet, - const struct flow *key, - const struct nlattr *action, bool may_steal); +typedef void (*odp_execute_cb)(void *dp, struct ofpbuf *packet, + struct flow *metadata, + const struct nlattr *action, bool may_steal); +/* Actions that need to be executed in the context of a datapath are handed + * to 'dp_execute_action', if non-NULL. Currently this is called only for + * 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 *key, +odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *metadata, const struct nlattr *actions, size_t actions_len, - odp_output_cb output, odp_userspace_cb userspace); + odp_execute_cb dp_execute_action); #endif diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 558598c51..11e0855dc 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2093,7 +2093,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len, &ctx->mpls_depth_delta); odp_execute_actions(NULL, packet, &key, ctx->xout->odp_actions.data, - ctx->xout->odp_actions.size, NULL, NULL); + ctx->xout->odp_actions.size, NULL); pin = xmalloc(sizeof *pin); pin->up.packet_len = packet->size; -- 2.43.0