Send outgoing packets for every address#462
Send outgoing packets for every address#462anti-social wants to merge 1 commit intokeepsimple1:mainfrom
Conversation
anti-social
force-pushed
the
multi-addr-if
branch
4 times, most recently
from
d157e97 to
555d157
Compare
|
Thanks for submitting the PR! I understand it's still in Draft, but I have a general comment: can we split this PR into smaller ones that are relatively independent (or built on-top) of each other? That will make review and merge much easier. For example, those cleanup changes can be in one PR:
|
anti-social
force-pushed
the
multi-addr-if
branch
from
5fcd11e to
392a3ac
Compare
| /// Resend unregister packet. | ||
| UnregisterResend(Vec<u8>, u32, bool), // (packet content, if_index, is_ipv4) | ||
| /// Resend unregister packets. | ||
| UnregisterResend(Vec<DnsOutPacket>, u32, bool), // (packets to resend, if_index, is_ipv4) |
There was a problem hiding this comment.
We changed this variant thus DnsOutPacket should implement std::fmt::Debug
anti-social
force-pushed
the
multi-addr-if
branch
3 times, most recently
from
95ca00f to
0814fba
Compare
There was a problem hiding this comment.
As this is somewhat non-trivial change, could you please add a detailed PR description:
- What's the problem? (motivation)
- Changes (approach or the solution)
That will also help the reader understand which part of #459 is addressed by this PR.
And could you please also add new test case to cover the multi-address-per-interface case that motivated this change?
Thanks!
|
|
||
| send_dns_outgoing_impl(out, if_name, my_intf.index, if_addr, sock, port) | ||
| let mut invalid_interfaces = HashSet::new(); | ||
| let packets = out.to_packets(); |
There was a problem hiding this comment.
It seems it would return packets even when no address was actually sent on.
| }; | ||
| sent_vec.into_iter().next().unwrap_or_default() | ||
| let (sent_packets, invalid_intf_addrs) = send_dns_outgoing(&out, intf, sock, self.port); | ||
| let _ = self.send_cmd_to_self(Command::InvalidIntfAddrs(invalid_intf_addrs)); |
There was a problem hiding this comment.
This sends the InvalidIntfAddrs command even if invalid_intf_addrs is empty.
| let is_ipv4 = sock.domain() == Domain::IPV4; | ||
| if let Some(out) = prepare_announce(info, intf, dns_registry, is_ipv4) { | ||
| let _ = send_dns_outgoing(&out, intf, sock, port, None)?; | ||
| let _ = send_dns_outgoing(&out, intf, sock, port); |
There was a problem hiding this comment.
This call used to propagate the error back to the caller. Now it silently ignores the error.
|
|
||
| /// Returns a list of actual DNS packet data to be sent on the wire. | ||
| pub fn to_data_on_wire(&self) -> Vec<Vec<u8>> { | ||
| pub(crate) fn _to_data_on_wire(&self) -> Vec<Vec<u8>> { |
There was a problem hiding this comment.
This change seems odd as it's still used. If you removed it completely and replace the call sites with direct alternative calls, I'm ok with that.
2 participants
Partially fixes #459