Fixed according to comments from Jesse and Ben. --8<--------------------------cut here-------------------------->8--
Following patch adds skb-priority to flow key. So userspace will know what was priority when packet arrived and we can remove the pop/reset priority action. It's no longer necessary to have a special action for pop that is based on the kernel remembering original skb->priority. Userspace can just emit a set priority action with the original value. Since the priority field is a match field with just a normal set action, we can convert it into the new model for actions that are based on matches. Signed-off-by: Pravin B Shelar <[email protected]> Bug #7715 --- datapath/actions.c | 13 +++-------- datapath/datapath.c | 18 ++++++---------- datapath/flow.c | 45 +++++++++++++++++++++++++++++++----------- datapath/flow.h | 14 ++++++++---- include/linux/openvswitch.h | 6 +---- lib/classifier.c | 1 + lib/dpif-linux.c | 2 +- lib/dpif-netdev.c | 11 +++------ lib/dpif-provider.h | 3 +- lib/dpif.c | 6 ++-- lib/flow.c | 4 ++- lib/flow.h | 10 +++++--- lib/learning-switch.c | 2 +- lib/odp-util.c | 35 +++++++++++++++++++++++++------- lib/odp-util.h | 7 +++-- lib/ofp-print.c | 2 +- ofproto/ofproto-dpif.c | 43 +++++++++++++++++++---------------------- ofproto/ofproto.c | 4 +- tests/odp.at | 4 +++ tests/test-flows.c | 2 +- 20 files changed, 133 insertions(+), 99 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index 8ca243d..4db2563 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -314,6 +314,10 @@ static int execute_set_action(struct sk_buff *skb, int err = 0; switch (nla_type(nested_attr)) { + case OVS_KEY_ATTR_PRIORITY: + skb->priority = nla_get_u32(nested_attr); + break; + case OVS_KEY_ATTR_TUN_ID: OVS_CB(skb)->tun_id = nla_get_be64(nested_attr); break; @@ -347,7 +351,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, * then freeing the original skbuff is wasteful. So the following code * is slightly obscure just to avoid that. */ int prev_port = -1; - u32 priority = skb->priority; const struct nlattr *a; int rem; @@ -385,14 +388,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, err = execute_set_action(skb, nla_data(a)); break; - case OVS_ACTION_ATTR_SET_PRIORITY: - skb->priority = nla_get_u32(a); - break; - - case OVS_ACTION_ATTR_POP_PRIORITY: - skb->priority = priority; - break; - case OVS_ACTION_ATTR_SAMPLE: err = sample(dp, skb, a); break; diff --git a/datapath/datapath.c b/datapath/datapath.c index be90d54..87056cf 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -558,6 +558,7 @@ static int validate_action_key(const struct nlattr *a, const struct ovs_key_ipv4 *ipv4_key; const struct ovs_key_8021q *q_key; + case ACTION(OVS_ACTION_ATTR_SET, OVS_KEY_ATTR_PRIORITY): case ACTION(OVS_ACTION_ATTR_SET, OVS_KEY_ATTR_TUN_ID): case ACTION(OVS_ACTION_ATTR_SET, OVS_KEY_ATTR_ETHERNET): break; @@ -652,8 +653,6 @@ static int validate_actions(const struct nlattr *attr, [OVS_ACTION_ATTR_PUSH] = (u32)-1, [OVS_ACTION_ATTR_POP] = 2, [OVS_ACTION_ATTR_SET] = (u32)-1, - [OVS_ACTION_ATTR_SET_PRIORITY] = 4, - [OVS_ACTION_ATTR_POP_PRIORITY] = 0, [OVS_ACTION_ATTR_SAMPLE] = (u32)-1 }; int type = nla_type(a); @@ -667,11 +666,6 @@ static int validate_actions(const struct nlattr *attr, case OVS_ACTION_ATTR_UNSPEC: return -EINVAL; - case OVS_ACTION_ATTR_SET_PRIORITY: - case OVS_ACTION_ATTR_POP_PRIORITY: - /* No validation needed. */ - break; - case OVS_ACTION_ATTR_USERSPACE: err = validate_userspace(a); if (err) @@ -770,8 +764,9 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info) if (err) goto err_flow_put; - err = flow_metadata_from_nlattrs(&flow->key.eth.in_port, - &flow->key.eth.tun_id, + err = flow_metadata_from_nlattrs(&flow->key.phy.priority, + &flow->key.phy.in_port, + &flow->key.phy.tun_id, a[OVS_PACKET_ATTR_KEY]); if (err) goto err_flow_put; @@ -789,6 +784,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info) rcu_assign_pointer(flow->sf_acts, acts); OVS_CB(packet)->flow = flow; + packet->priority = flow->key.phy.priority; rcu_read_lock(); dp = get_dp(ovs_header->dp_ifindex); @@ -796,9 +792,9 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info) if (!dp) goto err_unlock; - if (flow->key.eth.in_port < DP_MAX_PORTS) + if (flow->key.phy.in_port < DP_MAX_PORTS) OVS_CB(packet)->vport = get_vport_protected(dp, - flow->key.eth.in_port); + flow->key.phy.in_port); local_bh_disable(); err = execute_actions(dp, packet); diff --git a/datapath/flow.c b/datapath/flow.c index b6023a0..0084ca2 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -638,8 +638,10 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key, struct ethhdr *eth; memset(key, 0, sizeof(*key)); - key->eth.tun_id = OVS_CB(skb)->tun_id; - key->eth.in_port = in_port; + + key->phy.priority = skb->priority; + key->phy.tun_id = OVS_CB(skb)->tun_id; + key->phy.in_port = in_port; skb_reset_mac_header(skb); @@ -849,6 +851,7 @@ static int parse_tos_frag(struct sw_flow_key *swkey, u8 tos, u8 frag) /* The size of the argument for each %OVS_KEY_ATTR_* Netlink attribute. */ const u32 ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = { + [OVS_KEY_ATTR_PRIORITY] = 4, [OVS_KEY_ATTR_TUN_ID] = 8, [OVS_KEY_ATTR_IN_PORT] = 4, [OVS_KEY_ATTR_ETHERNET] = sizeof(struct ovs_key_ethernet), @@ -874,7 +877,7 @@ const u32 ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = { * This state machine accepts the following forms, with [] for optional * elements and | for alternatives: * - * [tun_id] [in_port] ethernet [8021q] [ethertype \ + * [priority] [tun_id] [in_port] ethernet [8021q] [ethertype \ * [IPv4 [TCP|UDP|ICMP] | IPv6 [TCP|UDP|ICMPv6 [ND]] | ARP]] * * except that IPv4 or IPv6 terminates the sequence if its @ipv4_frag or @@ -891,7 +894,7 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp, int key_len; memset(swkey, 0, sizeof(*swkey)); - swkey->eth.in_port = USHRT_MAX; + swkey->phy.in_port = USHRT_MAX; swkey->eth.type = htons(ETH_P_802_2); key_len = SW_FLOW_KEY_OFFSET(eth); @@ -915,18 +918,25 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp, #define TRANSITION(PREV_TYPE, TYPE) (((PREV_TYPE) << 16) | (TYPE)) switch (TRANSITION(prev_type, type)) { + case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_PRIORITY): + swkey->phy.priority = nla_get_u32(nla); + break; + case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_TUN_ID): - swkey->eth.tun_id = nla_get_be64(nla); + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_TUN_ID): + swkey->phy.tun_id = nla_get_be64(nla); break; case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_IN_PORT): + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_IN_PORT): case TRANSITION(OVS_KEY_ATTR_TUN_ID, OVS_KEY_ATTR_IN_PORT): if (nla_get_u32(nla) >= DP_MAX_PORTS) goto invalid; - swkey->eth.in_port = nla_get_u32(nla); + swkey->phy.in_port = nla_get_u32(nla); break; case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_ETHERNET): + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_ETHERNET): case TRANSITION(OVS_KEY_ATTR_TUN_ID, OVS_KEY_ATTR_ETHERNET): case TRANSITION(OVS_KEY_ATTR_IN_PORT, OVS_KEY_ATTR_ETHERNET): eth_key = nla_data(nla); @@ -1073,6 +1083,7 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp, case OVS_KEY_ATTR_UNSPEC: goto invalid; + case OVS_KEY_ATTR_PRIORITY: case OVS_KEY_ATTR_TUN_ID: case OVS_KEY_ATTR_IN_PORT: goto invalid; @@ -1148,7 +1159,7 @@ ok: * get the metadata, that is, the parts of the flow key that cannot be * extracted from the packet itself. */ -int flow_metadata_from_nlattrs(u16 *in_port, __be64 *tun_id, +int flow_metadata_from_nlattrs(u32 *priority, u16 *in_port, __be64 *tun_id, const struct nlattr *attr) { const struct nlattr *nla; @@ -1157,6 +1168,7 @@ int flow_metadata_from_nlattrs(u16 *in_port, __be64 *tun_id, *in_port = USHRT_MAX; *tun_id = 0; + *priority = 0; prev_type = OVS_KEY_ATTR_UNSPEC; nla_for_each_nested(nla, attr, rem) { @@ -1166,11 +1178,17 @@ int flow_metadata_from_nlattrs(u16 *in_port, __be64 *tun_id, return -EINVAL; switch (TRANSITION(prev_type, type)) { + case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_PRIORITY): + *priority = nla_get_u32(nla); + break; + case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_TUN_ID): + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_TUN_ID): *tun_id = nla_get_be64(nla); break; case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_IN_PORT): + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_IN_PORT): case TRANSITION(OVS_KEY_ATTR_TUN_ID, OVS_KEY_ATTR_IN_PORT): if (nla_get_u32(nla) >= DP_MAX_PORTS) return -EINVAL; @@ -1196,13 +1214,16 @@ int flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb) /* This is an imperfect sanity-check that FLOW_BUFSIZE doesn't need * to be updated, but will at least raise awareness when new * datapath key types are added. */ - BUILD_BUG_ON(__OVS_KEY_ATTR_MAX != 14); + BUILD_BUG_ON(__OVS_KEY_ATTR_MAX != 15); + + if (swkey->phy.priority) + NLA_PUT_U32(skb, OVS_KEY_ATTR_PRIORITY, swkey->phy.priority); - if (swkey->eth.tun_id != cpu_to_be64(0)) - NLA_PUT_BE64(skb, OVS_KEY_ATTR_TUN_ID, swkey->eth.tun_id); + if (swkey->phy.tun_id != cpu_to_be64(0)) + NLA_PUT_BE64(skb, OVS_KEY_ATTR_TUN_ID, swkey->phy.tun_id); - if (swkey->eth.in_port != USHRT_MAX) - NLA_PUT_U32(skb, OVS_KEY_ATTR_IN_PORT, swkey->eth.in_port); + if (swkey->phy.in_port != USHRT_MAX) + NLA_PUT_U32(skb, OVS_KEY_ATTR_IN_PORT, swkey->phy.in_port); nla = nla_reserve(skb, OVS_KEY_ATTR_ETHERNET, sizeof(*eth_key)); if (!nla) diff --git a/datapath/flow.h b/datapath/flow.h index 484ea62..e68269e 100644 --- a/datapath/flow.h +++ b/datapath/flow.h @@ -36,8 +36,11 @@ struct sw_flow_actions { struct sw_flow_key { struct { - __be64 tun_id; /* Encapsulating tunnel ID. */ - u16 in_port; /* Input switch port (or USHRT_MAX). */ + __be64 tun_id; /* Encapsulating tunnel ID. */ + u32 priority; /* Packet QoS priority. */ + u16 in_port; /* Input switch port (or USHRT_MAX). */ + } phy; + struct { u8 src[ETH_ALEN]; /* Ethernet source address. */ u8 dst[ETH_ALEN]; /* Ethernet destination address. */ __be16 tci; /* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */ @@ -138,6 +141,7 @@ u64 flow_used_time(unsigned long flow_jiffies); * * struct pad nl hdr total * ------ --- ------ ----- + * OVS_KEY_ATTR_PRIORITY 4 -- 4 8 * OVS_KEY_ATTR_TUN_ID 8 -- 4 12 * OVS_KEY_ATTR_IN_PORT 4 -- 4 8 * OVS_KEY_ATTR_ETHERNET 12 -- 4 16 @@ -147,14 +151,14 @@ u64 flow_used_time(unsigned long flow_jiffies); * OVS_KEY_ATTR_ICMPV6 2 2 4 8 * OVS_KEY_ATTR_ND 28 -- 4 32 * ------------------------------------------------- - * total 132 + * total 140 */ -#define FLOW_BUFSIZE 132 +#define FLOW_BUFSIZE 140 int flow_to_nlattrs(const struct sw_flow_key *, struct sk_buff *); int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp, const struct nlattr *); -int flow_metadata_from_nlattrs(u16 *in_port, __be64 *tun_id, +int flow_metadata_from_nlattrs(u32 *priority, u16 *in_port, __be64 *tun_id, const struct nlattr *); #define TBL_MIN_BUCKETS 1024 diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h index 087655c..57ce4ef 100644 --- a/include/linux/openvswitch.h +++ b/include/linux/openvswitch.h @@ -263,6 +263,7 @@ struct ovs_flow_stats { enum ovs_key_attr { OVS_KEY_ATTR_UNSPEC, + OVS_KEY_ATTR_PRIORITY, /* 32-bit skb->priority */ OVS_KEY_ATTR_TUN_ID, /* 64-bit tunnel ID */ OVS_KEY_ATTR_IN_PORT, /* 32-bit OVS dp port number */ OVS_KEY_ATTR_ETHERNET, /* struct ovs_key_ethernet */ @@ -449,9 +450,6 @@ enum ovs_userspace_attr { * header. * @OVS_ACTION_ATTR_SET: Replaces the contents of an existing header. * The argument takes the same form as %OVS_ACTION_ATTR_PUSH. - * @OVS_ACTION_ATTR_SET_PRIORITY: Sets skb->priority to 32-bit number passed - * as argument. - * @OVS_ACTION_ATTR_POP_PRIORITY: Restore skb->priority to original value. * @OVS_ACTION_ATTR_SAMPLE: Probabilitically executes actions, as specified in * the nested %OVS_SAMPLE_ATTR_* attributes. * @@ -466,8 +464,6 @@ enum ovs_action_attr { OVS_ACTION_ATTR_PUSH, /* One nested OVS_KEY_ATTR_*. */ OVS_ACTION_ATTR_POP, /* u16 OVS_KEY_ATTR_*. */ OVS_ACTION_ATTR_SET, /* One nested OVS_KEY_ATTR_*. */ - OVS_ACTION_ATTR_SET_PRIORITY, /* u32 skb->priority value. */ - OVS_ACTION_ATTR_POP_PRIORITY, /* No argument. */ OVS_ACTION_ATTR_SAMPLE, /* Nested OVS_SAMPLE_ATTR_*. */ __OVS_ACTION_ATTR_MAX }; diff --git a/lib/classifier.c b/lib/classifier.c index 869029f..8ffc96f 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -83,6 +83,7 @@ cls_rule_init_exact(const struct flow *flow, unsigned int priority, struct cls_rule *rule) { rule->flow = *flow; + rule->flow.priority = 0; flow_wildcards_init_exact(&rule->wc); rule->priority = priority; } diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index c5628b7..634031d 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -1275,7 +1275,7 @@ dpif_linux_vport_send(int dp_ifindex, uint32_t port_no, uint64_t action; ofpbuf_use_const(&packet, data, size); - flow_extract(&packet, htonll(0), 0, &flow); + flow_extract(&packet, htonll(0), 0, 0, &flow); ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); odp_flow_key_from_flow(&key, &flow); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 1d05dd8..fc15a64 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -881,7 +881,7 @@ dpif_netdev_execute(struct dpif *dpif, ofpbuf_reserve(©, DP_NETDEV_HEADROOM); ofpbuf_put(©, packet->data, packet->size); - flow_extract(©, 0, -1, &key); + flow_extract(©, 0, -1, 0, &key); error = dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key); if (!error) { dp_netdev_execute_actions(dp, ©, &key, @@ -981,7 +981,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port, if (packet->size < ETH_HEADER_LEN) { return; } - flow_extract(packet, 0, port->port_no, &key); + flow_extract(packet, 0, port->port_no, 0, &key); flow = dp_netdev_lookup_flow(dp, &key); if (flow) { dp_netdev_flow_used(flow, &key, packet); @@ -1239,6 +1239,8 @@ execute_set_action(struct ofpbuf *packet, const struct nlattr *a) enum ovs_key_attr type = nl_attr_type(a); switch (type) { case OVS_KEY_ATTR_TUN_ID: + case OVS_KEY_ATTR_PRIORITY: + /* not implemented */ break; case OVS_KEY_ATTR_ETHERNET: @@ -1319,11 +1321,6 @@ dp_netdev_execute_actions(struct dp_netdev *dp, dp_netdev_sample(dp, packet, key, a); break; - case OVS_ACTION_ATTR_SET_PRIORITY: - case OVS_ACTION_ATTR_POP_PRIORITY: - /* not implemented */ - break; - case OVS_ACTION_ATTR_UNSPEC: case __OVS_ACTION_ATTR_MAX: NOT_REACHED(); diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 83d56d6..429cc9d 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -318,8 +318,7 @@ struct dpif_class { int (*recv_set_mask)(struct dpif *dpif, int listen_mask); /* Translates OpenFlow queue ID 'queue_id' (in host byte order) into a - * priority value for use in the OVS_ACTION_ATTR_SET_PRIORITY action in - * '*priority'. */ + * priority value used for setting packet priority. */ int (*queue_to_priority)(const struct dpif *dpif, uint32_t queue_id, uint32_t *priority); diff --git a/lib/dpif.c b/lib/dpif.c index 68a95f6..cc6e805 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1129,9 +1129,9 @@ dpif_get_netflow_ids(const struct dpif *dpif, } /* Translates OpenFlow queue ID 'queue_id' (in host byte order) into a priority - * value for use in the OVS_ACTION_ATTR_SET_PRIORITY action. On success, - * returns 0 and stores the priority into '*priority'. On failure, returns a - * positive errno value and stores 0 into '*priority'. */ + * value used for setting packet priority. + * On success, returns 0 and stores the priority into '*priority'. + * On failure, returns a positive errno value and stores 0 into '*priority'. */ int dpif_queue_to_priority(const struct dpif *dpif, uint32_t queue_id, uint32_t *priority) diff --git a/lib/flow.c b/lib/flow.c index 06cc822..2785554 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -325,7 +325,7 @@ invalid: */ void flow_extract(struct ofpbuf *packet, ovs_be64 tun_id, uint16_t ofp_in_port, - struct flow *flow) + uint32_t priority, struct flow *flow) { struct ofpbuf b = *packet; struct eth_header *eth; @@ -335,6 +335,7 @@ flow_extract(struct ofpbuf *packet, ovs_be64 tun_id, uint16_t ofp_in_port, memset(flow, 0, sizeof *flow); flow->tun_id = tun_id; flow->in_port = ofp_in_port; + flow->priority = priority; packet->l2 = b.data; packet->l3 = NULL; @@ -484,6 +485,7 @@ flow_zero_wildcards(struct flow *flow, const struct flow_wildcards *wildcards) if (wc & FWW_ND_TARGET) { memset(&flow->nd_target, 0, sizeof flow->nd_target); } + flow->priority = 0; } char * diff --git a/lib/flow.h b/lib/flow.h index e9da2ad..f190e71 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -54,6 +54,7 @@ BUILD_ASSERT_DECL(FLOW_FRAG_LATER == NX_IP_FRAG_LATER); struct flow { ovs_be64 tun_id; /* Encapsulating tunnel ID. */ + uint32_t priority; /* Packet priority for QoS. */ uint32_t regs[FLOW_N_REGS]; /* Registers. */ ovs_be32 nw_src; /* IPv4 source address. */ ovs_be32 nw_dst; /* IPv4 destination address. */ @@ -71,21 +72,22 @@ struct flow { struct in6_addr ipv6_src; /* IPv6 source address. */ struct in6_addr ipv6_dst; /* IPv6 destination address. */ struct in6_addr nd_target; /* IPv6 neighbor discovery (ND) target. */ + uint32_t reserved; /* Reserved for 64-bit packing. */ }; /* Assert that there are FLOW_SIG_SIZE bytes of significant data in "struct * flow", followed by FLOW_PAD_SIZE bytes of padding. */ -#define FLOW_SIG_SIZE (100 + FLOW_N_REGS * 4) -#define FLOW_PAD_SIZE 0 +#define FLOW_SIG_SIZE (104 + FLOW_N_REGS * 4) +#define FLOW_PAD_SIZE 4 BUILD_ASSERT_DECL(offsetof(struct flow, nd_target) == FLOW_SIG_SIZE - 16); BUILD_ASSERT_DECL(sizeof(((struct flow *)0)->nd_target) == 16); BUILD_ASSERT_DECL(sizeof(struct flow) == FLOW_SIG_SIZE + FLOW_PAD_SIZE); /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */ -BUILD_ASSERT_DECL(FLOW_SIG_SIZE == 120 && FLOW_WC_SEQ == 3); +BUILD_ASSERT_DECL(FLOW_SIG_SIZE == 124 && FLOW_WC_SEQ == 3); void flow_extract(struct ofpbuf *, ovs_be64 tun_id, uint16_t in_port, - struct flow *); + uint32_t priority, struct flow *); void flow_zero_wildcards(struct flow *, const struct flow_wildcards *); char *flow_to_string(const struct flow *); diff --git a/lib/learning-switch.c b/lib/learning-switch.c index b260862..5b6b56a 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -419,7 +419,7 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn, pkt_ofs = offsetof(struct ofp_packet_in, data); pkt_len = ntohs(opi->header.length) - pkt_ofs; ofpbuf_use_const(&pkt, opi->data, pkt_len); - flow_extract(&pkt, 0, in_port, &flow); + flow_extract(&pkt, 0, in_port, 0, &flow); /* Choose output port. */ out_port = lswitch_choose_destination(sw, &flow); diff --git a/lib/odp-util.c b/lib/odp-util.c index 33672c8..fa2f34c 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -64,8 +64,6 @@ odp_action_len(uint16_t type) case OVS_ACTION_ATTR_PUSH: return -2; case OVS_ACTION_ATTR_POP: return 2; case OVS_ACTION_ATTR_SET: return -2; - case OVS_ACTION_ATTR_SET_PRIORITY: return 4; - case OVS_ACTION_ATTR_POP_PRIORITY: return 0; case OVS_ACTION_ATTR_SAMPLE: return -2; case OVS_ACTION_ATTR_UNSPEC: @@ -205,12 +203,6 @@ format_odp_action(struct ds *ds, const struct nlattr *a) ds_put_format(ds, "pop(key%"PRIu16")", nl_attr_get_u16(a)); } break; - case OVS_ACTION_ATTR_SET_PRIORITY: - ds_put_format(ds, "set_priority(%#"PRIx32")", nl_attr_get_u32(a)); - break; - case OVS_ACTION_ATTR_POP_PRIORITY: - ds_put_cstr(ds, "pop_priority"); - break; case OVS_ACTION_ATTR_SAMPLE: format_odp_sample_action(ds, a); break; @@ -257,6 +249,7 @@ odp_flow_key_attr_len(uint16_t type) } switch ((enum ovs_key_attr) type) { + case OVS_KEY_ATTR_PRIORITY: return 4; case OVS_KEY_ATTR_TUN_ID: return 8; case OVS_KEY_ATTR_IN_PORT: return 4; case OVS_KEY_ATTR_ETHERNET: return sizeof(struct ovs_key_ethernet); @@ -338,6 +331,10 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds) } switch (nl_attr_type(a)) { + case OVS_KEY_ATTR_PRIORITY: + ds_put_format(ds, "priority(%"PRIu32")", nl_attr_get_u32(a)); + break; + case OVS_KEY_ATTR_TUN_ID: ds_put_format(ds, "tun_id(%#"PRIx64")", ntohll(nl_attr_get_be64(a))); break; @@ -528,6 +525,16 @@ parse_odp_key_attr(const char *s, struct ofpbuf *key) * type larger than 64 bits. */ { + unsigned long long int priority; + int n = -1; + + if (sscanf(s, "priority(%lli)%n", &priority, &n) > 0 && n > 0) { + nl_msg_put_u32(key, OVS_KEY_ATTR_PRIORITY, priority); + return n; + } + } + + { char tun_id_s[32]; int n = -1; @@ -824,6 +831,10 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow) { struct ovs_key_ethernet *eth_key; + if (flow->priority) { + nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, flow->priority); + } + if (flow->tun_id != htonll(0)) { nl_msg_put_be64(buf, OVS_KEY_ATTR_TUN_ID, flow->tun_id); } @@ -991,11 +1002,17 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, #define TRANSITION(PREV_TYPE, TYPE) (((PREV_TYPE) << 16) | (TYPE)) switch (TRANSITION(prev_type, type)) { + case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_PRIORITY): + flow->priority = nl_attr_get_u32(nla); + break; + case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_TUN_ID): + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_TUN_ID): flow->tun_id = nl_attr_get_be64(nla); break; case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_IN_PORT): + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_IN_PORT): case TRANSITION(OVS_KEY_ATTR_TUN_ID, OVS_KEY_ATTR_IN_PORT): if (nl_attr_get_u32(nla) >= UINT16_MAX) { return EINVAL; @@ -1004,6 +1021,7 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, break; case TRANSITION(OVS_KEY_ATTR_UNSPEC, OVS_KEY_ATTR_ETHERNET): + case TRANSITION(OVS_KEY_ATTR_PRIORITY, OVS_KEY_ATTR_ETHERNET): case TRANSITION(OVS_KEY_ATTR_TUN_ID, OVS_KEY_ATTR_ETHERNET): case TRANSITION(OVS_KEY_ATTR_IN_PORT, OVS_KEY_ATTR_ETHERNET): eth_key = nl_attr_get(nla); @@ -1137,6 +1155,7 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, case OVS_KEY_ATTR_UNSPEC: return EINVAL; + case OVS_KEY_ATTR_PRIORITY: case OVS_KEY_ATTR_TUN_ID: case OVS_KEY_ATTR_IN_PORT: return EINVAL; diff --git a/lib/odp-util.h b/lib/odp-util.h index 8c7d4d1..eceeae6 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -66,6 +66,7 @@ void format_odp_actions(struct ds *, const struct nlattr *odp_actions, * * struct pad nl hdr total * ------ --- ------ ----- + * OVS_KEY_ATTR_PRIORITY 4 -- 4 8 * OVS_KEY_ATTR_TUN_ID 8 -- 4 12 * OVS_KEY_ATTR_IN_PORT 4 -- 4 8 * OVS_KEY_ATTR_ETHERNET 12 -- 4 16 @@ -75,14 +76,14 @@ void format_odp_actions(struct ds *, const struct nlattr *odp_actions, * OVS_KEY_ATTR_ICMPV6 2 2 4 8 * OVS_KEY_ATTR_ND 28 -- 4 32 * ------------------------------------------------- - * total 132 + * total 140 */ -#define ODPUTIL_FLOW_KEY_BYTES 132 +#define ODPUTIL_FLOW_KEY_BYTES 140 /* This is an imperfect sanity-check that ODPUTIL_FLOW_KEY_BYTES doesn't * need to be updated, but will at least raise awareness when new OVS * datapath key types are added. */ -BUILD_ASSERT_DECL(__OVS_KEY_ATTR_MAX == 14); +BUILD_ASSERT_DECL(__OVS_KEY_ATTR_MAX == 15); /* A buffer with sufficient size and alignment to hold an nlattr-formatted flow * key. An array of "struct nlattr" might not, in theory, be sufficiently diff --git a/lib/ofp-print.c b/lib/ofp-print.c index a6e41eb..2b67202 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -140,7 +140,7 @@ ofp_print_packet_in(struct ds *string, const struct ofp_packet_in *op, struct ofpbuf packet; ofpbuf_use_const(&packet, op->data, data_len); - flow_extract(&packet, 0, ntohs(op->in_port), &flow); + flow_extract(&packet, 0, ntohs(op->in_port), 0, &flow); flow_format(string, &flow); ds_put_char(string, '\n'); } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index aba8e3c..b1602dc 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -205,9 +205,8 @@ struct action_xlate_ctx { * reason to look at them. */ int recurse; /* Recursion level, via xlate_table_action. */ - uint32_t priority; /* Current flow priority. 0 if none. */ struct flow base_flow; /* Flow at the last commit. */ - uint32_t base_priority; /* Priority at the last commit. */ + uint32_t original_priority; /* Priority when packet arrived. */ uint8_t table_id; /* OpenFlow table ID where flow was found. */ uint32_t sflow_n_outputs; /* Number of output ports. */ uint16_t sflow_odp_port; /* Output port for composing sFlow action. */ @@ -2256,7 +2255,8 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall *upcalls, /* Obtain in_port and tun_id, at least, then set 'flow''s header * pointers. */ odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow); - flow_extract(upcall->packet, flow.tun_id, flow.in_port, &flow); + flow_extract(upcall->packet, flow.tun_id, flow.in_port, + flow.priority, &flow); /* Handle 802.1ag, LACP, and STP specially. */ if (process_special(ofproto, &flow, upcall->packet)) { @@ -3448,7 +3448,7 @@ send_packet(struct ofproto_dpif *ofproto, uint32_t odp_port, struct flow flow; int error; - flow_extract((struct ofpbuf *) packet, 0, 0, &flow); + flow_extract((struct ofpbuf *) packet, 0, 0, 0, &flow); ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); odp_flow_key_from_flow(&key, &flow); @@ -3721,19 +3721,17 @@ commit_set_port_action(const struct flow *flow, struct flow *base, } static void -commit_priority_action(struct action_xlate_ctx *ctx) +commit_set_priority_action(const struct flow *flow, struct flow *base, + struct ofpbuf *odp_actions) { - if (ctx->base_priority == ctx->priority) { + if (base->priority == flow->priority) { return; } + base->priority = flow->priority; - if (ctx->priority) { - nl_msg_put_u32(ctx->odp_actions, - OVS_ACTION_ATTR_SET_PRIORITY, ctx->priority); - } else { - nl_msg_put_flag(ctx->odp_actions, OVS_ACTION_ATTR_POP_PRIORITY); - } - ctx->base_priority = ctx->priority; + commit_action__(odp_actions, OVS_ACTION_ATTR_SET, + OVS_KEY_ATTR_PRIORITY, &base->priority, + sizeof(base->priority)); } static void @@ -3748,7 +3746,7 @@ commit_odp_actions(struct action_xlate_ctx *ctx) commit_vlan_action(ctx, flow->vlan_tci); commit_set_nw_action(flow, base, odp_actions); commit_set_port_action(flow, base, odp_actions); - commit_priority_action(ctx); + commit_set_priority_action(flow, base, odp_actions); } static void @@ -3957,7 +3955,7 @@ xlate_enqueue_action(struct action_xlate_ctx *ctx, const struct ofp_action_enqueue *oae) { uint16_t ofp_port, odp_port; - uint32_t ctx_priority, priority; + uint32_t flow_priority, priority; int error; error = dpif_queue_to_priority(ctx->ofproto->dpif, ntohl(oae->queue_id), @@ -3978,10 +3976,10 @@ xlate_enqueue_action(struct action_xlate_ctx *ctx, odp_port = ofp_port_to_odp_port(ofp_port); /* Add datapath actions. */ - ctx_priority = ctx->priority; - ctx->priority = priority; + flow_priority = ctx->flow.priority; + ctx->flow.priority = priority; add_output_action(ctx, odp_port); - ctx->priority = ctx_priority; + ctx->flow.priority = flow_priority; /* Update NetFlow output port. */ if (ctx->nf_output_iface == NF_OUT_DROP) { @@ -4006,7 +4004,7 @@ xlate_set_queue_action(struct action_xlate_ctx *ctx, return; } - ctx->priority = priority; + ctx->flow.priority = priority; } struct xlate_reg_state { @@ -4200,7 +4198,7 @@ do_xlate_actions(const union ofp_action *in, size_t n_in, break; case OFPUTIL_NXAST_POP_QUEUE: - ctx->priority = 0; + ctx->flow.priority = ctx->original_priority; break; case OFPUTIL_NXAST_REG_MOVE: @@ -4295,8 +4293,7 @@ xlate_actions(struct action_xlate_ctx *ctx, ctx->has_normal = false; ctx->nf_output_iface = NF_OUT_DROP; ctx->recurse = 0; - ctx->priority = 0; - ctx->base_priority = 0; + ctx->original_priority = ctx->flow.priority; ctx->base_flow = ctx->flow; ctx->base_flow.tun_id = 0; ctx->table_id = 0; @@ -5274,7 +5271,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_, ds_put_cstr(&result, s); free(s); - flow_extract(packet, tun_id, in_port, &flow); + flow_extract(packet, tun_id, in_port, 0, &flow); } else { unixctl_command_reply(conn, 501, "Bad command syntax"); goto exit; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 7543e11..927919b 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1607,7 +1607,7 @@ rule_execute(struct rule *rule, uint16_t in_port, struct ofpbuf *packet) assert(ofpbuf_headroom(packet) >= sizeof(struct ofp_packet_in)); - flow_extract(packet, 0, in_port, &flow); + flow_extract(packet, 0, in_port, 0, &flow); return rule->ofproto->ofproto_class->rule_execute(rule, &flow, packet); } @@ -1769,7 +1769,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh) } /* Send out packet. */ - flow_extract(&payload, 0, ntohs(opo->in_port), &flow); + flow_extract(&payload, 0, ntohs(opo->in_port), 0, &flow); error = p->ofproto_class->packet_out(p, &payload, &flow, ofp_actions, n_ofp_actions); ofpbuf_delete(buffer); diff --git a/tests/odp.at b/tests/odp.at index 65a9fb9..7157cea 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -33,6 +33,10 @@ in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x0806),arp sed 's/eth([[^)]]*)/&,vlan(vid=99,pcp=7)/' odp-base.txt echo + echo '# Valid forms with QoS priority.' + sed 's/^/priority(1234),/' odp-base.txt + + echo echo '# Valid forms with tun_id and VLAN headers.' sed 's/^/tun_id(0xfedcba9876543210),/ s/eth([[^)]]*)/&,vlan(vid=99,pcp=7)/' odp-base.txt diff --git a/tests/test-flows.c b/tests/test-flows.c index 57157c9..2ee94fa 100644 --- a/tests/test-flows.c +++ b/tests/test-flows.c @@ -68,7 +68,7 @@ main(int argc OVS_UNUSED, char *argv[]) ovs_fatal(retval, "error reading pcap file"); } - flow_extract(packet, 0, 1, &flow); + flow_extract(packet, 0, 1, 0, &flow); cls_rule_init_exact(&flow, 0, &rule); ofputil_cls_rule_to_match(&rule, &extracted_match); -- 1.7.1 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
