[ovs-dev] [PATCH 46/62] dpif-netdev: Fix snat pkt offload bug

Tao YunXiang taoyunxiang at cmss.chinamobile.com
Mon Dec 28 09:25:04 UTC 2020


From: Taoyunxiang <taoyunxiang at cmss.chinamobile.com>

Code Source From: Self Code
Description:
For snat reply pkt , we should offload from action ct(nat),
fix it in parse_netdev_flow_put.

Jira:  #[Optional]
市场项目编号(名称):[Optional]
---
 lib/conntrack.c   |  3 +++
 lib/dpif-netdev.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 95d48c5..5318df0 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -32,6 +32,7 @@
 #include "flow.h"
 #include "netdev.h"
 #include "odp-netlink.h"
+#include "odp-execute.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/vlog.h"
 #include "ovs-rcu.h"
@@ -883,6 +884,7 @@ un_nat_packet(struct dp_packet *pkt, const struct conn *conn,
             struct ip_header *nh = dp_packet_l3(pkt);
             packet_set_ipv4_addr(pkt, &nh->ip_dst,
                                  conn->key.src.addr.ipv4);
+            pkt->md.skb_priority |= 1 << SET_ACTION_SET ;
         } else {
             struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
             packet_set_ipv6_addr(pkt, conn->key.nw_proto,
@@ -901,6 +903,7 @@ un_nat_packet(struct dp_packet *pkt, const struct conn *conn,
             struct ip_header *nh = dp_packet_l3(pkt);
             packet_set_ipv4_addr(pkt, &nh->ip_src,
                                  conn->key.dst.addr.ipv4);
+            pkt->md.skb_priority |= 1 << SET_ACTION_SET ;
         } else {
             struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
             packet_set_ipv6_addr(pkt, conn->key.nw_proto,
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b96d12d..96eaeb2 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2594,7 +2594,9 @@ parse_netdev_flow_put(struct match *match, const struct nlattr *actions,
     bool action_has_recirc = false;
     bool action_has_set = false;
     bool action_has_dnat = false;
+    bool action_has_null_nat = false;
     bool action_has_ct = false;
+    bool action_has_ct_nat = false;
     bool ret = false;
     struct nlattr *nla;
     size_t left;
@@ -2630,6 +2632,9 @@ parse_netdev_flow_put(struct match *match, const struct nlattr *actions,
                     bool proto_num_min_specified = false;
                     bool ip_max_specified = false;
                     bool proto_num_max_specified = false;
+
+                    action_has_ct_nat = true;
+                    action_has_null_nat = true;
                     memset(nat_action, 0, sizeof *nat_action);
 
                     NL_NESTED_FOR_EACH_UNSAFE (b_nest, left_nest, b) {
@@ -2643,6 +2648,7 @@ parse_netdev_flow_put(struct match *match, const struct nlattr *actions,
                                     ? NAT_ACTION_SRC : NAT_ACTION_DST);
                             action_has_dnat = (sub_type_nest == OVS_NAT_ATTR_DST)?
                                                true : action_has_dnat;
+                            action_has_null_nat = false;
                             break;
                         case OVS_NAT_ATTR_IP_MIN:
                             memcpy(&(nat_action->min_addr),
@@ -2710,7 +2716,12 @@ parse_netdev_flow_put(struct match *match, const struct nlattr *actions,
      */
     if (mod_flag) {
         if (match->wc.masks.ct_state &&
-            !(match->wc.masks.ct_state & match->flow.ct_state & CS_ESTABLISHED) ){
+            (match->wc.masks.ct_state & match->flow.ct_state & CS_TRACKED) &&
+            !(match->wc.masks.ct_state & match->flow.ct_state & CS_ESTABLISHED) &&
+            !(action_has_ct_nat && !action_has_null_nat) ) {
+            /* for nat(src= or nat(dst= , we should try to offload no matter
+             * what ct_state is
+             */
             ret = false;
         } else {
             ret = true;
@@ -2718,38 +2729,54 @@ parse_netdev_flow_put(struct match *match, const struct nlattr *actions,
         goto out;
     }
 
-    /*if only has one conntrack action, won't need to offload */
-    if ((actions_len == 1) && action_has_ct) {
-        goto out;
-    }
     /* no mod_flag means: 1. no set action at all
-     *                    2. set action but is the first translated pkt
-     *                    3. nat action then set mac/ttl: such as DNAT
+     *                    2. first translated pkt with set action
+     *                    3. first translated pkt with reply dir nat action
+     *                    4. first translated pkt with req dir dnat action
+     *                    5. others
      * For 1 , only established pkt should try to offload
-     * For 2 , still established pkt should be offload
-     * For 3 , we don't care ct_state,and try to offload
-     * so for 1 && 2: we cannot offload non-est pkt
-     * for 3, we try to offload non-est pkt
+     * For 2 and 3 , still established pkt should be offload
+     * For 4 , we don't care ct_state,and try to offload
+     * so for 1 - 3: we cannot offload non-est pkt
+     * for 4, we try to offload non-est pkt
+     * for 5, no need to offload
      */
+
+    /* scenario:4 */
     if (action_has_dnat) {
         ret = true;
         goto out;
     }
     if (match->wc.masks.ct_state &&
+        (match->wc.masks.ct_state & match->flow.ct_state & CS_TRACKED) &&
         !(match->wc.masks.ct_state & match->flow.ct_state & CS_ESTABLISHED) ){
         goto out;
     }
 
+    /* scenario 3: if in reply direction's nat and also +est ,
+     * we should try to offload it
+     */
+    if (match->wc.masks.ct_state &&
+        action_has_ct_nat &&
+        action_has_null_nat &&
+        (match->wc.masks.ct_state & match->flow.ct_state & CS_ESTABLISHED) &&
+        (match->wc.masks.ct_state & match->flow.ct_state & CS_REPLY_DIR) ){
+        ret = true;
+        goto out;
+    }
+    /* scenario:1 */
     /* no mod_flag and no recirc flow: we should try to offload */
     if (action_has_recirc==false) {
         ret = true;
         goto out;
     }
+    /* scenario:2 */
     /* no mod_flag and recirc and set and +est,means it's first loopback pkt
      * we should try to offload jump action and "no" group id
      */
     if (action_has_set==true) {
         if (match->wc.masks.ct_state &&
+            (match->wc.masks.ct_state & match->flow.ct_state & CS_TRACKED) &&
             (match->wc.masks.ct_state & match->flow.ct_state & CS_ESTABLISHED)) {
             ret = true;
             goto out;
@@ -2758,6 +2785,7 @@ parse_netdev_flow_put(struct match *match, const struct nlattr *actions,
 
 
 out:
+    /* scenario:5 */
     return ret;
 }
 
-- 
1.8.3.1





More information about the dev mailing list