[v3] neighbour: confirm neigh entries when ARP packet is received

Message ID 20180913181203.4207-1-vasilykh@arista.com
State Accepted
Delegated to: David Miller
Headers show
Series
  • [v3] neighbour: confirm neigh entries when ARP packet is received
Related show

Commit Message

Vasiliy Khoruzhick Sept. 13, 2018, 6:12 p.m.
Update 'confirmed' timestamp when ARP packet is received. It shouldn't
affect locktime logic and anyway entry can be confirmed by any higher-layer
protocol. Thus it makes sense to confirm it when ARP packet is received.

Fixes: 77d7123342 ("neighbour: update neigh timestamps iff update is effective")
Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
---
v2: - update comment to match new code.
v3: - fix wording in commit message, make 'Fixes' tag one line, remove extra new line
      between 'Fixes' and 'Signed-off-by'.

 net/core/neighbour.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Sergei Shtylyov Sept. 13, 2018, 6:26 p.m. | #1
Hello!

On 09/13/2018 09:12 PM, Vasily Khoruzhick wrote:

> Update 'confirmed' timestamp when ARP packet is received. It shouldn't
> affect locktime logic and anyway entry can be confirmed by any higher-layer
> protocol. Thus it makes sense to confirm it when ARP packet is received.
> 
> Fixes: 77d7123342 ("neighbour: update neigh timestamps iff update is effective")

   Need at least 12 hex digits... Perhaps could be fixed when applying?

> Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
[...]
MBR, Sergei
David Miller Sept. 13, 2018, 7:02 p.m. | #2
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Thu, 13 Sep 2018 21:26:42 +0300

> Hello!
> 
> On 09/13/2018 09:12 PM, Vasily Khoruzhick wrote:
> 
>> Update 'confirmed' timestamp when ARP packet is received. It shouldn't
>> affect locktime logic and anyway entry can be confirmed by any higher-layer
>> protocol. Thus it makes sense to confirm it when ARP packet is received.
>> 
>> Fixes: 77d7123342 ("neighbour: update neigh timestamps iff update is effective")
> 
>    Need at least 12 hex digits... Perhaps could be fixed when applying?

Yeah I took care of that.

> 
>> Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
> [...]

Applied and queued up for -stable.
Vasiliy Khoruzhick Sept. 13, 2018, 7:10 p.m. | #3
On Thu, Sep 13, 2018 at 12:02 PM, David Miller <davem@davemloft.net> wrote:
> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Date: Thu, 13 Sep 2018 21:26:42 +0300
>
>> Hello!
>>
>> On 09/13/2018 09:12 PM, Vasily Khoruzhick wrote:
>>
>>> Update 'confirmed' timestamp when ARP packet is received. It shouldn't
>>> affect locktime logic and anyway entry can be confirmed by any higher-layer
>>> protocol. Thus it makes sense to confirm it when ARP packet is received.
>>>
>>> Fixes: 77d7123342 ("neighbour: update neigh timestamps iff update is effective")
>>
>>    Need at least 12 hex digits... Perhaps could be fixed when applying?
>
> Yeah I took care of that.
>
>>
>>> Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
>> [...]
>
> Applied and queued up for -stable.

Thanks!

Patch

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index aa19d86937af..91592fceeaad 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1180,6 +1180,12 @@  int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		lladdr = neigh->ha;
 	}
 
+	/* Update confirmed timestamp for neighbour entry after we
+	 * received ARP packet even if it doesn't change IP to MAC binding.
+	 */
+	if (new & NUD_CONNECTED)
+		neigh->confirmed = jiffies;
+
 	/* If entry was valid and address is not changed,
 	   do not change entry state, if new one is STALE.
 	 */
@@ -1201,15 +1207,12 @@  int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		}
 	}
 
-	/* Update timestamps only once we know we will make a change to the
+	/* Update timestamp 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;
+	if (new != old || lladdr != neigh->ha)
 		neigh->updated = jiffies;
-	}
 
 	if (new != old) {
 		neigh_del_timer(neigh);