You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There are two issues with struct confirm_query that will make it harder to understand and maintain going forward. These should be easy to fix, so seems worth doing:
IPv4 and IPv4 fields should be placed into namedstructs inside confirm_query. Instead of
union {
/* `ip4_id` is not used to identify probes (in
* `confirm_query_cmp`) as it is not returned in
* `ECHO_REPLY` packets. */
uint16_t ipid;
struct {
uint8_t traffic_class;
uint32_t flow_label;
};
};
We should have something like:
union {
/* `ip4_id` is not used to identify probes (in
* `confirm_query_cmp`) as it is not returned in
* `ECHO_REPLY` packets. */
struct {
uint16_t id;
} ip4;
struct {
uint8_t traffic_class;
uint32_t flow_label;
} ip6;
};
Another (more subtle) problem is that struct confirm_query stores TCP header fields by name, but stores (what we use to set) the content of ICMP header fields by content (e.g., flowid and revflow). This should be streamlined (without further thinking, it seems we should accept only header fields and let the user use whatever field it wants as the flow identifier. (Note that this would require us to move the reverse flow identifier functionality (revflow) somewhere else.)
The text was updated successfully, but these errors were encountered:
There are two issues with
struct confirm_query
that will make it harder to understand and maintain going forward. These should be easy to fix, so seems worth doing:structs
insideconfirm_query
. Instead ofWe should have something like:
struct confirm_query
stores TCP header fields by name, but stores (what we use to set) the content of ICMP header fields by content (e.g.,flowid
andrevflow
). This should be streamlined (without further thinking, it seems we should accept only header fields and let the user use whatever field it wants as the flow identifier. (Note that this would require us to move the reverse flow identifier functionality (revflow
) somewhere else.)The text was updated successfully, but these errors were encountered: