Update to GeoNet with current standards (Version 1)#1392
Update to GeoNet with current standards (Version 1)#1392danieluliedguevara wants to merge 2 commits into
Conversation
This PR updates ETSI GeoNetworking protocol to version 1 (ETSI EN 302 636-4-1 V1.4.1 (2020-01)). The update includes dissectors for GeoNet Beacon messages and TopoScopeBcast-SH. Additionally, the Basic Transport Protocol (ETSI EN 302 636-5-1 V2.2.1 (2019-05)) has also been updated. It includes BTP-A and BTP-B in all its variants.
|
@infrastation just a reminder of this PR! Thank you! |
| case HT_BEACON: | ||
| process_beacon_header_from_bytes(ndo, bp, length); | ||
| return; | ||
| break; |
| lon = GET_BE_S_4(*bp); | ||
| *bp += 4; | ||
| *length -= 4; | ||
| uint32_t value = GET_BE_U_4(*bp); |
There was a problem hiding this comment.
I remember commenting on this before: would it be simpler to fetch one field at a time?
There was a problem hiding this comment.
Yes, the issue here is that some values, e.g., PAI, are 1 bit long. Which means that there must be some kind of bitwise operation. Although I've removed the bitwise operation for h (heading).
| } | ||
|
|
||
| /* Process Long Position Vector as per Section 9.5.2 of ETSI EN 302 636-4-1 V1.4.1 (2020-01)*/ | ||
| static void process_long_position_vector_from_bytes(netdissect_options *ndo, const u_char **bp, u_int *length) |
There was a problem hiding this comment.
Here and elsewhere: the common convention is to return the processed/consumed number of bytes from a helper function, then the calling function can increment bp and decrement length once.
There was a problem hiding this comment.
Done, please review and see if the solution I've proposed is the "common convention" way of doing it.
| (*bp)++; | ||
| (*length)--; | ||
| version = (value >> 4) & FOUR_BITS_MASK; | ||
| if (!is_value_in_list(version, implemented_gn_versions, IMPLEMENTED_GN_VERSIONS_NUM)) |
There was a problem hiding this comment.
Please do not over-engineer this, an if would do the job just fine.
| const char *next_header_text = tok2str(common_header_next_header_values, "Unknown", *next_header); | ||
| const char *header_type_text = tok2str(header_type_tok, "Unknown", HT_HST(*header_type, *header_subtype)); | ||
| const char *flags_text = tok2str(flags_text_from_bytes, "Unknown", flags); | ||
| if (ndo->ndo_vflag == 1) |
There was a problem hiding this comment.
This looks like a switch block.
infrastation
left a comment
infrastation
left a comment
There was a problem hiding this comment.
Thank you for waiting. This requires a bit more work, as noted.
Including bitwise operation simplifications, and other related tasks
|
Please take your time to address the issues with this draft I have already pointed out earlier. Due to other important work I do not expect to have significant amounts of time any time soon to drive this. |
|
I updated the changes you mentioned last commit! Take a look whenever you can! |
|
Can this be revisited? All proposed changes were fixed. |
|
Unfortunately, the next review will have to wait a bit longer because of unexpected other backlog, please be patient and meanwhile improve what you can on your own. |
3 participants
This PR updates ETSI GeoNetworking protocol to version 1 (ETSI EN 302 636-4-1 V1.4.1 (2020-01)).
The update includes dissectors for GeoNet Beacon messages and TopoScopeBcast-SH. Additionally, the Basic Transport Protocol (ETSI EN 302 636-5-1 V2.2.1 (2019-05)) has also been updated. It includes BTP-A and BTP-B in all its variants.