Fix synchronization of lbgroup access.
ClosedPublic
Actions

Authored by markj on Sep 4 2018, 9:36 PM.

Details

Reviewers

sbruno
mmacy
johalun0_gmail.com

Group Reviewers

network

Commits

rS338571: Fix synchronization of LB group access.

Summary

lbgroup hash table lookups are protected by epoch sections, but the hash
chains are regular LISTs instead of CK_LISTs. We also were not
deferring frees of inpcblbgroups.

Test Plan

Regression test in D17110.

gallatin tested the patch on a box at Netflix.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 19396
Build 18996: arc lint + arc unit

Event Timeline

markj created this revision.Sep 4 2018, 9:36 PM

Herald added a subscriber: ae. · View Herald TranscriptSep 4 2018, 9:36 PM

Harbormaster completed remote builds in B19396: Diff 47667.Sep 4 2018, 9:36 PM

markj edited the test plan for this revision. (Show Details)Sep 4 2018, 9:37 PM

markj added reviewers: network, sbruno, mmacy, johalun0_gmail.com.

gallatin added a subscriber: gallatin.Sep 5 2018, 7:23 PM

markj edited the test plan for this revision. (Show Details)Sep 6 2018, 3:24 PM

sbruno accepted this revision.Sep 6 2018, 3:27 PM

This revision is now accepted and ready to land.Sep 6 2018, 3:27 PM

Yeah, CK work was committed soon after so_reuseport_lb was merged so this was missed. Thanks for fixing!

markj edited the test plan for this revision. (Show Details)Sep 10 2018, 6:30 PM

FWIW, as I mentioned in private email, the testing went fine. I verified that there were no apparent mem leaks, and that in_pcblbgroup_free_deferred() was called when I bounced nginx several times on a heavily loaded (> 90Gb/s) box.

In D17031#364754, @gallatin wrote:

FWIW, as I mentioned in private email, the testing went fine. I verified that there were no apparent mem leaks, and that in_pcblbgroup_free_deferred() was called when I bounced nginx several times on a heavily loaded (> 90Gb/s) box.

Indeed, thanks. I also wrote some regression tests; see D17110. I submitted this diff to re@ already.

Closed by commit rS338571: Fix synchronization of LB group access. (authored by markj). · Explain WhySep 10 2018, 7:00 PM

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: imp. · View Herald TranscriptSep 10 2018, 7:00 PM