Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Evan Huus
It sounds to me like it shouldn’t be a set or a list, but a tree?

Evan

On Fri, Oct 6, 2017 at 08:17 Michael Mann via Wireshark-dev <
wireshark-dev@wireshark.org> wrote:

> There's also this explanation:
> https://www.wireshark.org/lists/wireshark-dev/201701/msg5.html
>
>
> -Original Message-
> From: Pascal Quantin <pascal.quan...@gmail.com>
> To: Developer support list for Wireshark <wireshark-dev@wireshark.org>
> Sent: Fri, Oct 6, 2017 3:06 am
> Subject: Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to
> frame.protocols field
>
> Hi Roland,
>
> 2017-10-06 8:23 GMT+02:00 Roland Knall <rkn...@gmail.com>:
>
> Personally I think moving to a set would reduce functionality for some
> applications. Industrial ethernet applications for instance heavily rely on
> multiple protocols being transported in single frames multiple times (one
> UDP packet contains a lot of openSAFETY frames, which themselve could
> contain data dissectors).
>
> So -1 for me for moving to a set.
>
>  @Pascal - could you point me in the direction of Michael's change you
> mentioned (pino stuff)?
>
>
> Here it is: https://code.wireshark.org/review/19464
>
>
> On Fri, Oct 6, 2017 at 7:01 AM, Pascal Quantin <pascal.quan...@gmail.com>
> wrote:
>
> Hi Guy,
>
> Le 5 oct. 2017 23:20, "Guy Harris" <g...@alum.mit.edu> a écrit :
>
> A given frame's dissection can have multiple packets for a given protocol,
> if, at any protocol layer, a PDU can contain multiple PDUs for the next
> layer above it (or parts of multiple PDUs, as with byte-stream protocols
> such as TCP).
>
> Some recent changes have been submitted to fix that for particular
> protocols.
>
> However, the underlying problem is that frame.protocols is intended to be
> a set (in which a given item can occur only once) rather than a bag (in
> which a given item can occur multiple times).  Perhaps it should be
> implemented as a set, with uniqueness enforced, so that individual
> dissectors don't need to keep from putting another  in the bag if
> there's already one there?
>
>
> What I like also with frame.protocols field is that it shows the protocol
> encapsulation order within the packet. So in case of an IP packet
> encapsulated inside a protocol running in top of IP, I think it makes sense
> to display up twice. Changing it to a set would lose this property.
>
> The problem with S1AP and Co is that it uses some dissector tables
> internally to decode the fields, leading to fake multiple occurrences
> within frame.protocols field. By the way, I realize that the pino
> functionality introduced by Michael might have been used here also instead
> of the simple patch I did. It might be an opportunity for me to see how
> this pino stuff behaves exactly ;)
>
> Cheers,
> Pascal.
>
> ___
> Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
>
>
> ___
> Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
>
> ___
> Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org>
> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe:
> https://www.wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
> <wireshark-dev-requ...@wireshark.org?subject=unsubscribe>
> ___
> Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Michael Mann via Wireshark-dev

There's also this explanation: 
https://www.wireshark.org/lists/wireshark-dev/201701/msg5.html
 
 
-Original Message-
From: Pascal Quantin <pascal.quan...@gmail.com>
To: Developer support list for Wireshark <wireshark-dev@wireshark.org>
Sent: Fri, Oct 6, 2017 3:06 am
Subject: Re: [Wireshark-dev] : avoid appending xxxx multiple times to 
frame.protocols field



Hi Roland,



2017-10-06 8:23 GMT+02:00 Roland Knall <rkn...@gmail.com>:

Personally I think moving to a set would reduce functionality for some 
applications. Industrial ethernet applications for instance heavily rely on 
multiple protocols being transported in single frames multiple times (one UDP 
packet contains a lot of openSAFETY frames, which themselve could contain data 
dissectors). 


So -1 for me for moving to a set.


 @Pascal - could you point me in the direction of Michael's change you 
mentioned (pino stuff)?



Here it is: https://code.wireshark.org/review/19464

 





On Fri, Oct 6, 2017 at 7:01 AM, Pascal Quantin <pascal.quan...@gmail.com> wrote:




Hi Guy,



Le 5 oct. 2017 23:20, "Guy Harris" <g...@alum.mit.edu> a écrit :

A given frame's dissection can have multiple packets for a given protocol, if, 
at any protocol layer, a PDU can contain multiple PDUs for the next layer above 
it (or parts of multiple PDUs, as with byte-stream protocols such as TCP).

Some recent changes have been submitted to fix that for particular protocols.

However, the underlying problem is that frame.protocols is intended to be a set 
(in which a given item can occur only once) rather than a bag (in which a given 
item can occur multiple times).  Perhaps it should be implemented as a set, 
with uniqueness enforced, so that individual dissectors don't need to keep from 
putting another  in the bag if there's already one there?





What I like also with frame.protocols field is that it shows the protocol 
encapsulation order within the packet. So in case of an IP packet encapsulated 
inside a protocol running in top of IP, I think it makes sense to display up 
twice. Changing it to a set would lose this property.


