datapath: Convert kernel priority actions into match/set.

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 <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #7715

diff --git a/datapath/actions.c b/datapath/actions.c
index 8ca243d..4db2563 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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..7d0628c 100644 (file)
--- 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, 0, htonll(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..b4e788f 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -881,7 +881,7 @@ dpif_netdev_execute(struct dpif *dpif,
     ofpbuf_reserve(&copy, DP_NETDEV_HEADROOM);
     ofpbuf_put(&copy, packet->data, packet->size);
 
-    flow_extract(&copy, 0, -1, &key);
+    flow_extract(&copy, 0, 0, -1, &key);
     error = dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key);
     if (!error) {
         dp_netdev_execute_actions(dp, &copy, &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, 0, port->port_no, &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 (file)
--- 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 (file)
--- 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..1263734 100644 (file)
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -324,8 +324,8 @@ invalid:
  *      present and has a correct length, and otherwise NULL.
  */
 void
-flow_extract(struct ofpbuf *packet, ovs_be64 tun_id, uint16_t ofp_in_port,
-             struct flow *flow)
+flow_extract(struct ofpbuf *packet, uint32_t priority, ovs_be64 tun_id,
+             uint16_t ofp_in_port, 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 *
@@ -499,8 +501,14 @@ flow_format(struct ds *ds, const struct flow *flow)
 {
     int frag;
 
-    ds_put_format(ds, "tunnel%#"PRIx64":in_port%04"PRIx16":tci(",
-                  ntohll(flow->tun_id), flow->in_port);
+    ds_put_format(ds, "priority%"PRIu32
+                      ":tunnel%#"PRIx64
+                      ":in_port%04"PRIx16,
+                      flow->priority,
+                      ntohll(flow->tun_id),
+                      flow->in_port);
+
+    ds_put_format(ds, ":tci(");
     if (flow->vlan_tci) {
         ds_put_format(ds, "vlan%"PRIu16",pcp%d",
                       vlan_tci_to_vid(flow->vlan_tci),

diff --git a/lib/flow.h b/lib/flow.h
index e9da2ad..0abfe5a 100644 (file)
--- 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 *);
+void flow_extract(struct ofpbuf *, uint32_t priority, ovs_be64 tun_id,
+                  uint16_t in_port, 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..ecc5509 100644 (file)
--- 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, 0, in_port, &flow);
 
     /* Choose output port. */
     out_port = lswitch_choose_destination(sw, &flow);

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 1e20f49..1e9289a 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -65,8 +65,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:
@@ -206,12 +204,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;
@@ -258,6 +250,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);
@@ -339,6 +332,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)
      * The tun_id parser has to use an alternative approach because there is no
      * 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;
@@ -825,6 +832,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);
     }
@@ -992,11 +1003,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;
@@ -1005,6 +1022,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);
@@ -1138,6 +1156,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 (file)
--- 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 6278395..fe852b4 100644 (file)
--- 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, 0, ntohs(op->in_port), &flow);
         flow_format(string, &flow);
         ds_put_char(string, '\n');
     }

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d0a5e00..3cceb6b 100644 (file)
--- 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. */
@@ -2257,7 +2256,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.priority, flow.tun_id,
+                     flow.in_port, &flow);
 
         /* Handle 802.1ag, LACP, and STP specially. */
         if (process_special(ofproto, &flow, upcall->packet)) {
@@ -3449,7 +3449,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);
 
