[ovs-dev] [PATCH] ofproto-dpif: Do not give stats to rules bypassed by "drop" frag policy.

Ethan Jackson ethan at nicira.com
Tue Jun 4 02:28:44 UTC 2013


Acked-by: Ethan Jackson <ethan at nicira.com>

On Tue, May 28, 2013 at 1:16 PM, Ben Pfaff <blp at nicira.com> wrote:
> 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 at gmail.com>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  ofproto/ofproto-dpif.c |   20 ++++++++++++++++----
>  tests/ofproto-dpif.at  |    9 +++++++--
>  2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 280fd57..17af5c6 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -698,6 +698,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. */
>
>      /* Statistics. */
>      uint64_t n_matches;
> @@ -1505,6 +1506,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;
>  }
>
> @@ -5611,20 +5618,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);
>      }
> @@ -8262,6 +8271,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 ff007cd..7fc896f 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 ovs-dummy "$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
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list