<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Kevin, Joshua,</p>
<p>Your opinions on <a class="moz-txt-link-freetext" href="https://gerrit.asterisk.org/c/asterisk/+/13370">https://gerrit.asterisk.org/c/asterisk/+/13370</a>
would be greatly appreciated.</p>
<p>I ended up doing a combination of the described by creating and
_internal variant, where purpose basically becomes log_prefix ...
and if that's supplied it enables logging.</p>
<p>There are then two wrapper functions, one with identical
interface to existing, which if purpose is given as NULL it'll
pass "" to _internal (thus enabling logging as per current
implementation), and _nolog variant which will always pass
log_prefix = NULL.</p>
<p><br>
</p>
<div class="moz-signature">
<style type="text/css">
* { padding: 0px; margin: 0px; }
body, html { font-family: Arial, San-Serif; font-size: small; color: black; padding-left: 10px; padding-top: 3px; }
a { text-decoration: none; color: #818285; }
h1 { font-size: large; }
table { font-size: 12px; }
p + p { padding-top: 1em; }
</style>
<p>Kind Regards,<br>
Jaco Kroon</p>
<p>On 2019/12/04 17:51, Kevin Harwell wrote:<br>
</p>
</div>
<blockquote type="cite"
cite="mid:CAM-yhnm-Hzh76ht1c0VGVfPc1N-bHXAKyzFzrDYwcKpsS8ZUgA@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div dir="ltr">On Wed, Dec 4, 2019 at 5:05 AM Joshua C. Colp
<<a href="mailto:jcolp@sangoma.com" moz-do-not-send="true">jcolp@sangoma.com</a>>
wrote:<br>
</div>
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div dir="ltr">On Wed, Dec 4, 2019 at 5:05 AM Jaco Kroon
<<a href="mailto:jaco@uls.co.za" target="_blank"
moz-do-not-send="true">jaco@uls.co.za</a>> wrote:<br>
</div>
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<p>Hi All,</p>
<p>In ast_apply_acl (main/acl.c) there is two lines
that's issuing a LOG_WARNING when an ACL gets
denied.</p>
<p>The first happens if the ACL is invalid. I'm not
too worried about this specific one, it's probably
a good thing if this gets logged always.</p>
<p>The latter, in the case of AST_SENSE_DENY is a
bit problematic for me. I've submitted patches
now to use ACLs in res_rtp_asterisk, and with
large number of rejects this can quickly spam the
logs, and frankly, confuse consumers.</p>
<p>As I see it, there are two possible solutions:</p>
<p>Solution 1:<br>
</p>
<p>1. Add AST_SENSE_INVALID as a possible return.<br>
2. Rename the current function to
ast_apply_acl_(silent|nolog), and remove the
logging.<br>
3. Add a replacement ast_apply_acl function which
will generate the log entries as per current.</p>
<p>Solution 2:<br>
</p>
<p>Simply don't log at all if the purpose argument
is NULL.</p>
<p>Solution two is the simpler fix, but it's
probably also the less ideal one.</p>
<p>The adding of the AST_SENSE_INVALID will also
mean that the replacement function will need to
rewrite AST_SENSE_INVALID => AST_SENSE_DENY, or
we need to audit all consumers of the function
(there fortunately isn't a great many of these)
and wherever ast_apply_acl(...) == AST_SENSE_DENY
is found, it should be rewritten as
ast_apply_acl(...) != AST_SENSE_ALLOW.</p>
<p>Would dearly like some opinions on the matter.<br>
</p>
<p>PS: The advantage for me on using ACL over HA is
simply the named ACL functionality, so in rtp.conf
I can state ice_acl = named_acl instead of having
to embed the ACL into rtp.conf<br>
</p>
</div>
</blockquote>
</div>
<div><br>
</div>
I would prefer #1 however the existing function should
behave as it does now. Each consumer should not need to be
touched, unless they are to be switched to silent. We have
an obligation to maintain ABI and behavior of API
functions as best we can in case there are any outside
consumers as well.</div>
</blockquote>
<div><br>
</div>
<div>Unfortunately due to how consumer possibly use of the
current return value I think the only way to handle this is
with #2. Or well by adding another
function ast_apply_acl_silent, and removing the logging that
is. Your new changes could call this function, and nothing
else would be potentially affected.</div>
</div>
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">
<div>
<div dir="ltr">
<div dir="ltr">Kevin Harwell
<div>Senior Software Developer</div>
<div>Sangoma Technologies<br>
<div>Check us out at: <a href="https://sangoma.com/"
target="_blank" moz-do-not-send="true">https://sangoma.com</a> &
<a href="https://asterisk.org" target="_blank"
moz-do-not-send="true">https://asterisk.org</a></div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
</blockquote>
</body>
</html>