@@ -3722,19 +3722,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
@@ -3749,7 +3747,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
@@ -3958,7 +3956,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),
@@ -3979,10 +3977,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) {
@@ -4007,7 +4005,7 @@ xlate_set_queue_action(struct action_xlate_ctx *ctx,
         return;
     }
 
-    ctx->priority = priority;
+    ctx->flow.priority = priority;
 }
 
 struct xlate_reg_state {
@@ -4205,7 +4203,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:
@@ -4304,8 +4302,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;
@@ -5215,7 +5212,7 @@ static void
 ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
                       void *aux OVS_UNUSED)
 {
-    char *dpname, *arg1, *arg2, *arg3;
+    char *dpname, *arg1, *arg2, *arg3, *arg4;
     char *args = xstrdup(args_);
     char *save_ptr = NULL;
     struct ofproto_dpif *ofproto;
@@ -5233,7 +5230,8 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
     dpname = strtok_r(args, " ", &save_ptr);
     arg1 = strtok_r(NULL, " ", &save_ptr);
     arg2 = strtok_r(NULL, " ", &save_ptr);
-    arg3 = strtok_r(NULL, "", &save_ptr); /* Get entire rest of line. */
+    arg3 = strtok_r(NULL, " ", &save_ptr);
+    arg4 = strtok_r(NULL, "", &save_ptr); /* Get entire rest of line. */
     if (dpname && arg1 && (!arg2 || !strcmp(arg2, "-generate")) && !arg3) {
         /* ofproto/trace dpname flow [-generate] */
         int error;
@@ -5258,18 +5256,20 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const char *args_,
             packet = ofpbuf_new(0);
             flow_compose(packet, &flow);
         }
-    } else if (dpname && arg1 && arg2 && arg3) {
-        /* ofproto/trace dpname tun_id in_port packet */
+    } else if (dpname && arg1 && arg2 && arg3 && arg4) {
+        /* ofproto/trace dpname priority tun_id in_port packet */
         uint16_t in_port;
         ovs_be64 tun_id;
+        uint32_t priority;
 
-        tun_id = htonll(strtoull(arg1, NULL, 0));
-        in_port = ofp_port_to_odp_port(atoi(arg2));
+        priority = atoi(arg1);
+        tun_id = htonll(strtoull(arg2, NULL, 0));
+        in_port = ofp_port_to_odp_port(atoi(arg3));
 
         packet = ofpbuf_new(strlen(args) / 2);
-        arg3 = ofpbuf_put_hex(packet, arg3, NULL);
-        arg3 += strspn(arg3, " ");
-        if (*arg3 != '\0') {
+        arg4 = ofpbuf_put_hex(packet, arg4, NULL);
+        arg4 += strspn(arg4, " ");
+        if (*arg4 != '\0') {
             unixctl_command_reply(conn, 501, "Trailing garbage in command");
             goto exit;
         }
@@ -5284,7 +5284,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, priority, tun_id, in_port, &flow);
     } else {
         unixctl_command_reply(conn, 501, "Bad command syntax");
         goto exit;

diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man
index 2cf7bfa..5ae90f2 100644 (file)
--- a/ofproto/ofproto-unixctl.man
+++ b/ofproto/ofproto-unixctl.man
@@ -6,13 +6,15 @@ These commands manage the core OpenFlow switch implementation (called
 Lists the names of the running ofproto instances.  These are the names
 that may be used on \fBofproto/trace\fR.
 .
-.IP "\fBofproto/trace \fIswitch tun_id in_port packet\fR"
+.IP "\fBofproto/trace \fIswitch priority tun_id in_port packet\fR"
 .IQ "\fBofproto/trace \fIswitch odp_flow \fB\-generate\fR"
 Traces the path of an imaginary packet through \fIswitch\fR.  Both
 forms require \fIswitch\fR, the switch on which the packet arrived
 (one of those listed by \fBofproto/list\fR).  The first form specifies
 a packet's contents explicitly:
 .RS
+.IP "\fIpriority\fR"
+Packet QoS priority. Use \fB0\fR if QoS is not setup.
 .IP "\fItun_id\fR"
 The tunnel ID on which the packet arrived.  Use
 \fB0\fR if the packet did not arrive through a tunnel.

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9a302d5..50c5298 100644 (file)
--- 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, 0, in_port, &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, 0, ntohs(opo->in_port), &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 (file)
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -32,6 +32,10 @@ in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x0806),arp
  echo '# Valid forms with VLAN header.'
  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),/

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 4c190a1..1f5de78 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -237,7 +237,7 @@ dnl The tcpdump output format differs slightly from one version to another,
 dnl so trim off the end of the line where differences appear.
 AT_CHECK([sed 's/\(length 60:\).*/\1 .../' stdout], [0], [dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=3 data_len=60 buffer=0x00000111
-tunnel0:in_port0003:tci(0) mac50:54:00:00:00:05->50:54:00:00:00:06 type0800 proto6 tos0 ip192.168.0.1->192.168.0.2 port10031->0
+priority0:tunnel0:in_port0003:tci(0) mac50:54:00:00:00:05->50:54:00:00:00:06 type0800 proto6 tos0 ip192.168.0.1->192.168.0.2 port10031->0
 50:54:00:00:00:05 > 50:54:00:00:00:06, ethertype IPv4 (0x0800), length 60: ...
 ])
 AT_CLEANUP

diff --git a/tests/test-flows.c b/tests/test-flows.c
index 57157c9..9d36eb1 100644 (file)
--- 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, 0, 1, &flow);
         cls_rule_init_exact(&flow, 0, &rule);
         ofputil_cls_rule_to_match(&rule, &extracted_match);