arp/nd: Cope with late calls to iflladdr_event
ClosedPublic
Actions

Authored by kp on Feb 22 2021, 2:33 PM.

Details

Reviewers

donner
melifaro

Group Reviewers

network

Commits

rGa285c84d517f: arp/nd: Cope with late calls to iflladdr_event
rG05ac80ac0f3a: arp/nd: Cope with late calls to iflladdr_event
rGc139b3c19b52: arp/nd: Cope with late calls to iflladdr_event

Summary

When tearing down vnet jails we can move an if_bridge out (as
part of the normal vnet_if_return()). This can, when it's clearing out
its list of member interfaces, change its link layer address.
That sends an iflladdr_event, but at that point we've already freed the
AF_INET/AF_INET6 if_afdata pointers.

In other words: when the iflladdr_event callbacks fire we can't assume
that ifp->if_afdata[AF_INET] will be set.

MFC after: 1 week
Sponsored by: Orange Business Services

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 37269
Build 34158: arc lint + arc unit

Event Timeline

kp created this revision.Feb 22 2021, 2:33 PM

Herald added subscribers: melifaro, bz, ae, imp. · View Herald TranscriptFeb 22 2021, 2:33 PM

kp requested review of this revision.Feb 22 2021, 2:33 PM

Harbormaster completed remote builds in B37269: Diff 84455.Feb 22 2021, 2:33 PM

kp added a child revision: D28861: bridge tests: Test STP on top of VLAN devices.Feb 22 2021, 2:33 PM

kp added a parent revision: D28859: bridge: Remove members when assigned to a new vnet.

Would it not better to not setting the callback on shutdown?

In D28860#646170, @donner wrote:

Would it not better to not setting the callback on shutdown?

It'd require a reordering of stack setup/teardown, and that'd be a far more impactful and risky change.
We currently NULL out if_afdata[] in if_detach_internal(), which is called by if_vmove() before the if_reassign callback which triggers the bridge cleanup that results in the iflladdr_event that triggers the event this fixes.

To remove the callback first we'd have to shut down the arp/nd6 code before we move interfaces out. That feels like the wrong order, and would almost certainly result in other bugs.

donner accepted this revision as: donner.Feb 22 2021, 7:21 PM

This revision is now accepted and ready to land.Feb 22 2021, 7:21 PM

melifaro accepted this revision.Feb 22 2021, 8:07 PM

melifaro added inline comments.