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: 2021-05-11 13:24 UTC (History)
5 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 2020-07-23 03:27:00 UTC
Fix will appear in 12.2; thanks!
Comment 5 Andrey V. Elsukov freebsd_committer freebsd_triage 2021-04-26 20:06:04 UTC
After this patch IPv6 link prefix routes on vlan(4) interfaces have 1500 MTU instead of MTU configured on vlan, i.e. after upgrade all IPv6 link prefix routes have 1500 MTU instead of 9000.
Comment 6 Andrey V. Elsukov freebsd_committer freebsd_triage 2021-04-27 08:37:29 UTC
How it worked before patch:
# ifconfig vlan1000 create vlandev mce3 vlan 1000 up
# ifconfig vlan1000
vlan1000: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 9000
	options=1c680003<RXCSUM,TXCSUM,LINKSTATE,RXCSUM_IPV6,TXCSUM_IPV6,NOMAP,TXTLS4,TXTLS6>
	ether 24:8a:07:b4:32:43
	inet6 fe80::268a:7ff:feb4:3243%vlan1000 prefixlen 64 scopeid 0x8
	groups: vlan
	vlan: 1000 vlanpcp: 0 parent interface: mce3
	media: Ethernet autoselect <full-duplex,rxpause,txpause> (100GBase-CR4 <full-duplex,rxpause,txpause>)
	status: active
	nd6 options=21<PERFORMNUD,AUTO_LINKLOCAL>
# netstat -Wrnf inet6 | grep vlan1000
fe80::%vlan1000/64                link#8                        U          11   9000 vlan1000
fe80::268a:7ff:feb4:3243%vlan1000 link#3                        UHS         1  16384      lo0
# ndp -i vlan1000
linkmtu=0, maxmtu=0, curhlim=64, basereachable=30s0ms, reachable=27s, retrans=1s0ms
Flags: nud auto_linklocal 

After this patch:
# netstat -Wrnf inet6 | grep vlan1000
fe80::%vlan1000/64                link#8                        U          11   1500 vlan1000
fe80::268a:7ff:feb4:3243%vlan1000 link#3                        UHS         1  16384      lo0
# ndp -i vlan1000
linkmtu=0, maxmtu=1500, curhlim=64, basereachable=30s0ms, reachable=18s, retrans=1s0ms
Flags: nud auto_linklocal