<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/13804">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_outbound_registration: Fix SRV failover on timeout<br><br>In order to retry outbound registrations for some situations, we<br>need access to the tdata from the original request. For instance<br>for 401/407 responses, we need it to properly construct the<br>subsequent request with the authentication. For timeouts, we need<br>it so we can skip any failed SRV entries. We used to get the tdata<br>from the response rdata/transaction but that only worked for the<br>failures where there was actually a response. For a timeout<br>there's no response and therefore no rdata or transaction. This<br>wasn't an issue if we we're traversing SRV entries, we just retried<br>that same server. If there were SRV entries though, without the<br>original tdata, we kept doing DNS queries and trying the same<br>failed server over and over instead of skipping to the next one in<br>the list.<br><br>* Added a "last_tdata" member to the client state structure to keep<br> track of the sent tdata.<br><br>* Updated registration_client_send() to save the tdata it used into<br> the client_state.<br><br>* Updated sip_outbound_registration_response_cb() to use the tdata<br> saved in client_state instead of getting it from the response<br> rdata/transaction.<br><br>Change-Id: I199b8274392d17661dd3ce3b4d69a3968368fa06<br>---<br>M res/res_pjsip_outbound_registration.c<br>1 file changed, 37 insertions(+), 6 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/04/13804/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c</span><br><span>index 9ea6054..420c262 100644</span><br><span>--- a/res/res_pjsip_outbound_registration.c</span><br><span>+++ b/res/res_pjsip_outbound_registration.c</span><br><span>@@ -334,6 +334,14 @@</span><br><span> * module unload.</span><br><span> */</span><br><span> pjsip_regc *client;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief Last tdata sent</span><br><span style="color: hsl(120, 100%, 40%);">+ * We need the original tdata to resend a request on auth failure</span><br><span style="color: hsl(120, 100%, 40%);">+ * or timeout. On an auth failure, we use the original tdata</span><br><span style="color: hsl(120, 100%, 40%);">+ * to initialize the new tdata for the authorized response. On a timeout</span><br><span style="color: hsl(120, 100%, 40%);">+ * we need it to skip failed SRV entries if any.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_tx_data *last_tdata;</span><br><span> /*! \brief Timer entry for retrying on temporal responses */</span><br><span> pj_timer_entry timer;</span><br><span> /*! \brief Optional line parameter placed into Contact */</span><br><span>@@ -544,6 +552,13 @@</span><br><span> /* Due to the message going out the callback may now be invoked, so bump the count */</span><br><span> ao2_ref(client_state, +1);</span><br><span> /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * We also bump tdata in expectation of saving it to client_state->last_tdata.</span><br><span style="color: hsl(120, 100%, 40%);">+ * We have to do it BEFORE pjsip_regc_send because if it succeeds, it decrements</span><br><span style="color: hsl(120, 100%, 40%);">+ * the ref count on its own.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_tx_data_add_ref(tdata);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span> * Set the transport in case transports were reloaded.</span><br><span> * When pjproject removes the extraneous error messages produced,</span><br><span> * we can check status and only set the transport and resend if there was an error</span><br><span>@@ -552,13 +567,26 @@</span><br><span> pjsip_regc_set_transport(client_state->client, &selector);</span><br><span> status = pjsip_regc_send(client_state->client, tdata);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* If the attempt to send the message failed and the callback was not invoked we need to</span><br><span style="color: hsl(0, 100%, 40%);">- * drop the reference we just added</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * If the attempt to send the message failed and the callback was not invoked we need to</span><br><span style="color: hsl(120, 100%, 40%);">+ * drop the references we just added</span><br><span> */</span><br><span> if ((status != PJ_SUCCESS) && !(*callback_invoked)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_tx_data_dec_ref(tdata);</span><br><span> ao2_ref(client_state, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ return status;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * Decref the old last_data before replacing it.</span><br><span style="color: hsl(120, 100%, 40%);">+ * BTW, it's quite possible that last_data == tdata</span><br><span style="color: hsl(120, 100%, 40%);">+ * if we're trying successive servers in an SRV set.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (client_state->last_tdata) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_tx_data_dec_ref(client_state->last_tdata);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ client_state->last_tdata = tdata;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> return status;</span><br><span> }</span><br><span> </span><br><span>@@ -1072,17 +1100,17 @@</span><br><span> </span><br><span> if (param->rdata) {</span><br><span> struct pjsip_retry_after_hdr *retry_after;</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_transaction *tsx;</span><br><span> </span><br><span> retry_after = pjsip_msg_find_hdr(param->rdata->msg_info.msg, PJSIP_H_RETRY_AFTER,</span><br><span> NULL);</span><br><span> response->retry_after = retry_after ? retry_after->ivalue : 0;</span><br><span style="color: hsl(0, 100%, 40%);">- tsx = pjsip_rdata_get_tsx(param->rdata);</span><br><span style="color: hsl(0, 100%, 40%);">- response->old_request = tsx->last_tx;</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_tx_data_add_ref(response->old_request);</span><br><span> pjsip_rx_data_clone(param->rdata, 0, &response->rdata);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* old_request steals the reference */</span><br><span style="color: hsl(120, 100%, 40%);">+ response->old_request = client_state->last_tdata;</span><br><span style="color: hsl(120, 100%, 40%);">+ client_state->last_tdata = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*</span><br><span> * Transfer response reference to serializer task so the</span><br><span> * nominal path will not dec the response ref in this</span><br><span>@@ -1127,6 +1155,9 @@</span><br><span> ast_taskprocessor_unreference(client_state->serializer);</span><br><span> ast_free(client_state->transport_name);</span><br><span> ast_free(client_state->registration_name);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (client_state->last_tdata) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_tx_data_dec_ref(client_state->last_tdata);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> /*! \brief Allocator function for registration state */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/13804">change 13804</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/13804"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I199b8274392d17661dd3ce3b4d69a3968368fa06 </div>
<div style="display:none"> Gerrit-Change-Number: 13804 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>