Repository navigation
Feat/x4b hpfw provider - #6
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce20c3d22b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static inline u16 banset_expiry(u32 timeout) | ||
| { | ||
| u32 epochs = DIV_ROUND_UP(timeout, 1U << BANSET_EPOCH_SHIFT); | ||
|
|
||
| epochs = clamp_t(u32, epochs, 1, S16_MAX); | ||
| return banset_epoch() + epochs; |
There was a problem hiding this comment.
For entries added late in a 32-second epoch, this computes expiry from the truncated current epoch rather than from the current time. For example, an entry added at boot second 31 with a 32-second timeout gets expiry epoch 1 and is considered expired at second 32, after only one second; even the documented 600-second timeout can expire about 23 seconds early. Round the absolute expiry time instead so entries cannot expire before their requested timeout.
Useful? React with 👍 / 👎.
| u32 epochs = DIV_ROUND_UP(timeout, 1U << BANSET_EPOCH_SHIFT); | ||
|
|
||
| epochs = clamp_t(u32, epochs, 1, S16_MAX); | ||
| return banset_epoch() + epochs; |
There was a problem hiding this comment.
Any timeout above S16_MAX * 32 seconds is silently reduced to that limit by this clamp. Thus a valid 30-day set or element timeout is reported and accepted through the ipset interface but expires after roughly 12.1 days, allowing traffic earlier than configured. Either use a representation that covers the supported timeout range or reject out-of-range values instead of shortening them.
Useful? React with 👍 / 👎.
| list_for_each_entry(set, &banset_bindings, bindings) | ||
| if (set->family == family && set->net == net && | ||
| !strncmp(set->set->name, name, | ||
| IPSET_MAXNAMELEN)) |
There was a problem hiding this comment.
When two bansets are exchanged with ipset swap, the core swaps their data while the name-bearing struct ip_set objects stay in place, but each struct banset retains the creator-time set->set backpointer. This lookup therefore returns the pre-swap backend for a name, so existing iptables and native lookups continue enforcing the old contents instead of the atomically swapped replacement; destroying the other name afterward can make those rules stop matching entirely.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
No description provided.