[Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
Nathaniel McCallum
npmccallum at redhat.com
Thu Nov 13 19:20:14 UTC 2014
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 <mkosek at redhat.com> wrote:
> > >>
> > >> On 11/07/2014 03:03 PM, Simo Sorce wrote:
> > >>> On Fri, 07 Nov 2014 14:53:17 +0100
> > >>> Martin Kosek <mkosek at redhat.com> wrote:
> > >>>
> > >>>> On 11/07/2014 02:51 PM, Simo Sorce wrote:
> > >>>>> On Fri, 07 Nov 2014 14:26:06 +0100
> > >>>>> Martin Kosek <mkosek at redhat.com> wrote:
> > >>>>>
> > >>>>>> On 11/07/2014 02:20 PM, Simo Sorce wrote:
> > >>>>>>> On Fri, 07 Nov 2014 08:02:02 +0100
> > >>>>>>> Martin Kosek <mkosek at redhat.com> wrote:
> > >>>>>>>
> > >>>>>>>> On 11/07/2014 01:46 AM, Simo Sorce wrote:
> > >>>>>>>>> On Thu, 06 Nov 2014 18:00:21 -0500
> > >>>>>>>>> Nathaniel McCallum <npmccallum at 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?
Nathaniel
[1] -
https://www.redhat.com/archives/freeipa-devel/2014-November/msg00098.html
More information about the Freeipa-devel
mailing list