ofproto-dpif: Do not give stats to rules bypassed by "drop" frag policy.

When the OFPC_FRAG_DROP policy is in effect, IP fragments are supposed to
be dropped before they reach the flow table.  Open vSwitch properly dropped
IP fragments in this case, but still accounted them to the packet and byte
counters for the flow that they would have hit if the OFPC_FRAG_NX_MATCh
policy had been in effect.

Reported-by: love you <thunder.love07@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

2 files changed, 26 insertions, 8 deletions

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 2ac3cd52e..d491f91e5 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -681,6 +681,7 @@ struct ofproto_dpif {
     /* Special OpenFlow rules. */
     struct rule_dpif *miss_rule; /* Sends flow table misses to controller. */
     struct rule_dpif *no_packet_in_rule; /* Drops flow table misses. */
+    struct rule_dpif *drop_frags_rule; /* Used in OFPC_FRAG_DROP mode. */
 
     /* Bridging. */
     struct netflow *netflow;
@@ -1509,6 +1510,12 @@ add_internal_flows(struct ofproto_dpif *ofproto)
     ofpbuf_clear(&ofpacts);
     error = add_internal_flow(ofproto, id++, &ofpacts,
                               &ofproto->no_packet_in_rule);
+    if (error) {
+        return error;
+    }
+
+    error = add_internal_flow(ofproto, id++, &ofpacts,
+                              &ofproto->drop_frags_rule);
     return error;
 }
 
@@ -1771,7 +1778,7 @@ get_tables(struct ofproto *ofproto_, struct ofp12_table_stats *ots)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct dpif_dp_stats s;
-    uint64_t n_miss, n_no_pkt_in, n_bytes;
+    uint64_t n_miss, n_no_pkt_in, n_bytes, n_dropped_frags;
     uint64_t n_lookup;
 
     strcpy(ots->name, "classifier");
@@ -1779,8 +1786,9 @@ get_tables(struct ofproto *ofproto_, struct ofp12_table_stats *ots)
     dpif_get_dp_stats(ofproto->backer->dpif, &s);
     rule_get_stats(&ofproto->miss_rule->up, &n_miss, &n_bytes);
     rule_get_stats(&ofproto->no_packet_in_rule->up, &n_no_pkt_in, &n_bytes);
+    rule_get_stats(&ofproto->drop_frags_rule->up, &n_dropped_frags, &n_bytes);
 
-    n_lookup = s.n_hit + s.n_missed;
+    n_lookup = s.n_hit + s.n_missed - n_dropped_frags;
     ots->lookup_count = htonll(n_lookup);
     ots->matched_count = htonll(n_lookup - n_miss - n_no_pkt_in);
 }
@@ -5455,20 +5463,22 @@ rule_dpif_lookup__(struct ofproto_dpif *ofproto, const struct flow *flow,
 {
     struct cls_rule *cls_rule;
     struct classifier *cls;
+    bool frag;
 
     if (table_id >= N_TABLES) {
         return NULL;
     }
 
     cls = &ofproto->up.tables[table_id].cls;
-    if (flow->nw_frag & FLOW_NW_FRAG_ANY
-        && ofproto->up.frag_handling == OFPC_FRAG_NORMAL) {
-        /* For OFPC_NORMAL frag_handling, we must pretend that transport ports
-         * are unavailable. */
+    frag = (flow->nw_frag & FLOW_NW_FRAG_ANY) != 0;
+    if (frag && ofproto->up.frag_handling == OFPC_FRAG_NORMAL) {
+        /* We must pretend that transport ports are unavailable. */
         struct flow ofpc_normal_flow = *flow;
         ofpc_normal_flow.tp_src = htons(0);
         ofpc_normal_flow.tp_dst = htons(0);
         cls_rule = classifier_lookup(cls, &ofpc_normal_flow);
+    } else if (frag && ofproto->up.frag_handling == OFPC_FRAG_DROP) {
+        cls_rule = &ofproto->drop_frags_rule->up.cr;
     } else {
         cls_rule = classifier_lookup(cls, flow);
     }
@@ -8187,6 +8197,9 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
     } else if (rule == ofproto->no_packet_in_rule) {
         ds_put_cstr(ds, "\nNo match, packets dropped because "
                     "OFPPC_NO_PACKET_IN is set on in_port.\n");
+    } else if (rule == ofproto->drop_frags_rule) {
+        ds_put_cstr(ds, "\nPackets dropped because they are IP fragments "
+                    "and the fragment handling mode is \"drop\".\n");
     }
 
     if (rule) {
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 4eecd21d1..27c440e56 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -804,9 +804,14 @@ do
   AT_CHECK([ovs-ofctl set-frags br0 $mode])
   for type in no first later; do
     eval flow=\$${type}_flow exp_output=\$$type
+    printf "\n%s\n" "----$mode $type-----"
     AT_CHECK([ovs-appctl ofproto/trace br0 "$flow"], [0], [stdout])
-    AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath actions: $exp_output
-])
+    : > expout
+    if test $mode = drop && test $type != no; then
+        echo 'Packets dropped because they are IP fragments and the fragment handling mode is "drop".' >> expout
+    fi
+    echo "Datapath actions: $exp_output" >> expout
+    AT_CHECK([grep 'IP fragments' stdout; tail -1 stdout], [0], [expout])
   done
 done
 OVS_VSWITCHD_STOP