On Tue, Aug 13, 2019 at 8:03 PM Darrell Ball <dlu...@gmail.com> wrote:
>> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
>> index e988626ea05b..d12b5a91c2eb 100644
>> --- a/lib/dpif-provider.h
>> +++ b/lib/dpif-provider.h
>> @@ -542,6 +542,16 @@ struct dpif_class {
>> struct ct_dpif_timeout_policy *tp);
>> int (*ct_timeout_policy_dump_done)(struct dpif *, void *state);
>>
>> + /* Gets timeout policy name based on 'tp_id', 'dl_type' and 'nw_proto'.
>> + * On success, returns 0, stores the timeout policy name in 'tp_name',
>> + * and sets 'unwildcard'. 'unwildcard' is true if the timeout
>> + * policy in 'dpif' is 'dl_type' and 'nw_proto' specific, .i.e. in
>> + * kernel datapath. Sets 'unwildcard' to false if the timeout policy
>> + * is generic to all supported 'dl_type' and 'nw_proto'. */
>> + int (*ct_get_timeout_policy_name)(struct dpif *, uint32_t tp_id,
>> + uint16_t dl_type, uint8_t nw_proto,
>> + struct ds *tp_name, bool *unwildcard);
>
> I think adding the 'unwildcard' parameter to this particular API is not
> intuitive;
> I would create a simple dedicated API for it.
As our offline discussion, we will keep this interface as is but
update comment to make the API easier to understand. I also add some
comment in the caller (in ofproto-dpif.c) to make the 'unwildcard'
idea to be more clear.
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 3013d83e96a0..8bbc596e2ce0 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> +bool
>> +ofproto_dpif_ct_zone_timeout_policy_get_name(
>> + const struct dpif_backer *backer, uint16_t zone, uint16_t dl_type,
>> + uint8_t nw_proto, struct ds *tp_name, bool *unwildcard)
>> +{
>> + struct ct_zone *ct_zone;
>> +
>> + if (!ct_dpif_timeout_policy_support_ipproto(nw_proto)) {
>> + return false;
>> + }
>> +
>> + ct_zone = ct_zone_lookup(&backer->ct_zones, zone);
>
>
> struct ct_zone *ct_zone = ct_zone_lookup(&backer->ct_zones, zone);
Done in v4.
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> +dnl Wait until the timeout expire.
>> +dnl We intend to wait a bit longer, because conntrack does not recycle the
>> entry right after it is expired.
>> +sleep 4
>
> Once the issue with sending additional packets after the first timeout expiry
> is fixed, we should enhance the test
> to resend and re-timeout to make sure it works.
Sure, will modify the test case once the kernel fix is upstream.
>> diff --git a/tests/system-userspace-macros.at
>> b/tests/system-userspace-macros.at
>> index 9d5f3bf419d3..8950a4de7287 100644
>> --- a/tests/system-userspace-macros.at
>> +++ b/tests/system-userspace-macros.at
>> +# VSCTL_ADD_ZONE_TIMEOUT_POLICY([parameters])
>> +#
>> +# Add zone based timeout policy to userspace datapath
>> +m4_define([VSCTL_ADD_ZONE_TIMEOUT_POLICY],
>
>
> TBH, does not seems useful; just hides the what is happening
Thanks for the diff in the other e-mail. I will fold in the proposed
diff in v4.
Thanks,
-Yi-Hung
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
