[asterisk-dev] [Code Review] 3207: HEP: Add a Homer Encapsulation Protocol (HEP) v3 capture agent module and a packet logger for PJSIP

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3207/#review10882
-----------------------------------------------------------

Ship it!


The only things I could comment on are minor stylistic things, so ship it! My endorsement is made with the knowledge of the C code being sound; however, my lack of knowledge about Homer means there could be protocol errors I do not know about.


/branches/12/include/asterisk/res_hep.h
<https://reviewboard.asterisk.org/r/3207/#comment20498>

    I had recommended making this unsigned before.



/branches/12/res/res_hep.c
<https://reviewboard.asterisk.org/r/3207/#comment20497>

    Any particular reason you lowercased all these? It makes it less clear now that these are constant values when used later.


- Mark Michelson


On Feb. 14, 2014, 3:58 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3207/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2014, 3:58 a.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp and Olle E Johansson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds the following:
> (1) A new module, res_hep, which implements a generic packet capture agent for the Homer Encapsulation Protocol (HEP) version 3. Note that this code is heavily based on a patch provided by Alexandr Dubovikov; I basically just wrapped it up, added configuration via the configuration framework, and threw in a taskprocessor.
> (2) A new module, res_hep_pjsip, which performs packet capturing for the PJSIP SIP stack. This is one of those modules that I think really showcases how nice the new stack is - we're able to add a new module that inserts itself into the stack and forwards the message traffic off to the res_hep module without modifying the core parts of the stack itself. This means a system administrator could load this at will on certain Asterisk systems and - if the capturing isn't needed - unload it and keep the stack 'slim'.
> 
> A few notes:
> 
> * This code exists in the following branch:
>   http://svn.asterisk.org/svn/asterisk/team/mjordan/12-hep
> * The code in the branch also contains a module for RTCP. While that actually *does* send RTCP information over HEP, it does so as a JSON blob, which is not super useful. It's an open question as to what the formatting should be, i.e., a SNOM-esque encoding, RFC 6035, etc. I'm open to suggestions on this, which is why I deferred that functionality for a later review.
> * Much thanks to Alexandr for his Asterisk patch for this code and for a *lot* of patience waiting for me to port it to 12/trunk. Due to some dithering on my part, this has taken the better part of a year to port forward (I still blame CDRs for the delay).
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_hep_pjsip.c PRE-CREATION 
>   /branches/12/res/res_hep.exports.in PRE-CREATION 
>   /branches/12/res/res_hep.c PRE-CREATION 
>   /branches/12/include/asterisk/res_hep.h PRE-CREATION 
>   /branches/12/configs/hep.conf.sample PRE-CREATION 
>   /branches/12/CHANGES 407945 
> 
> Diff: https://reviewboard.asterisk.org/r/3207/diff/
> 
> 
> Testing
> -------
> 
> An automated test that emulates a SIP capture server was written and is up for review here: https://reviewboard.asterisk.org/r/3206
> 
> This admittedly needs some *real* testing, as I have yet to stand up Kamailio with HEP. I think the code is far enough along to get some eyes on it however.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140214/ef2fc922/attachment-0001.html>