Page MenuHomeFreeBSD

Use ratecheck(9) in in_pcbinslbgrouphash().
ClosedPublic

Authored by markj on Sep 6 2018, 5:43 PM.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 19441
Build 19036: arc lint + arc unit

Event Timeline

bz added a subscriber: bz.

LGTM

This revision is now accepted and ready to land.Sep 6 2018, 6:29 PM
This revision was automatically updated to reflect the committed changes.

Hi. I know I'm late to the party, but I have three comments:
a) I could be wrong, but I don't think there is any guarantee this won't be called simultaneously for two different groups at the same time. (The groups could be in different VNETs, for example.) In that case, two different invocations could be working on the function's static variables at the same time. That may produce unexpected results. (Granted, it would take an unusual series of events. But, I think we've all seen highly unusual events occur.)
b) I don't think the const variable also needs to be static.
c) It seems like the rate limiter should really be per-group, so I would suggest adding the lastprint variable to the inpcblbgroup struct.

In D17065#364086, @jtl wrote:

Hi. I know I'm late to the party, but I have three comments:
a) I could be wrong, but I don't think there is any guarantee this won't be called simultaneously for two different groups at the same time. (The groups could be in different VNETs, for example.) In that case, two different invocations could be working on the function's static variables at the same time. That may produce unexpected results. (Granted, it would take an unusual series of events. But, I think we've all seen highly unusual events occur.)
b) I don't think the const variable also needs to be static.
c) It seems like the rate limiter should really be per-group, so I would suggest adding the lastprint variable to the inpcblbgroup struct.

Hmm, right, thanks. I'll fix this up and post a separate review.