On Wed, Aug 7, 2019 at 11:40 AM Darrell Ball <dlu...@gmail.com> wrote:
> > > On Tue, Aug 6, 2019 at 9:57 PM Justin Pettit <jpet...@ovn.org> wrote: > >> >> > On Aug 5, 2019, at 8:07 PM, Darrell Ball <dlu...@gmail.com> wrote: >> > >> > On Thu, Aug 1, 2019 at 3:10 PM Yi-Hung Wei <yihung....@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 >> >> >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev