[Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients

[Martin Kosek]

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.

Martin