<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://reviewboard.asterisk.org/r/3304/">https://reviewboard.asterisk.org/r/3304/</a>
     </td>
    </tr>
   </table>
   <br />



 <p>Ship it!</p>



 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I verified it's working consistently and correctly.  Looking forward to phase 2!</pre>
 <br />









<p>- George Joseph</p>


<br />
<p>On March 5th, 2014, 1:19 p.m. MST, Jonathan Rose wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Asterisk Developers, George Joseph, Kevin Harwell, and Matt Jordan.</div>
<div>By Jonathan Rose.</div>


<p style="color: grey;"><i>Updated March 5, 2014, 1:19 p.m.</i></p>







<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="https://issues.asterisk.org/jira/browse/ASTERISK-23235">ASTERISK-23235</a>


</div>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">As was discussed on the mailing list, current behavior of the 'tos' setting for PJSIP transports is to interpret this value as a DSCP value. The tos settings (tos_audio/tos_video) for PJSIP endpoints on the other hand are interpreted as TOS values as their name would imply. This patch changes the transport TOS value to comply with the old behavior and allows for all TOS values to be set as strings from the DSCP string pool using the ast_str2tos function.

Upon examining the alembic scripts (which needed to be updated to allow for string values on the TOS settings), I noticed that both TOS and COS values (all of them) were set to YES/NO enum values. Obviously that's not proper since COS is an integer between 0 and 7 and TOS was previously an integer as well. Since I had to fix TOS anyway, I went ahead and updated the COS values to use integers as well.

NOTE: This is phase 1 of the effort. Phase 2 will deprecate the TOS values for both object types in favor of more modern DSCP fields. The TOS values will remain usable, but the DSCP values will override them if present.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested that wireshark would send the same DSCP values in SIP packets when using a tos value for transport that the RTP would have when using tos_audio/video.
Tested output for various TOS values as strings to see what would happen in the following cases:
  string input (numerical) - result: successful configuration (if a tos value can't be matched to a DSCP string value though, it will show as 'unknown' when examining the object configuration)
  string input (alph) from DSCP string pool - result: successful configuration
  string input (alpha) not from DSCP string pool - result: failed configuration

Tested upgrading and downgrading with alembic. Made sure the fields ended up matching my expectations in each case.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>/branches/12/res/res_pjsip/pjsip_configuration.c <span style="color: grey">(409678)</span></li>

 <li>/branches/12/res/res_pjsip/config_transport.c <span style="color: grey">(409678)</span></li>

 <li>/branches/12/main/acl.c <span style="color: grey">(409678)</span></li>

 <li>/branches/12/include/asterisk/acl.h <span style="color: grey">(409678)</span></li>

 <li>/branches/12/contrib/ast-db-manage/config/versions/4c573e7135bd_fix_tos_field_types.py <span style="color: grey">(PRE-CREATION)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/3304/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>