first batch for local/global ct #659

borkmann · 2017-05-06T02:00:27Z

Remaining bit on todo is to hash out pining of own service via lb for the global CT, rest of the tests work fine already.

- support both global 5 tuple / container local 4 tuple conntrack - cilium daemon can configure this support using #define CONNTRACK_LOCAL - defaults to global Signed-off-by: Madhu Challa <madhu@cilium.io> [ dbkm: squashed some follow-up fixes into it ]

Use a single huge global connection tracking table by default, and have also the option to switch to container local one instead as we have prior to this patch. The switch can even happen during runtime per reconfiguration via: cilium endpoint config <ep-ID> ConntrackLocal=false // global table cilium endpoint config <ep-ID> ConntrackLocal=true // local table Dumping the global table works as usual: cilium bpf ct list global // dump global table cilium bpf ct list <ep-ID> // dump local table It's also listed as a config option via cli, f.e.: # cilium config Conntrack Enabled ConntrackAccounting Enabled ConntrackLocal Enabled DropNotification Enabled NAT46 Enabled Policy Enabled PolicyTracing Disabled I changed the 01-ct tests to include local/global table tests and they were all passing. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

Signed-off-by: Madhu Challa <madhu@cilium.io>

Never really worked well on IPv6. The ping tests also need to run with --privileged. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

In v4 (as opposed to v6), we call both __ct_lookup()'s with a ct_state buffer that will get updated once an entry is found, whereas we only want to store the nat index etc when in reverse direction, before we called ipv4_ct_tuple_reverse(tuple) to put it into forward direction. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

We don't need to get the tuple once again since unnecessary. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

Updating to the service target will make the reverse lookup fail when we ping our own service. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

Otherwise it's easy to miss when auditing the code. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

The offset for l3_csum_replace() is wrong, but it also never worked since as size we pass 1, which is invalid from kernel side. But also the skb_store_bytes() used the old ttl value. Fix the csum offset and use 2 as size, which is fine since it just zero extends. Then, store new_ttl into the packet. Signed-off-by: Madhu Challa <madhu@cilium.io> Signed-off-by: Daniel Borkmann <daniel@cilium.io>

Use common helpers that will chose constant vs. runtime variants automatically. Reduces also #insns for runtime cases. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

The type is really just __u8, not int. We do skb_load_bytes() on this and write the first byte of it, and later read the full int. IPv6 is correct, and using __u8. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

Add IPv6 test for pinging own service. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

Update go-bindata due to prior changes under bpf/. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

houndci-bot · 2017-05-06T02:00:38Z

pkg/option/option.go

@@ -170,6 +170,10 @@ func (bo *BoolOptions) IsEnabled(key string) bool {
 	return exists && set
 }

+func (bo *BoolOptions) IsDisabled(key string) bool {


exported method BoolOptions.IsDisabled should have comment or be unexported

houndci-bot · 2017-05-06T02:00:38Z

pkg/maps/ctmap/ctmap.go

+	flags   uint8
+}
+
+func (key CtKey4Global) Dump(buffer *bytes.Buffer) bool {


exported method CtKey4Global.Dump should have comment or be unexported

houndci-bot · 2017-05-06T02:00:38Z

pkg/maps/ctmap/ctmap.go

+	return true
+}
+
+type CtKey4Global struct {


exported type CtKey4Global should have comment or be unexported

houndci-bot · 2017-05-06T02:00:38Z

pkg/maps/ctmap/ctmap.go

+	flags   uint8
+}
+
+func (key CtKey6Global) Dump(buffer *bytes.Buffer) bool {


exported method CtKey6Global.Dump should have comment or be unexported

houndci-bot · 2017-05-06T02:00:38Z

pkg/maps/ctmap/ctmap.go

@@ -123,6 +130,78 @@ func (key CtKey4) Dump(buffer *bytes.Buffer) bool {
 	return true
 }

+type CtKey6Global struct {


exported type CtKey6Global should have comment or be unexported

houndci-bot · 2017-05-06T02:00:38Z

pkg/maps/ctmap/ctmap.go

@@ -32,8 +32,13 @@ type CtMap struct {
 }

 const (
-	MapName6 = "cilium_ct6_"
-	MapName4 = "cilium_ct4_"
+	MapName6       = "cilium_ct6_"


exported const MapName6 should have comment (or a comment on this block) or be unexported

tgraf

Looks great. Can you add a commit add the end which updates the NEWS.rst file?

borkmann · 2017-05-06T15:46:03Z

Yeah, will do that later today.

Mention the main change in this set also in news section. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

borkmann · 2017-05-06T22:10:47Z

Pushed above, I only described the high level change on the ct table. The rest might be too detailed for the news log (?), but if preferred I can also add further entries, just let me know.

mchalla and others added 13 commits May 6, 2017 03:52

bpf: Global 5 tuple conntrack.

3aa2122

- support both global 5 tuple / container local 4 tuple conntrack - cilium daemon can configure this support using #define CONNTRACK_LOCAL - defaults to global Signed-off-by: Madhu Challa <madhu@cilium.io> [ dbkm: squashed some follow-up fixes into it ]

Add global, local ctmaps with different key sizes.

2f0284b

Signed-off-by: Madhu Challa <madhu@cilium.io>

tests: get rid of bombardier tool/test and fixup ping tests

73c1221

Never really worked well on IPv6. The ping tests also need to run with --privileged. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

bpf: minor cleanups in ct and lxr code

147b99c

We don't need to get the tuple once again since unnecessary. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

bpf: make service pinging of local ct working again

b5e3c88

Updating to the service target will make the reverse lookup fail when we ping our own service. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

bpf: use defines for ct directions

6dc6c04

Otherwise it's easy to miss when auditing the code. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

bpf: add and use bpf_hton{s,l}, bpf_ntoh{s,l}

eb7257c

Use common helpers that will chose constant vs. runtime variants automatically. Reduces also #insns for runtime cases. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

bpf: fix icmp v4 type load

c5fd805

The type is really just __u8, not int. We do skb_load_bytes() on this and write the first byte of it, and later read the full int. IPv6 is correct, and using __u8. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

tests: extend 06-lb test wrt own service ping

76c1526

Add IPv6 test for pinging own service. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

cilium: update bindata due to bpf code changes

cadf234

Update go-bindata due to prior changes under bpf/. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

borkmann requested a review from tgraf May 6, 2017 02:00

houndci-bot reviewed May 6, 2017

View reviewed changes

borkmann requested a review from mchalla May 6, 2017 02:00

This was referenced May 6, 2017

cilium, bpf: make global vs local CT table configurable #443

Closed

Global 5 tuple conntrack. #462

Closed

tgraf requested changes May 6, 2017

View reviewed changes

docs: update news entry

01c0e4d

Mention the main change in this set also in news section. Signed-off-by: Daniel Borkmann <daniel@cilium.io>

tgraf approved these changes May 6, 2017

View reviewed changes

tgraf merged commit 790ca89 into master May 6, 2017

tgraf deleted the bpf-ct7 branch May 6, 2017 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

first batch for local/global ct #659

first batch for local/global ct #659

borkmann commented May 6, 2017

houndci-bot May 6, 2017

houndci-bot May 6, 2017

houndci-bot May 6, 2017

houndci-bot May 6, 2017

houndci-bot May 6, 2017

houndci-bot May 6, 2017

tgraf left a comment

borkmann commented May 6, 2017

borkmann commented May 6, 2017

first batch for local/global ct #659

first batch for local/global ct #659

Conversation

borkmann commented May 6, 2017

houndci-bot May 6, 2017

Choose a reason for hiding this comment

houndci-bot May 6, 2017

Choose a reason for hiding this comment

houndci-bot May 6, 2017

Choose a reason for hiding this comment

houndci-bot May 6, 2017

Choose a reason for hiding this comment

houndci-bot May 6, 2017

Choose a reason for hiding this comment

houndci-bot May 6, 2017

Choose a reason for hiding this comment

tgraf left a comment

Choose a reason for hiding this comment

borkmann commented May 6, 2017

borkmann commented May 6, 2017