datapath: Update flow key before recirc

When flow key becomes invalid due to push or pop actions, current
implementation leaves it as invalid, only rebuild the flow key used
for recirculation.

This works, but is less efficient in case of multiple recirc
actions. Each recirc action will have to re-extract
its own flow keys.

This patch update the original flow key as soon as the first recirc
action is encountered, avoiding expensive flow extract call for any
future recirc actions as long as the flow key remains valid.

Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

diff --git a/datapath/actions.c b/datapath/actions.c
index 25c5d77..b16e0b2 100644 (file)
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -40,6 +40,17 @@
 #include "vlan.h"
 #include "vport.h"
 
+static void flow_key_clone(struct sk_buff *skb, struct sw_flow_key *new_key)
+{
+       *new_key = *OVS_CB(skb)->pkt_key;
+       OVS_CB(skb)->pkt_key = new_key;
+}
+
+static void flow_key_set_recirc_id(struct sk_buff *skb, u32 recirc_id)
+{
+       OVS_CB(skb)->pkt_key->recirc_id = recirc_id;
+}
+
 static void flow_key_set_priority(struct sk_buff *skb, u32 priority)
 {
        OVS_CB(skb)->pkt_key->phy.priority = priority;
@@ -695,8 +706,7 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
                        /* Skip the sample action when out of memory. */
                        return 0;
 
-               sample_key = *OVS_CB(skb)->pkt_key;
-               OVS_CB(sample_skb)->pkt_key = &sample_key;
+               flow_key_clone(skb, &sample_key);
        }
 
        /* Note that do_execute_actions() never consumes skb.
@@ -775,18 +785,6 @@ static int execute_set_action(struct sk_buff *skb,
        return err;
 }
 
-static void flow_key_clone_recirc(struct sk_buff *skb, u32 recirc_id,
-                                 struct sw_flow_key *recirc_key)
-{
-       *recirc_key = *OVS_CB(skb)->pkt_key;
-       recirc_key->recirc_id = recirc_id;
-       OVS_CB(skb)->pkt_key = recirc_key;
-}
-
-static void flow_key_set_recirc_id(struct sk_buff *skb, u32 recirc_id)
-{
-       OVS_CB(skb)->pkt_key->recirc_id = recirc_id;
-}
 
 static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
                          const struct nlattr *a, int rem)
@@ -805,22 +803,19 @@ static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
                        return 0;
        }
 
-       if (is_skb_flow_key_valid(skb)) {
-               if (!last_action(a, rem))
-                       flow_key_clone_recirc(skb, nla_get_u32(a), &recirc_key);
-               else
-                       flow_key_set_recirc_id(skb, nla_get_u32(a));
-       } else {
-               struct sw_flow_key *pkt_key = OVS_CB(skb)->pkt_key;
-
-               err = ovs_flow_key_extract_recirc(nla_get_u32(a), pkt_key,
-                                                 skb, &recirc_key);
+       if (!is_skb_flow_key_valid(skb)) {
+               err = ovs_flow_key_update(skb, OVS_CB(skb)->pkt_key);
                if (err) {
                        kfree_skb(skb);
                        return err;
                }
        }
+       BUG_ON(!is_skb_flow_key_valid(skb));
+
+       if (!last_action(a, rem))
+               flow_key_clone(skb, &recirc_key);
 
+       flow_key_set_recirc_id(skb, nla_get_u32(a));
        ovs_dp_process_packet(skb, true);
        return 0;
 }

diff --git a/datapath/flow.c b/datapath/flow.c
index d56812a..12ebc5b 100644 (file)
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -673,6 +673,11 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
        return 0;
 }
 
+int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
+{
+       return key_extract(skb, key);
+}
+
 int ovs_flow_key_extract(const struct ovs_tunnel_info *tun_info,
                         struct sk_buff *skb,
                         struct sw_flow_key *key)
@@ -718,13 +723,3 @@ int ovs_flow_key_extract_userspace(const struct nlattr *attr,
 
        return key_extract(skb, key);
 }
-
-int ovs_flow_key_extract_recirc(u32 recirc_id,
-                               const struct sw_flow_key *key,
-                               struct sk_buff *skb,
-                               struct sw_flow_key *new_key)
-{
-       memcpy(new_key, key, OVS_SW_FLOW_KEY_METADATA_SIZE);
-       new_key->recirc_id = recirc_id;
-       return key_extract(skb, new_key);
-}

diff --git a/datapath/flow.h b/datapath/flow.h
index 106feb8..127127f 100644 (file)
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -228,9 +228,7 @@ int ovs_flow_key_extract(const struct ovs_tunnel_info *tun_info,
 int ovs_flow_key_extract_userspace(const struct nlattr *attr,
                                   struct sk_buff *skb,
                                   struct sw_flow_key *key);
-int ovs_flow_key_extract_recirc(u32 recirc_id,
-                               const struct sw_flow_key *key,
-                               struct sk_buff *skb,
-                               struct sw_flow_key *new_key);
+/* Update the non-metadata part of the flow key using skb. */
+int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key);
 
 #endif /* flow.h */