[ovs-dev] [PATCH v2 09/15] dpif-netdev: Execute conntrack action.

Joe Stringer joe at ovn.org
Tue Apr 19 21:52:54 UTC 2016


On 15 April 2016 at 17:02, Daniele Di Proietto <diproiettod at vmware.com> wrote:
> This commit implements the OVS_ACTION_ATTR_CT action in dpif-netdev.
>
> To allow ofproto-dpif to detect the conntrack feature, flow_put will not
> discard anymore flows with ct_* fields set. We still shouldn't allow
> flows with NAT bits set, since there is no support for NAT.
>
> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> ---

...

> @@ -2604,6 +2617,9 @@ dpif_netdev_run(struct dpif *dpif)
>      ovs_mutex_unlock(&dp->non_pmd_mutex);
>      dp_netdev_pmd_unref(non_pmd);
>
> +    /* XXX: If workload is too heavy we could add a separate thread. */
> +    conntrack_run(&dp->conntrack);
> +

Have you tried, eg, portscanning with several threads against the
implementation to see how bad it gets?


> @@ -3850,12 +3866,48 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
>          VLOG_WARN("Packet dropped. Max recirculation depth exceeded.");
>          break;
>
> -    case OVS_ACTION_ATTR_CT:
> -        /* If a flow with this action is slow-pathed, datapath assistance is
> -         * required to implement it. However, we don't support this action
> -         * in the userspace datapath. */
> -        VLOG_WARN("Cannot execute conntrack action in userspace.");
> +    case OVS_ACTION_ATTR_CT: {
> +        const struct nlattr *b;
> +        bool commit = false;
> +        unsigned int left;
> +        uint16_t zone = 0;
> +        const char *helper = NULL;
> +        const uint32_t *setmark = NULL;
> +        const struct ovs_key_ct_labels *setlabel = NULL;
> +
> +
> +        /* XXX parsing this everytime is expensive.  We should do like kernel
> +         * does and create a structure. */

Seems reasonable. You say it's expensive (how expensive?), but it also
seems a little cleaner to store it in a more accessible manner.



More information about the dev mailing list