Conversation
When I implemented local mode in 14b84a7 ("treewide: Introduce 'local mode' for disconnected setups"), I didn't consider the possibility that, also in that case, the user might want to override addresses, default gateway or netmask, even though I expressly mentioned this in the man page: In this case, **unless configured otherwise**, they will assign the IPv4 link-local address 169.254.2.1 to the guest or target namespace, and no IPv6 address. Fix this by checking if an address, gateway, or netmask length was explicitly set by the user, before overriding them with the default parameters for local mode. This might lead to invalid configurations where we won't be able to set the default gateway passed by the user, but we print a warning message, and we assume users know what they're doing in that case. Link: https://bugs.passt.top/show_bug.cgi?id=217 Fixes: 14b84a7 ("treewide: Introduce 'local mode' for disconnected setups") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
We might have situations, such as the one described in https://bugs.passt.top/show_bug.cgi?id=217, where using a link-local address as source in a given namespace doesn't guarantee that we can reach the intended destination, because, for instance, the inbound traffic we forward is in turn forwarded to a different interface, such as a bridge. In that case, the assumption from 9618d24 ("ndp, dhcpv6, tcp, udp: Always use link-local as source if gateway isn't") isn't a safe one: the user might have specified a valid gateway address, matching the scope of the destination address, but we won't use it as address of last resort, and prefer a link-local address with a mismatch in scope instead. So, if the user specifies a given default gateway address for IPv6, note that as 'our_tap_addr', like we would do with with IPv4, and stick to Rule 2 of RFC 6724, Section 5, when selecting a source address, by preferring an address with the same scope, if available. Note that this preference will be applied only if, at the point of the source address selection, the destination address is already known, which, in general, only happens if there's an explicit mapping rule specifying the target destination address. Otherwise, if there's no explicit rule picking the destination address, we select the destination address after selecting the source address. This should probably be improved at some point, to select matching address pairs (when a compatible one is available) instead of sequentially picking source and destination addresses. That appears to be beyond the scope of this patch, though. Reported-by: Paul Holzinger <pholzing@redhat.com> Link: https://bugs.passt.top/show_bug.cgi?id=217 Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
No sunsetting or rightsizing for you, qrap: dig, bury, and cover. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
… messages When ports are forwarded by exclusion, we might fail to bind all privileged ports (typically lower than 1024), but that's actually expected, and we shouldn't log warnings just because of that. Now, if those ports are automatically forwarded, and we have a container binding some low ports, we'll log those warnings messages every second, which is just unnecessary noise in the system log or in log files. Use LOG_DEBUG as severity for those messages, instead of LOG_WARNING, so that they are only printed when --debug is given: that should be convenient enough to investigate things, while avoiding excess noise during regular operations. There might be more sophisticated ways to limit this noise, but this might be a significant regression in some setups, introduced by recent changes in port forwarding handling, so I'm going for a somewhat minimal fix for the moment, which can be improved later on if needed. Reported-by: frajo <frajo@frajo.fi> Link: https://bugs.passt.top/show_bug.cgi?id=213 Fixes: b223bec ("fwd, tcp, udp: Set up listening sockets based on forward table") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )