On Thu, 13 Nov 2014 14:20:14 -0500 Nathaniel McCallum <npmccal...@redhat.com> wrote:
> On Thu, 2014-11-13 at 14:02 -0500, Nathaniel McCallum wrote: > > On Fri, 2014-11-07 at 15:39 +0100, Martin Kosek wrote: > > > On 11/07/2014 03:28 PM, Nathaniel McCallum wrote: > > > > > > > >> On Nov 7, 2014, at 9:21 AM, Martin Kosek <mko...@redhat.com> > > > >> wrote: > > > >> > > > >> On 11/07/2014 03:03 PM, Simo Sorce wrote: > > > >>> On Fri, 07 Nov 2014 14:53:17 +0100 > > > >>> Martin Kosek <mko...@redhat.com> wrote: > > > >>> > > > >>>> On 11/07/2014 02:51 PM, Simo Sorce wrote: > > > >>>>> On Fri, 07 Nov 2014 14:26:06 +0100 > > > >>>>> Martin Kosek <mko...@redhat.com> wrote: > > > >>>>> > > > >>>>>> On 11/07/2014 02:20 PM, Simo Sorce wrote: > > > >>>>>>> On Fri, 07 Nov 2014 08:02:02 +0100 > > > >>>>>>> Martin Kosek <mko...@redhat.com> wrote: > > > >>>>>>> > > > >>>>>>>> On 11/07/2014 01:46 AM, Simo Sorce wrote: > > > >>>>>>>>> On Thu, 06 Nov 2014 18:00:21 -0500 > > > >>>>>>>>> Nathaniel McCallum <npmccal...@redhat.com> wrote: > > > >>>>>>>>> > > > >>>>>>>>>> On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote: > > > >>>>>>>>>>> > > > >>>>>>>>>>> ----- Original Message ----- > > > >>>>>>>>>>>> On 3.10.2013 23:43, Nathaniel McCallum wrote: > > > >>>>>>>>>>>>> Patch attached. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> I'm curious - what is the purpose of this patch? To > > > >>>>>>>>>>>> prevent 1 second timeouts and re-transmits when OTP > > > >>>>>>>>>>>> is in place? > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> What is the expected performance impact? Could it be > > > >>>>>>>>>>>> configured for OTP separately - somehow? (I guess > > > >>>>>>>>>>>> that it is not possible now ...) > > > >>>>>>>>>>> > > > >>>>>>>>>>> It benefits also communication of large packets (when > > > >>>>>>>>>>> large MS-PAC or CAMMAC AD Data are attached), so it > > > >>>>>>>>>>> is a better choice for IPA in general. Especially > > > >>>>>>>>>>> given we have multiple KDC processes configured we do > > > >>>>>>>>>>> not want clients wasting KDC resources by making > > > >>>>>>>>>>> multiple processes do the same operation. > > > >>>>>>>>>> > > > >>>>>>>>>> So apparently this patch never got reviewed over a > > > >>>>>>>>>> year ago. > > > >>>>>>>>>> > > > >>>>>>>>>> It was related to a bug which was opened in SSSD. > > > >>>>>>>>>> However, when it became clear we wanted to solve this > > > >>>>>>>>>> in FreeIPA, the SSSD bug was closed but no > > > >>>>>>>>>> corresponding FreeIPA bug was opened. The patch then > > > >>>>>>>>>> fell through the cracks. > > > >>>>>>>> > > > >>>>>>>> Right - without an associated ticket tracking the patch, > > > >>>>>>>> it is too easy to loose it unless the author prods > > > >>>>>>>> people to review it. > > > >>>>>>>> > > > >>>>>>>>>> Without this patch, if OTP validation runs long we get > > > >>>>>>>>>> retransmits and failures. > > > >>>>>>>>>> > > > >>>>>>>>>> One question I have is how to handle this for upgrades > > > >>>>>>>>>> since (I think) this patch only handles new installs. > > > >>>>>>>>>> > > > >>>>>>>>>> Anyway, this patch is somewhat urgent now. So help is > > > >>>>>>>>>> appreciated. > > > >>>>>>>>>> > > > >>>>>>>>>> I have attached a rebased version which has no other > > > >>>>>>>>>> changes. > > > >>>>>>>>>> > > > >>>>>>>>>> Nathaniel > > > >>>>>>>>> > > > >>>>>>>>> I am not sure we can do much on updates, we do not have > > > >>>>>>>>> a client-update tool, I would just document it I guess. > > > >>>>>>>>> Otherwise we'd have to go back to sssd which can inject > > > >>>>>>>>> additional values in krb5.conf, however I am not sure > > > >>>>>>>>> it would be ok to set something like this in the sssd's > > > >>>>>>>>> pubconf includes ... > > > >>>>>>>> > > > >>>>>>>> Agreed, pubconf update does not sound right. > > > >>>>>>>> > > > >>>>>>>> However, we already update krb5.conf on client updates, > > > >>>>>>>> in %post: > > > >>>>>>>> > > > >>>>>>>> %post client > > > >>>>>>>> if [ $1 -gt 1 ] ; then > > > >>>>>>>> # Has the client been configured? > > > >>>>>>>> restore=0 > > > >>>>>>>> test -f > > > >>>>>>>> '/var/lib/ipa-client/sysrestore/sysrestore.index' && > > > >>>>>>>> restore=$(wc -l > > > >>>>>>>> '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk > > > >>>>>>>> '{print $1}') > > > >>>>>>>> > > > >>>>>>>> if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2 > > > >>>>>>>> ]; then if ! grep -E -q > > > >>>>>>>> '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf > > > >>>>>>>> 2>/dev/null ; then > > > >>>>>>>> echo > > > >>>>>>>> "includedir /var/lib/sss/pubconf/krb5.include.d/" > > > >>>>>>>>> /etc/krb5.conf.ipanew cat /etc/krb5.conf > > > >>>>>>>>>>> /etc/krb5.conf.ipanew > > > >>>>>>>> mv /etc/krb5.conf.ipanew /etc/krb5.conf > > > >>>>>>>> /sbin/restorecon /etc/krb5.conf > > > >>>>>>>> fi > > > >>>>>>>> fi > > > >>>>>>>> ... > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> This particular update is more difficult as not a first > > > >>>>>>>> line needs to be changed. Without adding ipa client > > > >>>>>>>> update tool with some reasonable krb5.conf parser, we > > > >>>>>>>> could do something along the lines of > > > >>>>>>>> > > > >>>>>>>> sed -E 0i 's/(forwardable = \w+)/\1\n > > > >>>>>>>> udp_preference_limit = 0/g' /etc/krb5.conf > > > >>>>>>>> > > > >>>>>>>> (tested), but it is not pretty. > > > >>>>>>> > > > >>>>>>> What happen the next time you run it again ? > > > >>>>>>> > > > >>>>>>> Simo. > > > >>>>>>> > > > >>>>>> > > > >>>>>> It would of course be added again, you would need to first > > > >>>>>> grep for presence of udp_preference_limit setting. > > > >>>>>> Question is if this approach is safe enough to be in our > > > >>>>>> client %post upgrade. We already upgrade krb5.conf here, > > > >>>>>> just the change is much easier as we just add a line to > > > >>>>>> the beginning of the file. > > > >>>>> > > > >>>>> Well the concern (aside of duplication) is that an admin > > > >>>>> may "correct" the krb5.conf file to remove that option (for > > > >>>>> example because his clients also connect to a differen > > > >>>>> (older) KDC and must use UDP in preference. But now we end > > > >>>>> up messing with its krb5.conf every time an update is > > > >>>>> released. An update tool that keep tracks of whether a > > > >>>>> specific update has already been applied and does not retry > > > >>>>> every time would be needed IMO. > > > >>>>> > > > >>>>> Simo. > > > >>>> > > > >>>> In 4.1.x (as there is not much time to develop a separate > > > >>>> client update tool), we could grep just for > > > >>>> "udp_preference_limit" presence so that if admin changes > > > >>>> it's value or comment it, it would not be added again. > > > >>> > > > >>> Ok then maybe we add this: > > > >>> > > > >>> # The following value has been added by a freeipa client > > > >>> update # if you want to disable it, please comment it, do not > > > >>> delete it # or it will be re-added on the next update > > > >>> udp_preference_limit = 0 > > > >>> > > > >>> What do you think ? > > > >>> Simo. > > > >>> > > > >> > > > >> Sure, this could work (though it is quite lengthy). > > > > > > > > I think it would be sufficient to only perform the addition of > > > > udp_preference_limit if it does not already exist and if the > > > > version number of the package we are upgrading from is less > > > > than the one where we introduced the change. > > > > > > > > Nathaniel > > > > > > > > > > Or to simply call python and get the value of our state store > > > keeping track what was done on upgrades. This would work: > > > > > > # python2 -c "from ipaserver.install import sysupgrade; import > > > sys; sys.exit(1 if sysupgrade.get_upgrade_state('krb5.conf', > > > 'udp_preference_limit_set') else 0)" # echo $? > > > 0 > > > > > > # python2 -c "from ipaserver.install import sysupgrade; > > > sysupgrade.set_upgrade_state('krb5.conf', > > > 'udp_preference_limit_set', True)" > > > > > > # python2 -c "from ipaserver.install import sysupgrade; import > > > sys; sys.exit(1 if sysupgrade.get_upgrade_state('krb5.conf', > > > 'udp_preference_limit_set') else 0)" # echo $? > > > 1 > > > > > > You would just need to come up with something that's present on > > > the client, this is just on the server sub package. > > > > What about the attached approach? It is untested, but I can test it > > if we like this method. > > > > Basically, create an include directory that we ship defaults in. > > Admins can update /etc/krb5.conf to override the defaults. If we > > need to change the default, we just change the file we ship. > > > > We can enable this on upgrades by ensuring that the appropriate > > includedir statement appears at the top of the file. The nice thing > > about this approach is that is preserves admin modifications (unless > > they removed the configuration directive rather than changing it). > > > > Thoughts? > > Simo has just informed me that we will be getting /etc/krb5.conf.d at > some point in the near future. So I think for now I'd like to have the > original patch[1] reviewed (not the includedir experiment patch). We > then document the TCP issue for upgrades. Then, in the future, when we > get /etc/krb5.conf.d, we can just stick the default in there. > > Is everyone cool with this? +1 > Nathaniel > > [1] - > https://www.redhat.com/archives/freeipa-devel/2014-November/msg00098.html > -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel