pjsip configuration: Make transport TOS values consistent with endpoints

Transport TOS values were interpreted as DSCP values without being documented
as such. Endpoint TOS values (tos_audio/tos_video) behaved normally as TOS
values have historically. This patch makes the transport TOS values behave as
TOS values and makes all TOS values readable as string values (e.g. AF11).
In addition, alembic scripts have been updated to use the proper field types
for all TOS/COS values.

(issue ASTERISK-23235)
Reported by: George Joseph
Review: https://reviewboard.asterisk.org/r/3304/
........

Merged revisions 410028 from http://svn.asterisk.org/svn/asterisk/branches/12

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

diff --git a/UPGRADE.txt b/UPGRADE.txt
index b996f072c2e4a4eaf0afa74adb8b445be3a26103..a3e808c7f71849fd79eca23a220ab2595b18b63a 100644 (file)
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -27,6 +27,13 @@ PJSIP:
    PJSIP contacts, this means that the schema has been updated to add a user_agent
    column. An alembic revision has been added to facilitate this update.
 
+Realtime Configuration:
+ - PJSIP endpoint columns 'tos_audio' and 'tos_video' have been changed from yes/no
+   enumerators to string values. 'cos_audio' and 'cos_video' have been changed from
+   yes/no enumerators to integer values. PJSIP transport column 'tos' has been
+   changed from a yes/no enumerator to a string value. 'cos' has been changed from
+   a yes/no enumerator to an integer value.
+
 From 12.0.0 to 12.1.0:
 * The sound_place_into_conference sound used in Confbridge is now deprecated
   and is no longer functional since it has been broken since its inception

