On 19/04/2016 14:52, "Joe Stringer" <j...@ovn.org> wrote:
>On 15 April 2016 at 17:02, Daniele Di Proietto <diproiet...@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 <diproiet...@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?
Good point, I'm doing some testing and I'll revisit this.
>
>
>> @@ -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.
I tried creating a structure, but it doesn't seem to make a difference in
performance. It is a little cleaner, but the I guess the complexity of
maintaining a (not so different) action format doesn't seem worth for now.
I think I'll remove this comment.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
