Set appropriate TCP flags in reverse flow at creation

When agent creates a reverse flow for an existing forward flow, appropriate TCP flags should also be set based on the TCP flags that are set in the forward flow. Otherwise, eviction (and other features that depend on flags in both forward and reverse entries) might not work. Case in point is the D flag. Any fragment other than the first fragment of the packet should not be allowed to create a new flow. While creating a defer call back, unset the evict flags only if the context that led to creation of the defer was because of eviction. Otherwise, there could be some misbehavior. Allow eviction for flows even if there is only one way link between forward and the reverse flow Change-Id: I7bccd256e4d33eaf4623a89e49c40d0928c372c4 Closes-BUG: 1362701
Juniper · Dec 9, 2015 · d752dcf · d752dcf
1 parent 6d01d73
commit d752dcf
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 19 deletions.
diff --git a/dp-core/vr_flow.c b/dp-core/vr_flow.c
@@ -361,11 +361,12 @@ vr_flow_evict_flow(struct vrouter *router, struct vr_flow_entry *fe)
             (fe->fe_flags & VR_FLOW_FLAG_EVICT_CANDIDATE)) {
         flags = fe->fe_flags | VR_FLOW_FLAG_ACTIVE |
             VR_FLOW_FLAG_EVICT_CANDIDATE;
-        (void)__sync_bool_compare_and_swap(&fe->fe_flags, flags,
+        if (__sync_bool_compare_and_swap(&fe->fe_flags, flags,
                 (flags ^ VR_FLOW_FLAG_EVICT_CANDIDATE) |
-                VR_FLOW_FLAG_EVICTED);
-        vr_flow_stop_modify(router, fe);
-        vr_flow_reset_active_entry(router, fe);
+                VR_FLOW_FLAG_EVICTED)) {
+            vr_flow_stop_modify(router, fe);
+            vr_flow_reset_active_entry(router, fe);
+        }
     }
 
     return;
