Page MenuHomeFreeBSD

nd6: Make the DAD callout MPSAFE
ClosedPublic

Authored by markj on Sep 3 2021, 6:07 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 14 2024, 7:30 AM
Unknown Object (File)
Apr 8 2024, 12:12 PM
Unknown Object (File)
Apr 7 2024, 7:18 PM
Unknown Object (File)
Mar 25 2024, 5:42 AM
Unknown Object (File)
Mar 7 2024, 4:33 PM
Unknown Object (File)
Dec 23 2023, 2:06 AM
Unknown Object (File)
Dec 20 2023, 7:41 PM
Unknown Object (File)
Nov 18 2023, 9:51 PM

Details

Summary

Addresses with pending DAD live in a global queue. In this case, a
callout is associated with each entry. The callout transmits neighbour
solicitations until the system decides the address is no longer
tentative, or until a duplicate address is discovered. At this point
the entry is dequeued and freed. DAD may be manually stopped as well.

The callout currently runs with Giant held, which is not really nice
since it transmits packets. Reorganize DAD queue locking to interlock
properly with the callout:

- Configure the callout to acquire the DAD queue lock before running.
  The lock is dropped before transmitting any packets.
- When looking up DAD queue entries for an incoming NS or NA, don't
  bother fiddling with the DAD queue entry reference count.
- Split nd6_dad_starttimer() so that the caller is responsible to
  transmitting a NS if it so desires.
- Remove the DAD entry from the queue before stopping the timer.  Use a
  temporary reference to make sure that the entry doesn't get freed by
  the callout while we're draining.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.Sep 3 2021, 6:07 PM

While I haven't fully checked all the code paths and probably won't be able to the next two weeks; the change in general looks ok to me. Accept-er-ish for that.
And I am happy nd6_dad_starttimer() no longer requires epoch.

sys/netinet6/nd6_nbr.c
1193

unrelated white space; can you just commit that upfront?

1215

again

1222

again

1232

again?

1399

also here { }

This revision is now accepted and ready to land.Sep 3 2021, 8:01 PM
In D31826#717820, @bz wrote:

While I haven't fully checked all the code paths and probably won't be able to the next two weeks; the change in general looks ok to me. Accept-er-ish for that.

Thanks for taking a look. Maybe "discussed with" rather than "reviewed by"? I'm not sure.

I will commit the whitespace changes separately.

hrs added a subscriber: hrs.

If you need a second pair of eyes, I say this change looks good.

This revision was automatically updated to reflect the committed changes.