Skip to contentSkip to conversation

net/ipv6: Only update MTU metric if it set

PATCH In review [email protected] 4 participants 4 messages Aug 31, 2018 – Sep 3, 2018

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

PATCH 1/1 net/ipv6: Only update MTU metric if it set

From: David Ahern <[email protected]>

Jan reported a regression after an update to 4.18.5. In this case ipv6 default route is setup by systemd-networkd based on data from an RA. The RA contains an MTU of 1492 which is used when the route is first inserted but then systemd-networkd pushes down updates to the default route without the mtu set.

Prior to the change to fib6_info, metrics such as MTU were held in the dst_entry and rt6i_pmtu in rt6_info contained an update to the mtu if any. ip6_mtu would look at rt6i_pmtu first and use it if set. If not, the value from the metrics is used if it is set and finally falling back to the idev value.

After the fib6_info change metrics are contained in the fib6_info struct and there is no equivalent to rt6i_pmtu. To maintain consistency with the old behavior the new code should only reset the MTU in the metrics if the route update has it set.

Fixes d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
Reported-by Jan Janssen <[email protected]>
Signed-off-by David Ahern <[email protected]>
---
 net/ipv6/ip6_fib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
net/ipv6/ip6_fib.c +4 -1
index d212738e9d10..f43d278e0040 100644
@@ -987,7 +987,10 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
 					fib6_clean_expires(iter);
 				else
 					fib6_set_expires(iter, rt->expires);
-				fib6_metric_set(iter, RTAX_MTU, rt->fib6_pmtu);
+
+				if (rt->fib6_pmtu)
+					fib6_metric_set(iter, RTAX_MTU,
+							rt->fib6_pmtu);
 				return -EEXIST;
 			}
 			/* If we have the same destination and the same metric,
Signature · 1 line
2.15.2 (Apple Git-101.1)
#1 lore raw
Quoted 32 lines · On Donnerstag, 30. August 2018 23:15:43 CEST [email protected] wrote:

From: David Ahern <[email protected]>

Jan reported a regression after an update to 4.18.5. In this case ipv6 default route is setup by systemd-networkd based on data from an RA. The RA contains an MTU of 1492 which is used when the route is first inserted but then systemd-networkd pushes down updates to the default route without the mtu set.

Prior to the change to fib6_info, metrics such as MTU were held in the dst_entry and rt6i_pmtu in rt6_info contained an update to the mtu if any. ip6_mtu would look at rt6i_pmtu first and use it if set. If not, the value from the metrics is used if it is set and finally falling back to the idev value.

After the fib6_info change metrics are contained in the fib6_info struct and there is no equivalent to rt6i_pmtu. To maintain consistency with the old behavior the new code should only reset the MTU in the metrics if the route update has it set.

Fixes d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
Reported-by Jan Janssen <[email protected]>
Signed-off-by David Ahern <[email protected]>
---
 net/ipv6/ip6_fib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
net/ipv6/ip6_fib.c +0 -0
index d212738e9d10..f43d278e0040 100644
@@ -987,7 +987,10 @@ static int fib6_add_rt2node(struct fib6_node *fn,
 struct fib6_info *rt, fib6_clean_expires(iter);

net/ipv6/ip6_fib.c static int fib6_add_rt2node(struct fib6_node *fn, · line 996

Quoted 10 lines
net/ipv6/ip6_fib.c +4 -1
 				else
 					fib6_set_expires(iter, rt->expires);
-				fib6_metric_set(iter, RTAX_MTU, rt->fib6_pmtu);
+
+				if (rt->fib6_pmtu)
+					fib6_metric_set(iter, RTAX_MTU,
+							rt->fib6_pmtu);
 				return -EEXIST;
 			}
 			/* If we have the same destination and the same metric,

I just tested this and can confirm that it fixes my problem.

You mentioned that systemd-networkd is doing something silly here, so I was wondering if you could make a bug report to fix the underlying issue? I don't mind doing it myself but you're the expert and can explain things better.

Thank you for fixing this.

#2 lore raw
Quoted 3 lines · Jan Janssen

You mentioned that systemd-networkd is doing something silly here, so I was wondering if you could make a bug report to fix the underlying issue? I don't mind doing it myself but you're the expert and can explain things better.

It is silly in the sense that systemd-networkd installs a default route with an MTU and then pushes down updates (route replace) without the MTU.

maintainer
#3 lore raw
In reply to [email protected]

From: [email protected]
Date: Thu, 30 Aug 2018 14:15:43 -0700

Quoted 14 lines

From: David Ahern <[email protected]>

Jan reported a regression after an update to 4.18.5. In this case ipv6 default route is setup by systemd-networkd based on data from an RA. The RA contains an MTU of 1492 which is used when the route is first inserted but then systemd-networkd pushes down updates to the default route without the mtu set.

Prior to the change to fib6_info, metrics such as MTU were held in the dst_entry and rt6i_pmtu in rt6_info contained an update to the mtu if any. ip6_mtu would look at rt6i_pmtu first and use it if set. If not, the value from the metrics is used if it is set and finally falling back to the idev value.

Quoted 8 lines

After the fib6_info change metrics are contained in the fib6_info struct and there is no equivalent to rt6i_pmtu. To maintain consistency with the old behavior the new code should only reset the MTU in the metrics if the route update has it set.

Fixes d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
Reported-by Jan Janssen <[email protected]>
Signed-off-by David Ahern <[email protected]>

Applied and queued up for -stable, thanks David.