[Asterisk-code-review] res_fax: wrap v21 detected Asterisk initiated negotiation with config... (asterisk[13])

Joshua Colp asteriskteam at digium.com
Thu Jan 2 08:44:03 CST 2020


Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13430 )

Change subject: res_fax: wrap v21 detected Asterisk initiated negotiation with config option
......................................................................

res_fax: wrap v21 detected Asterisk initiated negotiation with config option

A previous patch:

Gerrit Change-Id: I73bb24799bfe1a48adae9c034a2edbae54cc2a39

made it so a T.38 Gateway tries to negotiate with both sides by sending T.38
negotiation request to both endpoints supported T.38 versus the previous
behavior of forwarding negotiation to the "other" channel once a preamble
was detected.

This had the unfortunate side effect of breaking some setups. Specifically
ones that set the max datagram option on an endpoint configuration (configured
max datagram was not propagated since Asterisk now initiates negotiations).

This patch adds a configuration option, "negotiate_both", that when enabled
makes it so Asterisk initiates the negotiation requests to both endpoints vs.
the previous behavior of waiting, and forwarding the request.

The default is disabled keeping with the old behavior.

ASTERISK-28660

Change-Id: I5deb875f3485e20bc75119ec743090655d864a1a
---
A doc/CHANGES-staging/res_fax_negotiate_both
M include/asterisk/res_fax.h
M res/res_fax.c
3 files changed, 35 insertions(+), 1 deletion(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/doc/CHANGES-staging/res_fax_negotiate_both b/doc/CHANGES-staging/res_fax_negotiate_both
new file mode 100644
index 0000000..73be756
--- /dev/null
+++ b/doc/CHANGES-staging/res_fax_negotiate_both
@@ -0,0 +1,7 @@
+Subject: res_fax
+
+Added configuration option "negotiate_both". This option is only used
+when a gateway is enabled, and a v21 preamble is detected. If this option
+is enabled, once a preamble is detected Asterisk will initiate negotiation
+requests to both T.38 enabled endpoint versus waiting, and forwarding a
+request from an initiating endpoint. Defaults to disabled.
diff --git a/include/asterisk/res_fax.h b/include/asterisk/res_fax.h
index c1dd378..dc455bb 100644
--- a/include/asterisk/res_fax.h
+++ b/include/asterisk/res_fax.h
@@ -189,6 +189,9 @@
 	int faxdetect_flags;
 	/*! Non-zero if T.38 is negotiated */
 	int is_t38_negotiated;
+	/*! Upon v21 detection the gateway sends negotiation requests to both
+		T.38 endpoints, and do not wait on the "other" side to initiate */
+	int negotiate_both;
 };
 
 struct ast_fax_tech;
diff --git a/res/res_fax.c b/res/res_fax.c
index 8650945..1b51dbf 100644
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -228,6 +228,9 @@
 					<enum name="t38timeout">
 						<para>R/W The timeout used for T.38 negotiation.</para>
 					</enum>
+					<enum name="negotiate_both">
+						<para>R/W Upon v21 detection allow gateway to send negotiation requests to both T.38 endpoints, and do not wait on the "other" side to initiate (yes|no)</para>
+					</enum>
 				</enumlist>
 			</parameter>
 		</syntax>
@@ -724,6 +727,7 @@
 	d->gateway_id = -1;
 	d->faxdetect_id = -1;
 	d->gateway_timeout = 0;
+	d->negotiate_both = 0;
 
 	return d;
 }
@@ -3023,6 +3027,22 @@
 		enum ast_t38_state state_other;
 		enum ast_t38_state state_active;
 		struct ast_frame *fp;
+		struct ast_fax_session_details *details;
+		int negotiate_both = 0;
+
+		/*
+		 * The default behavior is to wait for the active endpoint to initiate negotiation.
+		 * Find out if this has been overridden. If so, instead of waiting have Asterisk
+		 * initiate the negotiation requests out to both endpoints.
+		 */
+		details = find_or_create_details(active);
+		if (details) {
+			negotiate_both = details->negotiate_both;
+			ao2_ref(details, -1);
+		} else {
+			ast_log(LOG_WARNING, "Detect v21 - no session details for channel '%s'\n",
+					ast_channel_name(chan));
+		}
 
 		destroy_v21_sessions(gateway);
 
@@ -3039,7 +3059,7 @@
 			}
 			/* May be called endpoint is improperly configured to rely on the calling endpoint
 			 * to initiate T.38 re-INVITEs, send T.38 negotiation request to called endpoint */
-			if (state_active == T38_STATE_UNKNOWN) {
+			if (negotiate_both && state_active == T38_STATE_UNKNOWN) {
 				ast_debug(1, "sending T.38 negotiation request to %s\n", ast_channel_name(active));
 				if (active == chan) {
 					ast_channel_unlock(chan);
@@ -4557,6 +4577,8 @@
 		ast_fax_modem_to_str(details->modems, buf, len);
 	} else if (!strcasecmp(data, "t38timeout")) {
 		snprintf(buf, len, "%u", details->t38timeout);
+	} else if (!strcasecmp(data, "negotiate_both")) {
+		ast_copy_string(buf, details->negotiate_both != -1 ? "yes" : "no", len);
 	} else {
 		ast_log(LOG_WARNING, "channel '%s' can't read FAXOPT(%s) because it is unhandled!\n", ast_channel_name(chan), data);
 		res = -1;
@@ -4695,6 +4717,8 @@
 		}
 	} else if ((!strcasecmp(data, "modem")) || (!strcasecmp(data, "modems"))) {
 		update_modem_bits(&details->modems, value);
+	} else if (!strcasecmp(data, "negotiate_both")) {
+		details->negotiate_both = ast_true(ast_skip_blanks(value));
 	} else {
 		ast_log(LOG_WARNING, "channel '%s' set FAXOPT(%s) to '%s' is unhandled!\n", ast_channel_name(chan), data, value);
 		res = -1;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13430
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I5deb875f3485e20bc75119ec743090655d864a1a
Gerrit-Change-Number: 13430
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.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-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200102/899f0d7b/attachment-0001.html>


More information about the asterisk-code-review mailing list