On 8/12/24 19:30, Vladislav Odintsov wrote:
> 
> On 13.08.2024 00:14, Dumitru Ceara wrote:
>> On 8/12/24 18:21, Martin Kalcok wrote:
>>> This change adds two new LRP options:
>>>   * routing-protocol-redirect
>>>   * routing-protocols
>>>
>>> These allow redirection of a routing protocol traffic to
>>> an Logical Switch Port. This enables external routing daemons
>>> to listen on an interface bound to an LSP and effectively act
>>> as if they were listening on (and speaking from) LRP's IP address.
>>>
>>> Option 'routing-protocols' takes a comma-separated list of routing
>>> protocols whose traffic should be redirected. Currently supported
>>> are BGP (tcp 179) and BFD (udp 3784).
>>>
>>> Option 'routing-protocol-redirect' expects a string with an LSP
>>> name.
>>>
>>> When both of these options are set, any traffic entering LS
>>> that's destined for LRP's IP addresses (including IPv6 LLA) and
>>> routing protocol's port number, is redirected to the LSP specified
>>> in the 'routing-protocol-redirect' value.
>>>
>>> NOTE: this feature is experimental and may be subject to
>>> removal/change in the future.
>>>
>>> Signed-off-by: Martin Kalcok<[email protected]>
>>> ---
>>>
>>>   v9 contains small changes based on the review of v8:
>>>   * Simplified search for the port specified in
>>> 'routing-protocol-redirect',
>>>     using 'ovn_port_find'
>>>     * As a result this change a new possible warning was added when LRP
>>>       is not connected to the same LS as LSP specified in
>>>       'routing-protocol-redirect'.
>>>   * Datapath test for this feature now includes verification of BFD's
>>>     UDP traffic.
>>>     * These tests required some more care as Ncat produced false
>>> positive
>>>       results even when sending to a port where nothing was
>>> listening. My
>>>       assumption is that Ncat tries to assert succes of a UDP connection
>>>       based on lack of ICMP Port Unreachable message, and LR probably
>>>       does not generate these?
>>>   * nit: null pointer checks changed from 'if (p == NULL)' to 'if (!p)'
>>>     for consistency.
>>>   
>> This version looks good to me!
>>
>> Acked-by: Dumitru Ceara<[email protected]>
>>
>> Vladislav, do you happen to have some time to try this version out on
>> your end too?
> 
> Yes, just now finished testing.
> 
> With centralized routing the BGP and BFD works well (in my setup there
> are two VRFs with BGP and BFD peerings configured as a haipnit inside
> one node to each other):
> 
> # sh run
> <...snip...>
> router bgp 64512 vrf dxvif-62C25580
>  bgp router-id 169.254.252.1
>  no bgp ebgp-requires-policy
>  no bgp network import-check
>  neighbor 169.254.252.2 remote-as 64513
>  neighbor 169.254.252.2 bfd
> exit
> !
> router bgp 64513 vrf dxvif-9ED34880
>  bgp router-id 169.254.252.2
>  no bgp ebgp-requires-policy
>  no bgp network import-check
>  neighbor 169.254.252.1 remote-as 64512
>  neighbor 169.254.252.1 bfd
>  !
>  address-family ipv4 unicast
>   network 10.0.0.0/24
>  exit-address-family
> exit
> 
> # sh ip bgp vrf all
> 
> Instance dxvif-62C25580:
> BGP table version is 1, local router ID is 169.254.252.1, vrf id 760
> Default local pref 100, local AS 64512
> Status codes:  s suppressed, d damped, h history, * valid, > best, =
> multipath,
>                i internal, r RIB-failure, S Stale, R Removed
> Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
> Origin codes:  i - IGP, e - EGP, ? - incomplete
> RPKI validation codes: V valid, I invalid, N Not found
> 
>    Network          Next Hop            Metric LocPrf Weight Path
> *> 10.0.0.0/24      169.254.252.2            0             0 64513 i
> 
> Displayed  1 routes and 1 total paths
> 
> Instance dxvif-9ED34880:
> BGP table version is 1, local router ID is 169.254.252.2, vrf id 763
> Default local pref 100, local AS 64513
> Status codes:  s suppressed, d damped, h history, * valid, > best, =
> multipath,
>                i internal, r RIB-failure, S Stale, R Removed
> Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
> Origin codes:  i - IGP, e - EGP, ? - incomplete
> RPKI validation codes: V valid, I invalid, N Not found
> 
>    Network          Next Hop            Metric LocPrf Weight Path
> *> 10.0.0.0/24      0.0.0.0                  0         32768 i
> 
> Displayed  1 routes and 1 total paths
> 
> # sh bfd peers
> BFD Peers:
>     peer 169.254.252.1 local-address 169.254.252.2 vrf dxvif-9ED34880
> interface dx-9ED34880-v
>         ID: 3199966869
>         Remote ID: 2401437121
>         Active mode
>         Status: up
>         Uptime: 26 second(s)
>         Diagnostics: ok
>         Remote diagnostics: ok
>         Peer Type: dynamic
>         RTT min/avg/max: 0/0/0 usec
>         Local timers:
>             Detect-multiplier: 3
>             Receive interval: 300ms
>             Transmission interval: 300ms
>             Echo receive interval: 50ms
>             Echo transmission interval: disabled
>         Remote timers:
>             Detect-multiplier: 3
>             Receive interval: 300ms
>             Transmission interval: 300ms
>             Echo receive interval: 50ms
> 
>     peer 169.254.252.2 local-address 169.254.252.1 vrf dxvif-62C25580
> interface dx-62C25580-v
>         ID: 2401437121
>         Remote ID: 3199966869
>         Active mode
>         Status: up
>         Uptime: 26 second(s)
>         Diagnostics: ok
>         Remote diagnostics: ok
>         Peer Type: dynamic
>         RTT min/avg/max: 0/0/0 usec
>         Local timers:
>             Detect-multiplier: 3
>             Receive interval: 300ms
>             Transmission interval: 300ms
>             Echo receive interval: 50ms
>             Echo transmission interval: disabled
>         Remote timers:
>             Detect-multiplier: 3
>             Receive interval: 300ms
>             Transmission interval: 300ms
>             Echo receive interval: 50ms
> 
> At this point this looks good with a note, that for LB VIPs and NAT
> addresses we do use ha-chassis-group with primary/secondary chassis,
> which is currently not supported by the redirect feature.
> 
> This probably should be somehow addressed in a future development.
> 

Ack.

> Tested-by: Vladislav Odintsov <[email protected]>
> 

Thanks, Vladislav!

>>
>> Mark, Numan, Han, as discussed before branching, I think it's fine to
>> include this experimental feature on branch 24.09 (and in v24.09.0) too.
>>
>> Do you guys agree?
>>

I synced up with Mark offline about this and he was OK with pushing the
patch to 24.09 too.  So I went ahead and pushed the patch to main (I
added a NEWS entry as we were missing one) and then backported it to
branch-24.09.

Thanks again for the work on this, Martin and Vladislav!

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to