[ovs-dev] [PATCH v2 5/9] ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables
Darrell Ball
dlu998 at gmail.com
Wed Aug 7 18:51:46 UTC 2019
On Wed, Aug 7, 2019 at 11:40 AM Darrell Ball <dlu998 at gmail.com> wrote:
>
>
> On Tue, Aug 6, 2019 at 9:57 PM Justin Pettit <jpettit at ovn.org> wrote:
>
>>
>> > On Aug 5, 2019, at 8:07 PM, Darrell Ball <dlu998 at gmail.com> wrote:
>> >
>> > On Thu, Aug 1, 2019 at 3:10 PM Yi-Hung Wei <yihung.wei at gmail.com>
>> wrote:
>> >
>> >> +struct ct_timeout_policy {
>> >> + struct uuid uuid;
>> >> + unsigned int last_used_seqno;
>> >> + struct ct_dpif_timeout_policy cdtp;
>> >> + struct cmap_node node; /* Element in struct dpif_backer's
>> >> + * "ct_tps" cmap. */
>> >>
>> >
>> >
>> > This looks like a layering violation
>> > should be in dpif-netlink or netlink-conntrack for kernel side
>>
>> Hi, Darrell. As I mentioned in my code review, I had my own concerns
>> about layering, but mine were from the top-down. Yi-Hung and I didn't
>> understand your concern here, since these seem to be structures that would
>> be useful regardless of the implementation. Can you explain a bit more
>> about your layering concerns?
>>
>
>
> I was off yesterday afternoon.
>
> There are 3 behaviors with the patchset that are datapath specific
>
> 1/ Unwildcarding of commit flows with timeout policies
> As we discussed, the userspace conntrack does not need to do this and
> would not since it is suboptimal
> since unnecessary flows are generated.
> This is because userspace conntrack would use a single shared profile
> across all dl_types and ip_proto
> rather than expanding to 6 profiles as in the case of kernel across
> dl_types and ip_protos.
>
> 2/ Userspace datepath/conntrack can easily manage cleanup of deleted
> profiles using a refcount approach.
> For userspace conntrack, we don't need to read all the timeouts
> profiles continually and to continually try to
> delete them from top down hoping to catch a window when the profile is
> not referenced by a flow.
>
> 3/ In terms of timeout profile naming in userspace conntrack, we don't
> need to manage a separate profile ID space for
> userspace conntrack. We can simply use the uuid directly. This
> simplifies the management of profiles and always
> keeps knowledge of the profile name in sync across layers.
>
Hence, the comments for this patch center around where the implementation
code is.
I think the code should live in datapath type specific code/files.
Of course, wrappers are needed at higher layers to call the datapath
specific implementations.
>
> Thanks Darrell
>
>
>
>
>>
>> Thanks,
>>
>> --Justin
>>
>>
>>
More information about the dev
mailing list