[ovs-dev] [PATCH 0/3] Initial support for new SIP Alg.

Mark Michelson mmichels at redhat.com
Wed Jan 10 15:46:37 UTC 2018


Hi Tiago,

Apologies if my message came across as dismissive or offensive (and to 
you as well, Darrell). I realize after re-reading it that I could have 
stated my intentions better. I never intended to discourage you in what 
you are contributing.

Thanks for the mailing list link. I had missed that conversation. I read 
through the thread and it sounds like the main goal is to be able to 
associate potential media addresses/ports with the corresponding signaling.

Several of the scenarios I thought of yesterday (forking, early media, 
SIP UPDATE, RTCP-MUX, and BUNDLE) are not as problematic as I had 
previously thought. My concern had been that any of the above could 
change the addresses used for media beyond a simple reading of an SDP. 
However, for conntrack purposes, this isn't probably that big a deal 
since you're not trying to zero in on *the* exact negotiated addresses 
that will be used for media. You're more concerned with being prepared 
for what addresses/ports potentially will be used for media and 
associating traffic on those addresses/ports appropriately. If you're 
prepared to handle traffic from some address/port but don't actually 
receive any media from there, it's not a big issue.

The biggest item in my list that still could be of concern is ICE. With 
ICE, endpoints list candidate addresses and ports to use for media and 
then negotiate with each other to find the best candidate pair to use. 
So for conntrack purposes, this would require some more in-depth SDP 
parsing.

The rest of my notes are on more general findings in the code submission 
itself (and in some cases, you've acknowledged the potential problems 
yourself). I'll perform a code review sometime soon with this information.

What's important is clearly defining the scope and use cases that will 
be supported. For a traditional telephone scenario, what you have 
written is on the right track. If we have no intentions of supporting 
more "advanced" scenarios (e.g. multiple media streams, WebRTC, ICE) 
then that's fine, as long as we're clear about it.


On 01/10/2018 05:47 AM, Tiago Lam wrote:
> Hi Mark,
> 
> Your email strikes me as if you haven't read the previous email exchange 
> at:
> 
>      https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341089.html
> 
>      (TL;DR I've asked similar questions there)
> 
> Having said that, and knowing this is happening, my intention is exactly 
> this; Contribute with knowledge, where and how I can, in the form of 
> code or otherwise, _to help_ bring that into life.
> 
> I'm sure no-one is underestimating how complex SIP is, I certainly am 
> not. As a side note, it is also true that we are not looking into 
> writing a complete SIP stack, but something that allows us to verify the 
> exchanged traffic; It is my understanding that many shortcuts can be 
> taken with this in mind.
> 
> Hopefully this, together with the email I pointed to, clarifies things a 
> bit better.
> 
> Tiago
> 
> On 01/09/2018 07:33 PM, Mark Michelson wrote:
>> On 12/22/2017 01:53 PM, Tiago Lam wrote:
>>> This patch-set is an initial approach at implementing the new SIP Alg,
>>> mentioned by Aaron at [1].
>>>
>>> I'm mostly interested in getting to know your thoughts of how this is
>>> headed. There are a couple of points that are worth bringing up:
>>> - As mentioned in patches 1/3 and 2/3, this is still a preliminary
>>>     implementation, and some work will be needed to move away from some
>>>     assuptions, like assuming the SIP traffic is always going over IPv4
>>>     and TCP;
>>> - At the moment, the sip state is being stored in the conn struct. I
>>>     followed the example of seq_skew_dir here, which is also stored 
>>> there,
>>>     but realise this is not ideal. It seems storing it somewhere 
>>> agnostic
>>>     will be ideal in the future, to avoid polluting that struct with
>>>     different Alg's details;
>>> - The SIP helpers functions and structures are in conntrack-sip.h and
>>>     conntrack-sip.c. This can create confusion when comparing to
>>>     conntrack-tcp.c and other protocols since SIP is an Alg and is at a
>>>     different level.
>>>
>>> With regards to testing, for now, this has been tested manually, by
>>> setting up the flows mentioned in patch 2/3 and having two VMs connected
>>> to OvS, both using SIPp to simulate real traffic both ways. I'm going to
>>> have a look at how this can be automated and added to
>>> tests/system-traffic.at, together with the rest of the already existing
>>> tests.
>>>
>>> [1] [CONNTRACK] Discussions at OvS 2017:
>>>       
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341089.html
>>>
>>> Tiago Lam (3):
>>>     Conntrack: Add new API for future SIP Alg.
>>>     Conntrack: Add initial support for new SIP Alg.
>>>     Conntrack: Support asymmetric RTP port for SIP.
>>>
>>>    include/openvswitch/ofp-actions.h |   4 +
>>>    lib/automake.mk                   |   2 +
>>>    lib/conntrack-private.h           |   2 +
>>>    lib/conntrack-sip.c               | 491 
>>> ++++++++++++++++++++++++++++++++++++++
>>>    lib/conntrack-sip.h               | 123 ++++++++++
>>>    lib/conntrack.c                   | 254 +++++++++++++++++++-
>>>    lib/ofp-parse.c                   |   5 +
>>>    ofproto/ofproto-dpif-xlate.c      |   3 +
>>>    8 files changed, 883 insertions(+), 1 deletion(-)
>>>    create mode 100644 lib/conntrack-sip.c
>>>    create mode 100644 lib/conntrack-sip.h
>>>
>>
>> Hi Tiago,
>>
>> Before starting in my current role, I worked for ten years doing VoIP
>> development with a strong focus on SIP.
>>
>> SIP is a beast of a protocol, and seeing this patchset, I made some
>> notes about how things might possibly go wrong, and ... let's just say
>> there are quite a few :). I know that you are writing this as a proof of
>> concept and that you have many TODO-style comments owning up to the fact
>> that you know about some things that are missing, but I think it may go
>> deeper than you or anyone on the OVS development team realize.
>>
>> I'm willing to help out in this effort by donating SIP knowledge, but
>> before that, I'm curious what the end goal of the SIP ALG actually is. I
>> have a feeling that the effort and expense of writing, and more
>> importantly maintaining, a proper SIP ALG may not be worth it.
>>
>> As a side point, when I was working on VoIP software, one of the first
>> things we'd tell people who came to us with connectivity issues was
>> "turn off the SIP ALG in your router". This on its own resolved issues
>> with an alarming frequency.
>>
>> Mark
>>



More information about the dev mailing list