Skip to content

Conversation

jrfastab
Copy link
Contributor

@jrfastab jrfastab commented Apr 29, 2020

PR #10926 is completely reworked and converted to asm to enable old clang-7 compiler. From #10928 we only apply last remaining test commit. Without the backport of 10926 we were failing previous backports PR #11109 so it was moved here.

$ for pr in 10926 10928; do contrib/backporting/set-labels.py $pr done 1.7; done

[ upstream commit f25d8b9 ]

When Cilium is used in chaining mode with portmap, the hostPort is
translated using iptables DNAT as inserted by the portmap plugin.  When
this happens all within a node, we can preserve the source identity for
the reply traffic for correct visibility. The traffic will be allowed
anyway based on the connection tracking state.

To work with clang-7 and avoid the pattern where the ctx is read into
a register and then incremented then finally a value assigned to it,

  r1 = %[ctx]
  r1 += 8
  ...
  *(u32)(r1 +=8) = %[mark]

We wrote the code block in asm which is not the same as master branch
which was able to use C code due to use of clang-11. We attempted to
update the branch to clang-10 but that created a separate set of issue
that was causing more code churn than we wanted.

Updates: #9784

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
@jrfastab jrfastab requested a review from a team as a code owner April 29, 2020 20:21
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.7 kind/backports This PR provides functionality previously merged into master. labels Apr 29, 2020
@jrfastab
Copy link
Contributor Author

test-me-please

@joestringer
Copy link
Member

joestringer commented Apr 29, 2020

Precheck failure:

13:33:33  bpf_lxc.c:77:20: error: unused function 'asm_set_seclabel_identity' [-Werror,-Wunused-function]
13:33:33  static inline void asm_set_seclabel_identity(struct __sk_buff *skb)
13:33:33                     ^
13:33:33  1 error generated.

@jrfastab jrfastab force-pushed the pr/v1.7-backport-2020-04-29-bpf branch from 5771b83 to 4953e81 Compare April 29, 2020 20:53
@jrfastab
Copy link
Contributor Author

test-me-please

Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

lgtm

[ upstream commit 9499596 ]

Requires the following tests to pass:
```
NAME                                                     READY   STATUS    RESTARTS   AGE
echo-a-5f555bbc8b-9cxv9                                  1/1     Running   0          41s
echo-b-659766fb56-zw2wl                                  1/1     Running   0          41s
echo-b-host-65d7db76d8-5wmhm                             1/1     Running   0          41s
host-to-b-multi-node-clusterip-c7557d4f8-gv6ws           1/1     Running   0          41s
host-to-b-multi-node-headless-5dfcdf9b76-9hcqn           1/1     Running   0          41s
pod-to-a-6cf58894b7-mqg67                                1/1     Running   0          40s
pod-to-a-allowed-cnp-5898f7d8c9-bdfxz                    1/1     Running   0          41s
pod-to-a-external-1111-5779fb7cb9-tgdlh                  1/1     Running   0          39s
pod-to-a-l3-denied-cnp-74b9566cc7-zjhhh                  1/1     Running   0          41s
pod-to-b-intra-node-77b485d996-xfv45                     1/1     Running   0          40s
pod-to-b-intra-node-hostport-6c55bf8459-vddt2            1/1     Running   0          40s
pod-to-b-multi-node-clusterip-75f5c78f68-2lk8x           1/1     Running   0          40s
pod-to-b-multi-node-headless-5df88f9bd4-f5jlt            1/1     Running   0          40s
pod-to-b-multi-node-hostport-d7c8d659f-4xqpt             1/1     Running   0          39s
pod-to-external-fqdn-allow-google-cnp-74466b4c6f-dxvlq   1/1     Running   0          39s
```

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
@jrfastab jrfastab force-pushed the pr/v1.7-backport-2020-04-29-bpf branch from 4953e81 to 01d2c33 Compare April 29, 2020 21:15
@jrfastab
Copy link
Contributor Author

never-tell-me-the-odds

1 similar comment
@jrfastab
Copy link
Contributor Author

never-tell-me-the-odds

@jrfastab
Copy link
Contributor Author

19:52:41      k8s2-1.17: This node has joined the cluster:
19:52:41      k8s2-1.17: * Certificate signing request was sent to apiserver and a response was received.
19:52:41      k8s2-1.17: * The Kubelet was informed of the new secure connection details.
19:52:41      k8s2-1.17: 
19:52:41      k8s2-1.17: Run 'kubectl get nodes' on the control-plane to see this node join the cluster.
19:52:41      k8s2-1.17: 12417445dac012e23b70c439db063e93e8716de15e242bf418dc5c28c4e2fae4
19:52:41      k8s2-1.17: e4069b2efab6796de3c1dbe64644470d245b8111feed7016b7dd3a7dc1bc535b
19:52:42      k8s2-1.17: 0c3366023f00cc6e4ff4e2a68a869e6e4588a6f405cc3ea4adb00800826efe79
19:52:42  getting vagrant kubeconfig from provisioned vagrant cluster
19:52:44  checking whether kubeconfig works for vagrant cluster
19:52:44  Unable to connect to the server: tls: first record does not look like a TLS handshake

@jrfastab
Copy link
Contributor Author

never-tell-me-the-odds

@aanm
Copy link
Member

aanm commented May 1, 2020

it seems the tests failures are related with changes:

	 2097       Enabled            Enabled           5          reserved:init                                     f00d::a0a:0:0:33de   10.10.0.169   ready   
	 3004       Disabled           Disabled          4          reserved:health                                   f00d::a0a:0:0:4dd7   10.10.0.14    ready   
	 3296       Enabled            Enabled           5          reserved:init                                     f00d::a0a:0:0:b37d   10.10.0.237   ready   

@jrfastab
Copy link
Contributor Author

jrfastab commented May 2, 2020

@tgraf any guesses on what we might be missing here. Did a quick scan of the git log and didn't see much that was promising in the ./bpf side.

@jrfastab
Copy link
Contributor Author

jrfastab commented May 7, 2020

never-tell-me-the-odds

@joestringer
Copy link
Member

Upstream k8s build timed out fetching box:
https://jenkins.cilium.io/job/Cilium-PR-Kubernetes-Upstream/2091/execution/node/33/log/

@joestringer
Copy link
Member

test-upstream-k8s

1 similar comment
@jrfastab
Copy link
Contributor Author

jrfastab commented May 7, 2020

test-upstream-k8s

@jrfastab
Copy link
Contributor Author

jrfastab commented May 7, 2020

�[KAn error occurred while downloading the remote file. The error

@jrfastab
Copy link
Contributor Author

jrfastab commented May 7, 2020

test-upstream-k8s

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 7, 2020
@joestringer joestringer merged commit 5f3886b into v1.7 May 7, 2020
@joestringer joestringer deleted the pr/v1.7-backport-2020-04-29-bpf branch May 7, 2020 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants