On Thu, 2017-10-26 at 22:14 +0200, Sumit Bose wrote: > On Thu, Oct 26, 2017 at 02:43:29PM -0400, Simo Sorce wrote: > > On Thu, 2017-10-26 at 12:16 +0200, Jakub Hrozek wrote: > > > On Wed, Oct 25, 2017 at 05:39:21PM +0200, Sumit Bose wrote: > > > > Hi, > > > > > > > > please find below the design document for the enhanced NSS API > > > > which > > > > makes e.g. the client side timeouts which where recently > > > > refactored > > > > available to callers. > > > > > > > > A more visual friendly version can be found at: > > > > https://pagure.io/fork/sbose/SSSD/docs/blob/07514ce52845d47fe6b > > > > b327 > > > > f782a865bfa75628a/f/design_pages/enhanced_nss_api.rst > > > > > > > > bye, > > > > Sumit > > > > > > LGTM! > > > > > > > Looking at this I have some questions, if you are going to create a > > new > > library and just need to set a timeout it seem it would be a much > > better interface to use a context handle you allocate and pass into > > each call, and then have getters setters to set timeouts or any > > other > > flags that should influence the whole behavior. This will allow you > > also to control how many concurrent connections you want to have > > against sssd, as each new context will create a new socket > > connection > > and all. > > > > In the original libnss_sss.so we could not do that because the > > glibc > > interface does not offer any way to hold a context, but there is no > > reason to continue along that line in a *new* API. And not using > > contexts in threaded applications is generally a bad idea, as you > > end > > up *requiring* te use of mutexes when that is really not always a > > need > > (separated threads can open separate connections and not share any > > data > > that require mutexes). > > This sounds like a good 2.0 feature, are you interested in creating a > more detailed design page for this?
Sure. > Currently my goal was to reuse as > much or the current trusted code we already have. > > > > > On the responder side I also do not see why new calls are being > > created. You clearly want a client-wide behavior, introduce ONE new > > call that sets flags for the rest of the connection, and just reuse > > the > > usual commands otherwise. > > The current flags, like invalidating a cached entry are per-request, > only the single object address by the current request should be > invalidate not all object which are requested on the same connection. I would probably add a command to explicitly invalidate individual caches, this would avoid having special paths on every other call, resulting in cleaner code, at the cost of one more roundtrip though, so I guess it is a matter of figuring out what is the right balance here. > > > > I do not understand what is the point of nss_truste_users why a > > force > > reload is a privileged operation ? > > Since it can force expensive operations on the backends which will > hit servers I think not everybody should be allowed to do this. You can already force this by requesting unexisting users/groups, I am not convinced this necessarily needs to be a privileged operation as there are already ways to cause work for SSSD. I would rather drop it. If we really want to deal with potential abuse we should introduce rate-limiting per uid, and basically slow down to a halt abusing clients by giving weights and possibly quotas, doling out privileges is cumbersome anyway and does not really prevent a malicious client to cause hard ATM. IMHO nss_trusted_users gets a NACK as a concept and should be dropped. > > > > I guess DNLSGTM ? > > A test-builds with a simplified version of the related patches solved > bottleneck issues with 389ds IPA modules. So at least it would help > without introducing other issues. I did not know we already had code, and I am not against the goal of improving the bottlenecks at all, but we should definitely improve the design here as this design is way too conservative and introduces APIs that we probably want to toss with a better design. So please mark those functions "private/tech preview/whatever" by putting them behind a guarding #ifdef in the header so that people really need to know what they are doing to use them and we can more easily deprecate and remove them quickly. Simo. > bye, > Sumit > > > > > Simo. > > > > -- > > Simo Sorce > > Sr. Principal Software Engineer > > Red Hat, Inc > > _______________________________________________ > > sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org > > To unsubscribe send an email to sssd-devel-leave@lists.fedorahosted > > .org > > _______________________________________________ > sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org > To unsubscribe send an email to sssd-devel-leave@lists.fedorahosted.o > rg -- Simo Sorce Sr. Principal Software Engineer Red Hat, Inc _______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
