Skip to content

Conversation

ysksuzuki
Copy link
Member

@ysksuzuki ysksuzuki commented Oct 25, 2023

This PR fixes the issue that neighbor entries can be installed on devices where no route to the destination host exists.

netlink.RouteGetWithOptions (equivalent to ip route get) with oif returns a route even if the destination is unreachable from the specified interface. The neighbor entries can be installed on all devices because of this.

netlink.RouteGetWithOptions with FIBMatch option (equivalent to fibmatch flag) returns full fib lookup matched route. It returns No route to host if the destination is non-routable from the specified device. This PR adds FIBMatch option to avoid installing the unnecessary entries.

Note:

  • With IPv6, it returns Network is unreachable if the destination is unreachable with or without fibmatch.
  • With FIBMatch option, netlink.RouteGetWithOptions returns MultiPath field if there are multiple paths to the dest. It returns GW field without FIBMatch. See examples below.
// FIBMatch: false
netlink.Route{ GW: 8.8.8.250, MultiPath: [] }
netlink.Route{ GW: 9.9.9.250, MultiPath: [] }

// FIBMatch: true
netlink.Route{ GW: <nil>, MultiPath: [{Ifindex: 1218 Weight: 1 Gw: 9.9.9.250 Flags: []}, {Ifindex: 1220 Weight: 1 Gw: 8.8.8.250 Flags: []}]}

ip route get examples

$ ip route
10.0.0.0/24 dev veth1 proto kernel scope link src 10.0.0.1
10.0.1.0/24 dev veth3 proto kernel scope link src 10.0.1.1

$ ping -I veth1 10.0.0.1
PING 10.0.0.1 (10.0.0.1) from 10.0.0.1 veth1: 56(84) bytes of data.
64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=0.066 ms
64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=0.056 ms
64 bytes from 10.0.0.1: icmp_seq=3 ttl=64 time=0.057 ms

$ ping -I veth3 10.0.0.1
PING 10.0.0.1 (10.0.0.1) from 10.0.1.1 veth3: 56(84) bytes of data.
From 10.0.1.1 icmp_seq=1 Destination Host Unreachable
From 10.0.1.1 icmp_seq=2 Destination Host Unreachable
From 10.0.1.1 icmp_seq=3 Destination Host Unreachable

$ ip route get 10.0.0.1 oif veth1
local 10.0.0.1 dev lo table local src 10.0.0.1 uid 1000
    cache <local>

// `ip route get` returns a route even if the destination is unreachable
// from the specified interface
$ ip route get 10.0.0.1 oif veth3
10.0.0.1 dev veth3 src 10.0.1.1 uid 1000
    cache

// With fibmatch flag, it returns full fib lookup matched route
$ ip route get fibmatch 10.0.0.1 oif veth1
local 10.0.0.1 dev veth1 proto kernel scope host src 10.0.0.1

$ ip route get fibmatch 10.0.0.1 oif veth3
RTNETLINK answers: No route to host

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #28660

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 25, 2023
This PR fixes the issue that neighbor entries can be installed on devices
where no route to the destination host exists.

`netlink.RouteGetWithOptions` (equivalent to `ip route get`) with oif
returns a route even if the destination is unreachable from the specified interface.
The neighbor entries can be installed on all devices because of this.

`netlink.RouteGetWithOptions` with `FIBMatch` option (equivalent to `fibmatch` flag)
returns full fib lookup matched route. It returns `No route to host`
if the destination is non-routable from the specified device.
This PR adds `FIBMatch` option to avoid installing the unnecessary entries.

Note:

- With IPv6, it returns `Network is unreachable` if the destination
  is unreachable with or without fibmatch.
- With `FIBMatch` option, `netlink.RouteGetWithOptions` returns `MultiPath` field
  if there are multiple paths to the dest. It returns `GW` field without `FIBMatch`.
  See examples below.

```
// FIBMatch: false
netlink.Route{ GW: 8.8.8.250, MultiPath: [] }
netlink.Route{ GW: 9.9.9.250, MultiPath: [] }

// FIBMatch: true
netlink.Route{ GW: <nil>, MultiPath: [{Ifindex: 1218 Weight: 1 Gw: 9.9.9.250 Flags: []},
                                      {Ifindex: 1220 Weight: 1 Gw: 8.8.8.250 Flags: []}]}
```

`ip route get` examples

```
$ ip route
10.0.0.0/24 dev veth1 proto kernel scope link src 10.0.0.1
10.0.1.0/24 dev veth3 proto kernel scope link src 10.0.1.1

$ ping -I veth1 10.0.0.1
PING 10.0.0.1 (10.0.0.1) from 10.0.0.1 veth1: 56(84) bytes of data.
64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=0.066 ms
64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=0.056 ms
64 bytes from 10.0.0.1: icmp_seq=3 ttl=64 time=0.057 ms

$ ping -I veth3 10.0.0.1
PING 10.0.0.1 (10.0.0.1) from 10.0.1.1 veth3: 56(84) bytes of data.
From 10.0.1.1 icmp_seq=1 Destination Host Unreachable
From 10.0.1.1 icmp_seq=2 Destination Host Unreachable
From 10.0.1.1 icmp_seq=3 Destination Host Unreachable

$ ip route get 10.0.0.1 oif veth1
local 10.0.0.1 dev lo table local src 10.0.0.1 uid 1000
    cache <local>

// `ip route get` returns a route even if the destination is unreachable
// from the specified interface
$ ip route get 10.0.0.1 oif veth3
10.0.0.1 dev veth3 src 10.0.1.1 uid 1000
    cache

// With fibmatch flag, it returns full fib lookup matched route
$ ip route get fibmatch 10.0.0.1 oif veth1
local 10.0.0.1 dev veth1 proto kernel scope host src 10.0.0.1

$ ip route get fibmatch 10.0.0.1 oif veth3
RTNETLINK answers: No route to host
```

Fixes: cilium#28660

Signed-off-by: Yusuke Suzuki <ysuzuki4112@gmail.com>
@ysksuzuki
Copy link
Member Author

/test

@ysksuzuki ysksuzuki marked this pull request as ready for review October 27, 2023 14:00
@ysksuzuki ysksuzuki requested review from a team as code owners October 27, 2023 14:00
@ysksuzuki ysksuzuki requested review from brb and borkmann October 27, 2023 14:00
@dylandreimerink dylandreimerink added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Oct 30, 2023
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ysksuzuki ysksuzuki added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Nov 8, 2023
@brb brb added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 9, 2023
@squeed squeed merged commit 2058ed6 into cilium:main Nov 10, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Neighbor entry is installed on every NIC on a MultiNIC node
4 participants