chan_sip: Clarify 'Forcerport' Setting Displayed When Running "sip show peers"

While looking at ASTERISK-22236, Walter Doekes pointed out that when running
"sip show peers", the setting being displayed can be confusing.  The display of
"N" used to mean NAT (i.e. yes).  The NAT setting has gone through many
different changes resulting in the display of different characters to try and
convey what the current setting is for 'Forcerport' (A for Auto and Forcerport
is currently on, a for Auto but Forcerport is off, Y for yes, and N for no).
During the initial code review to try and clarify these settings (especially
since "N" no longer meant what it used to mean in prior versions of Asterisk),
Mark Michelson suggested using the full space available to display the settings
which helped to make the settings very clear.  That was a great suggestion.

Therefore, this patch does the following:

* The column for 'Forcerport' now will show: Auto (Yes), Auto (No), Yes, or No.

* A column for the 'Comedia' setting has been added.  It too will display the
  setting in a non-cryptic way: Auto (Yes), Auto (No), Yes, or No.

* UPGRADE.txt has been updated to document this change.

(closes issue ASTERISK-22728)
Reported by: Walter Doekes
Tested by: Michael L. Young
Patches:
    asterisk-forcerport-display-clarification_v3.diff
                                     uploaded by Michael L. Young (license 5026)

Review: https://reviewboard.asterisk.org/r/2941

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/11@402111 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/UPGRADE.txt b/UPGRADE.txt
index 37859df8ed3d29ba1aa8d20f1446cf38c92340b3..b970574f6f22160b1eed90c5747fdf1f38f1df97 100644 (file)
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -25,6 +25,16 @@ ConfBridge
  - ConfBridge now has the ability to set the language of announcements to the
    conference.  The language can be set on a bridge profile in confbridge.conf
    or by the dialplan function CONFBRIDGE(bridge,language)=en.
+chan_sip - Clarify The "sip show peers" Forcerport Column And Add Comedia
+ - Under the "Forcerport" column, the "N" used to mean NAT (i.e. Yes).  With
+   the additon of auto_* NAT settings, the meaning changed and there was a
+   certain combination of letters added to indicate the current setting. The
+   combination of using "Y", "N", "A" or "a", can be confusing.  Therefore, we
+   now display clearly what the current Forcerport setting is: "Yes", "No",
+   "Auto (Yes)", "Auto (No)".
+ - Since we are clarifying the Forcerport column, we have added a column to
+   display the Comedia setting since this is useful information as well.  We
+   no longer have a simple "NAT" setting like other versions before 11.
 
 From 11.5 to 11.6:
 * res_agi will now properly indicate if there was an error in streaming an

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index c736b67e880be1fbd1e3324fc73639d3b8d0b7ba..f43405d41c9b808cc085d38ce63fdac5dfb4e2e4 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -19019,7 +19019,7 @@ int peercomparefunc(const void *a, const void *b)
 }
 
 /* the last argument is left-aligned, so we don't need a size anyways */
-#define PEERS_FORMAT2 "%-25.25s %-39.39s %-3.3s %-10.10s %-3.3s %-8s %-11s %-32.32s %s\n"
+#define PEERS_FORMAT2 "%-25.25s %-39.39s %-3.3s %-10.10s %-10.10s %-3.3s %-8s %-11s %-32.32s %s\n"
 
 /*! \brief Used in the sip_show_peers functions to pass parameters */
 struct show_peers_context {
@@ -19081,7 +19081,7 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
 
        if (!s) {
                /* Normal list */
-               ast_cli(fd, PEERS_FORMAT2, "Name/username", "Host", "Dyn", "Forcerport", "ACL", "Port", "Status", "Description", (cont.realtimepeers ? "Realtime" : ""));
+               ast_cli(fd, PEERS_FORMAT2, "Name/username", "Host", "Dyn", "Forcerport", "Comedia", "ACL", "Port", "Status", "Description", (cont.realtimepeers ? "Realtime" : ""));
        }
 
        ao2_lock(peers);
@@ -19197,9 +19197,8 @@ static struct sip_peer *_sip_show_peers_one(int fd, struct mansession *s, struct
                ast_cli(fd, PEERS_FORMAT2, name,
                tmp_host,
                peer->host_dynamic ? " D " : "   ",     /* Dynamic or not? */
-               ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_RPORT) ?
-                       ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? " A " : " a " :
-                       ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? " N " : "   ",    /* NAT=yes? */
+               force_rport_string(peer->flags),
+               comedia_string(peer->flags),
                (!ast_acl_list_is_empty(peer->acl)) ? " A " : "   ",       /* permit/deny */
                tmp_port, status,
                peer->description ? peer->description : "",
@@ -19230,7 +19229,7 @@ static struct sip_peer *_sip_show_peers_one(int fd, struct mansession *s, struct
                ast_sockaddr_isnull(&peer->addr) ? "0" : tmp_port,
                peer->host_dynamic ? "yes" : "no",      /* Dynamic or not? */
                ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_RPORT) ? "yes" : "no",
-               ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? "yes" : "no",     /* NAT=yes? */
+               ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? "yes" : "no",
                ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_COMEDIA) ? "yes" : "no",
                ast_test_flag(&peer->flags[1], SIP_PAGE2_SYMMETRICRTP) ? "yes" : "no",
                ast_test_flag(&peer->flags[1], SIP_PAGE2_VIDEOSUPPORT) ? "yes" : "no",  /* VIDEOSUPPORT=yes? */