From: Michael L. Young Date: Mon, 28 Oct 2013 14:50:12 +0000 (+0000) Subject: chan_sip: Clarify 'Forcerport' Setting Displayed When Running "sip show peers" X-Git-Tag: 11.8.0-rc1~52 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9731431f08d8ffe0f2022894e4b76b1ed91e88b6;p=thirdparty%2Fasterisk.git 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 37859df8ed..b970574f6f 100644 --- 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 c736b67e88..f43405d41c 100644 --- 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? */