[Asterisk-code-review] chan_sip: in case of tcp/tls, be less annoying about tx errors. (asterisk[13])

[Jaco Kroon]

Jaco Kroon has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/13366 )

Change subject: chan_sip:  in case of tcp/tls, be less annoying about tx errors.
......................................................................


Patch Set 1:

> Patch Set 1: Code-Review+1
> 
> This is probably okay. However it'll miss reporting an error in at least one code path that it was previously doing. If you wanted you could log an error just before returning after the "tcptls_write_setup_error" label in the sip_tcptls_write function to catch that one.
> 
> Also please cherry pick to the 16, 17, and master branches when ready.

To be complete:

There is two code paths that previously resulted in (incorrect) errors being printed (and after having finally read the code these code paths have caused me personally numerous hours of explaining that these errors are bogus, even though I only just "fixed" this on impulse after they caused me headaches trouble-shooting another (unrelated) issue due to flooding the logs.

1.  if (!tcptls_session) .... obviously we can't write if we don't have a connection - so keep quiet about it now and if it's a reliable transmission hope the peer reconnects before it times out (nothing better we can do).
2.  The code path you mention is more complex, there are four possible failure modes:
2.1 fd == -1 - I'm assuming no connection - whould probably be treated like 1 above.
2.2 issues getting some thread - doesn't look like ao2_t_find will print an error ...
2.3 ao2_alloc error for packet; and 
2.4 packet string alloc error - both presumably memory errors.  Not sure if they'll get logged.

I suspect the four paths for 2 should be split up, and more detailed logs be printed for each sub-branch, before going to the tcptls_write_setup_error label.

If you're happy with that as a solution, it'll end up that:

1 and 2.1 never gets logged.  It'll be like sending a udp frame that gets lost on the wire somewhere.
2.2 will be logged as "Unable to locate tcptls helper thread"
2.3 and 2.4 will get logged as "memory allocation failure"

Happy?


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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I46eb82924beeff9dfd86fa6c7eb87d2651b950f2
Gerrit-Change-Number: 13366
Gerrit-PatchSet: 1
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Thu, 05 Dec 2019 18:43:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191205/bb0e327a/attachment.html>