Page MenuHomeFreeBSD

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

Authored by vangyzen on Feb 7 2018, 9:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 4, 9:49 AM
Unknown Object (File)
Jan 12 2024, 10:07 AM
Unknown Object (File)
Dec 20 2023, 2:45 AM
Unknown Object (File)
Sep 3 2023, 5:51 PM
Unknown Object (File)
Jul 1 2023, 7:22 PM
Unknown Object (File)
Jul 1 2023, 7:22 PM
Unknown Object (File)
Jul 1 2023, 7:19 PM
Unknown Object (File)
Jul 1 2023, 7:19 PM
Subscribers

Details

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.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 14881
Build 14993: arc lint + arc unit

Event Timeline

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()?

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?

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?

  • Revert "IPv6: consider interface MTU in path MTU calculation"
  • IPv6: consider interface MTU in path MTU calculation
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.

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?

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.

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.

  • Revert "IPv6: consider interface MTU in path MTU calculation"
  • Update the MTU in affected routes when IPv6 RA changes the MTU
vangyzen retitled this revision from IPv6: consider interface MTU in path MTU calculation to Update the MTU in affected routes when IPv6 RA changes the MTU.Feb 9 2018, 3:42 PM
vangyzen edited the summary of this revision. (Show Details)

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.

This revision is now accepted and ready to land.Feb 9 2018, 5:21 PM

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.

This revision was automatically updated to reflect the committed changes.