[Ocfs2-tools-devel] [PATCH 1/1] wireshark-ocfs2: add dlm_query_join response handler

[Sunil Mushran]

jeff.liu wrote:
> Sunil Mushran 写道:
>> jeff.liu wrote:
>>>>> +static void dissect_dlm_query_join_response(proto_tree *tree,
>>>>> + proto_item *item,
>>>>> + tvbuff_t *tvb,
>>>>> + int offset)
>>>>> +{
>>>>> + union dlm_query_join_response response;
>>>>> + struct dlm_query_join_packet packet;
>>>>> + guint32 status;
>>>>> +
>>>>> + status = tvb_get_ntohl(tvb, offset);
>>>>> +
>>>>> + /* NOP if ocfs2-1.2.x message coming */
>>>>> + if (status < JOIN_PROTOCOL_MISMATCH)
>>>>> + return ;
>>>>
>>>> Not sure I understand this. Can't you key off the size of the packet?
>>> here my idea is, by comparing the status value with 
>>> JOIN_PROTOCOL_MISMATCH to determine the message version, since the 
>>> status value defined in old ocfs-1.2.9 should be dissected from 0 to 
>>> 3, but for 1.4.2 or newer, the status value is bigger than 3 due to 
>>> the structure changed and it was packed in big-endian.
>>>
>>> It's also easy to compute the packet size to determine the message 
>>> version by figure out the buffer size start from the offset of 
>>> "Magic". I'll submit the next version in this way if you think the 
>>> current approach is not reasonable.
>>
>> We cannot rely on a specific value unless that value is hard coded
>> in the packet. Like a signature or message type, etc. In this specific
>> case, the assumption is that 1.4/mainline will not return JOIN_OK.
>> That's incorrect. Go with the packet size logic.
> I have made a wrong estimation by computing the message size to 
> determine the message version before.
> according to the comparison the size of o2net_msg structure defined in 
> tcp.h, it is same for all ocfs2 versions.
> So, in this way, the dissected package size should no different since 
> this message contains the ocfs2 header only.

True. I looked at it again. The only change between the two is that
the response could be PROTOCOL_MISMATCH in 1.4. Meaning, we simply
add the new enum and leave it as is. Should be a one line change.