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

[jeff.liu]

Hi Sunil,

Sorry for my later response, I somehow skip over this mail before.
Please check my comments inline.

Sunil Mushran 写道:
> Comment inline.
>
> Jeff Liu wrote:
>> this patch add the handler against dlm_query_join_response message, it
>> could deal with both ocfs2-1.2.x and the newer packet layout.
>>
>> for ocfs2-1.4.x and the newer, the response status will be dissected 
>> and shown as:
>> Status: Value (response_code:dlm_minor:fs_minor)
>>
>> Signed-off-by: Jeff Liu <jeff.liu at oracle.com>
>> ---
>> epan/dissectors/packet-ocfs2.c | 54 
>> +++++++++++++++++++++++++++++++++++++--
>> 1 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/epan/dissectors/packet-ocfs2.c 
>> b/epan/dissectors/packet-ocfs2.c
>> index ce57c3f..a4f07cc 100644
>> --- a/epan/dissectors/packet-ocfs2.c
>> +++ b/epan/dissectors/packet-ocfs2.c
>> @@ -229,12 +229,27 @@ const char * 
>> decode_enumerated_bitfield_full(guint32 val, guint32 mask, int widt
>> #define DLM_LVB_LEN 64
>> #define DLM_MOD_KEY (0x666c6172)
>>
>> -enum dlm_query_join_response {
>> +enum dlm_query_join_response_code {
>> JOIN_DISALLOW = 0,
>> JOIN_OK,
>> JOIN_OK_NO_MAP,
>> + JOIN_PROTOCOL_MISMATCH,
>> };
>>
>> +struct dlm_query_join_packet {
>> + guint8 code; /* Response code. dlm_minor and fs_minor
>> + are only valid if this is JOIN_OK */
>> + guint8 dlm_minor; /* The minor version of the protocol the
>> + dlm is speaking. */
>> + guint8 fs_minor; /* The minor version of the protocol the
>> + filesystem is speaking. */
>> + guint8 reserved;
>> +};
>> +
>> +union dlm_query_join_response {
>> + guint32 intval;
>> + struct dlm_query_join_packet packet;
>> +};
>>
>> /* DLM lock modes */
>> enum {
>> @@ -1148,10 +1163,37 @@ static void 
>> msg_struct_enumerate_fields(struct dlm_msg_struct_def *def, proto_tr
>> }
>> }
>>
>> +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.
>
>> +
>> + /* for ocfs2-1.4.x and newer */
>> + response.intval = htonl(status);
>> + packet = response.packet;
>> + proto_item_append_text(item, " (%u:", packet.code);
>> +
>> + if (packet.code == JOIN_OK)
>> + proto_item_append_text(item, "%u:%u",
>> + packet.dlm_minor, packet.fs_minor);
>> +
>> + proto_item_append_text(item, ")");
>> +}
>> +
>> static int dissect_ocfs2(tvbuff_t *tvb, packet_info *pinfo, 
>> proto_tree *tree)
>> {
>> proto_tree *subtree = NULL;
>> - proto_item *ti;
>> + proto_item *ti, *si;
>> guint16 len, magic, msg_type;
>> int ret;
>>
>> @@ -1188,7 +1230,7 @@ static int dissect_ocfs2(tvbuff_t *tvb, 
>> packet_info *pinfo, proto_tree *tree)
>> proto_tree_add_item(subtree, hf_msg_sys_status,
>> tvb, O2NET_MSG_HDR_OFF_SYS_STATUS, 4,
>> FALSE);
>> - proto_tree_add_item(subtree, hf_msg_status,
>> + si = proto_tree_add_item(subtree, hf_msg_status,
>> tvb, O2NET_MSG_HDR_OFF_STATUS, 4,
>> FALSE);
>> proto_tree_add_item(subtree, hf_msg_key,
>> @@ -1240,6 +1282,12 @@ static int dissect_ocfs2(tvbuff_t *tvb, 
>> packet_info *pinfo, proto_tree *tree)
>> }
>> }
>>
>> + if (msg_type == DLM_QUERY_JOIN_MSG) {
>> + if (magic == O2NET_MSG_STATUS_MAGIC)
>> + dissect_dlm_query_join_response(subtree,
>> + si, tvb, O2NET_MSG_HDR_OFF_STATUS);
>> + goto out;
>> + }
>>
>> if (magic == O2NET_MSG_MAGIC) {
>> struct dlm_msg_struct_def *def;
>