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 named
structs 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.)
There are two issues with
struct confirm_querythat will make it harder to understand and maintain going forward. These should be easy to fix, so seems worth doing:structsinsideconfirm_query. Instead ofWe should have something like:
struct confirm_querystores TCP header fields by name, but stores (what we use to set) the content of ICMP header fields by content (e.g.,flowidandrevflow). 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.)