Support VRF for TCP MD5#3459
Draft
YutaroHayakawa wants to merge 3 commits into
Draft
Conversation
Contributor
Author
|
This is still a draft (also seems like the new scenario test I introduced is failing somehow). |
YutaroHayakawa
force-pushed
the
yutaro/bgp-md5-for-vrf
branch
from
c1d4f68 to
792f8e5
Compare
Linux has per-VRF TCP MD5 key registry. torvalds/linux@6b102db Setting unix.TCPMD5Sig.Ifindex to the VRF's ifindex makes the key scoped within the VRF. GoBGP already has a support for binding Listener/Neighbor socket to VRF, so what we need to do is propagating the interface ifindex to SetTCPMD5SigSockopt call. Actually, we already have a logic to set unix.TCPMD5Sig.Ifindex, but the current usage is wrong. osrg#3370 added it as a part of the MD5 support for IPv6 link-local address peering (a.k.a. BGP Unnumbered). It derives Ifindex from the IPv6 zone information, but this doesn't have any effect as Ifindex only accepts VRF device's ifindex. Also, it uses unix.TCP_MD5SIG instead of unix.TCP_MD5SIG_EXT and it doesn't set unix.TCP_MD5SIG_FLAG_IFINDEX flag, so the kernel even ignores the parameter. Remove the current logic and support setting bindInterface derived from the (Dynamic)Neighbor.Transport.Config.BindInterface configuration. Note that technically it is still possible to specify non-VRF interface to this parameter, so the new implementation always checks the interface type via netlink and if the interface is not VRF device, skip setting the parameter. As a minor cleanup, always use unix.TCP_MD5SIG_EXT instead of unix.TCP_MD5SIG as EXT is a superset of the non-EXT one. We believe the risk of having Linux kernel version that doesn't have EXT support is low these days as it is v4.13 which is almost 10 years old. Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Add bind-to-device to the GoBGP YANG listen config and regenerate the OpenConfig config structs so file-based global config can carry it. Propagate the value through config-to-API and API-to-OpenConfig conversion so the BGP listener can be bound to the configured device. Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
YutaroHayakawa
force-pushed
the
yutaro/bgp-md5-for-vrf
branch
7 times, most recently
from
f181723 to
883263f
Compare
Add some basic scenario test scenario for TCP MD5: 1. Basic IPv4 peering 2. BGP Unnumbered peering 3. DynamicNeighbor peering 4. IPv4 peering with VRF Move the existing BGP Unnumbered link-local address discovery helper out from bgp_unnumbered_test.py to lib/utils.py. Also made some fixes to the unnumbered related tests: 1. Don't rely on all-node multicast address as it will end up having three link-local addresses in the neighbor table which raises "not p2p link" error. One is for the actual remote node, one is the local node, and the last one is the one of bridge network's gateway's. Since we are using local container, we can directly obtain the link-local address and what we need to do is ping it and warm-up the neighbor cache with it. 2. Use dedicated Bridge network for unnumbered peering because the default bridge network may be shared with someone else. Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
YutaroHayakawa
force-pushed
the
yutaro/bgp-md5-for-vrf
branch
from
883263f to
c95fe07
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Copying the commit message from the first commit for convenience.
===
Linux has per-VRF TCP MD5 key registry.
torvalds/linux@6b102db
Setting unix.TCPMD5Sig.Ifindex to the VRF's ifindex makes the key scoped within the VRF. GoBGP already has a support for binding Listener/Neighbor socket to VRF, so what we need to do is propagating the interface ifindex to SetTCPMD5SigSockopt call.
Actually, we already have a logic to set unix.TCPMD5Sig.Ifindex, but the current usage is wrong. #3370 added it as a part of the MD5 support for IPv6 link-local address peering (a.k.a. BGP Unnumbered).
It derives Ifindex from the IPv6 zone information, but this doesn't have any effect as Ifindex only accepts VRF device's ifindex. Also, it uses unix.TCP_MD5SIG instead of unix.TCP_MD5SIG_EXT and it doesn't set unix.TCP_MD5SIG_FLAG_IFINDEX flag, so the kernel even ignores the parameter.
Remove the current logic and support setting bindInterface derived from the (Dynamic)Neighbor.Transport.Config.BindInterface configuration. Note that technically it is still possible to specify non-VRF interface to this parameter, so the new implementation always checks the interface type via netlink and if the interface is not VRF device, skip setting the parameter.
As a minor cleanup, always use unix.TCP_MD5SIG_EXT instead of unix.TCP_MD5SIG as EXT is a superset of the non-EXT one. We believe the risk of having Linux kernel version that doesn't have EXT support is low these days as it is v4.13 which is almost 10 years old.