Set rule handle during flush #299
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change makes it possible to delete rules after inserting them, without needing to query the rules first. Rules can be deleted both before and after they are flushed. Additionally, this allows positioning a new rule next to an existing rule, both before and after the existing rule is flushed. There are two ways to refer to a rule: Either by ID or by handle. The ID is assigned by userspace, and is only valid within a transaction, so it can only be used before the flush. The handle is assigned by the kernel when the transaction is committed, and can thus only be used after the flush. We thus need to set an ID on each newly created rule, and retrieve the handle of the rule during the flush. There was an existing mechanism to allocate IDs for sets, but this was using a global counter without any synchronization to prevent data races. I replaced this by a new mechanism which uses a connection-scoped counter. I implemented a new mechanism for retrieving replies in Flush, and handling these replies by adding a callback to netlink messages. There was some existing code to handle "overrun", which I deleted, because it was nonsensical and just worked by accident. NLMSG_OVERRUN is in fact not a flag, but a complete message type, so the (re&netlink.Overrun) masking makes no sense. Even better, NLMSG_OVERRUN is never actually used by Linux. What this code was actually doing was skipping over the NFT_MSG_NEWRULE replies, and possibly a NFT_MSG_NEWGEN reply. I had to update all existing tests which compared generated netlink messages against a reference, by inserting the newly added ID attribute. We also need to generate replies for the NFT_MSG_NEWRULE messages with a handle added.
The functionality added in a46119e never worked: If you set NFTA_RULE_POSITION to 0, the kernel will just complain that a rule with this handle does not exist. This removes the broken functionality, leaving the field deprecated. google#126 mentions that the nft command allows referring to rules by index. But here is a quote from the nft manpage: > The add and insert commands support an optional location specifier, > which is either a handle or the index (starting at zero) of an > existing rule. Internally, rule locations are always identified by > handle and the translation from index happens in userspace. In other words, identifiying rules by index is a feature of nft and is not part of the kernel interface.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change makes it possible to delete rules after inserting them, without needing to query the rules first. Rules can be deleted both before and after they are flushed. Additionally, this allows positioning a new rule next to an existing rule, both before and after the existing rule is flushed.
This is a new and improved implementation of the feature proposed in #88. I tried to address the unresolved comments there. See commit messages for more details.