Wireshark-dev: Re: [Wireshark-dev] wireshark -- atm n:1 dissector

From: "Tamazov, Artem" <artem.tamazov@xxxxxxxxxxx>
Date: Wed, 8 Apr 2009 11:28:03 -0500
Hello, core WS developers,

In this patch you can find my ATM N:1 PW implementation and 
some contents of other patch (with SAToP/CESoPSN dissectors which 
were submitted recently as bug# 3397). The latter is needed because my ATM PW 
dissector uses some code from it.

Now you can answer questions asked by FF.

At the moment I am going to change my implementation in a way which 
will combine best approaches from both implementations.

Please have a look, thanks!

ATA 

-----Original Message-----
From: Francesco Fondelli [mailto:francesco.fondelli@xxxxxxxxx] 
Sent: Wednesday, April 08, 2009 6:30 PM
To: Tamazov, Artem
Cc: Developer support list for Wireshark
Subject: Re: wireshark -- atm n:1 dissector

On Tue, Apr 7, 2009 at 4:17 PM, Tamazov, Artem
<artem.tamazov@xxxxxxxxxxx> wrote:
>
> Hello Francesco,

Hi Artem,

first of all thanks for making my PW dissection framework better
than the original :-)

> Recently I've implemented ATM N:1 dissection and get into merge conflict
> after SVN merge.
> It looks that we have the same plans regarding Wireshark ;)
> I mean:
>     DONE:
>         - ATM N-to-One Cell Mode (with CW)
>         - ATM N-to-One Cell Mode (no CW)
>     TODO:
>         - ATM One-to-One Cell Mode
>         - ATM AAL5 SDU Mode
>         - ATM AAL5 PDU Mode

:-)

> Well, let's decide how to handle this.
>
> It first glance, my implementation hase more features related to validation
> of packets.

yes

> Also it supports some old equipment (it has a couple of options for this).

I didn't know someone was using CW length/reserved bits with a value != 0, if
they use the length to determine if a packet is valid or not they will have
some interoperability problems, it is nice that WS is able to highlight it.

> Your implementation more extensively uses existing ATM dissector, which is
> important.

filter for ATM and ATM fields is a MUST, IMHO, given the fact that there are
ATM cells within that PW.  I had to call the ATM dissector and put part
of the work there.  This looked to me the more appropriate way to accomplish
the task.  Any idea from WS people?

> For now I am going to look at your ATM PW dissector with more attention.
> Also I would like you to look at my code.
> Then we can decide how to resolve this situation.
>

ok no problem

> If you agree, I will send my patches to you (I will prepare them so you will
> not experience merge conflicts).

thanks

> What is your opinion?

a) re-spin your patch using mine as a base and get good things from
both approaches
b) revert mine (a part of it, i.e. packet-pw-eth.c is fine as it is
now, don't revert
   packet-atm.c stuff about NNI dissection option because this is fine
'per se') and apply
   your patch.

Any core WS developer opinion?

> Regards,
> Artem Tamazov//

thanks
Ciao
FF
============================================================
The information contained in this message may be privileged
and confidential and protected from disclosure. If the reader
of this message is not the intended recipient, or an employee
or agent responsible for delivering this message to the
intended recipient, you are hereby notified that any reproduction,
dissemination or distribution of this communication is strictly
prohibited. If you have received this communication in error,
please notify us immediately by replying to the message and
deleting it from your computer. Thank you. Tellabs
============================================================

Attachment: tlab-pw-atm-008-svn27983-PLUS-TDM-TEMP.patch
Description: tlab-pw-atm-008-svn27983-PLUS-TDM-TEMP.patch