diff --git a/contrib/ast-db-manage/config/versions/4c573e7135bd_fix_tos_field_types.py b/contrib/ast-db-manage/config/versions/4c573e7135bd_fix_tos_field_types.py
new file mode 100755 (executable)
index 0000000..51e8fa9
--- /dev/null
+++ b/contrib/ast-db-manage/config/versions/4c573e7135bd_fix_tos_field_types.py
@@ -0,0 +1,61 @@
+"""Fix tos and cos field types
+
+Revision ID: 4c573e7135bd
+Revises: 21e526ad3040
+Create Date: 2014-03-05 12:16:56.618630
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '4c573e7135bd'
+down_revision = '21e526ad3040'
+
+from alembic import op
+from alembic import context
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import ENUM
+
+YESNO_NAME = 'yesno_values'
+YESNO_VALUES = ['yes', 'no']
+
+def upgrade():
+    op.alter_column('ps_endpoints', 'tos_audio',
+                    type_=sa.String(10))
+    op.alter_column('ps_endpoints', 'tos_video',
+                    type_=sa.String(10))
+    op.alter_column('ps_transports', 'tos',
+                    type_=sa.String(10))
+
+    # Can't cast YENO_VALUES to Integers, so dropping and adding is required
+    op.drop_column('ps_endpoints', 'cos_audio')
+    op.drop_column('ps_endpoints', 'cos_video')
+    op.drop_column('ps_transports', 'cos')
+
+    op.add_column('ps_endpoints', sa.Column('cos_audio', sa.Integer))
+    op.add_column('ps_endpoints', sa.Column('cos_video', sa.Integer))
+    op.add_column('ps_transports', sa.Column('cos', sa.Integer))
+    pass
+
+
+def downgrade():
+
+    yesno_values = ENUM(*YESNO_VALUES, name=YESNO_NAME, create_type=False)
+
+    # Can't cast string to YESNO_VALUES, so dropping and adding is required
+    op.drop_column('ps_endpoints', 'tos_audio')
+    op.drop_column('ps_endpoints', 'tos_video')
+    op.drop_column('ps_transports', 'tos')
+
+    op.add_column('ps_endpoints', sa.Column('tos_audio', yesno_values))
+    op.add_column('ps_endpoints', sa.Column('tos_video', yesno_values))
+    op.add_column('ps_transports', sa.Column('tos', yesno_values))
+
+    # Can't cast integers to YESNO_VALUES, so dropping and adding is required
+    op.drop_column('ps_endpoints', 'cos_audio')
+    op.drop_column('ps_endpoints', 'cos_video')
+    op.drop_column('ps_transports', 'cos')
+
+    op.add_column('ps_endpoints', sa.Column('cos_audio', yesno_values))
+    op.add_column('ps_endpoints', sa.Column('cos_video', yesno_values))
+    op.add_column('ps_transports', sa.Column('cos', yesno_values))
+    pass

diff --git a/include/asterisk/acl.h b/include/asterisk/acl.h
index d1773b6b16b6a916b2b85a3a845800d51326bb60..502e7f447593c1f84f0e3caa2aee41d7ee3e584e 100644 (file)
--- a/include/asterisk/acl.h
+++ b/include/asterisk/acl.h
@@ -357,6 +357,17 @@ int ast_str2tos(const char *value, unsigned int *tos);
  */
 const char *ast_tos2str(unsigned int tos);
 
+/*!
+ * \brief Convert a TOS value into its string representation
+ *        and create a dynamically allocated copy
+ *
+ * \param tos The TOS value to look up
+ * \param buf pointer to character pointer where string will be duplicated to
+ *
+ * \note The string allocated at buf must be free'd
+ */
+void ast_tos2str_buf(unsigned int tos, char **buf);
+
 /*!
  * \brief Retrieve a named ACL
  *

diff --git a/main/acl.c b/main/acl.c
index c341125fd681afe3d91220a98433ba69cb2a8d14..a55e87458da7e605997a25cf6de629855f765e08 100644 (file)
--- a/main/acl.c
+++ b/main/acl.c
@@ -894,6 +894,12 @@ const char *ast_tos2str(unsigned int tos)
        return "unknown";
 }
 
+void ast_tos2str_buf(unsigned int tos, char **buf)
+{
+       const char *tos_string = ast_tos2str(tos);
+       *buf = ast_strdup(tos_string);
+}
+
 int ast_get_ip(struct ast_sockaddr *addr, const char *hostname)
 {
        return ast_get_ip_or_srv(addr, hostname, NULL);

diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c
index 3c11dcc70301674b3a24b71457d84f06eaa44294..58d0f157754139e5b22e63970c381874e2ddd953 100644 (file)
--- a/res/res_pjsip/config_transport.c
+++ b/res/res_pjsip/config_transport.c
@@ -121,9 +121,11 @@ static void *transport_alloc(const char *name)
 
 static void set_qos(struct ast_sip_transport *transport, pj_qos_params *qos)
 {
+       int tos_as_dscp = transport->tos >> 2;
+
        if (transport->tos) {
                qos->flags |= PJ_QOS_PARAM_HAS_DSCP;
-               qos->dscp_val = transport->tos;
+               qos->dscp_val = tos_as_dscp;
        }
        if (transport->cos) {
                qos->flags |= PJ_QOS_PARAM_HAS_SO_PRIO;
@@ -448,6 +450,39 @@ static int localnet_to_str(const void *obj, const intptr_t *args, char **buf)
        return 0;
 }
 
+/*! \brief Custom handler for TOS setting */
+static int transport_tos_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
+{
+       struct ast_sip_transport *transport = obj;
+       unsigned int value;
+
+       if (ast_str2tos(var->value, &value)) {
+               ast_log(LOG_ERROR, "Error configuring transport '%s' - Could not "
+                       "interpret 'tos' value '%s'\n",
+                       ast_sorcery_object_get_id(transport), var->value);
+               return -1;
+       }
+
+       if (value % 4) {
+               value = value >> 2;
+               value = value << 2;
+               ast_log(LOG_WARNING,
+                       "transport '%s' - 'tos' value '%s' uses bits that are "
+                       "discarded when converted to DSCP. Using equivalent %d instead.\n",
+                       ast_sorcery_object_get_id(transport), var->value, value);
+       }
+
+       transport->tos = value;
+       return 0;
+}
+
+static int tos_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+       const struct ast_sip_transport *transport = obj;
+       ast_tos2str_buf(transport->tos, buf);
+       return 0;
+}
+
 static struct ao2_container *cli_get_container(void)
 {
        RAII_VAR(struct ao2_container *, container, NULL, ao2_cleanup);
@@ -581,7 +616,7 @@ int ast_sip_initialize_sorcery_transport(void)
        ast_sorcery_object_field_register_custom(sorcery, "transport", "method", "", transport_tls_method_handler, tls_method_to_str, 0, 0);
        ast_sorcery_object_field_register_custom(sorcery, "transport", "cipher", "", transport_tls_cipher_handler, transport_tls_cipher_to_str, 0, 0);
        ast_sorcery_object_field_register_custom(sorcery, "transport", "local_net", "", transport_localnet_handler, localnet_to_str, 0, 0);
-       ast_sorcery_object_field_register(sorcery, "transport", "tos", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_transport, tos));
+       ast_sorcery_object_field_register_custom(sorcery, "transport", "tos", "0", transport_tos_handler, tos_to_str, 0, 0);
        ast_sorcery_object_field_register(sorcery, "transport", "cos", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_transport, cos));
 
        ast_sip_register_endpoint_formatter(&endpoint_transport_formatter);

diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index deb374e6e3f2c371c36498255caba55804b5e904..994987b7a91cb7956d71c4787200f44470e2db85 100644 (file)
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -758,6 +758,45 @@ static int t38udptl_ec_to_str(const void *obj, const intptr_t *args, char **buf)
        return 0;
 }
 
+static int tos_handler(const struct aco_option *opt,
+       struct ast_variable *var, void *obj)
+{
+       struct ast_sip_endpoint *endpoint = obj;
+       unsigned int value;
+
+       if (ast_str2tos(var->value, &value)) {
+               ast_log(LOG_ERROR, "Error configuring endpoint '%s' - Could not "
+                       "interpret '%s' value '%s'\n",
+                       ast_sorcery_object_get_id(endpoint), var->name, var->value);
+               return -1;
+       }
+
+       if (!strcmp(var->name, "tos_audio")) {
+               endpoint->media.tos_audio = value;
+       } else if (!strcmp(var->name, "tos_video")) {
+               endpoint->media.tos_video = value;
+       } else {
+               /* If we reach this point, someone called the tos_handler when they shouldn't have. */
+               ast_assert(0);
+               return -1;
+       }
+       return 0;
+}
+
+static int tos_audio_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+       const struct ast_sip_endpoint *endpoint = obj;
+       ast_tos2str_buf(endpoint->media.tos_audio, buf);
+       return 0;
+}
+
+static int tos_video_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+       const struct ast_sip_endpoint *endpoint = obj;
+       ast_tos2str_buf(endpoint->media.tos_video, buf);
+       return 0;
+}
+
 static int set_var_handler(const struct aco_option *opt,
        struct ast_variable *var, void *obj)
 {
@@ -1586,8 +1625,8 @@ int ast_res_pjsip_initialize_configuration(const struct ast_module_info *ast_mod
        ast_sorcery_object_field_register(sip_sorcery, "endpoint", "allow_transfer", "yes", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, allowtransfer));
        ast_sorcery_object_field_register(sip_sorcery, "endpoint", "sdp_owner", "-", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, media.sdpowner));
        ast_sorcery_object_field_register(sip_sorcery, "endpoint", "sdp_session", "Asterisk", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, media.sdpsession));
-       ast_sorcery_object_field_register(sip_sorcery, "endpoint", "tos_audio", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_endpoint, media.tos_audio));
-       ast_sorcery_object_field_register(sip_sorcery, "endpoint", "tos_video", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_endpoint, media.tos_video));
+       ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "tos_audio", "0", tos_handler, tos_audio_to_str, 0, 0);
+       ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "tos_video", "0", tos_handler, tos_video_to_str, 0, 0);
        ast_sorcery_object_field_register(sip_sorcery, "endpoint", "cos_audio", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_endpoint, media.cos_audio));
        ast_sorcery_object_field_register(sip_sorcery, "endpoint", "cos_video", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_endpoint, media.cos_video));
        ast_sorcery_object_field_register(sip_sorcery, "endpoint", "allow_subscribe", "yes", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, subscription.allow));