Bug 248005 - nd6 initializes nd_ifinfo->maxmtu = 0 on newly inserted network interfaces
Summary: nd6 initializes nd_ifinfo->maxmtu = 0 on newly inserted network interfaces
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Kyle Evans
URL:
Keywords: easy
Depends on:
Blocks:
 
Reported: 2020-07-15 18:44 UTC by Mathew
Modified: 2020-07-23 03:27 UTC (History)
4 users (show)

See Also:
kevans: maintainer-feedback+
koobs: maintainer-feedback?
kevans: mfc-stable12+
kevans: mfc-stable11-


Attachments
Proposed patch (456 bytes, patch)
2020-07-15 18:44 UTC, Mathew
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mathew 2020-07-15 18:44:23 UTC
Created attachment 216476 [details]
Proposed patch

The issue is that if_attach is called prior to setting the interface MTU, which will cause nd6_ifattach to attach the interface with an uninitialized MTU value(0).

This issue will typically go unnoticed unless you start adding new network interface after the system has finished starting-up since drivers are mounted/attached then at some time after domainattach will explicitly be called by:
SYSINIT(domainifattach, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_SECOND,
    if_attachdomain, NULL);

To reproduce the observed behaviour:
1. Start-up without a USB Ethernet device plugged in
2. Insert one and load the driver.
3. ndp -i <ifname> will show maxmtu=0 and not maxmtu=1500.

I've attached a diff that will resolve the issue.
Comment 1 Kyle Evans freebsd_committer 2020-07-16 05:38:08 UTC
I agree both with your assessment and your patch. Indeed, early boot will be fine since if_attach_internal will not call if_attachdomain1 due to being too early for domains to be ready.

Take, and I will look at committing this tomorrow since it's trivially correct.
Comment 2 commit-hook freebsd_committer 2020-07-16 13:37:36 UTC
A commit references this bug:

Author: kevans
Date: Thu Jul 16 13:37:32 UTC 2020
New revision: 363244
URL: https://svnweb.freebsd.org/changeset/base/363244

Log:
  ether_ifattach: set mtu before calling if_attach()

  if_attach() -> if_attach_internal() will call if_attachdomain1(ifp) any time
  an ethernet interface is setup *after*
  SI_SUB_PROTO_IFATTACHDOMAIN/SI_ORDER_FIRST.  This eventually leads to
  nd6_ifattach() -> nd6_setmtu0() stashing off ifp->if_mtu in ndi->maxmtu
  *before* ifp->if_mtu has been properly set in some scenarios, e.g., USB
  ethernet adapter plugged in later on.

  For interfaces that are created in early boot, we don't have this issue as
  domains aren't constructed enough for them to attach and thus it gets
  deferred to domainifattach at SI_SUB_PROTO_IFATTACHDOMAIN/SI_ORDER_SECOND
  *after* the mtu has been set earlier in ether_ifattach().

  PR:		248005
  Submitted by:	Mathew <mjanelle blackberry com>
  MFC after:	1 week

Changes:
  head/sys/net/if_ethersubr.c
Comment 3 commit-hook freebsd_committer 2020-07-23 03:25:30 UTC
A commit references this bug:

Author: kevans
Date: Thu Jul 23 03:24:35 UTC 2020
New revision: 363442
URL: https://svnweb.freebsd.org/changeset/base/363442

Log:
  MFC r363244: ether_ifattach: set mtu before calling if_attach()

  if_attach() -> if_attach_internal() will call if_attachdomain1(ifp) any time
  an ethernet interface is setup *after*
  SI_SUB_PROTO_IFATTACHDOMAIN/SI_ORDER_FIRST.  This eventually leads to
  nd6_ifattach() -> nd6_setmtu0() stashing off ifp->if_mtu in ndi->maxmtu
  *before* ifp->if_mtu has been properly set in some scenarios, e.g., USB
  ethernet adapter plugged in later on.

  For interfaces that are created in early boot, we don't have this issue as
  domains aren't constructed enough for them to attach and thus it gets
  deferred to domainifattach at SI_SUB_PROTO_IFATTACHDOMAIN/SI_ORDER_SECOND
  *after* the mtu has been set earlier in ether_ifattach().

  PR:		248005

Changes:
_U  stable/12/
  stable/12/sys/net/if_ethersubr.c
Comment 4 Kyle Evans freebsd_committer 2020-07-23 03:27:00 UTC
Fix will appear in 12.2; thanks!