From cdee00fd635d1e0f1eeb5d9c009daeb59abd4777 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 10 Dec 2010 10:40:58 -0800 Subject: [PATCH] datapath: Replace "struct odp_action" by Netlink attributes. In the medium term, we plan to migrate the datapath to use Netlink as its communication channel. In the short term, we need to be able to have actions with 64-bit arguments but "struct odp_action" only has room for 48 bits. So this patch shifts to variable-length arguments using Netlink attributes, which starts in on the Netlink transition and makes 64-bit arguments possible at the same time. Signed-off-by: Ben Pfaff Acked-by: Jesse Gross --- datapath/actions.c | 101 +++++----- datapath/actions.h | 3 +- datapath/datapath.c | 147 ++++++++------ datapath/flow.c | 12 +- datapath/flow.h | 6 +- datapath/loop_counter.c | 2 +- datapath/odp-compat.h | 4 +- include/openvswitch/datapath-protocol.h | 133 +++---------- lib/dpif-linux.c | 6 +- lib/dpif-netdev.c | 129 ++++++------ lib/dpif-provider.h | 9 +- lib/dpif.c | 66 +++---- lib/dpif.h | 5 +- lib/odp-util.c | 141 +++++++++---- lib/odp-util.h | 31 +-- lib/ofp-util.c | 2 +- ofproto/in-band.c | 12 +- ofproto/in-band.h | 4 +- ofproto/ofproto-sflow.c | 67 +++---- ofproto/ofproto.c | 253 ++++++++++++------------ ofproto/ofproto.h | 7 +- utilities/ovs-dpctl.c | 6 +- vswitchd/bridge.c | 35 ++-- 23 files changed, 589 insertions(+), 592 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index 9f738e4c..7290a898 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -73,9 +73,9 @@ static struct sk_buff *vlan_pull_tag(struct sk_buff *skb) static struct sk_buff *modify_vlan_tci(struct datapath *dp, struct sk_buff *skb, const struct odp_flow_key *key, - const union odp_action *a, int n_actions) + const struct nlattr *a, u32 actions_len) { - __be16 tci = a->dl_tci.tci; + __be16 tci = nla_get_be16(a); skb = make_writable(skb, VLAN_HLEN); if (!skb) @@ -132,6 +132,8 @@ static struct sk_buff *modify_vlan_tci(struct datapath *dp, struct sk_buff *skb, * support hardware-accelerated VLAN tagging without VLAN * groups configured). */ if (skb_is_gso(skb)) { + const struct nlattr *actions_left; + u32 actions_len_left; struct sk_buff *segs; segs = skb_gso_segment(skb, 0); @@ -139,6 +141,9 @@ static struct sk_buff *modify_vlan_tci(struct datapath *dp, struct sk_buff *skb, if (unlikely(IS_ERR(segs))) return ERR_CAST(segs); + actions_len_left = actions_len; + actions_left = nla_next(a, &actions_len_left); + do { struct sk_buff *nskb = segs->next; int err; @@ -151,9 +156,9 @@ static struct sk_buff *modify_vlan_tci(struct datapath *dp, struct sk_buff *skb, segs = __vlan_put_tag(segs, ntohs(tci)); err = -ENOMEM; if (segs) { - err = execute_actions(dp, segs, - key, a + 1, - n_actions - 1); + err = execute_actions( + dp, segs, key, actions_left, + actions_len_left); } if (unlikely(err)) { @@ -199,20 +204,6 @@ static struct sk_buff *strip_vlan(struct sk_buff *skb) return skb; } -static struct sk_buff *set_dl_addr(struct sk_buff *skb, - const struct odp_action_dl_addr *a) -{ - skb = make_writable(skb, 0); - if (skb) { - struct ethhdr *eh = eth_hdr(skb); - if (a->type == ODPAT_SET_DL_SRC) - memcpy(eh->h_source, a->dl_addr, ETH_ALEN); - else - memcpy(eh->h_dest, a->dl_addr, ETH_ALEN); - } - return skb; -} - static bool is_ip(struct sk_buff *skb, const struct odp_flow_key *key) { return (key->dl_type == htons(ETH_P_IP) && @@ -234,8 +225,9 @@ static __sum16 *get_l4_checksum(struct sk_buff *skb, const struct odp_flow_key * static struct sk_buff *set_nw_addr(struct sk_buff *skb, const struct odp_flow_key *key, - const struct odp_action_nw_addr *a) + const struct nlattr *a) { + __be32 new_nwaddr = nla_get_be32(a); struct iphdr *nh; __sum16 *check; __be32 *nwaddr; @@ -248,21 +240,21 @@ static struct sk_buff *set_nw_addr(struct sk_buff *skb, return NULL; nh = ip_hdr(skb); - nwaddr = a->type == ODPAT_SET_NW_SRC ? &nh->saddr : &nh->daddr; + nwaddr = nla_type(a) == ODPAT_SET_NW_SRC ? &nh->saddr : &nh->daddr; check = get_l4_checksum(skb, key); if (likely(check)) - inet_proto_csum_replace4(check, skb, *nwaddr, a->nw_addr, 1); - csum_replace4(&nh->check, *nwaddr, a->nw_addr); + inet_proto_csum_replace4(check, skb, *nwaddr, new_nwaddr, 1); + csum_replace4(&nh->check, *nwaddr, new_nwaddr); - *nwaddr = a->nw_addr; + *nwaddr = new_nwaddr; return skb; } static struct sk_buff *set_nw_tos(struct sk_buff *skb, - const struct odp_flow_key *key, - const struct odp_action_nw_tos *a) + const struct odp_flow_key *key, + u8 nw_tos) { if (unlikely(!is_ip(skb, key))) return skb; @@ -275,7 +267,7 @@ static struct sk_buff *set_nw_tos(struct sk_buff *skb, u8 new; /* Set the DSCP bits and preserve the ECN bits. */ - new = a->nw_tos | (nh->tos & INET_ECN_MASK); + new = nw_tos | (nh->tos & INET_ECN_MASK); csum_replace4(&nh->check, (__force __be32)old, (__force __be32)new); *f = new; @@ -285,7 +277,7 @@ static struct sk_buff *set_nw_tos(struct sk_buff *skb, static struct sk_buff *set_tp_port(struct sk_buff *skb, const struct odp_flow_key *key, - const struct odp_action_tp_port *a) + const struct nlattr *a) { struct udphdr *th; __sum16 *check; @@ -311,9 +303,9 @@ static struct sk_buff *set_tp_port(struct sk_buff *skb, * supports those protocols. */ th = udp_hdr(skb); - port = a->type == ODPAT_SET_TP_SRC ? &th->source : &th->dest; - inet_proto_csum_replace2(check, skb, *port, a->tp_port, 0); - *port = a->tp_port; + port = nla_type(a) == ODPAT_SET_TP_SRC ? &th->source : &th->dest; + inet_proto_csum_replace2(check, skb, *port, nla_get_be16(a), 0); + *port = nla_get_be16(a); return skb; } @@ -376,21 +368,20 @@ static int output_control(struct datapath *dp, struct sk_buff *skb, u32 arg) /* Send a copy of this packet up to the sFlow agent, along with extra * information about what happened to it. */ static void sflow_sample(struct datapath *dp, struct sk_buff *skb, - const union odp_action *a, int n_actions, + const struct nlattr *a, u32 actions_len, struct vport *vport) { struct odp_sflow_sample_header *hdr; - unsigned int actlen = n_actions * sizeof(union odp_action); unsigned int hdrlen = sizeof(struct odp_sflow_sample_header); struct sk_buff *nskb; - nskb = skb_copy_expand(skb, actlen + hdrlen, 0, GFP_ATOMIC); + nskb = skb_copy_expand(skb, actions_len + hdrlen, 0, GFP_ATOMIC); if (!nskb) return; - memcpy(__skb_push(nskb, actlen), a, actlen); + memcpy(__skb_push(nskb, actions_len), a, actions_len); hdr = (struct odp_sflow_sample_header*)__skb_push(nskb, hdrlen); - hdr->n_actions = n_actions; + hdr->actions_len = actions_len; hdr->sample_pool = atomic_read(&vport->sflow_pool); dp_output_control(dp, nskb, _ODPL_SFLOW_NR, 0); } @@ -398,7 +389,7 @@ static void sflow_sample(struct datapath *dp, struct sk_buff *skb, /* Execute a list of actions against 'skb'. */ int execute_actions(struct datapath *dp, struct sk_buff *skb, const struct odp_flow_key *key, - const union odp_action *a, int n_actions) + const struct nlattr *actions, u32 actions_len) { /* Every output action needs a separate clone of 'skb', but the common * case is just a single output action, so that doing a clone and @@ -406,7 +397,8 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb, * is slightly obscure just to avoid that. */ int prev_port = -1; u32 priority = skb->priority; - int err; + const struct nlattr *a; + int rem, err; if (dp->sflow_probability) { struct vport *p = OVS_CB(skb)->vport; @@ -414,25 +406,25 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb, atomic_inc(&p->sflow_pool); if (dp->sflow_probability == UINT_MAX || net_random() < dp->sflow_probability) - sflow_sample(dp, skb, a, n_actions, p); + sflow_sample(dp, skb, actions, actions_len, p); } } OVS_CB(skb)->tun_id = 0; - for (; n_actions > 0; a++, n_actions--) { + for (a = actions, rem = actions_len; rem > 0; a = nla_next(a, &rem)) { if (prev_port != -1) { do_output(dp, skb_clone(skb, GFP_ATOMIC), prev_port); prev_port = -1; } - switch (a->type) { + switch (nla_type(a)) { case ODPAT_OUTPUT: - prev_port = a->output.port; + prev_port = nla_get_u32(a); break; case ODPAT_CONTROLLER: - err = output_control(dp, skb, a->controller.arg); + err = output_control(dp, skb, nla_get_u32(a)); if (err) { kfree_skb(skb); return err; @@ -440,11 +432,11 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb, break; case ODPAT_SET_TUNNEL: - OVS_CB(skb)->tun_id = a->tunnel.tun_id; + OVS_CB(skb)->tun_id = nla_get_be32(a); break; case ODPAT_SET_DL_TCI: - skb = modify_vlan_tci(dp, skb, key, a, n_actions); + skb = modify_vlan_tci(dp, skb, key, a, rem); if (IS_ERR(skb)) return PTR_ERR(skb); break; @@ -454,26 +446,35 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb, break; case ODPAT_SET_DL_SRC: + skb = make_writable(skb, 0); + if (!skb) + return -ENOMEM; + memcpy(eth_hdr(skb)->h_source, nla_data(a), ETH_ALEN); + break; + case ODPAT_SET_DL_DST: - skb = set_dl_addr(skb, &a->dl_addr); + skb = make_writable(skb, 0); + if (!skb) + return -ENOMEM; + memcpy(eth_hdr(skb)->h_dest, nla_data(a), ETH_ALEN); break; case ODPAT_SET_NW_SRC: case ODPAT_SET_NW_DST: - skb = set_nw_addr(skb, key, &a->nw_addr); + skb = set_nw_addr(skb, key, a); break; case ODPAT_SET_NW_TOS: - skb = set_nw_tos(skb, key, &a->nw_tos); + skb = set_nw_tos(skb, key, nla_get_u8(a)); break; case ODPAT_SET_TP_SRC: case ODPAT_SET_TP_DST: - skb = set_tp_port(skb, key, &a->tp_port); + skb = set_tp_port(skb, key, a); break; case ODPAT_SET_PRIORITY: - skb->priority = a->priority.priority; + skb->priority = nla_get_u32(a); break; case ODPAT_POP_PRIORITY: diff --git a/datapath/actions.h b/datapath/actions.h index a2585516..3067bf72 100644 --- a/datapath/actions.h +++ b/datapath/actions.h @@ -15,10 +15,9 @@ struct datapath; struct sk_buff; struct odp_flow_key; -union odp_action; int execute_actions(struct datapath *dp, struct sk_buff *skb, const struct odp_flow_key *key, - const union odp_action *, int n_actions); + const struct nlattr *, u32 actions_len); #endif /* actions.h */ diff --git a/datapath/datapath.c b/datapath/datapath.c index aaf2a531..7a70163a 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -516,7 +516,7 @@ void dp_process_received_packet(struct vport *p, struct sk_buff *skb) /* Execute actions. */ execute_actions(dp, skb, &OVS_CB(skb)->flow->key, acts->actions, - acts->n_actions); + acts->actions_len); stats_counter_off = offsetof(struct dp_stats_percpu, n_hit); /* Check whether sub-actions looped too much. */ @@ -658,50 +658,76 @@ static int flush_flows(struct datapath *dp) return 0; } -static int validate_actions(const struct sw_flow_actions *actions) +static int validate_actions(const struct nlattr *actions, u32 actions_len) { - unsigned int i; - - for (i = 0; i < actions->n_actions; i++) { - const union odp_action *a = &actions->actions[i]; - - switch (a->type) { - case ODPAT_CONTROLLER: - case ODPAT_STRIP_VLAN: - case ODPAT_SET_DL_SRC: - case ODPAT_SET_DL_DST: - case ODPAT_SET_NW_SRC: - case ODPAT_SET_NW_DST: - case ODPAT_SET_TP_SRC: - case ODPAT_SET_TP_DST: - case ODPAT_SET_TUNNEL: - case ODPAT_SET_PRIORITY: - case ODPAT_POP_PRIORITY: - case ODPAT_DROP_SPOOFED_ARP: - /* No validation needed. */ - break; + const struct nlattr *a; + int rem; + + nla_for_each_attr(a, actions, actions_len, rem) { + static const u32 action_lens[ODPAT_MAX + 1] = { + [ODPAT_OUTPUT] = 4, + [ODPAT_CONTROLLER] = 4, + [ODPAT_SET_DL_TCI] = 2, + [ODPAT_STRIP_VLAN] = 0, + [ODPAT_SET_DL_SRC] = ETH_ALEN, + [ODPAT_SET_DL_DST] = ETH_ALEN, + [ODPAT_SET_NW_SRC] = 4, + [ODPAT_SET_NW_DST] = 4, + [ODPAT_SET_NW_TOS] = 1, + [ODPAT_SET_TP_SRC] = 2, + [ODPAT_SET_TP_DST] = 2, + [ODPAT_SET_TUNNEL] = 4, + [ODPAT_SET_PRIORITY] = 4, + [ODPAT_POP_PRIORITY] = 0, + [ODPAT_DROP_SPOOFED_ARP] = 0, + }; + int type = nla_type(a); + + if (type > ODPAT_MAX || nla_len(a) != action_lens[type]) + return -EINVAL; + + switch (type) { + case ODPAT_UNSPEC: + return -EINVAL; - case ODPAT_OUTPUT: - if (a->output.port >= DP_MAX_PORTS) + case ODPAT_CONTROLLER: + case ODPAT_STRIP_VLAN: + case ODPAT_SET_DL_SRC: + case ODPAT_SET_DL_DST: + case ODPAT_SET_NW_SRC: + case ODPAT_SET_NW_DST: + case ODPAT_SET_TP_SRC: + case ODPAT_SET_TP_DST: + case ODPAT_SET_TUNNEL: + case ODPAT_SET_PRIORITY: + case ODPAT_POP_PRIORITY: + case ODPAT_DROP_SPOOFED_ARP: + /* No validation needed. */ + break; + + case ODPAT_OUTPUT: + if (nla_get_u32(a) >= DP_MAX_PORTS) + return -EINVAL; + + case ODPAT_SET_DL_TCI: + if (nla_get_be16(a) & htons(VLAN_CFI_MASK)) return -EINVAL; - break; + break; - case ODPAT_SET_DL_TCI: - if (a->dl_tci.tci & htons(VLAN_CFI_MASK)) - return -EINVAL; - break; + case ODPAT_SET_NW_TOS: + if (nla_get_u8(a) & INET_ECN_MASK) + return -EINVAL; + break; - case ODPAT_SET_NW_TOS: - if (a->nw_tos.nw_tos & INET_ECN_MASK) - return -EINVAL; - break; + default: + return -EOPNOTSUPP; + } + } - default: - return -EOPNOTSUPP; - } - } + if (rem > 0) + return -EINVAL; - return 0; + return 0; } static struct sw_flow_actions *get_actions(const struct odp_flow *flow) @@ -709,16 +735,15 @@ static struct sw_flow_actions *get_actions(const struct odp_flow *flow) struct sw_flow_actions *actions; int error; - actions = flow_actions_alloc(flow->n_actions); + actions = flow_actions_alloc(flow->actions_len); error = PTR_ERR(actions); if (IS_ERR(actions)) goto error; error = -EFAULT; - if (copy_from_user(actions->actions, flow->actions, - flow->n_actions * sizeof(union odp_action))) + if (copy_from_user(actions->actions, flow->actions, flow->actions_len)) goto error_free_actions; - error = validate_actions(actions); + error = validate_actions(actions->actions, actions->actions_len); if (error) goto error_free_actions; @@ -842,9 +867,9 @@ static int do_put_flow(struct datapath *dp, struct odp_flow_put *uf, if (IS_ERR(new_acts)) goto error; old_acts = rcu_dereference(flow->sf_acts); - if (old_acts->n_actions != new_acts->n_actions || + if (old_acts->actions_len != new_acts->actions_len || memcmp(old_acts->actions, new_acts->actions, - sizeof(union odp_action) * old_acts->n_actions)) { + old_acts->actions_len)) { rcu_assign_pointer(flow->sf_acts, new_acts); flow_deferred_free_acts(old_acts); } else { @@ -892,12 +917,12 @@ static int put_flow(struct datapath *dp, struct odp_flow_put __user *ufp) static int do_answer_query(struct sw_flow *flow, u32 query_flags, struct odp_flow_stats __user *ustats, - union odp_action __user *actions, - u32 __user *n_actionsp) + struct nlattr __user *actions, + u32 __user *actions_lenp) { struct sw_flow_actions *sf_acts; struct odp_flow_stats stats; - u32 n_actions; + u32 actions_len; spin_lock_bh(&flow->lock); get_stats(flow, &stats); @@ -907,17 +932,16 @@ static int do_answer_query(struct sw_flow *flow, u32 query_flags, spin_unlock_bh(&flow->lock); if (copy_to_user(ustats, &stats, sizeof(struct odp_flow_stats)) || - get_user(n_actions, n_actionsp)) + get_user(actions_len, actions_lenp)) return -EFAULT; - if (!n_actions) + if (!actions_len) return 0; sf_acts = rcu_dereference(flow->sf_acts); - if (put_user(sf_acts->n_actions, n_actionsp) || + if (put_user(sf_acts->actions_len, actions_lenp) || (actions && copy_to_user(actions, sf_acts->actions, - sizeof(union odp_action) * - min(sf_acts->n_actions, n_actions)))) + min(sf_acts->actions_len, actions_len)))) return -EFAULT; return 0; @@ -926,13 +950,13 @@ static int do_answer_query(struct sw_flow *flow, u32 query_flags, static int answer_query(struct sw_flow *flow, u32 query_flags, struct odp_flow __user *ufp) { - union odp_action *actions; + struct nlattr *actions; if (get_user(actions, &ufp->actions)) return -EFAULT; return do_answer_query(flow, query_flags, - &ufp->stats, actions, &ufp->n_actions); + &ufp->stats, actions, &ufp->actions_len); } static struct sw_flow *do_del_flow(struct datapath *dp, struct odp_flow_key *key) @@ -1073,18 +1097,17 @@ static int do_execute(struct datapath *dp, const struct odp_execute *execute) if (execute->length < ETH_HLEN || execute->length > 65535) goto error; - actions = flow_actions_alloc(execute->n_actions); + actions = flow_actions_alloc(execute->actions_len); if (IS_ERR(actions)) { err = PTR_ERR(actions); goto error; } err = -EFAULT; - if (copy_from_user(actions->actions, execute->actions, - execute->n_actions * sizeof *execute->actions)) + if (copy_from_user(actions->actions, execute->actions, execute->actions_len)) goto error_free_actions; - err = validate_actions(actions); + err = validate_actions(actions->actions, execute->actions_len); if (err) goto error_free_actions; @@ -1114,7 +1137,7 @@ static int do_execute(struct datapath *dp, const struct odp_execute *execute) goto error_free_skb; rcu_read_lock(); - err = execute_actions(dp, skb, &key, actions->actions, actions->n_actions); + err = execute_actions(dp, skb, &key, actions->actions, actions->actions_len); rcu_read_unlock(); kfree(actions); @@ -1498,7 +1521,7 @@ static int compat_get_flow(struct odp_flow *flow, const struct compat_odp_flow _ __copy_from_user(&flow->stats, &compat->stats, sizeof(struct odp_flow_stats)) || __copy_from_user(&flow->key, &compat->key, sizeof(struct odp_flow_key)) || __get_user(actions, &compat->actions) || - __get_user(flow->n_actions, &compat->n_actions) || + __get_user(flow->actions_len, &compat->actions_len) || __get_user(flow->flags, &compat->flags)) return -EFAULT; @@ -1536,7 +1559,7 @@ static int compat_answer_query(struct sw_flow *flow, u32 query_flags, return -EFAULT; return do_answer_query(flow, query_flags, &ufp->stats, - compat_ptr(actions), &ufp->n_actions); + compat_ptr(actions), &ufp->actions_len); } static int compat_del_flow(struct datapath *dp, struct compat_odp_flow __user *ufp) @@ -1659,7 +1682,7 @@ static int compat_execute(struct datapath *dp, const struct compat_odp_execute _ if (!access_ok(VERIFY_READ, uexecute, sizeof(struct compat_odp_execute)) || __get_user(actions, &uexecute->actions) || - __get_user(execute.n_actions, &uexecute->n_actions) || + __get_user(execute.actions_len, &uexecute->actions_len) || __get_user(data, &uexecute->data) || __get_user(execute.length, &uexecute->length)) return -EFAULT; diff --git a/datapath/flow.c b/datapath/flow.c index a3ed9e83..8a6ee23d 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -104,22 +104,24 @@ void flow_used(struct sw_flow *flow, struct sk_buff *skb) spin_unlock_bh(&flow->lock); } -struct sw_flow_actions *flow_actions_alloc(size_t n_actions) +struct sw_flow_actions *flow_actions_alloc(u32 actions_len) { struct sw_flow_actions *sfa; + if (actions_len % NLA_ALIGNTO) + return ERR_PTR(-EINVAL); + /* At least DP_MAX_PORTS actions are required to be able to flood a * packet to every port. Factor of 2 allows for setting VLAN tags, * etc. */ - if (n_actions > 2 * DP_MAX_PORTS) + if (actions_len > 2 * DP_MAX_PORTS * nla_total_size(4)) return ERR_PTR(-EINVAL); - sfa = kmalloc(sizeof *sfa + n_actions * sizeof(union odp_action), - GFP_KERNEL); + sfa = kmalloc(sizeof *sfa + actions_len, GFP_KERNEL); if (!sfa) return ERR_PTR(-ENOMEM); - sfa->n_actions = n_actions; + sfa->actions_len = actions_len; return sfa; } diff --git a/datapath/flow.h b/datapath/flow.h index b1e80057..d58196e7 100644 --- a/datapath/flow.h +++ b/datapath/flow.h @@ -24,8 +24,8 @@ struct sk_buff; struct sw_flow_actions { struct rcu_head rcu; - unsigned int n_actions; - union odp_action actions[]; + u32 actions_len; + struct nlattr actions[]; }; struct sw_flow { @@ -67,7 +67,7 @@ struct sw_flow *flow_alloc(void); void flow_deferred_free(struct sw_flow *); void flow_free_tbl(struct tbl_node *); -struct sw_flow_actions *flow_actions_alloc(size_t n_actions); +struct sw_flow_actions *flow_actions_alloc(u32 actions_len); void flow_deferred_free_acts(struct sw_flow_actions *); void flow_hold(struct sw_flow *); diff --git a/datapath/loop_counter.c b/datapath/loop_counter.c index fbfbdcfa..491305d9 100644 --- a/datapath/loop_counter.c +++ b/datapath/loop_counter.c @@ -20,7 +20,7 @@ void loop_suppress(struct datapath *dp, struct sw_flow_actions *actions) if (net_ratelimit()) pr_warn("%s: flow looped %d times, dropping\n", dp_name(dp), MAX_LOOPS); - actions->n_actions = 0; + actions->actions_len = 0; } #ifndef CONFIG_PREEMPT_RT diff --git a/datapath/odp-compat.h b/datapath/odp-compat.h index 5c9eb815..7ae5df69 100644 --- a/datapath/odp-compat.h +++ b/datapath/odp-compat.h @@ -32,7 +32,7 @@ struct compat_odp_flow { struct odp_flow_stats stats; struct odp_flow_key key; compat_uptr_t actions; - u32 n_actions; + u32 actions_len; u32 flags; }; @@ -48,7 +48,7 @@ struct compat_odp_flowvec { struct compat_odp_execute { compat_uptr_t actions; - u32 n_actions; + u32 actions_len; compat_uptr_t data; u32 length; diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h index 8e07b8bd..731a8493 100644 --- a/include/openvswitch/datapath-protocol.h +++ b/include/openvswitch/datapath-protocol.h @@ -152,12 +152,12 @@ struct odp_stats { * encapsulated this packet. It is 0 if the packet was not received on a tunnel. * * For @type == %_ODPL_ACTION_NR, the header is followed by packet data. The - * @arg member is copied from the &struct odp_action_controller that caused - * the &struct odp_msg to be composed. + * @arg member is copied from the %ODPAT_CONTROLLER action that caused the + * &struct odp_msg to be composed. * * For @type == %_ODPL_SFLOW_NR, the header is followed by &struct - * odp_sflow_sample_header, then by an array of &union odp_action (the number - * of which is specified in &struct odp_sflow_sample_header), then by packet + * odp_sflow_sample_header, then by a series of Netlink attributes (whose + * length is specified in &struct odp_sflow_sample_header), then by packet * data. */ struct odp_msg { @@ -172,15 +172,16 @@ struct odp_msg { * struct odp_sflow_sample_header - header added to sFlow sampled packet. * @sample_pool: Number of packets that were candidates for sFlow sampling, * regardless of whether they were actually chosen and sent down to userspace. - * @n_actions: Number of "union odp_action"s immediately following this header. + * @actions_len: Number of bytes of actions immediately following this header. * * This header follows &struct odp_msg when that structure's @type is - * %_ODPL_SFLOW_NR, and it is itself followed by an array of &union odp_action - * (the number of which is specified in @n_actions) and then by packet data. + * %_ODPL_SFLOW_NR, and it is itself followed by a series of Netlink attributes + * (the number of bytes of which is specified in @actions_len) and then by + * packet data. */ struct odp_sflow_sample_header { uint32_t sample_pool; - uint32_t n_actions; + uint32_t actions_len; }; #define VPORT_TYPE_SIZE 16 @@ -240,8 +241,8 @@ struct odp_flow_key { struct odp_flow { struct odp_flow_stats stats; struct odp_flow_key key; - union odp_action *actions; - uint32_t n_actions; + struct nlattr *actions; + uint32_t actions_len; uint32_t flags; }; @@ -262,101 +263,31 @@ struct odp_flowvec { }; /* Action types. */ -#define ODPAT_OUTPUT 0 /* Output to switch port. */ -#define ODPAT_CONTROLLER 2 /* Send copy to controller. */ -#define ODPAT_SET_DL_TCI 3 /* Set the 802.1q TCI value. */ -#define ODPAT_STRIP_VLAN 5 /* Strip the 802.1q header. */ -#define ODPAT_SET_DL_SRC 6 /* Ethernet source address. */ -#define ODPAT_SET_DL_DST 7 /* Ethernet destination address. */ -#define ODPAT_SET_NW_SRC 8 /* IP source address. */ -#define ODPAT_SET_NW_DST 9 /* IP destination address. */ -#define ODPAT_SET_NW_TOS 10 /* IP ToS/DSCP field (6 bits). */ -#define ODPAT_SET_TP_SRC 11 /* TCP/UDP source port. */ -#define ODPAT_SET_TP_DST 12 /* TCP/UDP destination port. */ -#define ODPAT_SET_TUNNEL 13 /* Set the encapsulating tunnel ID. */ -#define ODPAT_SET_PRIORITY 14 /* Set skb->priority. */ -#define ODPAT_POP_PRIORITY 15 /* Restore original skb->priority. */ -#define ODPAT_DROP_SPOOFED_ARP 16 /* Drop ARPs with spoofed source MAC. */ -#define ODPAT_N_ACTIONS 17 - -struct odp_action_output { - uint16_t type; /* ODPAT_OUTPUT. */ - uint16_t port; /* Output port. */ - uint16_t reserved1; - uint16_t reserved2; -}; - -struct odp_action_controller { - uint16_t type; /* ODPAT_OUTPUT_CONTROLLER. */ - uint16_t reserved; - uint32_t arg; /* Copied to struct odp_msg 'arg' member. */ -}; - -struct odp_action_tunnel { - uint16_t type; /* ODPAT_SET_TUNNEL. */ - uint16_t reserved; - ovs_be32 tun_id; /* Tunnel ID. */ -}; - -/* Action structure for ODPAT_SET_DL_TCI. */ -struct odp_action_dl_tci { - uint16_t type; /* ODPAT_SET_DL_TCI. */ - ovs_be16 tci; /* New TCI. CFI bit must be zero. */ - uint32_t reserved; +enum odp_action_type { + ODPAT_UNSPEC, + ODPAT_OUTPUT, /* Output to switch port. */ + ODPAT_CONTROLLER, /* Send copy to controller. */ + ODPAT_SET_DL_TCI, /* Set the 802.1q TCI value. */ + ODPAT_STRIP_VLAN, /* Strip the 802.1q header. */ + ODPAT_SET_DL_SRC, /* Ethernet source address. */ + ODPAT_SET_DL_DST, /* Ethernet destination address. */ + ODPAT_SET_NW_SRC, /* IP source address. */ + ODPAT_SET_NW_DST, /* IP destination address. */ + ODPAT_SET_NW_TOS, /* IP ToS/DSCP field (6 bits). */ + ODPAT_SET_TP_SRC, /* TCP/UDP source port. */ + ODPAT_SET_TP_DST, /* TCP/UDP destination port. */ + ODPAT_SET_TUNNEL, /* Set the encapsulating tunnel ID. */ + ODPAT_SET_PRIORITY, /* Set skb->priority. */ + ODPAT_POP_PRIORITY, /* Restore original skb->priority. */ + ODPAT_DROP_SPOOFED_ARP, /* Drop ARPs with spoofed source MAC. */ + __ODPAT_MAX }; -/* Action structure for ODPAT_SET_DL_SRC/DST. */ -struct odp_action_dl_addr { - uint16_t type; /* ODPAT_SET_DL_SRC/DST. */ - uint8_t dl_addr[6]; /* Ethernet address. */ -}; - -/* Action structure for ODPAT_SET_NW_SRC/DST. */ -struct odp_action_nw_addr { - uint16_t type; /* ODPAT_SET_TW_SRC/DST. */ - uint16_t reserved; - ovs_be32 nw_addr; /* IP address. */ -}; - -struct odp_action_nw_tos { - uint16_t type; /* ODPAT_SET_NW_TOS. */ - uint8_t nw_tos; /* IP ToS/DSCP field (6 bits). */ - uint8_t reserved1; - uint16_t reserved2; - uint16_t reserved3; -}; - -/* Action structure for ODPAT_SET_TP_SRC/DST. */ -struct odp_action_tp_port { - uint16_t type; /* ODPAT_SET_TP_SRC/DST. */ - ovs_be16 tp_port; /* TCP/UDP port. */ - uint16_t reserved1; - uint16_t reserved2; -}; - -/* Action structure for ODPAT_SET_PRIORITY. */ -struct odp_action_priority { - uint16_t type; /* ODPAT_SET_PRIORITY. */ - uint16_t reserved; - uint32_t priority; /* skb->priority value. */ -}; - -union odp_action { - uint16_t type; - struct odp_action_output output; - struct odp_action_controller controller; - struct odp_action_tunnel tunnel; - struct odp_action_dl_tci dl_tci; - struct odp_action_dl_addr dl_addr; - struct odp_action_nw_addr nw_addr; - struct odp_action_nw_tos nw_tos; - struct odp_action_tp_port tp_port; - struct odp_action_priority priority; -}; +#define ODPAT_MAX (__ODPAT_MAX - 1) struct odp_execute { - union odp_action *actions; - uint32_t n_actions; + struct nlattr *actions; + uint32_t actions_len; const void *data; uint32_t length; diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index 89cee1fe..6aa33355 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -392,13 +392,13 @@ dpif_linux_flow_list(const struct dpif *dpif_, struct odp_flow flows[], int n) static int dpif_linux_execute(struct dpif *dpif_, - const union odp_action actions[], int n_actions, + const struct nlattr *actions, unsigned int actions_len, const struct ofpbuf *buf) { struct odp_execute execute; memset(&execute, 0, sizeof execute); - execute.actions = (union odp_action *) actions; - execute.n_actions = n_actions; + execute.actions = (struct nlattr *) actions; + execute.actions_len = actions_len; execute.data = buf->data; execute.length = buf->size; return do_ioctl(dpif_, ODP_EXECUTE, &execute); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 3fa681c9..0f95c5fd 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -39,6 +40,7 @@ #include "hmap.h" #include "list.h" #include "netdev.h" +#include "netlink.h" #include "odp-util.h" #include "ofp-print.h" #include "ofpbuf.h" @@ -106,8 +108,8 @@ struct dp_netdev_flow { uint16_t tcp_ctl; /* Bitwise-OR of seen tcp_ctl values. */ /* Actions. */ - union odp_action *actions; - unsigned int n_actions; + struct nlattr *actions; + unsigned int actions_len; }; /* Interface to netdev-based datapath. */ @@ -139,7 +141,8 @@ static int dp_netdev_output_control(struct dp_netdev *, const struct ofpbuf *, int queue_no, int port_no, uint32_t arg); static int dp_netdev_execute_actions(struct dp_netdev *, struct ofpbuf *, struct flow *, - const union odp_action *, int n); + const struct nlattr *actions, + unsigned int actions_len); static struct dpif_class dpif_dummy_class; @@ -584,11 +587,10 @@ answer_flow_query(struct dp_netdev_flow *flow, uint32_t query_flags, odp_flow->stats.tcp_flags = TCP_FLAGS(flow->tcp_ctl); odp_flow->stats.reserved = 0; odp_flow->stats.error = 0; - if (odp_flow->n_actions > 0) { - unsigned int n = MIN(odp_flow->n_actions, flow->n_actions); + if (odp_flow->actions_len > 0) { memcpy(odp_flow->actions, flow->actions, - n * sizeof *odp_flow->actions); - odp_flow->n_actions = flow->n_actions; + MIN(odp_flow->actions_len, flow->actions_len)); + odp_flow->actions_len = flow->actions_len; } if (query_flags & ODPFF_ZERO_TCP_FLAGS) { @@ -618,34 +620,42 @@ dpif_netdev_flow_get(const struct dpif *dpif, struct odp_flow flows[], int n) } static int -dpif_netdev_validate_actions(const union odp_action *actions, int n_actions, - bool *mutates) +dpif_netdev_validate_actions(const struct nlattr *actions, + unsigned int actions_len, bool *mutates) { - unsigned int i; + const struct nlattr *a; + unsigned int left; *mutates = false; - for (i = 0; i < n_actions; i++) { - const union odp_action *a = &actions[i]; - switch (a->type) { + NL_ATTR_FOR_EACH (a, left, actions, actions_len) { + uint16_t type = nl_attr_type(a); + int len = odp_action_len(type); + + if (len != nl_attr_get_size(a)) { + return EINVAL; + } + + switch (type) { case ODPAT_OUTPUT: - if (a->output.port >= MAX_PORTS) { + if (nl_attr_get_u32(a) >= MAX_PORTS) { return EINVAL; } break; case ODPAT_CONTROLLER: + case ODPAT_DROP_SPOOFED_ARP: break; case ODPAT_SET_DL_TCI: *mutates = true; - if (a->dl_tci.tci & htons(VLAN_CFI)) { + if (nl_attr_get_be16(a) & htons(VLAN_CFI)) { return EINVAL; } break; case ODPAT_SET_NW_TOS: *mutates = true; - if (a->nw_tos.nw_tos & IP_ECN_MASK) { + if (nl_attr_get_u8(a) & IP_ECN_MASK) { return EINVAL; } break; @@ -660,6 +670,9 @@ dpif_netdev_validate_actions(const union odp_action *actions, int n_actions, *mutates = true; break; + case ODPAT_SET_TUNNEL: + case ODPAT_SET_PRIORITY: + case ODPAT_POP_PRIORITY: default: return EOPNOTSUPP; } @@ -670,23 +683,18 @@ dpif_netdev_validate_actions(const union odp_action *actions, int n_actions, static int set_flow_actions(struct dp_netdev_flow *flow, struct odp_flow *odp_flow) { - size_t n_bytes; bool mutates; int error; - if (odp_flow->n_actions >= 4096 / sizeof *odp_flow->actions) { - return EINVAL; - } error = dpif_netdev_validate_actions(odp_flow->actions, - odp_flow->n_actions, &mutates); + odp_flow->actions_len, &mutates); if (error) { return error; } - n_bytes = odp_flow->n_actions * sizeof *flow->actions; - flow->actions = xrealloc(flow->actions, n_bytes); - flow->n_actions = odp_flow->n_actions; - memcpy(flow->actions, odp_flow->actions, n_bytes); + flow->actions = xrealloc(flow->actions, odp_flow->actions_len); + flow->actions_len = odp_flow->actions_len; + memcpy(flow->actions, odp_flow->actions, odp_flow->actions_len); return 0; } @@ -793,7 +801,7 @@ dpif_netdev_flow_list(const struct dpif *dpif, struct odp_flow flows[], int n) static int dpif_netdev_execute(struct dpif *dpif, - const union odp_action actions[], int n_actions, + const struct nlattr *actions, unsigned int actions_len, const struct ofpbuf *packet) { struct dp_netdev *dp = get_dp_netdev(dpif); @@ -806,7 +814,7 @@ dpif_netdev_execute(struct dpif *dpif, return EINVAL; } - error = dpif_netdev_validate_actions(actions, n_actions, &mutates); + error = dpif_netdev_validate_actions(actions, actions_len, &mutates); if (error) { return error; } @@ -825,7 +833,7 @@ dpif_netdev_execute(struct dpif *dpif, copy = *packet; } flow_extract(©, 0, -1, &key); - error = dp_netdev_execute_actions(dp, ©, &key, actions, n_actions); + error = dp_netdev_execute_actions(dp, ©, &key, actions, actions_len); if (mutates) { ofpbuf_uninit(©); } @@ -928,7 +936,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port, if (flow) { dp_netdev_flow_used(flow, &key, packet); dp_netdev_execute_actions(dp, packet, &key, - flow->actions, flow->n_actions); + flow->actions, flow->actions_len); dp->n_hit++; } else { dp->n_missed++; @@ -1053,40 +1061,41 @@ is_ip(const struct ofpbuf *packet, const struct flow *key) static void dp_netdev_set_nw_addr(struct ofpbuf *packet, struct flow *key, - const struct odp_action_nw_addr *a) + const struct nlattr *a) { if (is_ip(packet, key)) { struct ip_header *nh = packet->l3; + ovs_be32 ip = nl_attr_get_be32(a); + uint16_t type = nl_attr_type(a); uint32_t *field; - field = a->type == ODPAT_SET_NW_SRC ? &nh->ip_src : &nh->ip_dst; + field = type == ODPAT_SET_NW_SRC ? &nh->ip_src : &nh->ip_dst; if (key->nw_proto == IP_TYPE_TCP && packet->l7) { struct tcp_header *th = packet->l4; - th->tcp_csum = recalc_csum32(th->tcp_csum, *field, a->nw_addr); + th->tcp_csum = recalc_csum32(th->tcp_csum, *field, ip); } else if (key->nw_proto == IP_TYPE_UDP && packet->l7) { struct udp_header *uh = packet->l4; if (uh->udp_csum) { - uh->udp_csum = recalc_csum32(uh->udp_csum, *field, a->nw_addr); + uh->udp_csum = recalc_csum32(uh->udp_csum, *field, ip); if (!uh->udp_csum) { uh->udp_csum = 0xffff; } } } - nh->ip_csum = recalc_csum32(nh->ip_csum, *field, a->nw_addr); - *field = a->nw_addr; + nh->ip_csum = recalc_csum32(nh->ip_csum, *field, ip); + *field = ip; } } static void -dp_netdev_set_nw_tos(struct ofpbuf *packet, struct flow *key, - const struct odp_action_nw_tos *a) +dp_netdev_set_nw_tos(struct ofpbuf *packet, struct flow *key, uint8_t nw_tos) { if (is_ip(packet, key)) { struct ip_header *nh = packet->l3; uint8_t *field = &nh->ip_tos; /* Set the DSCP bits and preserve the ECN bits. */ - uint8_t new = a->nw_tos | (nh->ip_tos & IP_ECN_MASK); + uint8_t new = nw_tos | (nh->ip_tos & IP_ECN_MASK); nh->ip_csum = recalc_csum16(nh->ip_csum, htons((uint16_t)*field), htons((uint16_t) new)); @@ -1096,20 +1105,23 @@ dp_netdev_set_nw_tos(struct ofpbuf *packet, struct flow *key, static void dp_netdev_set_tp_port(struct ofpbuf *packet, struct flow *key, - const struct odp_action_tp_port *a) + const struct nlattr *a) { if (is_ip(packet, key)) { + uint16_t type = nl_attr_type(a); + ovs_be16 port = nl_attr_get_be16(a); uint16_t *field; + if (key->nw_proto == IPPROTO_TCP && packet->l7) { struct tcp_header *th = packet->l4; - field = a->type == ODPAT_SET_TP_SRC ? &th->tcp_src : &th->tcp_dst; - th->tcp_csum = recalc_csum16(th->tcp_csum, *field, a->tp_port); - *field = a->tp_port; + field = type == ODPAT_SET_TP_SRC ? &th->tcp_src : &th->tcp_dst; + th->tcp_csum = recalc_csum16(th->tcp_csum, *field, port); + *field = port; } else if (key->nw_proto == IPPROTO_UDP && packet->l7) { struct udp_header *uh = packet->l4; - field = a->type == ODPAT_SET_TP_SRC ? &uh->udp_src : &uh->udp_dst; - uh->udp_csum = recalc_csum16(uh->udp_csum, *field, a->tp_port); - *field = a->tp_port; + field = type == ODPAT_SET_TP_SRC ? &uh->udp_src : &uh->udp_dst; + uh->udp_csum = recalc_csum16(uh->udp_csum, *field, port); + *field = port; } else { return; } @@ -1184,24 +1196,25 @@ dp_netdev_is_spoofed_arp(struct ofpbuf *packet, const struct flow *key) static int dp_netdev_execute_actions(struct dp_netdev *dp, struct ofpbuf *packet, struct flow *key, - const union odp_action *actions, int n_actions) + const struct nlattr *actions, + unsigned int actions_len) { - int i; - for (i = 0; i < n_actions; i++) { - const union odp_action *a = &actions[i]; + const struct nlattr *a; + unsigned int left; - switch (a->type) { + NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) { + switch (nl_attr_type(a)) { case ODPAT_OUTPUT: - dp_netdev_output_port(dp, packet, a->output.port); + dp_netdev_output_port(dp, packet, nl_attr_get_u32(a)); break; case ODPAT_CONTROLLER: dp_netdev_output_control(dp, packet, _ODPL_ACTION_NR, - key->in_port, a->controller.arg); + key->in_port, nl_attr_get_u32(a)); break; case ODPAT_SET_DL_TCI: - dp_netdev_set_dl_tci(packet, a->dl_tci.tci); + dp_netdev_set_dl_tci(packet, nl_attr_get_be16(a)); break; case ODPAT_STRIP_VLAN: @@ -1209,25 +1222,25 @@ dp_netdev_execute_actions(struct dp_netdev *dp, break; case ODPAT_SET_DL_SRC: - dp_netdev_set_dl_src(packet, a->dl_addr.dl_addr); + dp_netdev_set_dl_src(packet, nl_attr_get_unspec(a, ETH_ADDR_LEN)); break; case ODPAT_SET_DL_DST: - dp_netdev_set_dl_dst(packet, a->dl_addr.dl_addr); + dp_netdev_set_dl_dst(packet, nl_attr_get_unspec(a, ETH_ADDR_LEN)); break; case ODPAT_SET_NW_SRC: case ODPAT_SET_NW_DST: - dp_netdev_set_nw_addr(packet, key, &a->nw_addr); + dp_netdev_set_nw_addr(packet, key, a); break; case ODPAT_SET_NW_TOS: - dp_netdev_set_nw_tos(packet, key, &a->nw_tos); + dp_netdev_set_nw_tos(packet, key, nl_attr_get_u8(a)); break; case ODPAT_SET_TP_SRC: case ODPAT_SET_TP_DST: - dp_netdev_set_tp_port(packet, key, &a->tp_port); + dp_netdev_set_tp_port(packet, key, a); break; case ODPAT_DROP_SPOOFED_ARP: diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 26cd6b09..deb3bf28 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -247,11 +247,10 @@ struct dpif_class { * 'n' flows). On failure, returns a negative errno value. */ int (*flow_list)(const struct dpif *dpif, struct odp_flow flows[], int n); - /* Performs the 'n_actions' actions in 'actions' on the Ethernet frame - * specified in 'packet'. */ - int (*execute)(struct dpif *dpif, - const union odp_action actions[], int n_actions, - const struct ofpbuf *packet); + /* Performs the 'actions_len' bytes of actions in 'actions' on the Ethernet + * frame specified in 'packet'. */ + 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 diff --git a/lib/dpif.c b/lib/dpif.c index 03e13acc..d4d9b340 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -662,13 +662,13 @@ dpif_flow_flush(struct dpif *dpif) /* Queries 'dpif' for a flow entry matching 'flow->key'. * * If a flow matching 'flow->key' exists in 'dpif', stores statistics for the - * flow into 'flow->stats'. If 'flow->n_actions' is zero, then 'flow->actions' - * is ignored. If 'flow->n_actions' is nonzero, then 'flow->actions' should - * point to an array of the specified number of actions. At most that many of - * the flow's actions will be copied into that array. 'flow->n_actions' will - * be updated to the number of actions actually present in the flow, which may - * be greater than the number stored if the flow has more actions than space - * available in the array. + * flow into 'flow->stats'. If 'flow->actions_len' is zero, then + * 'flow->actions' is ignored. If 'flow->actions_len' is nonzero, then + * 'flow->actions' should point to an array of the specified number of bytes. + * At most that many bytes of the flow's actions will be copied into that + * array. 'flow->actions_len' will be updated to the number of bytes of + * actions actually present in the flow, which may be greater than the amount + * stored if the flow has more actions than space available in the array. * * If no flow matching 'flow->key' exists in 'dpif', returns ENOENT. On other * failure, returns a positive errno value. */ @@ -687,7 +687,7 @@ dpif_flow_get(const struct dpif *dpif, struct odp_flow *flow) if (error) { /* Make the results predictable on error. */ memset(&flow->stats, 0, sizeof flow->stats); - flow->n_actions = 0; + flow->actions_len = 0; } if (should_log_flow_message(error)) { log_flow_operation(dpif, "flow_get", error, flow); @@ -702,13 +702,13 @@ dpif_flow_get(const struct dpif *dpif, struct odp_flow *flow) * Stores 0 into 'flow->stats.error' and stores statistics for the flow * into 'flow->stats'. * - * If 'flow->n_actions' is zero, then 'flow->actions' is ignored. If - * 'flow->n_actions' is nonzero, then 'flow->actions' should point to an - * array of the specified number of actions. At most that many of the - * flow's actions will be copied into that array. 'flow->n_actions' will - * be updated to the number of actions actually present in the flow, which - * may be greater than the number stored if the flow has more actions than - * space available in the array. + * If 'flow->actions_len' is zero, then 'flow->actions' is ignored. If + * 'flow->actions_len' is nonzero, then 'flow->actions' should point to an + * array of the specified number of bytes. At most that amount of flow's + * actions will be copied into that array. 'flow->actions_len' will be + * updated to the number of bytes of actions actually present in the flow, + * which may be greater than the amount stored if the flow's actions are + * longer than the available space. * * - Flow-specific errors are indicated by a positive errno value in * 'flow->stats.error'. In particular, ENOENT indicates that no flow @@ -773,8 +773,8 @@ dpif_flow_put(struct dpif *dpif, struct odp_flow_put *put) /* Deletes a flow matching 'flow->key' from 'dpif' or returns ENOENT if 'dpif' * does not contain such a flow. * - * If successful, updates 'flow->stats', 'flow->n_actions', and 'flow->actions' - * as described for dpif_flow_get(). */ + * If successful, updates 'flow->stats', 'flow->actions_len', and + * 'flow->actions' as described for dpif_flow_get(). */ int dpif_flow_del(struct dpif *dpif, struct odp_flow *flow) { @@ -811,7 +811,7 @@ dpif_flow_list(const struct dpif *dpif, struct odp_flow flows[], size_t n, } else { for (i = 0; i < n; i++) { flows[i].actions = NULL; - flows[i].n_actions = 0; + flows[i].actions_len = 0; } } retval = dpif->dpif_class->flow_list(dpif, flows, n); @@ -873,20 +873,20 @@ dpif_flow_list_all(const struct dpif *dpif, return 0; } -/* Causes 'dpif' to perform the 'n_actions' actions in 'actions' on the - * Ethernet frame specified in 'packet'. +/* Causes 'dpif' to perform the 'actions_len' bytes of actions in 'actions' on + * the Ethernet frame specified in 'packet'. * * Returns 0 if successful, otherwise a positive errno value. */ int dpif_execute(struct dpif *dpif, - const union odp_action actions[], size_t n_actions, + const struct nlattr *actions, size_t actions_len, const struct ofpbuf *buf) { int error; COVERAGE_INC(dpif_execute); - if (n_actions > 0) { - error = dpif->dpif_class->execute(dpif, actions, n_actions, buf); + if (actions_len > 0) { + error = dpif->dpif_class->execute(dpif, actions, actions_len, buf); } else { error = 0; } @@ -895,7 +895,7 @@ dpif_execute(struct dpif *dpif, struct ds ds = DS_EMPTY_INITIALIZER; char *packet = ofp_packet_to_string(buf->data, buf->size, buf->size); ds_put_format(&ds, "%s: execute ", dpif_name(dpif)); - format_odp_actions(&ds, actions, n_actions); + format_odp_actions(&ds, actions, actions_len); if (error) { ds_put_format(&ds, " failed (%s)", strerror(error)); } @@ -1132,7 +1132,7 @@ static void log_flow_message(const struct dpif *dpif, int error, const char *operation, const struct odp_flow_key *flow, const struct odp_flow_stats *stats, - const union odp_action *actions, size_t n_actions) + const struct nlattr *actions, unsigned int actions_len) { struct ds ds = DS_EMPTY_INITIALIZER; ds_put_format(&ds, "%s: ", dpif_name(dpif)); @@ -1148,9 +1148,9 @@ log_flow_message(const struct dpif *dpif, int error, const char *operation, ds_put_cstr(&ds, ", "); format_odp_flow_stats(&ds, stats); } - if (actions || n_actions) { + if (actions || actions_len) { ds_put_cstr(&ds, ", actions:"); - format_odp_actions(&ds, actions, n_actions); + format_odp_actions(&ds, actions, actions_len); } vlog(THIS_MODULE, flow_message_log_level(error), "%s", ds_cstr(&ds)); ds_destroy(&ds); @@ -1161,11 +1161,11 @@ log_flow_operation(const struct dpif *dpif, const char *operation, int error, struct odp_flow *flow) { if (error) { - flow->n_actions = 0; + flow->actions_len = 0; } log_flow_message(dpif, error, operation, &flow->key, !error ? &flow->stats : NULL, - flow->actions, flow->n_actions); + flow->actions, flow->actions_len); } static void @@ -1190,12 +1190,12 @@ log_flow_put(struct dpif *dpif, int error, const struct odp_flow_put *put) } log_flow_message(dpif, error, ds_cstr(&s), &put->flow.key, !error ? &put->flow.stats : NULL, - put->flow.actions, put->flow.n_actions); + put->flow.actions, put->flow.actions_len); ds_destroy(&s); } /* There is a tendency to construct odp_flow objects on the stack and to - * forget to properly initialize their "actions" and "n_actions" members. + * forget to properly initialize their "actions" and "actions_len" members. * When this happens, we get memory corruption because the kernel * writes through the random pointer that is in the "actions" member. * @@ -1208,12 +1208,12 @@ log_flow_put(struct dpif *dpif, int error, const struct odp_flow_put *put) * easy-to-identify error later if it is dereferenced, etc. * * - Triggering a warning on uninitialized memory from Valgrind if - * "actions" or "n_actions" was not initialized. + * "actions" or "actions_len" was not initialized. */ static void check_rw_odp_flow(struct odp_flow *flow) { - if (flow->n_actions) { + if (flow->actions_len) { memset(&flow->actions[0], 0xcc, sizeof flow->actions[0]); } } diff --git a/lib/dpif.h b/lib/dpif.h index 927776cc..825a00c5 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -31,6 +31,7 @@ extern "C" { struct dpif; struct netdev; +struct nlattr; struct ofpbuf; struct svec; struct dpif_class; @@ -83,8 +84,8 @@ int dpif_flow_list(const struct dpif *, struct odp_flow[], size_t n, int dpif_flow_list_all(const struct dpif *, struct odp_flow **flowsp, size_t *np); -int dpif_execute(struct dpif *, const union odp_action[], size_t n_actions, - const struct ofpbuf *); +int dpif_execute(struct dpif *, const struct nlattr *actions, + size_t actions_len, const struct ofpbuf *); /* Minimum number of bytes of headroom for a packet returned by dpif_recv() * member function. This headroom allows "struct odp_msg" to be replaced by diff --git a/lib/odp-util.c b/lib/odp-util.c index 6b4f5fad..2ed551d2 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -19,26 +19,15 @@ #include #include #include +#include "byte-order.h" #include "coverage.h" #include "dynamic-string.h" #include "flow.h" +#include "netlink.h" #include "packets.h" #include "timeval.h" #include "util.h" -union odp_action * -odp_actions_add(struct odp_actions *actions, uint16_t type) -{ - union odp_action *a; - size_t idx; - - idx = actions->n_actions++ & (MAX_ODP_ACTIONS - 1); - a = &actions->actions[idx]; - memset(a, 0, sizeof *a); - a->type = type; - return a; -} - void format_odp_flow_key(struct ds *ds, const struct odp_flow_key *key) { @@ -59,54 +48,114 @@ format_odp_flow_key(struct ds *ds, const struct odp_flow_key *key) ntohs(key->tp_src), ntohs(key->tp_dst)); } +int +odp_action_len(uint16_t type) +{ + if (type > ODPAT_MAX) { + return -1; + } + + switch ((enum odp_action_type) type) { + case ODPAT_OUTPUT: return 4; + case ODPAT_CONTROLLER: return 4; + case ODPAT_SET_DL_TCI: return 2; + case ODPAT_STRIP_VLAN: return 0; + case ODPAT_SET_DL_SRC: return ETH_ADDR_LEN; + case ODPAT_SET_DL_DST: return ETH_ADDR_LEN; + case ODPAT_SET_NW_SRC: return 4; + case ODPAT_SET_NW_DST: return 4; + case ODPAT_SET_NW_TOS: return 1; + case ODPAT_SET_TP_SRC: return 2; + case ODPAT_SET_TP_DST: return 2; + case ODPAT_SET_TUNNEL: return 4; + case ODPAT_SET_PRIORITY: return 4; + case ODPAT_POP_PRIORITY: return 0; + case ODPAT_DROP_SPOOFED_ARP: return 0; + + case ODPAT_UNSPEC: + case __ODPAT_MAX: + return -1; + } + + return -1; +} + +static void +format_generic_odp_action(struct ds *ds, const struct nlattr *a) +{ + ds_put_format(ds, "action%"PRId16, nl_attr_type(a)); + if (a->nla_len) { + const uint8_t *unspec; + unsigned int i; + + unspec = nl_attr_get(a); + for (i = 0; i < a->nla_len; i++) { + ds_put_char(ds, i ? ' ': '('); + ds_put_format(ds, "%02x", unspec[i]); + } + ds_put_char(ds, ')'); + } +} + void -format_odp_action(struct ds *ds, const union odp_action *a) +format_odp_action(struct ds *ds, const struct nlattr *a) { - switch (a->type) { + const uint8_t *eth; + ovs_be32 ip; + + if (nl_attr_get_size(a) != odp_action_len(a->nla_len)) { + ds_put_format(ds, "***bad action: length is %zu, expected %d*** ", + nl_attr_get_size(a), odp_action_len(a->nla_len)); + format_generic_odp_action(ds, a); + return; + } + + switch (nl_attr_type(a)) { case ODPAT_OUTPUT: - ds_put_format(ds, "%"PRIu16, a->output.port); + ds_put_format(ds, "%"PRIu16, nl_attr_get_u32(a)); break; case ODPAT_CONTROLLER: - ds_put_format(ds, "ctl(%"PRIu32")", a->controller.arg); + ds_put_format(ds, "ctl(%"PRIu32")", nl_attr_get_u32(a)); break; case ODPAT_SET_TUNNEL: - ds_put_format(ds, "set_tunnel(%#"PRIx32")", ntohl(a->tunnel.tun_id)); + ds_put_format(ds, "set_tunnel(%#"PRIx32")", + ntohl(nl_attr_get_be32(a))); break; case ODPAT_SET_DL_TCI: ds_put_format(ds, "set_tci(vid=%"PRIu16",pcp=%d)", - vlan_tci_to_vid(a->dl_tci.tci), - vlan_tci_to_pcp(a->dl_tci.tci)); + vlan_tci_to_vid(nl_attr_get_be16(a)), + vlan_tci_to_pcp(nl_attr_get_be16(a))); break; case ODPAT_STRIP_VLAN: ds_put_format(ds, "strip_vlan"); break; case ODPAT_SET_DL_SRC: - ds_put_format(ds, "set_dl_src("ETH_ADDR_FMT")", - ETH_ADDR_ARGS(a->dl_addr.dl_addr)); + eth = nl_attr_get_unspec(a, ETH_ADDR_LEN); + ds_put_format(ds, "set_dl_src("ETH_ADDR_FMT")", ETH_ADDR_ARGS(eth)); break; case ODPAT_SET_DL_DST: - ds_put_format(ds, "set_dl_dst("ETH_ADDR_FMT")", - ETH_ADDR_ARGS(a->dl_addr.dl_addr)); + eth = nl_attr_get_unspec(a, ETH_ADDR_LEN); + ds_put_format(ds, "set_dl_dst("ETH_ADDR_FMT")", ETH_ADDR_ARGS(eth)); break; case ODPAT_SET_NW_SRC: - ds_put_format(ds, "set_nw_src("IP_FMT")", - IP_ARGS(&a->nw_addr.nw_addr)); + ip = nl_attr_get_be32(a); + ds_put_format(ds, "set_nw_src("IP_FMT")", IP_ARGS(&ip)); break; case ODPAT_SET_NW_DST: - ds_put_format(ds, "set_nw_dst("IP_FMT")", - IP_ARGS(&a->nw_addr.nw_addr)); + ip = nl_attr_get_be32(a); + ds_put_format(ds, "set_nw_dst("IP_FMT")", IP_ARGS(&ip)); break; case ODPAT_SET_NW_TOS: - ds_put_format(ds, "set_nw_tos(%"PRIu8")", a->nw_tos.nw_tos); + ds_put_format(ds, "set_nw_tos(%"PRIu8")", nl_attr_get_u8(a)); break; case ODPAT_SET_TP_SRC: - ds_put_format(ds, "set_tp_src(%"PRIu16")", ntohs(a->tp_port.tp_port)); + ds_put_format(ds, "set_tp_src(%"PRIu16")", ntohs(nl_attr_get_be16(a))); break; case ODPAT_SET_TP_DST: - ds_put_format(ds, "set_tp_dst(%"PRIu16")", ntohs(a->tp_port.tp_port)); + ds_put_format(ds, "set_tp_dst(%"PRIu16")", ntohs(nl_attr_get_be16(a))); break; case ODPAT_SET_PRIORITY: - ds_put_format(ds, "set_priority(0x%"PRIx32")", a->priority.priority); + ds_put_format(ds, "set_priority(%#"PRIx32")", nl_attr_get_u32(a)); break; case ODPAT_POP_PRIORITY: ds_put_cstr(ds, "pop_priority"); @@ -115,23 +164,29 @@ format_odp_action(struct ds *ds, const union odp_action *a) ds_put_cstr(ds, "drop_spoofed_arp"); break; default: - ds_put_format(ds, "***bad action 0x%"PRIx16"***", a->type); + format_generic_odp_action(ds, a); break; } } void -format_odp_actions(struct ds *ds, const union odp_action *actions, - size_t n_actions) +format_odp_actions(struct ds *ds, const struct nlattr *actions, + unsigned int actions_len) { - size_t i; - for (i = 0; i < n_actions; i++) { - if (i) { - ds_put_char(ds, ','); + if (actions_len) { + const struct nlattr *a; + unsigned int left; + + NL_ATTR_FOR_EACH (a, left, actions, actions_len) { + if (a != actions) { + ds_put_char(ds, ','); + } + format_odp_action(ds, a); } - format_odp_action(ds, &actions[i]); - } - if (!n_actions) { + if (left) { + ds_put_format(ds, " ***%u leftover bytes***", left); + } + } else { ds_put_cstr(ds, "drop"); } } @@ -157,7 +212,7 @@ format_odp_flow(struct ds *ds, const struct odp_flow *f) ds_put_cstr(ds, ", "); format_odp_flow_stats(ds, &f->stats); ds_put_cstr(ds, ", actions:"); - format_odp_actions(ds, f->actions, f->n_actions); + format_odp_actions(ds, f->actions, f->actions_len); } void diff --git a/lib/odp-util.h b/lib/odp-util.h index 813e29f3..6051c52d 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -29,30 +29,6 @@ struct ds; struct flow; -enum { MAX_ODP_ACTIONS = 16384 / sizeof(union odp_action) }; - -/* odp_actions_add() assumes that MAX_ODP_ACTIONS is a power of 2. */ -BUILD_ASSERT_DECL(IS_POW2(MAX_ODP_ACTIONS)); - -struct odp_actions { - size_t n_actions; - union odp_action actions[MAX_ODP_ACTIONS]; -}; - -static inline void -odp_actions_init(struct odp_actions *actions) -{ - actions->n_actions = 0; -} - -union odp_action *odp_actions_add(struct odp_actions *actions, uint16_t type); - -static inline bool -odp_actions_overflow(const struct odp_actions *actions) -{ - return actions->n_actions > MAX_ODP_ACTIONS; -} - static inline uint16_t ofp_port_to_odp_port(uint16_t ofp_port) { @@ -80,9 +56,10 @@ odp_port_to_ofp_port(uint16_t odp_port) } void format_odp_flow_key(struct ds *, const struct odp_flow_key *); -void format_odp_action(struct ds *, const union odp_action *); -void format_odp_actions(struct ds *, const union odp_action *actions, - size_t n_actions); +int odp_action_len(uint16_t type); +void format_odp_action(struct ds *, const struct nlattr *); +void format_odp_actions(struct ds *, const struct nlattr *odp_actions, + size_t actions_len); void format_odp_flow_stats(struct ds *, const struct odp_flow_stats *); void format_odp_flow(struct ds *, const struct odp_flow *); diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 88851f84..cdbbb0dc 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -130,7 +130,7 @@ ofputil_cls_rule_from_match(const struct ofp_match *match, rule->flow.tun_id = htonl(ntohll(cookie) >> 32); } else { wc->wildcards |= FWW_TUN_ID; - rule->flow.tun_id = 0; + rule->flow.tun_id = htonl(0); } if (ofpfw & OFPFW_DL_DST) { diff --git a/ofproto/in-band.c b/ofproto/in-band.c index aebdb7e4..9655f105 100644 --- a/ofproto/in-band.c +++ b/ofproto/in-band.c @@ -28,6 +28,7 @@ #include "dpif.h" #include "flow.h" #include "netdev.h" +#include "netlink.h" #include "odp-util.h" #include "ofproto.h" #include "ofpbuf.h" @@ -428,7 +429,7 @@ in_band_msg_in_hook(struct in_band *in_band, const struct flow *flow, * allowed to be set up in the datapath. */ bool in_band_rule_check(struct in_band *in_band, const struct flow *flow, - const struct odp_actions *actions) + const struct nlattr *actions, unsigned int actions_len) { if (!in_band) { return true; @@ -440,11 +441,12 @@ in_band_rule_check(struct in_band *in_band, const struct flow *flow, && flow->nw_proto == IP_TYPE_UDP && flow->tp_src == htons(DHCP_SERVER_PORT) && flow->tp_dst == htons(DHCP_CLIENT_PORT)) { - int i; + const struct nlattr *a; + unsigned int left; - for (i=0; in_actions; i++) { - if (actions->actions[i].output.type == ODPAT_OUTPUT - && actions->actions[i].output.port == ODPP_LOCAL) { + NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) { + if (nl_attr_type(a) == ODPAT_OUTPUT + && nl_attr_get_u32(a) == ODPP_LOCAL) { return true; } } diff --git a/ofproto/in-band.h b/ofproto/in-band.h index 23a30ced..972acaa8 100644 --- a/ofproto/in-band.h +++ b/ofproto/in-band.h @@ -21,7 +21,6 @@ struct dpif; struct in_band; -struct odp_actions; struct ofproto; struct rconn; struct settings; @@ -41,7 +40,8 @@ void in_band_wait(struct in_band *); bool in_band_msg_in_hook(struct in_band *, const struct flow *, const struct ofpbuf *packet); bool in_band_rule_check(struct in_band *, const struct flow *, - const struct odp_actions *); + const struct nlattr *odp_actions, + unsigned int actions_len); void in_band_flushed(struct in_band *); #endif /* in-band.h */ diff --git a/ofproto/ofproto-sflow.c b/ofproto/ofproto-sflow.c index 801614d2..77119fdb 100644 --- a/ofproto/ofproto-sflow.c +++ b/ofproto/ofproto-sflow.c @@ -25,6 +25,7 @@ #include "hash.h" #include "hmap.h" #include "netdev.h" +#include "netlink.h" #include "ofpbuf.h" #include "ofproto.h" #include "packets.h" @@ -484,41 +485,32 @@ ofproto_sflow_received(struct ofproto_sflow *os, struct odp_msg *msg) SFLFlow_sample_element switchElem; SFLSampler *sampler; const struct odp_sflow_sample_header *hdr; - const union odp_action *actions; - struct ofpbuf payload; - size_t n_actions, n_outputs; + const struct nlattr *actions, *a; + unsigned int left; + struct ofpbuf b; + size_t n_outputs; struct flow flow; - size_t min_size; - size_t i; - - /* Get odp_sflow_sample_header. */ - min_size = sizeof *msg + sizeof *hdr; - if (min_size > msg->length) { - VLOG_WARN_RL(&rl, "sFlow packet too small (%"PRIu32" < %zu)", - msg->length, min_size); - return; - } - hdr = (const struct odp_sflow_sample_header *) (msg + 1); - /* Get actions. */ - n_actions = hdr->n_actions; - if (n_actions > 65536 / sizeof *actions) { - VLOG_WARN_RL(&rl, "too many actions in sFlow packet (%zu > %zu)", - 65536 / sizeof *actions, n_actions); + /* Pull odp_msg header. */ + ofpbuf_use_const(&b, msg, msg->length); + ofpbuf_pull(&b, sizeof *msg); + + /* Pull odp_sflow_sample_header. */ + hdr = ofpbuf_try_pull(&b, sizeof *hdr); + if (!hdr) { + VLOG_WARN_RL(&rl, "missing odp_sflow_sample_header"); return; } - min_size += n_actions * sizeof *actions; - if (min_size > msg->length) { - VLOG_WARN_RL(&rl, "sFlow packet with %zu actions too small " - "(%"PRIu32" < %zu)", - n_actions, msg->length, min_size); + + /* Pull actions. */ + actions = ofpbuf_try_pull(&b, hdr->actions_len); + if (!actions) { + VLOG_WARN_RL(&rl, "missing odp actions"); return; } - actions = (const union odp_action *) (hdr + 1); - /* Get packet payload and extract flow. */ - ofpbuf_use_const(&payload, actions + n_actions, msg->length - min_size); - flow_extract(&payload, 0, msg->port, &flow); + /* Now only the payload is left. */ + flow_extract(&b, 0, msg->port, &flow); /* Build a flow sample */ memset(&fs, 0, sizeof fs); @@ -543,12 +535,11 @@ ofproto_sflow_received(struct ofproto_sflow *os, struct odp_msg *msg) header->header_protocol = SFLHEADER_ETHERNET_ISO8023; /* The frame_length should include the Ethernet FCS (4 bytes), but it has already been stripped, so we need to add 4 here. */ - header->frame_length = payload.size + 4; + header->frame_length = b.size + 4; /* Ethernet FCS stripped off. */ header->stripped = 4; - header->header_length = MIN(payload.size, - sampler->sFlowFsMaximumHeaderSize); - header->header_bytes = payload.data; + header->header_length = MIN(b.size, sampler->sFlowFsMaximumHeaderSize); + header->header_bytes = b.data; /* Add extended switch element. */ memset(&switchElem, 0, sizeof(switchElem)); @@ -562,18 +553,18 @@ ofproto_sflow_received(struct ofproto_sflow *os, struct odp_msg *msg) /* Figure out the output ports. */ n_outputs = 0; - for (i = 0; i < n_actions; i++) { - const union odp_action *a = &actions[i]; - uint16_t tci; + NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, hdr->actions_len) { + ovs_be16 tci; - switch (a->type) { + switch (nl_attr_type(a)) { case ODPAT_OUTPUT: - fs.output = ofproto_sflow_odp_port_to_ifindex(os, a->output.port); + fs.output = ofproto_sflow_odp_port_to_ifindex(os, + nl_attr_get_u32(a)); n_outputs++; break; case ODPAT_SET_DL_TCI: - tci = a->dl_tci.tci; + tci = nl_attr_get_be16(a); switchElem.flowType.sw.dst_vlan = vlan_tci_to_vid(tci); switchElem.flowType.sw.dst_priority = vlan_tci_to_pcp(tci); break; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 00b57311..86d0ae67 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -37,6 +37,7 @@ #include "mac-learning.h" #include "netdev.h" #include "netflow.h" +#include "netlink.h" #include "nx-match.h" #include "odp-util.h" #include "ofp-print.h" @@ -125,9 +126,7 @@ struct action_xlate_ctx { /* xlate_actions() initializes and uses these members. The client might want * to look at them after it returns. */ - /* Datapath action set. This is xlate_actions()'s primary output. */ - struct odp_actions out; - + struct ofpbuf *odp_actions; /* Datapath actions. */ tag_type tags; /* Tags associated with OFPP_NORMAL actions. */ bool may_set_up_flow; /* True ordinarily; false if the actions must * be reassessed for every packet. */ @@ -137,13 +136,15 @@ struct action_xlate_ctx { * reason to look at them. */ int recurse; /* Recursion level, via xlate_table_action. */ + int last_pop_priority; /* Offset in 'odp_actions' just past most + * recently added ODPAT_SET_PRIORITY. */ }; static void action_xlate_ctx_init(struct action_xlate_ctx *, struct ofproto *, const struct flow *, const struct ofpbuf *); -static int xlate_actions(struct action_xlate_ctx *ctx, - const union ofp_action *in, size_t n_in); +static struct ofpbuf *xlate_actions(struct action_xlate_ctx *, + const union ofp_action *in, size_t n_in); /* An OpenFlow flow. */ struct rule { @@ -224,8 +225,8 @@ struct facet { bool installed; /* Installed in datapath? */ bool may_install; /* True ordinarily; false if actions must * be reassessed for every packet. */ - int n_actions; /* Number of elements in actions[]. */ - union odp_action *actions; /* Datapath actions. */ + unsigned int actions_len; /* Number of bytes in actions[]. */ + struct nlattr *actions; /* Datapath actions. */ tag_type tags; /* Tags (set only by hooks). */ struct netflow_flow nf_flow; /* Per-flow NetFlow tracking data. */ }; @@ -1400,17 +1401,17 @@ ofproto_send_packet(struct ofproto *p, const struct flow *flow, const struct ofpbuf *packet) { struct action_xlate_ctx ctx; - int error; + struct ofpbuf *odp_actions; action_xlate_ctx_init(&ctx, p, flow, packet); - error = xlate_actions(&ctx, actions, n_actions); - if (error) { - return error; - } + odp_actions = xlate_actions(&ctx, actions, n_actions); /* XXX Should we translate the dpif_execute() errno value into an OpenFlow * error code? */ - dpif_execute(p->dpif, ctx.out.actions, ctx.out.n_actions, packet); + dpif_execute(p->dpif, odp_actions->data, odp_actions->size, packet); + + ofpbuf_delete(odp_actions); + return 0; } @@ -2049,10 +2050,11 @@ rule_has_out_port(const struct rule *rule, ovs_be16 out_port) * Takes ownership of 'packet'. */ static bool execute_odp_actions(struct ofproto *ofproto, uint16_t in_port, - const union odp_action *actions, size_t n_actions, + const struct nlattr *odp_actions, unsigned int actions_len, struct ofpbuf *packet) { - if (n_actions == 1 && actions[0].type == ODPAT_CONTROLLER) { + if (actions_len == NLA_ALIGN(NLA_HDRLEN + sizeof(uint32_t)) + && odp_actions->nla_type == ODPAT_CONTROLLER) { /* As an optimization, avoid a round-trip from userspace to kernel to * userspace. This also avoids possibly filling up kernel packet * buffers along the way. */ @@ -2063,7 +2065,7 @@ execute_odp_actions(struct ofproto *ofproto, uint16_t in_port, msg->length = sizeof(struct odp_msg) + packet->size; msg->port = in_port; msg->reserved = 0; - msg->arg = actions[0].controller.arg; + msg->arg = nl_attr_get_u32(odp_actions); send_packet_in(ofproto, packet); @@ -2071,7 +2073,7 @@ execute_odp_actions(struct ofproto *ofproto, uint16_t in_port, } else { int error; - error = dpif_execute(ofproto->dpif, actions, n_actions, packet); + error = dpif_execute(ofproto->dpif, odp_actions, actions_len, packet); ofpbuf_delete(packet); return !error; } @@ -2099,7 +2101,7 @@ facet_execute(struct ofproto *ofproto, struct facet *facet, flow_extract_stats(&facet->flow, packet, &stats); if (execute_odp_actions(ofproto, facet->flow.in_port, - facet->actions, facet->n_actions, packet)) { + facet->actions, facet->actions_len, packet)) { facet_update_stats(ofproto, facet, &stats); facet->used = time_msec(); netflow_flow_update_time(ofproto->netflow, @@ -2120,6 +2122,7 @@ rule_execute(struct ofproto *ofproto, struct rule *rule, uint16_t in_port, struct ofpbuf *packet) { struct action_xlate_ctx ctx; + struct ofpbuf *odp_actions; struct facet *facet; struct flow flow; size_t size; @@ -2147,18 +2150,15 @@ rule_execute(struct ofproto *ofproto, struct rule *rule, uint16_t in_port, /* We can't account anything to a facet. If we were to try, then that * facet would have a non-matching rule, busting our invariants. */ action_xlate_ctx_init(&ctx, ofproto, &flow, packet); - if (xlate_actions(&ctx, rule->actions, rule->n_actions)) { - ofpbuf_delete(packet); - return; - } - + odp_actions = xlate_actions(&ctx, rule->actions, rule->n_actions); size = packet->size; - if (execute_odp_actions(ofproto, in_port, - ctx.out.actions, ctx.out.n_actions, packet)) { + if (execute_odp_actions(ofproto, in_port, odp_actions->data, + odp_actions->size, packet)) { rule->used = time_msec(); rule->packet_count++; rule->byte_count += size; } + ofpbuf_delete(odp_actions); } /* Inserts 'rule' into 'p''s flow table. */ @@ -2246,22 +2246,20 @@ facet_make_actions(struct ofproto *p, struct facet *facet, const struct ofpbuf *packet) { const struct rule *rule = facet->rule; + struct ofpbuf *odp_actions; struct action_xlate_ctx ctx; - size_t actions_len; action_xlate_ctx_init(&ctx, p, &facet->flow, packet); - xlate_actions(&ctx, rule->actions, rule->n_actions); - facet->tags = ctx.tags; - facet->may_install = ctx.may_set_up_flow; - facet->nf_flow.output_iface = ctx.nf_output_iface; + odp_actions = xlate_actions(&ctx, rule->actions, rule->n_actions); - actions_len = ctx.out.n_actions * sizeof *ctx.out.actions; - if (facet->n_actions != ctx.out.n_actions - || memcmp(facet->actions, ctx.out.actions, actions_len)) { + if (facet->actions_len != odp_actions->size + || memcmp(facet->actions, odp_actions->data, odp_actions->size)) { free(facet->actions); - facet->n_actions = ctx.out.n_actions; - facet->actions = xmemdup(ctx.out.actions, actions_len); + facet->actions_len = odp_actions->size; + facet->actions = xmemdup(odp_actions->data, odp_actions->size); } + + ofpbuf_delete(odp_actions); } static int @@ -2271,7 +2269,7 @@ facet_put__(struct ofproto *ofproto, struct facet *facet, int flags, memset(&put->flow.stats, 0, sizeof put->flow.stats); odp_flow_key_from_flow(&put->flow.key, &facet->flow); put->flow.actions = facet->actions; - put->flow.n_actions = facet->n_actions; + put->flow.actions_len = facet->actions_len; put->flow.flags = 0; put->flags = flags; return dpif_flow_put(ofproto->dpif, put); @@ -2309,7 +2307,7 @@ facet_account(struct ofproto *ofproto, && total_bytes > facet->accounted_bytes) { ofproto->ofhooks->account_flow_cb( - &facet->flow, facet->tags, facet->actions, facet->n_actions, + &facet->flow, facet->tags, facet->actions, facet->actions_len, total_bytes - facet->accounted_bytes, ofproto->aux); facet->accounted_bytes = total_bytes; } @@ -2324,7 +2322,7 @@ facet_uninstall(struct ofproto *p, struct facet *facet) odp_flow_key_from_flow(&odp_flow.key, &facet->flow); odp_flow.actions = NULL; - odp_flow.n_actions = 0; + odp_flow.actions_len = 0; odp_flow.flags = 0; if (!dpif_flow_del(p->dpif, &odp_flow)) { facet_update_stats(p, facet, &odp_flow.stats); @@ -2429,8 +2427,8 @@ static bool facet_revalidate(struct ofproto *ofproto, struct facet *facet) { struct action_xlate_ctx ctx; + struct ofpbuf *odp_actions; struct rule *new_rule; - size_t actions_len; bool actions_changed; COVERAGE_INC(facet_revalidate); @@ -2445,15 +2443,14 @@ facet_revalidate(struct ofproto *ofproto, struct facet *facet) /* Calculate new ODP actions. * - * We are very cautious about actually modifying 'facet' state at this - * point, because we might need to, e.g., emit a NetFlow expiration and, if - * so, we need to have the old state around to properly compose it. */ + * We do not modify any 'facet' state yet, because we might need to, e.g., + * emit a NetFlow expiration and, if so, we need to have the old state + * around to properly compose it. */ action_xlate_ctx_init(&ctx, ofproto, &facet->flow, NULL); - xlate_actions(&ctx, new_rule->actions, new_rule->n_actions); - actions_len = ctx.out.n_actions * sizeof *ctx.out.actions; - actions_changed = (facet->n_actions != ctx.out.n_actions - || memcmp(facet->actions, ctx.out.actions, - actions_len)); + odp_actions = xlate_actions(&ctx, new_rule->actions, new_rule->n_actions); + actions_changed = (facet->actions_len != odp_actions->size + || memcmp(facet->actions, odp_actions->data, + facet->actions_len)); /* If the ODP actions changed or the installability changed, then we need * to talk to the datapath. */ @@ -2463,8 +2460,8 @@ facet_revalidate(struct ofproto *ofproto, struct facet *facet) memset(&put.flow.stats, 0, sizeof put.flow.stats); odp_flow_key_from_flow(&put.flow.key, &facet->flow); - put.flow.actions = ctx.out.actions; - put.flow.n_actions = ctx.out.n_actions; + put.flow.actions = odp_actions->data; + put.flow.actions_len = odp_actions->size; put.flow.flags = 0; put.flags = ODPPF_CREATE | ODPPF_MODIFY | ODPPF_ZERO_STATS; dpif_flow_put(ofproto->dpif, &put); @@ -2479,14 +2476,16 @@ facet_revalidate(struct ofproto *ofproto, struct facet *facet) facet_flush_stats(ofproto, facet); } + ofpbuf_delete(odp_actions); + /* Update 'facet' now that we've taken care of all the old state. */ facet->tags = ctx.tags; facet->nf_flow.output_iface = ctx.nf_output_iface; facet->may_install = ctx.may_set_up_flow; if (actions_changed) { free(facet->actions); - facet->n_actions = ctx.out.n_actions; - facet->actions = xmemdup(ctx.out.actions, actions_len); + facet->actions_len = odp_actions->size; + facet->actions = xmemdup(odp_actions->data, odp_actions->size); } if (facet->rule != new_rule) { COVERAGE_INC(facet_changed_rule); @@ -2619,13 +2618,6 @@ handle_set_config(struct ofconn *ofconn, const struct ofp_switch_config *osc) return 0; } -static void -add_controller_action(struct odp_actions *actions, uint16_t max_len) -{ - union odp_action *a = odp_actions_add(actions, ODPAT_CONTROLLER); - a->controller.arg = max_len; -} - /* Maximum depth of flow table recursion (due to NXAST_RESUBMIT actions) in a * flow translation. */ #define MAX_RESUBMIT_RECURSION 8 @@ -2651,7 +2643,7 @@ add_output_action(struct action_xlate_ctx *ctx, uint16_t port) */ } - odp_actions_add(&ctx->out, ODPAT_OUTPUT)->output.port = port; + nl_msg_put_u32(ctx->odp_actions, ODPAT_OUTPUT, port); ctx->nf_output_iface = port; } @@ -2695,14 +2687,14 @@ xlate_table_action(struct action_xlate_ctx *ctx, uint16_t in_port) static void flood_packets(struct ofproto *ofproto, uint16_t odp_in_port, uint32_t mask, - uint16_t *nf_output_iface, struct odp_actions *actions) + uint16_t *nf_output_iface, struct ofpbuf *odp_actions) { struct ofport *ofport; HMAP_FOR_EACH (ofport, hmap_node, &ofproto->ports) { uint16_t odp_port = ofport->odp_port; if (odp_port != odp_in_port && !(ofport->opp.config & mask)) { - odp_actions_add(actions, ODPAT_OUTPUT)->output.port = odp_port; + nl_msg_put_u32(odp_actions, ODPAT_OUTPUT, odp_port); } } *nf_output_iface = NF_OUT_FLOOD; @@ -2726,7 +2718,7 @@ xlate_output_action__(struct action_xlate_ctx *ctx, break; case OFPP_NORMAL: if (!ctx->ofproto->ofhooks->normal_cb(&ctx->flow, ctx->packet, - &ctx->out, &ctx->tags, + ctx->odp_actions, &ctx->tags, &ctx->nf_output_iface, ctx->ofproto->aux)) { COVERAGE_INC(ofproto_uninstallable); @@ -2735,14 +2727,14 @@ xlate_output_action__(struct action_xlate_ctx *ctx, break; case OFPP_FLOOD: flood_packets(ctx->ofproto, ctx->flow.in_port, OFPPC_NO_FLOOD, - &ctx->nf_output_iface, &ctx->out); + &ctx->nf_output_iface, ctx->odp_actions); break; case OFPP_ALL: flood_packets(ctx->ofproto, ctx->flow.in_port, 0, - &ctx->nf_output_iface, &ctx->out); + &ctx->nf_output_iface, ctx->odp_actions); break; case OFPP_CONTROLLER: - add_controller_action(&ctx->out, max_len); + nl_msg_put_u32(ctx->odp_actions, ODPAT_CONTROLLER, max_len); break; case OFPP_LOCAL: add_output_action(ctx, ODPP_LOCAL); @@ -2779,9 +2771,18 @@ xlate_output_action(struct action_xlate_ctx *ctx, static void remove_pop_action(struct action_xlate_ctx *ctx) { - size_t n = ctx->out.n_actions; - if (n > 0 && ctx->out.actions[n - 1].type == ODPAT_POP_PRIORITY) { - ctx->out.n_actions--; + if (ctx->odp_actions->size == ctx->last_pop_priority) { + ctx->odp_actions->size -= NLA_ALIGN(NLA_HDRLEN); + ctx->last_pop_priority = -1; + } +} + +static void +add_pop_action(struct action_xlate_ctx *ctx) +{ + if (ctx->odp_actions->size != ctx->last_pop_priority) { + nl_msg_put_flag(ctx->odp_actions, ODPAT_POP_PRIORITY); + ctx->last_pop_priority = ctx->odp_actions->size; } } @@ -2811,10 +2812,9 @@ xlate_enqueue_action(struct action_xlate_ctx *ctx, /* Add ODP actions. */ remove_pop_action(ctx); - odp_actions_add(&ctx->out, ODPAT_SET_PRIORITY)->priority.priority - = priority; + nl_msg_put_u32(ctx->odp_actions, ODPAT_SET_PRIORITY, priority); add_output_action(ctx, odp_port); - odp_actions_add(&ctx->out, ODPAT_POP_PRIORITY); + add_pop_action(ctx); /* Update NetFlow output port. */ if (ctx->nf_output_iface == NF_OUT_DROP) { @@ -2840,8 +2840,7 @@ xlate_set_queue_action(struct action_xlate_ctx *ctx, } remove_pop_action(ctx); - odp_actions_add(&ctx->out, ODPAT_SET_PRIORITY)->priority.priority - = priority; + nl_msg_put_u32(ctx->odp_actions, ODPAT_SET_PRIORITY, priority); } static void @@ -2849,10 +2848,10 @@ xlate_set_dl_tci(struct action_xlate_ctx *ctx) { ovs_be16 tci = ctx->flow.vlan_tci; if (!(tci & htons(VLAN_CFI))) { - odp_actions_add(&ctx->out, ODPAT_STRIP_VLAN); + nl_msg_put_flag(ctx->odp_actions, ODPAT_STRIP_VLAN); } else { - union odp_action *oa = odp_actions_add(&ctx->out, ODPAT_SET_DL_TCI); - oa->dl_tci.tci = tci & ~htons(VLAN_CFI); + nl_msg_put_be16(ctx->odp_actions, ODPAT_SET_DL_TCI, + tci & ~htons(VLAN_CFI)); } } @@ -2876,7 +2875,6 @@ xlate_nicira_action(struct action_xlate_ctx *ctx, const struct nx_action_resubmit *nar; const struct nx_action_set_tunnel *nast; const struct nx_action_set_queue *nasq; - union odp_action *oa; enum nx_action_subtype subtype = ntohs(nah->subtype); assert(nah->vendor == htonl(NX_VENDOR_ID)); @@ -2888,13 +2886,13 @@ xlate_nicira_action(struct action_xlate_ctx *ctx, case NXAST_SET_TUNNEL: nast = (const struct nx_action_set_tunnel *) nah; - oa = odp_actions_add(&ctx->out, ODPAT_SET_TUNNEL); - ctx->flow.tun_id = oa->tunnel.tun_id = nast->tun_id; + nl_msg_put_be32(ctx->odp_actions, ODPAT_SET_TUNNEL, nast->tun_id); + ctx->flow.tun_id = nast->tun_id; break; case NXAST_DROP_SPOOFED_ARP: if (ctx->flow.dl_type == htons(ETH_TYPE_ARP)) { - odp_actions_add(&ctx->out, ODPAT_DROP_SPOOFED_ARP); + nl_msg_put_flag(ctx->odp_actions, ODPAT_DROP_SPOOFED_ARP); } break; @@ -2904,7 +2902,7 @@ xlate_nicira_action(struct action_xlate_ctx *ctx, break; case NXAST_POP_QUEUE: - odp_actions_add(&ctx->out, ODPAT_POP_PRIORITY); + add_pop_action(ctx); break; case NXAST_REG_MOVE: @@ -2947,7 +2945,7 @@ do_xlate_actions(const union ofp_action *in, size_t n_in, for (ia = actions_first(&iter, in, n_in); ia; ia = actions_next(&iter)) { enum ofp_action_type type = ntohs(ia->type); - union odp_action *oa; + const struct ofp_action_dl_addr *oada; switch (type) { case OFPAT_OUTPUT: @@ -2973,44 +2971,47 @@ do_xlate_actions(const union ofp_action *in, size_t n_in, break; case OFPAT_SET_DL_SRC: - oa = odp_actions_add(&ctx->out, ODPAT_SET_DL_SRC); - memcpy(oa->dl_addr.dl_addr, - ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); - memcpy(ctx->flow.dl_src, - ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); + oada = ((struct ofp_action_dl_addr *) ia); + nl_msg_put_unspec(ctx->odp_actions, ODPAT_SET_DL_SRC, + oada->dl_addr, ETH_ADDR_LEN); + memcpy(ctx->flow.dl_src, oada->dl_addr, ETH_ADDR_LEN); break; case OFPAT_SET_DL_DST: - oa = odp_actions_add(&ctx->out, ODPAT_SET_DL_DST); - memcpy(oa->dl_addr.dl_addr, - ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); - memcpy(ctx->flow.dl_dst, - ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN); + oada = ((struct ofp_action_dl_addr *) ia); + nl_msg_put_unspec(ctx->odp_actions, ODPAT_SET_DL_DST, + oada->dl_addr, ETH_ADDR_LEN); + memcpy(ctx->flow.dl_dst, oada->dl_addr, ETH_ADDR_LEN); break; case OFPAT_SET_NW_SRC: - oa = odp_actions_add(&ctx->out, ODPAT_SET_NW_SRC); - ctx->flow.nw_src = oa->nw_addr.nw_addr = ia->nw_addr.nw_addr; + nl_msg_put_be32(ctx->odp_actions, ODPAT_SET_NW_SRC, + ia->nw_addr.nw_addr); + ctx->flow.nw_src = ia->nw_addr.nw_addr; break; case OFPAT_SET_NW_DST: - oa = odp_actions_add(&ctx->out, ODPAT_SET_NW_DST); - ctx->flow.nw_dst = oa->nw_addr.nw_addr = ia->nw_addr.nw_addr; + nl_msg_put_be32(ctx->odp_actions, ODPAT_SET_NW_DST, + ia->nw_addr.nw_addr); + ctx->flow.nw_dst = ia->nw_addr.nw_addr; break; case OFPAT_SET_NW_TOS: - oa = odp_actions_add(&ctx->out, ODPAT_SET_NW_TOS); - ctx->flow.nw_tos = oa->nw_tos.nw_tos = ia->nw_tos.nw_tos; + nl_msg_put_u8(ctx->odp_actions, ODPAT_SET_NW_TOS, + ia->nw_tos.nw_tos); + ctx->flow.nw_tos = ia->nw_tos.nw_tos; break; case OFPAT_SET_TP_SRC: - oa = odp_actions_add(&ctx->out, ODPAT_SET_TP_SRC); - ctx->flow.tp_src = oa->tp_port.tp_port = ia->tp_port.tp_port; + nl_msg_put_be16(ctx->odp_actions, ODPAT_SET_TP_SRC, + ia->tp_port.tp_port); + ctx->flow.tp_src = ia->tp_port.tp_port; break; case OFPAT_SET_TP_DST: - oa = odp_actions_add(&ctx->out, ODPAT_SET_TP_DST); - ctx->flow.tp_dst = oa->tp_port.tp_port = ia->tp_port.tp_port; + nl_msg_put_be16(ctx->odp_actions, ODPAT_SET_TP_DST, + ia->tp_port.tp_port); + ctx->flow.tp_dst = ia->tp_port.tp_port; break; case OFPAT_VENDOR: @@ -3039,31 +3040,29 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx, ctx->resubmit_hook = NULL; } -static int +static struct ofpbuf * xlate_actions(struct action_xlate_ctx *ctx, const union ofp_action *in, size_t n_in) { COVERAGE_INC(ofproto_ofp2odp); - odp_actions_init(&ctx->out); + + ctx->odp_actions = ofpbuf_new(512); ctx->tags = 0; ctx->may_set_up_flow = true; ctx->nf_output_iface = NF_OUT_DROP; ctx->recurse = 0; + ctx->last_pop_priority = -1; do_xlate_actions(in, n_in, ctx); remove_pop_action(ctx); /* Check with in-band control to see if we're allowed to set up this * flow. */ - if (!in_band_rule_check(ctx->ofproto->in_band, &ctx->flow, &ctx->out)) { + if (!in_band_rule_check(ctx->ofproto->in_band, &ctx->flow, + ctx->odp_actions->data, ctx->odp_actions->size)) { ctx->may_set_up_flow = false; } - if (odp_actions_overflow(&ctx->out)) { - COVERAGE_INC(odp_overflow); - odp_actions_init(&ctx->out); - return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_TOO_MANY); - } - return 0; + return ctx->odp_actions; } /* Checks whether 'ofconn' is a slave controller. If so, returns an OpenFlow @@ -3093,6 +3092,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh) struct ofpbuf payload, *buffer; union ofp_action *ofp_actions; struct action_xlate_ctx ctx; + struct ofpbuf *odp_actions; struct ofpbuf request; struct flow flow; size_t n_ofp_actions; @@ -3140,10 +3140,9 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh) /* Send. */ action_xlate_ctx_init(&ctx, p, &flow, &payload); - error = xlate_actions(&ctx, ofp_actions, n_ofp_actions); - if (!error) { - dpif_execute(p->dpif, ctx.out.actions, ctx.out.n_actions, &payload); - } + odp_actions = xlate_actions(&ctx, ofp_actions, n_ofp_actions); + dpif_execute(p->dpif, odp_actions->data, odp_actions->size, &payload); + ofpbuf_delete(odp_actions); exit: ofpbuf_delete(buffer); @@ -4321,12 +4320,12 @@ handle_odp_miss_msg(struct ofproto *p, struct ofpbuf *packet) /* Check with in-band control to see if this packet should be sent * to the local port regardless of the flow table. */ if (in_band_msg_in_hook(p->in_band, &flow, &payload)) { - union odp_action action; + struct ofpbuf odp_actions; - memset(&action, 0, sizeof(action)); - action.output.type = ODPAT_OUTPUT; - action.output.port = ODPP_LOCAL; - dpif_execute(p->dpif, &action, 1, &payload); + ofpbuf_init(&odp_actions, 32); + nl_msg_put_u32(&odp_actions, ODPAT_OUTPUT, ODPP_LOCAL); + dpif_execute(p->dpif, odp_actions.data, odp_actions.size, &payload); + ofpbuf_uninit(&odp_actions); } facet = facet_lookup_valid(p, &flow); @@ -5043,18 +5042,20 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_, trace_format_rule(&result, 0, rule); if (rule) { struct ofproto_trace trace; + struct ofpbuf *odp_actions; trace.result = &result; trace.flow = flow; action_xlate_ctx_init(&trace.ctx, ofproto, &flow, &packet); trace.ctx.resubmit_hook = trace_resubmit; - xlate_actions(&trace.ctx, rule->actions, rule->n_actions); + odp_actions = xlate_actions(&trace.ctx, + rule->actions, rule->n_actions); ds_put_char(&result, '\n'); trace_format_flow(&result, 0, "Final flow", &trace); ds_put_cstr(&result, "Datapath actions: "); - format_odp_actions(&result, - trace.ctx.out.actions, trace.ctx.out.n_actions); + format_odp_actions(&result, odp_actions->data, odp_actions->size); + ofpbuf_delete(odp_actions); } unixctl_command_reply(conn, 200, ds_cstr(&result)); @@ -5080,7 +5081,7 @@ ofproto_unixctl_init(void) static bool default_normal_ofhook_cb(const struct flow *flow, const struct ofpbuf *packet, - struct odp_actions *actions, tag_type *tags, + struct ofpbuf *odp_actions, tag_type *tags, uint16_t *nf_output_iface, void *ofproto_) { struct ofproto *ofproto = ofproto_; @@ -5111,9 +5112,9 @@ default_normal_ofhook_cb(const struct flow *flow, const struct ofpbuf *packet, NULL); if (out_port < 0) { flood_packets(ofproto, flow->in_port, OFPPC_NO_FLOOD, - nf_output_iface, actions); + nf_output_iface, odp_actions); } else if (out_port != flow->in_port) { - odp_actions_add(actions, ODPAT_OUTPUT)->output.port = out_port; + nl_msg_put_u32(odp_actions, ODPAT_OUTPUT, out_port); *nf_output_iface = out_port; } else { /* Drop. */ diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index fd089c5e..eeaeb6f7 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -31,7 +31,7 @@ extern "C" { #endif struct cls_rule; -struct odp_actions; +struct nlattr; struct ofhooks; struct ofproto; struct svec; @@ -136,10 +136,11 @@ void ofproto_flush_flows(struct ofproto *); /* Hooks for ovs-vswitchd. */ struct ofhooks { bool (*normal_cb)(const struct flow *, const struct ofpbuf *packet, - struct odp_actions *, tag_type *, + struct ofpbuf *odp_actions, tag_type *, uint16_t *nf_output_iface, void *aux); void (*account_flow_cb)(const struct flow *, tag_type tags, - const union odp_action *, size_t n_actions, + const struct nlattr *odp_actions, + size_t actions_len, unsigned long long int n_bytes, void *aux); void (*account_checkpoint_cb)(void *aux); }; diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c index bed50faa..8f2a2bc6 100644 --- a/utilities/ovs-dpctl.c +++ b/utilities/ovs-dpctl.c @@ -478,11 +478,11 @@ do_dump_flows(int argc OVS_UNUSED, char *argv[]) ds_init(&ds); for (i = 0; i < n_flows; i++) { struct odp_flow *f = &flows[i]; - enum { MAX_ACTIONS = 4096 / sizeof(union odp_action) }; - union odp_action actions[MAX_ACTIONS]; + enum { MAX_ACTIONS = 4096 }; /* An arbitrary but large number. */ + struct nlattr actions[MAX_ACTIONS]; f->actions = actions; - f->n_actions = MAX_ACTIONS; + f->actions_len = sizeof actions; if (!dpif_flow_get(dpif, f)) { ds_clear(&ds); format_odp_flow(&ds, f); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index c0984361..8ade873f 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -45,6 +45,7 @@ #include "list.h" #include "mac-learning.h" #include "netdev.h" +#include "netlink.h" #include "odp-util.h" #include "ofp-print.h" #include "ofpbuf.h" @@ -2424,7 +2425,7 @@ print_dsts(const struct dst *dsts, size_t n) static void compose_actions(struct bridge *br, const struct flow *flow, uint16_t vlan, const struct port *in_port, const struct port *out_port, - tag_type *tags, struct odp_actions *actions, + tag_type *tags, struct ofpbuf *actions, uint16_t *nf_output_iface) { struct dst dsts[DP_MAX_PORTS * (MAX_MIRRORS + 1)]; @@ -2440,19 +2441,18 @@ compose_actions(struct bridge *br, const struct flow *flow, uint16_t vlan, cur_vlan = OFP_VLAN_NONE; } for (p = dsts; p < &dsts[n_dsts]; p++) { - union odp_action *a; if (p->vlan != cur_vlan) { if (p->vlan == OFP_VLAN_NONE) { - odp_actions_add(actions, ODPAT_STRIP_VLAN); + nl_msg_put_flag(actions, ODPAT_STRIP_VLAN); } else { - a = odp_actions_add(actions, ODPAT_SET_DL_TCI); - a->dl_tci.tci = htons(p->vlan & VLAN_VID_MASK); - a->dl_tci.tci |= flow->vlan_tci & htons(VLAN_PCP_MASK); + ovs_be16 tci; + tci = htons(p->vlan & VLAN_VID_MASK); + tci |= flow->vlan_tci & htons(VLAN_PCP_MASK); + nl_msg_put_be16(actions, ODPAT_SET_DL_TCI, tci); } cur_vlan = p->vlan; } - a = odp_actions_add(actions, ODPAT_OUTPUT); - a->output.port = p->dp_ifidx; + nl_msg_put_u32(actions, ODPAT_OUTPUT, p->dp_ifidx); } } @@ -2645,7 +2645,7 @@ is_admissible(struct bridge *br, const struct flow *flow, bool have_packet, * not at all, if 'packet' was NULL. */ static bool process_flow(struct bridge *br, const struct flow *flow, - const struct ofpbuf *packet, struct odp_actions *actions, + const struct ofpbuf *packet, struct ofpbuf *actions, tag_type *tags, uint16_t *nf_output_iface) { struct port *in_port; @@ -2696,7 +2696,7 @@ done: static bool bridge_normal_ofhook_cb(const struct flow *flow, const struct ofpbuf *packet, - struct odp_actions *actions, tag_type *tags, + struct ofpbuf *actions, tag_type *tags, uint16_t *nf_output_iface, void *br_) { struct iface *iface; @@ -2718,14 +2718,15 @@ bridge_normal_ofhook_cb(const struct flow *flow, const struct ofpbuf *packet, static void bridge_account_flow_ofhook_cb(const struct flow *flow, tag_type tags, - const union odp_action *actions, - size_t n_actions, unsigned long long int n_bytes, - void *br_) + const struct nlattr *actions, + unsigned int actions_len, + unsigned long long int n_bytes, void *br_) { struct bridge *br = br_; - const union odp_action *a; + const struct nlattr *a; struct port *in_port; tag_type dummy = 0; + unsigned int left; int vlan; /* Feed information from the active flows back into the learning table to @@ -2743,9 +2744,9 @@ bridge_account_flow_ofhook_cb(const struct flow *flow, tag_type tags, if (!br->has_bonded_ports) { return; } - for (a = actions; a < &actions[n_actions]; a++) { - if (a->type == ODPAT_OUTPUT) { - struct port *out_port = port_from_dp_ifidx(br, a->output.port); + NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) { + if (nl_attr_type(a) == ODPAT_OUTPUT) { + struct port *out_port = port_from_dp_ifidx(br, nl_attr_get_u32(a)); if (out_port && out_port->n_ifaces >= 2) { uint16_t vlan = (flow->vlan_tci ? vlan_tci_to_vid(flow->vlan_tci) -- 2.30.2