Skip to content

Add support for iptables in nftables mode.#51

Merged
squeed merged 1 commit intocoreos:masterfrom
squeed:iptables-nft
Aug 3, 2018
Merged

Add support for iptables in nftables mode.#51
squeed merged 1 commit intocoreos:masterfrom
squeed:iptables-nft

Conversation

@squeed
Copy link
Contributor

@squeed squeed commented Aug 2, 2018

Iptables also has the ability to work in nftables mode, where it is supposed to act like iptables but use the nftables subsystem. Unfortunately, it isn't exactly the same.

The biggest difference is that counter output is iptables-save style, rather than with "-c N N".

Also, improve some tests.

Fixes: #49

@squeed
Copy link
Contributor Author

squeed commented Aug 2, 2018

@lucab would you mind taking a look?

@squeed squeed requested a review from lucab August 2, 2018 17:41
Copy link
Contributor

@lucab lucab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor nits, otherwise LGTM.

test Outdated
sudo -E bash -c "PATH=\$GOROOT/bin:\$PATH go test ${COVER} $@ ${TEST}"
sudo -E bash -c "${bin} $@ ${TEST}"
echo "Success"
rm ${bin}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and above, please quote variables in "": "${bin}" etc.

if groups := counterRegex.FindStringSubmatch(rule); groups != nil {
// drop the brackets
rule = rule[len(groups[0]):]
rule = fmt.Sprintf("%s -c %s %s", rule, groups[1], groups[2])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd personally prefer not shadowing input parameters and local variables, if possible.

Iptables also has the ability to work in nftables mode, where it is
supposed to act like iptables but use the nftables subsystem.
Unfortunately, it isn't exactly the same.

The biggest difference is that counter output is iptables-save style,
rather than with "-c N N".

Also, improve some tests.
@squeed
Copy link
Contributor Author

squeed commented Aug 3, 2018

Updated, and added another fix for a failing testcase; PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants