Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LTE message parsing #6

Closed
wants to merge 4 commits into from

Conversation

martin-heusse
Copy link
Contributor

Finds the size and copies to the LTE message from the diag structure. Up/down flag of RRC message looks better now.

Finds the size and copies to the LTE message from the diag structure. Up/down flag of RRC message looks better now.
@zecke
Copy link
Collaborator

zecke commented Jul 3, 2017

Thank you! Which wireshark patch do you use to look at the result?

@martin-heusse
Copy link
Contributor Author

martin-heusse commented Jul 3, 2017 via email

@zecke
Copy link
Collaborator

zecke commented Jul 3, 2017

Great. Then please push it to the Wireshark Gerrit, share the link and I can check how my traces look with your version of the modified dissector! And thank you for adding upstream support for LTE decoding!

@martin-heusse
Copy link
Contributor Author

martin-heusse commented Jul 4, 2017 via email

@E3V3A
Copy link

E3V3A commented Jul 6, 2017

Also note:
wireshark/wireshark#12 (comment)

diag_input.c Outdated
break;
default:
// Unhandled
return NULL;
}
// verify len
payload_len = ((uint16_t)dp->data[9]) << 8 | dp->data[8];
if (payload_len > len - 15) {
payload_len = dp->data[12]-1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that the length field is only 8bit? I would expect 16bit here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This length does not reflect the actual size of the message anyway.
This is corrected in a subsequent commit

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But these commits are not in this pull request yet? I am not a LTE expert but I would expect QXDM to have a 16bit length field and LTE to have longer messages too. bb.data can be > 256 as well.

I can merge anyway but I would expect us to see truncated messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 4 commits in this pull request
https://github.com/moiji-mobile/diag-parser/pull/6/commits

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. but I look at files changed which is "git diff base..4thcommit"? So payload_len = dp->data[12]-1 is the final one. Do you have an indication of why this field only needs to be 8bit and not 16bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with all 4 commits, it looks like this :
/* payload_len = dp->data[12]-1; */
payload_len=len-14;
printf("payload_len: %d\n",payload_len);
data = &dp->data[14];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should probably remove the comment

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Not sure how I ended up on the patch. I thought I clicked PR->files changed.. Hmm. Could you remove the /* payload_len .. */ and then merge it? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants