Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

'DEL' can not clean up all ip6tables #544

Closed
corporateselect opened this issue Oct 22, 2020 · 0 comments · Fixed by #563 · May be fixed by RoyceDavison/firecracker-go-sdk#4
Closed

'DEL' can not clean up all ip6tables #544

corporateselect opened this issue Oct 22, 2020 · 0 comments · Fixed by #563 · May be fixed by RoyceDavison/firecracker-go-sdk#4
Labels
bug low-hanging-fruit Easy issues, and good first issues.

Comments

@corporateselect
Copy link

We execute the plugins via commandline, because everything works fine i think we are using them right - but if
we add ipv6 it can not cleanup some ip6tables with an error msg;

{
   "cniVersion":"0.4.0",
   "name":"hynet",
   "type":"bridge",
   "bridge":"hybr0",
   "isGateway":true,
   "ipMasq":true,
   "ipam":{
      "type":"host-local",
      "ranges":[
         [
            {
               "subnet":"fd00:101::/64",
               "gateway":"fd00:101::1"
            }
         ],
         [
            {
               "subnet":"10.10.0.0/16",
               "gateway":"10.10.0.1"
            }
         ]
      ]
   }
}

After "ADD" everything works as expected and looks good:
(veth, bridge on host added, eth0 on ns added, iptables created, networking fine)

ip6tables -t nat --list

Chain POSTROUTING (policy ACCEPT)
target     prot opt source               destination         
CNI-63fd59f190a7b6095f99898e  all      fd00:101::9          anywhere             /* name: "hynet" id: "testler" */

Chain CNI-63fd59f190a7b6095f99898e (1 references)
target     prot opt source               destination         
ACCEPT     all      anywhere             fd00:101::/64        /* name: "hynet" id: "testler" */
MASQUERADE  all      anywhere            !ff00::/8             /* name: "hynet" id: "testler" */

If we now try to "DEL":
(with prevResult added)

{
    "code": 999,
    "msg": "running [/usr/sbin/ip6tables -t nat -D POSTROUTING -s fd00:101::9/64 -j CNI-63fd59f190a7b6095f99898e -m comment --comment name: \"hynet\" id: \"testler\" --wait]: exit status 1: iptables: Bad rule (does a matching rule exist in that chain?).\n"
}

We get the error above... but the rule is in fact removed, but not the CNI-Chain.

ip6tables -t nat --list

Chain POSTROUTING (policy ACCEPT)
target     prot opt source               destination         

Chain CNI-63fd59f190a7b6095f99898e (1 references)
target     prot opt source               destination         
ACCEPT     all      anywhere             fd00:101::/64        /* name: "hynet" id: "testler" */
MASQUERADE  all      anywhere            !ff00::/8             /* name: "hynet" id: "testler" */

It seems that it tries to delete the same rule twice and then get this error. If we remove ipv6 everything works, ADD and DEL, with all resources and iptables cleaned up.
@squeed squeed added bug low-hanging-fruit Easy issues, and good first issues. labels Nov 25, 2020
tlwr added a commit to tlwr/cni-plugins that referenced this issue Dec 31, 2020
@tlwr
deps: bump coreos/go-iptables
5d6fdc9 
Closes containernetworking#544

The above issue describes a situation where using the bridge plugin
with IPv6 addresses prevented `DEL` from working correctly.

`DEL` seems to be failing in the body of `TeardownIPMasq`

This arises because:

* twice delete postrouting rules: `ipn.String()` `ipn.IP.String()` containernetworking#279
* we are using a version of go-iptables which is bugged for v6

PR github.com/coreos/go-iptables/pull/74 describes why this does
not work. The error message is not being checked correctly.

Using a later version of go-iptables means that
* when the second `ipt.Delete` fails (this is okay)
* we will correctly interpret this as an non-fatal error
* `TeardownIPMasq` will not prematurely exit the method
* `ipt.ClearChain` now can run
* `ipt.DeleteChain` now can run

This explains why this was working for v4 but not v6

Signed-off-by: toby lorne <toby@toby.codes>
@tlwr tlwr mentioned this issue Dec 31, 2020
Merged
tlwr added a commit to tlwr/cni-plugins that referenced this issue Jan 5, 2021
@tlwr
deps: bump coreos/go-iptables
23a1d90 
Closes containernetworking#544

The above issue describes a situation where using the bridge plugin
with IPv6 addresses prevented `DEL` from working correctly.

`DEL` seems to be failing in the body of `TeardownIPMasq`

This arises because:

* twice delete postrouting rules: `ipn.String()` `ipn.IP.String()` containernetworking#279
* we are using a version of go-iptables which is bugged for v6

PR github.com/coreos/go-iptables/pull/74 describes why this does
not work. The error message is not being checked correctly.

Using a later version of go-iptables means that
* when the second `ipt.Delete` fails (this is okay)
* we will correctly interpret this as an non-fatal error
* `TeardownIPMasq` will not prematurely exit the method
* `ipt.ClearChain` now can run
* `ipt.DeleteChain` now can run

This explains why this was working for v4 but not v6

This commit was amended to include v0.5.0 instead of a pseudo-version
v0.4.6-0.20200318170312-12696f5c9108

Signed-off-by: toby lorne <toby@toby.codes>
@dependabot dependabot bot mentioned this issue Mar 18, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug low-hanging-fruit Easy issues, and good first issues.
Projects
None yet
Milestone
No milestone
2 participants
@squeed @corporateselect