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

Mark Michelson mmichels at redhat.com
Mon Jan 15 16:03:31 UTC 2018


Hi Tiago,

I've given patches 1 and 2 a review. I did not have any comments to add 
for patch 3. I did not add any comments for items that you already had 
XXX lines for, since you already understand what is missing.

Looking forward to the next patchset!
Mark

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
> 



More information about the dev mailing list