The problem with S1AP and Co is that it uses some dissector tables internally 
to decode the fields, leading to fake multiple occurrences within 
frame.protocols field. By the way, I realize that the pino functionality 
introduced by Michael might have been used here also instead of the simple 
patch I did. It might be an opportunity for me to see how this pino stuff 
behaves exactly ;)


Cheers,
Pascal. 


___
Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe




___
Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe





___Sent 
via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>Archives:
https://www.wireshark.org/lists/wireshark-devUnsubscribe: 
https://www.wireshark.org/mailman/options/wireshark-dev 
mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Roland Knall
There is code, that depends on the sequence. It is important to know
sometimes, if it is the first or the last entry.

I could do with a shortcut, but then we have to store the sequence
internally ( which is not a big issue ), but would prefer to have a signal
in the list. e.g. eth:epl:*opensafety fo determine, that that protocol is
repeated multiple times.

I do not like the solution in general, much rather would hae to have
individual packets, but this is for further changes down the line.

cheers
Roland

On Fri, Oct 6, 2017 at 9:26 AM, Guy Harris  wrote:

> On Oct 6, 2017, at 12:17 AM, Roland Knall  wrote:
>
> > Yeap, that is exactly the case with for instance openSAFETY. Usually a
> list would be eth:epl:opensafety|opensafety|opensafety (using | to better
> define the parrallel behavior).
>
> And there's code that depends on that entry being
>
> eth:epl:opensafety:opensafety:opensafety
>
> rather than just being
>
> eth:epl:opensafety
>
> even with three sequential openSAFETY packets atop Ethernet POWERLINK?
> 
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org?subject=
> unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Guy Harris
On Oct 6, 2017, at 12:17 AM, Roland Knall  wrote:

> Yeap, that is exactly the case with for instance openSAFETY. Usually a list 
> would be eth:epl:opensafety|opensafety|opensafety (using | to better define 
> the parrallel behavior).

And there's code that depends on that entry being

eth:epl:opensafety:opensafety:opensafety

rather than just being

eth:epl:opensafety

even with three sequential openSAFETY packets atop Ethernet POWERLINK?
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Roland Knall
Yeap, that is exactly the case with for instance openSAFETY. Usually a list
would be eth:epl:opensafety|opensafety|opensafety (using | to better define
the parrallel behavior).

Same goes for nearly all industrial ethernet protocols, who implement bus
coppler devices, where by definition multiple protocols can be seen on the
overlying fieldbus in a single packet.

cheers

On Fri, Oct 6, 2017 at 8:55 AM, Guy Harris  wrote:

> On Oct 5, 2017, at 11:23 PM, Roland Knall  wrote:
>
> > Personally I think moving to a set would reduce functionality for some
> applications. Industrial ethernet applications for instance heavily rely on
> multiple protocols being transported in single frames multiple times (one
> UDP packet contains a lot of openSAFETY frames, which themselve could
> contain data dissectors).
>
> So there are cases where, for example, for code that examines the protocol
> list, that code would need to see, for example, eth:ip:tcp:x11:x11:x11 for
> a TCP segment containing three X11 requests or replies, rather than just
> seeing eth:ip:tcp:x11?
>
> (BTW, the protocol list is a linearization of a structure that's not
> linear - x11:x11:x11 doesn't mean X11 inside X11 inside X11, it means 3
> X11's inside TCP.  Hopefully no software naively assumes that the protocol
> list is a tower of protocols, rather than just a representation of what you
> see if you move forward through the packet and any reassembled chunks.)
> 
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org?subject=
> unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Pascal Quantin
Hi Roland,

2017-10-06 8:23 GMT+02:00 Roland Knall :

> Personally I think moving to a set would reduce functionality for some
> applications. Industrial ethernet applications for instance heavily rely on
> multiple protocols being transported in single frames multiple times (one
> UDP packet contains a lot of openSAFETY frames, which themselve could
> contain data dissectors).
>
> So -1 for me for moving to a set.
>
>  @Pascal - could you point me in the direction of Michael's change you
> mentioned (pino stuff)?
>

Here it is: https://code.wireshark.org/review/19464


