[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