[Asterisk-code-review] res_pjsip: return all codecs on a re-INVITE without SDP (asterisk[16])
George Joseph
asteriskteam at digium.com
Thu Oct 27 14:46:00 CDT 2022
George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/18990 )
Change subject: res_pjsip: return all codecs on a re-INVITE without SDP
......................................................................
res_pjsip: return all codecs on a re-INVITE without SDP
Currently chan_pjsip on receiving a re-INVITE without SDP will only
return the codecs that are previously negotiated and not offering
all enabled codecs.
This causes interoperability issues with different equipment (e.g.
from Cisco) for some of our customers and probably also in other
scenarios involving 3PCC infrastructure.
According to RFC 3261, section 14.2 we SHOULD return all codecs
on a re-INVITE without SDP
The PR proposes a new parameter to configure this behaviour:
all_codecs_on_empty_reinvite. It includes the code, documentation,
alembic migrations, CHANGES file and example configuration additions.
ASTERISK-30193 #close
Change-Id: I69763708d5039d512f391e296ee8a4d43a1e2148
---
M configs/samples/pjsip.conf.sample
A contrib/ast-db-manage/config/versions/ccf795ee535f_all_codecs_on_empty_reinvite.py
A doc/CHANGES-staging/res_pjsip_all_codecs_on_empty_reinvite_option.txt
M include/asterisk/res_pjsip.h
M res/res_pjsip/config_global.c
M res/res_pjsip/pjsip_config.xml
M res/res_pjsip_session.c
7 files changed, 157 insertions(+), 12 deletions(-)
Approvals:
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index f093b59..e211128 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -1283,6 +1283,13 @@
; creating an implicit subscription (see RFC 4488).
; (default: "yes")
+;all_codecs_on_empty_reinvite=yes
+ ; On reception of a re-INVITE without SDP Asterisk will send an SDP
+ ; offer in the 200 OK response containing all configured codecs on the
+ ; endpoint, instead of simply those that have already been negotiated.
+ ; RFC 3261 specifies this as a SHOULD requirement.
+ ; (default: "no")
+
;allow_sending_180_after_183=yes ; Allow Asterisk to send 180 Ringing to an endpoint
; after 183 Session Progress has been send.
; If disabled Asterisk will instead send only a
diff --git a/contrib/ast-db-manage/config/versions/ccf795ee535f_all_codecs_on_empty_reinvite.py b/contrib/ast-db-manage/config/versions/ccf795ee535f_all_codecs_on_empty_reinvite.py
new file mode 100644
index 0000000..125684f
--- /dev/null
+++ b/contrib/ast-db-manage/config/versions/ccf795ee535f_all_codecs_on_empty_reinvite.py
@@ -0,0 +1,37 @@
+"""all_codecs_on_empty_reinvite
+
+Revision ID: ccf795ee535f
+Revises: 539f68bede2c
+Create Date: 2022-09-28 09:14:36.709781
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'ccf795ee535f'
+down_revision = '417c0247fd7e'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import ENUM
+
+AST_BOOL_NAME = 'ast_bool_values'
+# We'll just ignore the n/y and f/t abbreviations as Asterisk does not write
+# those aliases.
+AST_BOOL_VALUES = [ '0', '1',
+ 'off', 'on',
+ 'false', 'true',
+ 'no', 'yes' ]
+
+def upgrade():
+ ############################# Enums ##############################
+
+ # ast_bool_values has already been created, so use postgres enum object
+ # type to get around "already created" issue - works okay with mysql
+ ast_bool_values = ENUM(*AST_BOOL_VALUES, name=AST_BOOL_NAME, create_type=False)
+
+ op.add_column('ps_globals', sa.Column('all_codecs_on_empty_reinvite', ast_bool_values))
+
+def downgrade():
+ if op.get_context().bind.dialect.name == 'mssql':
+ op.drop_constraint('ck_ps_globals_all_codecs_on_empty_reinvite_ast_bool_values', 'ps_globals')
+ op.drop_column('ps_globals', 'all_codecs_on_empty_reinvite')
diff --git a/doc/CHANGES-staging/res_pjsip_all_codecs_on_empty_reinvite_option.txt b/doc/CHANGES-staging/res_pjsip_all_codecs_on_empty_reinvite_option.txt
new file mode 100644
index 0000000..99eccbb
--- /dev/null
+++ b/doc/CHANGES-staging/res_pjsip_all_codecs_on_empty_reinvite_option.txt
@@ -0,0 +1,8 @@
+Subject: res_pjsip
+
+A new option named "all_codecs_on_empty_reinvite" has been added to the
+global section. When this option is enabled, on reception of a re-INVITE
+without SDP, Asterisk will send an SDP offer in the 200 OK response containing
+all configured codecs on the endpoint, instead of simply those that have
+already been negotiated. RFC 3261 specifies this as a SHOULD requirement.
+The default value is "off".
\ No newline at end of file
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 087bbbc..e75bf38 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -3725,4 +3725,11 @@
*/
struct pjsip_param *ast_sip_pjsip_uri_get_other_param(pjsip_uri *uri, const pj_str_t *param_str);
+/*!
+ * \brief Retrieve the system setting 'all_codecs_on_empty_reinvite'.
+ *
+ * \retval non zero if we should return all codecs on empty re-INVITE
+ */
+unsigned int ast_sip_get_all_codecs_on_empty_reinvite(void);
+
#endif /* _RES_PJSIP_H */
diff --git a/res/res_pjsip/config_global.c b/res/res_pjsip/config_global.c
index 4620c3f..5a71b48 100644
--- a/res/res_pjsip/config_global.c
+++ b/res/res_pjsip/config_global.c
@@ -54,6 +54,7 @@
#define DEFAULT_SEND_CONTACT_STATUS_ON_UPDATE_REGISTRATION 0
#define DEFAULT_TASKPROCESSOR_OVERLOAD_TRIGGER TASKPROCESSOR_OVERLOAD_TRIGGER_GLOBAL
#define DEFAULT_NOREFERSUB 1
+#define DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE 0
/*!
* \brief Cached global config object
@@ -119,6 +120,8 @@
enum ast_sip_taskprocessor_overload_trigger overload_trigger;
/*! Nonzero if norefersub is to be sent in Supported header */
unsigned int norefersub;
+ /*! Nonzero if we should return all codecs on empty re-INVITE */
+ unsigned int all_codecs_on_empty_reinvite;
};
static void global_destructor(void *obj)
@@ -537,6 +540,21 @@
return norefersub;
}
+unsigned int ast_sip_get_all_codecs_on_empty_reinvite(void)
+{
+ unsigned int all_codecs_on_empty_reinvite;
+ struct global_config *cfg;
+
+ cfg = get_global_cfg();
+ if (!cfg) {
+ return DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE;
+ }
+
+ all_codecs_on_empty_reinvite = cfg->all_codecs_on_empty_reinvite;
+ ao2_ref(cfg, -1);
+ return all_codecs_on_empty_reinvite;
+}
+
static int overload_trigger_handler(const struct aco_option *opt,
struct ast_variable *var, void *obj)
{
@@ -744,6 +762,9 @@
ast_sorcery_object_field_register(sorcery, "global", "norefersub",
DEFAULT_NOREFERSUB ? "yes" : "no",
OPT_YESNO_T, 1, FLDSET(struct global_config, norefersub));
+ ast_sorcery_object_field_register(sorcery, "global", "all_codecs_on_empty_reinvite",
+ DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE ? "yes" : "no",
+ OPT_BOOL_T, 1, FLDSET(struct global_config, all_codecs_on_empty_reinvite));
if (ast_sorcery_instance_observer_add(sorcery, &observer_callbacks_global)) {
return -1;
diff --git a/res/res_pjsip/pjsip_config.xml b/res/res_pjsip/pjsip_config.xml
index 8d5cf1d..bbba3d0 100644
--- a/res/res_pjsip/pjsip_config.xml
+++ b/res/res_pjsip/pjsip_config.xml
@@ -2117,6 +2117,15 @@
</para>
</description>
</configOption>
+ <configOption name="all_codecs_on_empty_reinvite" default="no">
+ <synopsis>If we should return all codecs on re-INVITE without SDP</synopsis>
+ <description><para>
+ On reception of a re-INVITE without SDP Asterisk will send an SDP
+ offer in the 200 OK response containing all configured codecs on the
+ endpoint, instead of simply those that have already been negotiated.
+ RFC 3261 specifies this as a SHOULD requirement.
+ </para></description>
+ </configOption>
</configObject>
</configFile>
</configInfo>
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index bbb2b39..3ddc9ed 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -101,7 +101,7 @@
char stream_type[1];
};
-static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer);
+static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer, const unsigned int ignore_active_stream_topology);
static int sdp_handler_list_hash(const void *obj, int flags)
{
@@ -1619,7 +1619,7 @@
pjmedia_sdp_neg_get_active_local(inv_session->neg, &previous_sdp);
}
}
- SCOPE_EXIT_RTN_VALUE(create_local_sdp(inv_session, session, previous_sdp));
+ SCOPE_EXIT_RTN_VALUE(create_local_sdp(inv_session, session, previous_sdp, 0));
}
static void set_from_header(struct ast_sip_session *session)
@@ -2543,7 +2543,7 @@
pjmedia_sdp_neg_set_remote_offer(inv_session->pool, inv_session->neg, previous_offer);
}
- new_answer = create_local_sdp(inv_session, session, previous_offer);
+ new_answer = create_local_sdp(inv_session, session, previous_offer, 0);
if (!new_answer) {
ast_log(LOG_WARNING, "Could not create a new local SDP answer for channel '%s'\n",
ast_channel_name(session->channel));
@@ -2832,7 +2832,7 @@
{
pjmedia_sdp_session *offer;
- if (!(offer = create_local_sdp(session->inv_session, session, NULL))) {
+ if (!(offer = create_local_sdp(session->inv_session, session, NULL, 0))) {
pjsip_inv_terminate(session->inv_session, 500, PJ_FALSE);
return -1;
}
@@ -4034,10 +4034,10 @@
goto end;
}
/* We are creating a local SDP which is an answer to their offer */
- local = create_local_sdp(invite->session->inv_session, invite->session, sdp_info->sdp);
+ local = create_local_sdp(invite->session->inv_session, invite->session, sdp_info->sdp, 0);
} else {
/* We are creating a local SDP which is an offer */
- local = create_local_sdp(invite->session->inv_session, invite->session, NULL);
+ local = create_local_sdp(invite->session->inv_session, invite->session, NULL, 0);
}
/* If we were unable to create a local SDP terminate the session early, it won't go anywhere */
@@ -5041,7 +5041,7 @@
return 0;
}
-static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer)
+static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer, const unsigned int ignore_active_stream_topology)
{
static const pj_str_t STR_IN = { "IN", 2 };
static const pj_str_t STR_IP4 = { "IP4", 3 };
@@ -5074,11 +5074,19 @@
/* We've encountered a situation where we have been told to create a local SDP but noone has given us any indication
* of what kind of stream topology they would like. We try to not alter the current state of the SDP negotiation
* by using what is currently negotiated. If this is unavailable we fall back to what is configured on the endpoint.
+ * We will also do this if wanted by the ignore_active_stream_topology flag.
*/
+ ast_trace(-1, "no information about stream topology received\n");
ast_stream_topology_free(session->pending_media_state->topology);
- if (session->active_media_state->topology) {
+ if (session->active_media_state->topology && !ignore_active_stream_topology) {
+ ast_trace(-1, "using existing topology\n");
session->pending_media_state->topology = ast_stream_topology_clone(session->active_media_state->topology);
} else {
+ if (ignore_active_stream_topology) {
+ ast_trace(-1, "fall back to endpoint configuration - ignore active stream topolog\n");
+ } else {
+ ast_trace(-1, "fall back to endpoint configuration\n");
+ }
session->pending_media_state->topology = ast_stream_topology_clone(session->endpoint->media.topology);
}
if (!session->pending_media_state->topology) {
@@ -5212,7 +5220,7 @@
SCOPE_EXIT_RTN("%s: handle_incoming_sdp failed\n", ast_sip_session_get_name(session));
}
- if ((answer = create_local_sdp(inv, session, offer))) {
+ if ((answer = create_local_sdp(inv, session, offer, 0))) {
pjsip_inv_set_sdp_answer(inv, answer);
SCOPE_EXIT_RTN("%s: Set SDP answer\n", ast_sip_session_get_name(session));
}
@@ -5225,6 +5233,7 @@
const pjmedia_sdp_session *previous_sdp = NULL;
pjmedia_sdp_session *offer;
int i;
+ unsigned int ignore_active_stream_topology = 0;
/* We allow PJSIP to produce an SDP if no channel is present. This may result
* in an incorrect SDP occurring, but if no channel is present then we are in
@@ -5232,8 +5241,24 @@
* produce an SDP doesn't need to worry about a channel being present or not,
* just in case.
*/
+ SCOPE_ENTER(3, "%s\n", ast_sip_session_get_name(session));
if (!session->channel) {
- return;
+ SCOPE_EXIT_RTN("%s: No channel\n", ast_sip_session_get_name(session));
+ }
+
+ /* Some devices send a re-INVITE offer with empty SDP. Asterisk by default return
+ * an answer with the current used codecs, which is not strictly compliant to RFC
+ * 3261 (SHOULD requirement). So we detect this condition and include all
+ * configured codecs in the answer if the workaround is activated.
+ */
+ if (inv->invite_tsx && inv->state == PJSIP_INV_STATE_CONFIRMED
+ && inv->invite_tsx->method.id == PJSIP_INVITE_METHOD) {
+ ast_trace(-1, "re-INVITE\n");
+ if (inv->invite_tsx->role == PJSIP_ROLE_UAS && !pjmedia_sdp_neg_was_answer_remote(inv->neg)
+ && ast_sip_get_all_codecs_on_empty_reinvite()) {
+ ast_trace(-1, "no codecs in re-INIVTE, include all codecs in the answer\n");
+ ignore_active_stream_topology = 1;
+ }
}
if (inv->neg) {
@@ -5244,9 +5269,13 @@
}
}
- offer = create_local_sdp(inv, session, previous_sdp);
+ if (ignore_active_stream_topology) {
+ offer = create_local_sdp(inv, session, NULL, 1);
+ } else {
+ offer = create_local_sdp(inv, session, previous_sdp, 0);
+ }
if (!offer) {
- return;
+ SCOPE_EXIT_RTN("%s: create offer failed\n", ast_sip_session_get_name(session));
}
ast_queue_unhold(session->channel);
@@ -5292,6 +5321,7 @@
}
*p_offer = offer;
+ SCOPE_EXIT_RTN("%s: offer created\n", ast_sip_session_get_name(session));
}
static void session_inv_on_media_update(pjsip_inv_session *inv, pj_status_t status)
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18990
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I69763708d5039d512f391e296ee8a4d43a1e2148
Gerrit-Change-Number: 18990
Gerrit-PatchSet: 12
Gerrit-Owner: Henning Westerholt <hw at gilawa.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: N A <asterisk at phreaknet.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221027/91a2673f/attachment-0001.html>
More information about the asterisk-code-review
mailing list