| From foo@baz Mon Apr 9 17:09:24 CEST 2018 |
| From: Ihar Hrachyshka <ihrachys@redhat.com> |
| Date: Tue, 16 May 2017 08:44:24 -0700 |
| Subject: neighbour: update neigh timestamps iff update is effective |
| |
| From: Ihar Hrachyshka <ihrachys@redhat.com> |
| |
| |
| [ Upstream commit 77d7123342dcf6442341b67816321d71da8b2b16 ] |
| |
| It's a common practice to send gratuitous ARPs after moving an |
| IP address to another device to speed up healing of a service. To |
| fulfill service availability constraints, the timing of network peers |
| updating their caches to point to a new location of an IP address can be |
| particularly important. |
| |
| Sometimes neigh_update calls won't touch neither lladdr nor state, for |
| example if an update arrives in locktime interval. The neigh->updated |
| value is tested by the protocol specific neigh code, which in turn |
| will influence whether NEIGH_UPDATE_F_OVERRIDE gets set in the |
| call to neigh_update() or not. As a result, we may effectively ignore |
| the update request, bailing out of touching the neigh entry, except that |
| we still bump its timestamps inside neigh_update. |
| |
| This may be a problem for updates arriving in quick succession. For |
| example, consider the following scenario: |
| |
| A service is moved to another device with its IP address. The new device |
| sends three gratuitous ARP requests into the network with ~1 seconds |
| interval between them. Just before the first request arrives to one of |
| network peer nodes, its neigh entry for the IP address transitions from |
| STALE to DELAY. This transition, among other things, updates |
| neigh->updated. Once the kernel receives the first gratuitous ARP, it |
| ignores it because its arrival time is inside the locktime interval. The |
| kernel still bumps neigh->updated. Then the second gratuitous ARP |
| request arrives, and it's also ignored because it's still in the (new) |
| locktime interval. Same happens for the third request. The node |
| eventually heals itself (after delay_first_probe_time seconds since the |
| initial transition to DELAY state), but it just wasted some time and |
| require a new ARP request/reply round trip. This unfortunate behaviour |
| both puts more load on the network, as well as reduces service |
| availability. |
| |
| This patch changes neigh_update so that it bumps neigh->updated (as well |
| as neigh->confirmed) only once we are sure that either lladdr or entry |
| state will change). In the scenario described above, it means that the |
| second gratuitous ARP request will actually update the entry lladdr. |
| |
| Ideally, we would update the neigh entry on the very first gratuitous |
| ARP request. The locktime mechanism is designed to ignore ARP updates in |
| a short timeframe after a previous ARP update was honoured by the kernel |
| layer. This would require tracking timestamps for state transitions |
| separately from timestamps when actual updates are received. This would |
| probably involve changes in neighbour struct. Therefore, the patch |
| doesn't tackle the issue of the first gratuitous APR ignored, leaving |
| it for a follow-up. |
| |
| Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/core/neighbour.c | 14 ++++++++++---- |
| 1 file changed, 10 insertions(+), 4 deletions(-) |
| |
| --- a/net/core/neighbour.c |
| +++ b/net/core/neighbour.c |
| @@ -1130,10 +1130,6 @@ int neigh_update(struct neighbour *neigh |
| lladdr = neigh->ha; |
| } |
| |
| - if (new & NUD_CONNECTED) |
| - neigh->confirmed = jiffies; |
| - neigh->updated = jiffies; |
| - |
| /* If entry was valid and address is not changed, |
| do not change entry state, if new one is STALE. |
| */ |
| @@ -1155,6 +1151,16 @@ int neigh_update(struct neighbour *neigh |
| } |
| } |
| |
| + /* Update timestamps only once we know we will make a change to the |
| + * neighbour entry. Otherwise we risk to move the locktime window with |
| + * noop updates and ignore relevant ARP updates. |
| + */ |
| + if (new != old || lladdr != neigh->ha) { |
| + if (new & NUD_CONNECTED) |
| + neigh->confirmed = jiffies; |
| + neigh->updated = jiffies; |
| + } |
| + |
| if (new != old) { |
| neigh_del_timer(neigh); |
| if (new & NUD_PROBE) |