[ovs-dev] [PATCH v2] lib/dpif-netdev: Integrate megaflow classifier.

[Alex Wang]

>
> static inline struct netdev_flow_key *
> -miniflow_to_netdev_flow_key(const struct miniflow *mf)
> +miniflow_to_netdev_flow_key(const struct miniflow *miniflow)
>  {
> -    return (struct netdev_flow_key *) CONST_CAST(struct miniflow *, mf);
> +    struct netdev_flow_key *key;
> +
> +    INIT_CONTAINER(key, miniflow, mf);
> +
> +    return key;
>  }
>
>

Seems this function is no longer used,



> @ -2697,19 +2845,28 @@ fast_path_processing(struct dp_netdev_pmd_thread
> *pmd,
>                  ? &put_actions
>                  : &actions;
>
> -            ovs_mutex_lock(&dp->flow_mutex);
> -            /* XXX: There's a brief race where this flow could have
> already
> -             * been installed since we last did the flow lookup.  This
> could be
> -             * solved by moving the mutex lock outside the loop, but
> that's an
> -             * awful long time to be locking everyone out of making flow
> -             * installs.  If we move to a per-core classifier, it would be
> -             * reasonable. */
> -            if (OVS_LIKELY(error != ENOSPC)
> -                && !dp_netdev_lookup_flow(dp, mfs[i])) {
> -                dp_netdev_flow_add(dp, &match, ofpbuf_data(add_actions),
> -                                   ofpbuf_size(add_actions));
> +            if (OVS_LIKELY(error != ENOSPC)) {
> +                keys[i].hash = netdev_flow_key_hash(&keys[i]);
> +
> +                /* XXX: There's a race window where a flow covering this
> packet
> +                 * could have already been installed since we last did
> the flow
> +                 * lookup before upcall.  This could be solved by moving
> the
> +                 * mutex lock outside the loop, but that's an awful long
> time
> +                 * to be locking everyone out of making flow installs.
> If we
> +                 * move to a per-core classifier, it would be reasonable.
> */
> +                ovs_mutex_lock(&dp->flow_mutex);
> +                netdev_flow = dp_netdev_lookup_flow(dp, &keys[i]);
> +                if (OVS_LIKELY(!netdev_flow)) {
> +                    netdev_flow = dp_netdev_flow_add(dp, &match,
> +
>  ofpbuf_data(add_actions),
> +
>  ofpbuf_size(add_actions));
> +                }
> +                ovs_mutex_unlock(&dp->flow_mutex);
> +
> +                /* EMC uses different hash. */
> +                keys[i].hash = dpif_packet_get_dp_hash(packets[i]);
> +                emc_insert(flow_cache, &keys[i], netdev_flow);
>


Could we just assign the 'rules[i] = &netdev_flow->cr;' and install them in
the
end of function.

Also seems to me that if we do not do 'rules[i] = &netdev_flow->cr;', the
pkt
will not be queued in 'dp_netdev_queue_batches()' at the end of function.



>
> +/* Insert 'rule' into 'cls'. */
> +static void
> +dpcls_insert(struct dpcls *cls, struct dpcls_rule *rule,
> +             const struct netdev_flow_key *mask)
> +{
> +    struct dpcls_subtable *subtable = dpcls_find_subtable(cls, mask);
> +
> +    rule->mask = &subtable->mask;
> +    cmap_insert(&subtable->rules, &rule->cmap_node, rule->flow.hash);
> +}
> +
>

Could we add thread-safety tags to these dpctl_*()?