@@ -435,11 +436,13 @@ vr_flow_defer(struct vr_flow_md *flmd, struct vr_flow_entry *fe)
     struct vr_flow_defer_data *vfdd;
 
     if (!vdd || !vdd->vdd_data) {
-        if (fe->fe_rflow) {
-            rfe = vr_get_flow_entry(flmd->flmd_router, fe->fe_rflow);
-            vr_flow_reset_evict(flmd->flmd_router, rfe);
+        if (flmd->flmd_flags & VR_FLOW_FLAG_EVICT_CANDIDATE) {
+            if (fe->fe_rflow) {
+                rfe = vr_get_flow_entry(flmd->flmd_router, fe->fe_rflow);
+                vr_flow_reset_evict(flmd->flmd_router, rfe);
+            }
+            vr_flow_reset_evict(flmd->flmd_router, fe);
         }
-        vr_flow_reset_evict(flmd->flmd_router, fe);
 
         if (!(flmd->flmd_flags & VR_FLOW_FLAG_ACTIVE)) {
             vr_flow_reset_entry(flmd->flmd_router, fe);
@@ -646,8 +649,9 @@ vr_flow_mark_evict(struct vrouter *router, struct vr_flow_entry *fe)
     struct vr_flow_entry *rfe = NULL;
 
     /* start modifying the entry */
-    if (!vr_flow_start_modify(router, fe))
+    if (!vr_flow_start_modify(router, fe)) {
         return;
+    }
 
     if (fe->fe_rflow >= 0) {
         rfe = vr_get_flow_entry(router, fe->fe_rflow);
@@ -658,7 +662,7 @@ vr_flow_mark_evict(struct vrouter *router, struct vr_flow_entry *fe)
                     /* no modification. hence...*/
                     rfe = NULL;
                 } else {
-                    if (rfe->fe_rflow == index) {
+                    if ((rfe->fe_rflow == index) || (rfe->fe_rflow < 0)) {
                         evict_forward_flow = __vr_flow_mark_evict(router, rfe);
                     }
                 }
@@ -675,7 +679,7 @@ vr_flow_mark_evict(struct vrouter *router, struct vr_flow_entry *fe)
      */
     if (evict_forward_flow) {
         if (__vr_flow_mark_evict(router, fe)) {
-            if (__vr_flow_schedule_transition(router, fe,
+            if (!__vr_flow_schedule_transition(router, fe,
                         index, fe->fe_flags)) {
                 return;
             } else {
@@ -937,6 +941,39 @@ vr_flow_init_close(struct vrouter *router, struct vr_flow_entry *flow_e,
     return;
 }
 
+static void
+vr_flow_tcp_rflow_set(struct vrouter *router, struct vr_flow_entry *fe,
+        struct vr_flow_entry *rfe)
+{
+    uint16_t flags = 0;
+
+    if (!fe || !rfe)
+        return;
+
+    if (rfe->fe_tcp_flags & VR_FLOW_TCP_SYN) {
+        flags |= VR_FLOW_TCP_SYN_R;
+    }
+
+    if (rfe->fe_tcp_flags & VR_FLOW_TCP_RST) {
+        flags |= VR_FLOW_TCP_RST;
+    }
+
+    if (rfe->fe_tcp_flags & VR_FLOW_TCP_DEAD) {
+        flags |= VR_FLOW_TCP_DEAD;
+    }
+
+    if (rfe->fe_tcp_flags & VR_FLOW_TCP_FIN) {
+        flags |= VR_FLOW_TCP_FIN_R;
+    }
+
+    if (rfe->fe_tcp_flags & VR_FLOW_TCP_ESTABLISHED) {
+        flags |= (VR_FLOW_TCP_ESTABLISHED | VR_FLOW_TCP_ESTABLISHED_R);
+    }
+
+    (void)__sync_fetch_and_or(&fe->fe_tcp_flags, flags);
+    return;
+}
+
 static void
 vr_flow_tcp_digest(struct vrouter *router, struct vr_flow_entry *flow_e,
         struct vr_packet *pkt, struct vr_forwarding_md *fmd)
@@ -1088,6 +1125,26 @@ vr_flow_tcp_digest(struct vrouter *router, struct vr_flow_entry *flow_e,
     return;
 }
 
+static inline bool
+vr_flow_allow_new_flow(struct vrouter *router, struct vr_packet *pkt)
+{
+    bool allow;
+
+    if (pkt->vp_type == VP_TYPE_IP) {
+        allow = vr_inet_flow_allow_new_flow(router, pkt);
+        if (!allow)
+            return allow;
+    }
+
+    if ((vr_flow_hold_limit) &&
+            (vr_flow_table_hold_count(router) >
+             VR_MAX_FLOW_TABLE_HOLD_COUNT)) {
+        return false;
+    }
+
+    return true;
+}
+
 flow_result_t
 vr_flow_lookup(struct vrouter *router, struct vr_flow *key,
                struct vr_packet *pkt, struct vr_forwarding_md *fmd)
@@ -1097,16 +1154,13 @@ vr_flow_lookup(struct vrouter *router, struct vr_flow *key,
 
     pkt->vp_flags |= VP_FLAG_FLOW_SET;
 
-
     flow_e = vr_find_flow(router, key, pkt->vp_type,  &fe_index);
     if (!flow_e) {
         if (pkt->vp_nh &&
             (pkt->vp_nh->nh_flags & NH_FLAG_RELAXED_POLICY))
             return FLOW_FORWARD;
 
-        if ((vr_flow_hold_limit) &&
-                (vr_flow_table_hold_count(router) >
-                 VR_MAX_FLOW_TABLE_HOLD_COUNT)) {
+        if (!vr_flow_allow_new_flow(router, pkt)) {
             vr_pfree(pkt, VP_DROP_FLOW_UNUSABLE);
             return FLOW_CONSUMED;
         }
@@ -1729,10 +1783,7 @@ vr_flow_set(struct vrouter *router, vr_flow_req *req)
         if (fe->fe_flags & VR_RFLOW_VALID) {
             rfe = vr_get_flow_entry(router, fe->fe_rflow);
             if (rfe) {
-                if (rfe->fe_tcp_flags & VR_FLOW_TCP_SYN) {
-                    (void)__sync_fetch_and_or(&fe->fe_tcp_flags,
-                            VR_FLOW_TCP_SYN_R);
-                }
+                vr_flow_tcp_rflow_set(router, fe, rfe);
             }
         }
     }

diff --git a/dp-core/vr_proto_ip.c b/dp-core/vr_proto_ip.c
@@ -839,6 +839,17 @@ vr_inet_fragment_flow(struct vrouter *router, unsigned short vrf,
     return 0;
 }
 
+bool
+vr_inet_flow_allow_new_flow(struct vrouter *router, struct vr_packet *pkt)
+{
+    struct vr_ip *iph = (struct vr_ip *)pkt_network_header(pkt);
+
+    if (vr_ip_fragment(iph) && !vr_ip_fragment_head(iph))
+        return false;
+
+    return true;
+}
+
 bool
 vr_inet_flow_is_fat_flow(struct vrouter *router, struct vr_packet *pkt,
         struct vr_flow_entry *fe)

diff --git a/include/vr_flow.h b/include/vr_flow.h
@@ -337,6 +337,7 @@ extern void vr_inet_fill_flow(struct vr_flow *, unsigned short,
                 uint32_t, uint32_t, uint8_t, uint16_t, uint16_t);
 extern bool vr_inet_flow_is_fat_flow(struct vrouter *, struct vr_packet *,
         struct vr_flow_entry *);
+extern bool vr_inet_flow_allow_new_flow(struct vrouter *, struct vr_packet *);
 
 extern unsigned int vr_reinject_packet(struct vr_packet *,
         struct vr_forwarding_md *);