[asterisk-dev] [Code Review] RTP monitoring branch: initial review

[Tzafrir Cohen]

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

Review request for Asterisk Developers and Tzafrir Cohen.


Summary
-------

I'm working on the RTP monitoring branch:

  http://svn.digium.com/svn/asterisk/team/tzafrir/monitor-rtp/
  http://svn.digium.com/svn/asterisk/team/tzafrir/monitor-rtp/doc/monitor-rtp.txt

Basically this branch intends to allow sending all the recorded
(Monitor) files to a remote recording server instead of local files.
This could be recorded by some remote recording server or even by
a sniffer such as Wireshark.

At the moment the code basically works and now I want to figure out how
to best fit it in Asterisk.

I'm aware that the code could use some improvements, and I have some
items on my TODO list (at the end of monitor-rtp.txt). But the first
thing to do is to figure out where things are headed. Ref. the specific
questions in the description below. 

The changes involved:

1. Generation of dummy SIP calls from res_monitor.c
2. Allowing ast_writefile to write to a socket


Generating Dummy SIP Calls
--------------------------
The Recorded calls are sent over the network as faked SIP calls. That
is: dummy SIP INVITE header, an RTP stream, and a dummy BYE header.
The INVITE includes SDP that describes the port numbers of the RTP
streams. The BYE can include more information about the call (such as
the call duration from the CDR. Though see the TODO list). The recording
server will not answer us: it listens to network traffic
(using libpcap). So we basically have a faked, single-sided SIP call.

Note that there is such a call (and a socket) for each file that we
would have recorded using res_monitor. Thus we have two such "calls" for
each recorded channel.

My main target is the Oreka/Orecx recording server
(http://oreka.sourceforge.net/ ). However I also test the network
traffic with wireshark. Both of them identify what I send as "SIP calls"
(or rather: RTP stream, for wireshark, see the doc), which makes me more
cdonfident I'm not making up my own crazy format.

However such a one-sided forked SIP call doesn't fit well inside
Asterisk.

As I explain later on, we change main/file.c to allowed passing sockets to
ast_writefile. But we still have to open the socket somehow.
At the moment we treat it as a file format:

  http://svn.digium.com/svn/asterisk/team/tzafrir/monitor-rtp/formats/format_rtp.c

We can't simply pass the file format "rtp" to the monitoring
code: The format code only knows how to write, and doesn't handle
opening files, and we need to change "opening the file" (creating a
socket).

Thus extra code is added in res_monitor.c to create sockets for the RTP
stream and hand it over as the "filename".

The result, though, is ipv4 networking code inside res_monitor.c . The
nature of this one-sided faked SIP call is quite different from anything
else I can think of within Asterisk.

This is done by the bulk of the changes in res_monitor.c:

  http://svn.digium.com/svn/asterisk/team/tzafrir/monitor-rtp/res/res_monitor.c
  https://reviewboard.asterisk.org/r/394/diff/#6

Any better place for this networking code? Any interface I missed?


Writing to a socket
-------------------
We hook into a mechnism that writes to files and want to convert it to
sending packets to a remote server. Writing to a file and writing to a
socket is basically the same (write(2)). Opening is slightly different.

res_monitor writes to files (calls ast_writefile) and not to file
descriptors directly.  Our current workaround is to overload the
interface of ast_writefile:

  struct ast_filestream *ast_writefile(const char *filename, const char
        *type, const char *comment, int flags, int check, mode_t mode)

The parameter 'check' is currently unused: It seems to always get a '0'.
Hence we decided to use it to mark: if it is not 0, it is
considered to be a file descriptor. The filename pointer is the
considered to be a pointer to some state information.
(And yes, I know file descriptor 0 is a valid number).

An extra function is then needed to return the "filename" of a
filestream. I named it ast_filestream_xxx_get_filename() for now.

Any better way to allow ast_writefile to write to a socket? Any better
way to allow writing to a socket?

  http://svn.digium.com/svn/asterisk/team/tzafrir/monitor-rtp/main/file.c
  https://reviewboard.asterisk.org/r/394/diff/#5

(Not many changes. ast_writefile's declaration has not changed, but its
semantics has, as clearly seen in the small change following it)


Diffs
-----

  /trunk/configs/monitor.conf.sample PRE-CREATION 
  /trunk/doc/monitor-rtp.txt PRE-CREATION 
  /trunk/formats/format_rtp.c PRE-CREATION 
  /trunk/include/asterisk/file.h 222649 
  /trunk/include/asterisk/monitor.h 222649 
  /trunk/main/file.c 222649 
  /trunk/res/res_monitor.c 222649 

Diff: https://reviewboard.asterisk.org/r/394/diff


Testing
-------


Thanks,

Tzafrir