Update the MTU in affected routes when IPv6 RA changes the MTU

[vangyzen]

Summary

ip6_calcmtu() only looks at the interface MTU if neither the TCP hostcache nor the route provides an MTU. Update the routes so they do not provide stale MTUs.

Test Plan

This fixes UNH IPv6 conformance test cases v6LC_4_1_08 and v6LC_4_1_09,
which use a RA to reduce the link MTU from 1500 to 1280.

[vangyzen]

I do not plan to commit this change. We made this change to demonstrate that it fixes two conformance tests. However, I imagine there is a more correct change, since this one simply reverts part of rS274611. Perhaps rt_updatemtu() needs to be called in the nd_opts_mtu case in nd6_ra_input()?

[vangyzen]

Note to self: SCOS-47425

[vangyzen]

sys/netinet6/ip6_output.c

1385–1386

After further review, I think we should either do

mtu = min(mtu, ifmtu);

here, as was done prior to rS274611, or call rt_updatemtu() in nd6_ra_input(). Which is more correct?

[vangyzen]

sys/netinet6/ip6_output.c

1385–1386

Since RTM_ADD and RTM_CHANGE limit the route MTU to the IPv6 RA MTU, the intent seems to be that the route is always updated to reflect the actual MTU. If that is correct, I think we should call rt_updatemtu() in nd6_ra_input(). What do you think, @ae and @melifaro?

[vangyzen]

• Revert "IPv6: consider interface MTU in path MTU calculation"
• IPv6: consider interface MTU in path MTU calculation

[vangyzen]

sys/netinet6/ip6_output.c

1385–1386

The submitter tried adding rt_updatemtu() in nd6_ra_input(), but it did not work, so I now suggest this change.

[vangyzen]

sys/netinet6/ip6_output.c

1385–1386

Correction: Adding rt_updatemtu() in nd6_ra_input() does work. Sorry for the confusion. So, the question is still open: Which change is preferred?

[ae]

Invoking of rt_updatemtu() looks right to me. But I don't like how rt_updatemtu() works.
Recently we found that in6_mtutimo() on systems with many IPv6 routes produces noticeable delay for packets processing.
It holds RIB_WLOCK while all routes are processed, and thus normal packets processing is blocked on RIB_RLOCK for this time.
Also in6_mtutimo looks like dead code, that is doing nothing useful.

[melifaro]

In D14257#299355, @ae wrote:

Invoking of rt_updatemtu() looks right to me. But I don't like how rt_updatemtu() works.
Recently we found that in6_mtutimo() on systems with many IPv6 routes produces noticeable delay for packets processing.
It holds RIB_WLOCK while all routes are processed, and thus normal packets processing is blocked on RIB_RLOCK for this time.

There are other rt_foreach_fib() users as well. Probably we can change iteration logic the following way:

retry = 1
while (retry && retry_count < N) {
retry = 0
RADIX_RLOCK
generation = rnh_gen
foreach_route()
  if_match()
    if linked_list_add(route)
      refcount(route)
    else
     retry++; // allocation failure, retry
RADIX_RUNLOCK

if linked_list_notempty() {
RADIX_WLOCK
if rnh_gen != generation
  retry++;
foreach_list_item() {
  unref(route)
  call_function(route)
}
RADIX_WUNLOCK
}

}

sys/netinet6/ip6_output.c

1385–1386

In general, updating mtu is control plane change. We don't need to make data plane checks - this seem wrong. We should be able to maintain valid values in our dataplane structures, to optimise the data path. So, calling rt_updatemtu() should be preferred here.

[vangyzen]

• Revert "IPv6: consider interface MTU in path MTU calculation"
• Update the MTU in affected routes when IPv6 RA changes the MTU

[vangyzen]

• Pass the ifp...duh.

[vangyzen]

I would like to commit this change and leave improvements in rt_updatemtu() for another day. MTU changes by RA should be a very rare event, so the inefficiency of rt_updatemtu() is not so significant for this change.

[melifaro]

In D14257#299409, @vangyzen wrote:

I would like to commit this change and leave improvements in rt_updatemtu() for another day.

Sure, it's totally not the scope of this CR.