topologymapping / netlib Goto Github PK

destination is currently hardcoded

if we get a link local address, we will not sniff any actual network traffic

unclear why we need both. these files seem to have the same functionality

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.)

See the discussion here: #15 (comment)