> On Fri, Oct 6, 2017 at 7:01 AM, Pascal Quantin 
> wrote:
>
>> Hi Guy,
>>
>> Le 5 oct. 2017 23:20, "Guy Harris"  a écrit :
>>
>> A given frame's dissection can have multiple packets for a given
>> protocol, if, at any protocol layer, a PDU can contain multiple PDUs for
>> the next layer above it (or parts of multiple PDUs, as with byte-stream
>> protocols such as TCP).
>>
>> Some recent changes have been submitted to fix that for particular
>> protocols.
>>
>> However, the underlying problem is that frame.protocols is intended to be
>> a set (in which a given item can occur only once) rather than a bag (in
>> which a given item can occur multiple times).  Perhaps it should be
>> implemented as a set, with uniqueness enforced, so that individual
>> dissectors don't need to keep from putting another  in the bag if
>> there's already one there?
>>
>>
>> What I like also with frame.protocols field is that it shows the protocol
>> encapsulation order within the packet. So in case of an IP packet
>> encapsulated inside a protocol running in top of IP, I think it makes sense
>> to display up twice. Changing it to a set would lose this property.
>>
>> The problem with S1AP and Co is that it uses some dissector tables
>> internally to decode the fields, leading to fake multiple occurrences
>> within frame.protocols field. By the way, I realize that the pino
>> functionality introduced by Michael might have been used here also instead
>> of the simple patch I did. It might be an opportunity for me to see how
>> this pino stuff behaves exactly ;)
>>
>> Cheers,
>> Pascal.
>>
>> 
>> ___
>> Sent via:Wireshark-dev mailing list 
>> Archives:https://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>>  mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscr
>> ibe
>>
>
>
> 
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org?subject=
> unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Guy Harris
On Oct 5, 2017, at 11:23 PM, Roland Knall  wrote:

> Personally I think moving to a set would reduce functionality for some 
> applications. Industrial ethernet applications for instance heavily rely on 
> multiple protocols being transported in single frames multiple times (one UDP 
> packet contains a lot of openSAFETY frames, which themselve could contain 
> data dissectors).

So there are cases where, for example, for code that examines the protocol 
list, that code would need to see, for example, eth:ip:tcp:x11:x11:x11 for a 
TCP segment containing three X11 requests or replies, rather than just seeing 
eth:ip:tcp:x11?

(BTW, the protocol list is a linearization of a structure that's not linear - 
x11:x11:x11 doesn't mean X11 inside X11 inside X11, it means 3 X11's inside 
TCP.  Hopefully no software naively assumes that the protocol list is a tower 
of protocols, rather than just a representation of what you see if you move 
forward through the packet and any reassembled chunks.)
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-06 Thread Roland Knall
Personally I think moving to a set would reduce functionality for some
applications. Industrial ethernet applications for instance heavily rely on
multiple protocols being transported in single frames multiple times (one
UDP packet contains a lot of openSAFETY frames, which themselve could
contain data dissectors).

So -1 for me for moving to a set.

 @Pascal - could you point me in the direction of Michael's change you
mentioned (pino stuff)?

On Fri, Oct 6, 2017 at 7:01 AM, Pascal Quantin 
wrote:

> Hi Guy,
>
> Le 5 oct. 2017 23:20, "Guy Harris"  a écrit :
>
> A given frame's dissection can have multiple packets for a given protocol,
> if, at any protocol layer, a PDU can contain multiple PDUs for the next
> layer above it (or parts of multiple PDUs, as with byte-stream protocols
> such as TCP).
>
> Some recent changes have been submitted to fix that for particular
> protocols.
>
> However, the underlying problem is that frame.protocols is intended to be
> a set (in which a given item can occur only once) rather than a bag (in
> which a given item can occur multiple times).  Perhaps it should be
> implemented as a set, with uniqueness enforced, so that individual
> dissectors don't need to keep from putting another  in the bag if
> there's already one there?
>
>
> What I like also with frame.protocols field is that it shows the protocol
> encapsulation order within the packet. So in case of an IP packet
> encapsulated inside a protocol running in top of IP, I think it makes sense
> to display up twice. Changing it to a set would lose this property.
>
> The problem with S1AP and Co is that it uses some dissector tables
> internally to decode the fields, leading to fake multiple occurrences
> within frame.protocols field. By the way, I realize that the pino
> functionality introduced by Michael might have been used here also instead
> of the simple patch I did. It might be an opportunity for me to see how
> this pino stuff behaves exactly ;)
>
> Cheers,
> Pascal.
>
> 
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org?subject=
> unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] XXXX: avoid appending xxxx multiple times to frame.protocols field

2017-10-05 Thread Pascal Quantin
Hi Guy,

Le 5 oct. 2017 23:20, "Guy Harris"  a écrit :

A given frame's dissection can have multiple packets for a given protocol,
if, at any protocol layer, a PDU can contain multiple PDUs for the next
layer above it (or parts of multiple PDUs, as with byte-stream protocols
such as TCP).

Some recent changes have been submitted to fix that for particular
protocols.

However, the underlying problem is that frame.protocols is intended to be a
set (in which a given item can occur only once) rather than a bag (in which
a given item can occur multiple times).  Perhaps it should be implemented
as a set, with uniqueness enforced, so that individual dissectors don't
need to keep from putting another  in the bag if there's already one
there?


What I like also with frame.protocols field is that it shows the protocol
encapsulation order within the packet. So in case of an IP packet
encapsulated inside a protocol running in top of IP, I think it makes sense
to display up twice. Changing it to a set would lose this property.

The problem with S1AP and Co is that it uses some dissector tables
internally to decode the fields, leading to fake multiple occurrences
within frame.protocols field. By the way, I realize that the pino
functionality introduced by Michael might have been used here also instead
of the simple patch I did. It might be an opportunity for me to see how
this pino stuff behaves exactly ;)

Cheers,
Pascal.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe