[Ocfs2-tools-devel] [PATCH 1/1] wireshark-ocfs2: add dlm_query_join handler to support both ocfs2-1.2 and the newer format
Sunil Mushran
sunil.mushran at oracle.com
Tue Jul 14 16:58:25 PDT 2009
Can we split this into two. One for request and the other for response.
While they are related, the changes are enough that it would be easier
to review them separately.
Secondly, always treat the current version of the structure as the correct
one. Meaning, dlm_query_join_request_latest should be dlm_query_join_request
and the older one should be marked with some other decoration.
One small point I had not mentioned and may be causing some confusion wrt
dlm_query_join_request. The original structure in the dissector does not
include the node_map. Yes, we made this change in 1.2.5 or so. But we'll
ignore that case. All this dissector needs to do is handle these two
variations.
struct dlm_query_join_request {
guint8 node_idx;
guint8 pad1[2];
guint8 name_len;
struct dlm_protocol_version dlm_proto;
struct dlm_protocol_version fs_proto;
guint8 domain[O2NM_MAX_NAME_LEN];
guint8 node_map[BITS_TO_BYTES(O2NM_MAX_NODES)];
};
struct _dlm_query_join_request {
guint8 node_idx;
guint8 pad1[2];
guint8 name_len;
guint8 domain[O2NM_MAX_NAME_LEN];
guint8 node_map[BITS_TO_BYTES(O2NM_MAX_NODES)];
};
Lastly, don't write details source logic in the patch description. Let the
code talk for itself. The description should be what-it-is rather than
how-it-was-done.
Thanks
Sunil
Jeff Liu wrote:
> dlm_query_join(request/response) messages structure have been changed from ocfs2-1.4.
> we want the dissector can handle both ocfs2-1.2 and the newer format.
>
> the method used to distinct the different ocfs2 version:
> 1.to handle the dlm_query_join_request
> check whether the name_len is eqaul to the left msg length or not.
> if name_len == left_msg_len, dissect the domain_name as ocfs2-1.2,
> else if the name_len > left_msg_len, dissect domain_name as ocfs2-1.4 and newer.
>
> 2.to handle the dlm_query_join_response
> for ocfs-1.2, the response value is converted to big-endian before
> putting on the wire, so swap it back to little-endian in dissector,
> then check if its value is less-than or eual to 2(JOIN_OK_NO_MAP),
> if so, NOP here.
>
> for ocfs-1.4 and newer, the response code is in big-endian before sending over wire as well,
> tvb_get_ntohl() convert it to little-endian, but it is made up of four one-byte fields and in
> big-endian order originally, so convert it to big-endian first.
> Also, the 2nd and 3rd byte are valid only if the first byte is JOIN_OK. In this case, wrap these 3 values
> in parenthesis separated by colon, then append to status line, otherwise, just the code value appended.
>
> Signed-off-by: Jeff Liu <jeff.liu at oracle.com>
> ---
> epan/dissectors/packet-ocfs2.c | 123 ++++++++++++++++++++++++++++++++++-----
> 1 files changed, 107 insertions(+), 16 deletions(-)
>
> diff --git a/epan/dissectors/packet-ocfs2.c b/epan/dissectors/packet-ocfs2.c
> index ce57c3f..f89d85d 100644
> --- a/epan/dissectors/packet-ocfs2.c
> +++ b/epan/dissectors/packet-ocfs2.c
> @@ -229,12 +229,6 @@ 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 {
> - JOIN_DISALLOW = 0,
> - JOIN_OK,
> - JOIN_OK_NO_MAP,
> -};
> -
>
> /* DLM lock modes */
> enum {
> @@ -811,6 +805,46 @@ struct dlm_finalize_reco
> guint32 pad2; // unused
> };
>
> +struct dlm_protocol_version {
> + guint8 pv_major;
> + guint8 pv_minor;
> +};
> +
> +#define O2NM_MAX_NODES 255
> +#define BITS_PER_BYTE 8
> +#define BITS_TO_BYTES(bits) (((bits)+BITS_PER_BYTE-1)/BITS_PER_BYTE)
> +struct dlm_query_join_request_latest {
> + guint8 node_idx;
> + guint8 pad1[2];
> + guint8 name_len;
> + struct dlm_protocol_version dlm_proto;
> + struct dlm_protocol_version fs_proto;
> + guint8 domain[O2NM_MAX_NAME_LEN];
> + guint8 node_map[BITS_TO_BYTES(O2NM_MAX_NODES)];
> +};
> +
> +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;
> +};
> +
> #define DLM_FIELD_BYTES_LEN 24
> #define LVB_REMAIN_BYTES_LEN (DLM_LVB_LEN - (DLM_FIELD_BYTES_LEN << 1))
> #define LVB1_OFFSET(x) offsetof(x, lvb[0])
> @@ -946,12 +980,6 @@ static struct dlm_msg_struct_def dlm_struct_defs[] = {
> { "dead_node", &hf_dlm_dead_node, FIELD_OFFSET_AND_SIZE(struct dlm_begin_reco,dead_node), dlm_node_idx_handler},
> { DLM_MSG_STRUCT_DEF_END } }
> },
> -{ "dlm_query_join_request", "DLM Query Join Request", DLM_QUERY_JOIN_MSG, &ett_dlm_query_join, {
> - { "node_idx", &hf_dlm_node_idx, FIELD_OFFSET_AND_SIZE(struct dlm_query_join_request,node_idx), dlm_node_idx_handler},
> - { "name_len", &hf_dlm_domain_name_len,FIELD_OFFSET_AND_SIZE(struct dlm_query_join_request,name_len), dlm_domain_namelen_handler},
> - { "domain", &hf_dlm_domain_name, FIELD_OFFSET_AND_SIZE(struct dlm_query_join_request,domain), dlm_domain_name_handler},
> - { DLM_MSG_STRUCT_DEF_END } }
> -},
> { "dlm_assert_joined", "DLM Assert Joined", DLM_ASSERT_JOINED_MSG, &ett_dlm_assert_joined, {
> { "node_idx", &hf_dlm_node_idx, FIELD_OFFSET_AND_SIZE(struct dlm_assert_joined,node_idx), dlm_node_idx_handler},
> { "name_len", &hf_dlm_domain_name_len,FIELD_OFFSET_AND_SIZE(struct dlm_assert_joined,name_len), dlm_domain_namelen_handler},
> @@ -1148,10 +1176,64 @@ static void msg_struct_enumerate_fields(struct dlm_msg_struct_def *def, proto_tr
> }
> }
>
> +static void dissect_dlm_query_join_request(proto_tree *tree, tvbuff_t *tvb,
> + int offset)
> +{
> + guint8 namelen;
> + gint len;
> +
> + proto_tree_add_item(tree, hf_dlm_node_idx, tvb, offset, 1, FALSE);
> +
> + /* skip the following 2 padding bytes */
> + offset += 3;
> + namelen = tvb_get_guint8(tvb, offset);
> + proto_tree_add_item(tree, hf_dlm_namelen, tvb, offset, 1, FALSE);
> +
> + len = tvb_reported_length_remaining(tvb, offset);
> +
> + ++offset;
> + /* if the left message length more than namelen, its most likely
> + * ocfs2-1.4 or later we are dealing with, or else ocfs2-1.2 or
> + * mismatch */
> + if (len > namelen)
> + proto_tree_add_item(tree, hf_dlm_domain_name, tvb,
> + offset + 4, namelen, FALSE);
> + else if (len == namelen)
> + proto_tree_add_item(tree, hf_dlm_domain_name, tvb,
> + offset, namelen, FALSE);
> +}
> +
> +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 ;
> +
> + /* 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,9 +1270,9 @@ 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,
> - tvb, O2NET_MSG_HDR_OFF_STATUS, 4,
> - FALSE);
> + 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,
> tvb, O2NET_MSG_HDR_OFF_KEY, 4,
> FALSE);
> @@ -1247,6 +1329,15 @@ static int dissect_ocfs2(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
> if (def)
> msg_struct_enumerate_fields(def, subtree, tvb, O2NET_MSG_HDR_OFF_PAYLOAD);
> }
> +
> + if (msg_type == DLM_QUERY_JOIN_MSG) {
> + if (magic == O2NET_MSG_MAGIC)
> + dissect_dlm_query_join_request(subtree,
> + tvb, O2NET_MSG_HDR_OFF_PAYLOAD);
> + else if (magic == O2NET_MSG_STATUS_MAGIC)
> + dissect_dlm_query_join_response(subtree,
> + si, tvb, O2NET_MSG_HDR_OFF_STATUS);
> + }
> }
> out:
> return ret;
>
More information about the Ocfs2-tools-devel
mailing list