[asterisk-dev] [Code Review]: Add a SIP nat=auto setting

[Kevin P. Fleming]

On 02/08/2012 11:44 AM, Saúl Ibarra Corretgé wrote:
>> If we continue to not do combinable options, then we would have:
>>
>> nat=no
>> nat=force_rport
>> nat=comedia
>> nat=yes
>> nat=auto_force_rport
>> nat=auto_force_comedia
>> nat=auto ; to be consistent with the nat=yes for combining the non-auto options
>>
>> This doesn't seem too bad to me, to be honest. It seems cleaner than trying to combine auto and non-auto flags. No matter what we do, people will really need to read the docs to know what each value means--but only if we choose a default that doesn't work for them. :-)
>>
>
> I like this proposal, it is consistent and it has a sense of symmetry, +1.

Well, you are correct in that the current options aren't technically 
'combinable', but right now we have two boolean options and four 
possible 'nat' values that cover all combinations of those options.

We're talking about making one of those options tri-valued (off, on, 
auto), which breaks that model. To follow what we've done in the past 
with other config options (insecure, maybe others), I think we should 
convert to combinable options. Thus the options would be:

* no
* force_rport
* auto_force_rport (default)
* comedia
* auto_comedia
* yes (supported for legacy configurations, equal to 'force_rport,comedia')

The sample config file should only document the first five. If a 
configuration specified both the plain and 'auto' versions of an option, 
then the 'auto' version would take precedence (with a suitable warning 
emitted).

This doesn't break any existing configurations, and it supports 
additional options in the future should they be necessary.

-- 
Kevin P. Fleming
Digium, Inc. | Director of Software Technologies
Jabber: kfleming at digium.com | SIP: kpfleming at digium.com | Skype: kpfleming
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at www.digium.com & www.asterisk.org