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

expose VXLAN and health ports as a flags #15956

Closed
errordeveloper opened this issue Apr 30, 2021 · 11 comments · Fixed by #17975
Closed

expose VXLAN and health ports as a flags #15956

errordeveloper opened this issue Apr 30, 2021 · 11 comments · Fixed by #17975
Assignees
@errordeveloper
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow area/daemon Impacts operation of the Cilium daemon. area/health Relates to the cilium-health component kind/feature This introduces new functionality. pinned These issues are not marked stale by our issue bot. release-blocker/1.11 This issue will prevent the release of the next version of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Milestone
1.11

Comments

@errordeveloper
Copy link
Contributor

errordeveloper commented Apr 30, 2021 •
edited

Proposal / RFE

Is your feature request related to a problem?

When deploying Cilium on OpenShift user has to run custom script to modify security groups and enable Cilium ports. This step in the guide is quite error-prone.

OpenShift installer does make the IANA VXLAN port 4789 open, albeit Cilium uses default Linux VXLAN port 8472.

Additionally, having the ability to use a different port would make OpenShift CI setup work out of the box for all providers, presently (openshift/release#17380) Cilium job is enabled only in Azure (where the NSG rules are more permissive).

Also, the health port 4240 is not settable either, but OpenShift installations usually have ports 9900-9999 open and one of those can should be useable instead of 4240.

Describe the solution you'd like

Add a daemon flags and Helm chart values to set VXLAN and health ports, keep the default as is.
@errordeveloper errordeveloper added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. kind/feature This introduces new functionality. area/daemon Impacts operation of the Cilium daemon. area/CI-improvement Topic or proposal to improve the Continuous Integration workflow labels Apr 30, 2021
@errordeveloper errordeveloper changed the title expose VLAN port as a flag expose VLAN and health ports as a flag May 7, 2021
@errordeveloper errordeveloper added the area/health Relates to the cilium-health component label May 7, 2021
@errordeveloper errordeveloper changed the title expose VLAN and health ports as a flag expose VLAN and health ports as a flags May 7, 2021
@stale
Copy link

stale bot commented Jul 9, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jul 9, 2021
@errordeveloper errordeveloper changed the title expose VLAN and health ports as a flags expose VXLAN and health ports as a flags Jul 9, 2021
@stale stale bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jul 9, 2021
@errordeveloper errordeveloper added the pinned These issues are not marked stale by our issue bot. label Jul 9, 2021
@errordeveloper
Copy link
Contributor Author

errordeveloper commented Jul 12, 2021 •
edited

I am looking into this now, and have a few questions about setup logic.

Having looked at this line here:

cilium/bpf/init.sh

Lines 431 to 434 in 8876594

ip link show $ENCAP_DEV || {
ip link add name $ENCAP_DEV address $(rnd_mac_addr) type $TUNNEL_MODE external || encap_fail
}
ip link set $ENCAP_DEV mtu $MTU || encap_fail

I am wondering if I have to pass dstport parameter as part of ip link add invocation, or I can use ip link set?

I also found that there srcport, but assuming that it's enough to set dstport?

According to ip-link(8), it seems like dstport is only settable in an ip link add invocation?

I am just trying to figure out how complex this logic will need to be, because (ideally) port number should be settable after the interface was created...

@errordeveloper
Copy link
Contributor Author

errordeveloper commented Jul 12, 2021 •
edited

According to ip-link(8), it seems like dstport is only settable in an ip link add invocation?

Having looked at the source code briefly, it appears that it should be possible to use ip link set, as vxlan_parse_opt is passed as part of generalised struct link_util .

@errordeveloper errordeveloper mentioned this issue Jul 14, 2021
Merged
@errordeveloper
Copy link
Contributor Author

errordeveloper commented Jul 15, 2021 •
edited

TODOs

aanm pushed a commit that referenced this issue Sep 29, 2021
@errordeveloper @aanm
health: Add a flag to set HTTP port
e624868 
This change makes it possible for user to set a custom port
for Cilium heath-checks.

Default port remains unchanged.

Towards: #15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
errordeveloper added a commit to errordeveloper/cilium that referenced this issue Oct 14, 2021
@errordeveloper
datapath: Add a flag to set VXLAN and Geneve ports
223139f 
This change makes it possible for user to set a custom port
for VXLAN or Geneve. In order to anable that, the defaulting
logic was introduced in the agent, so kernel defaults are no
longer relied upon.

Default ports remain unchanged.

Towards: cilium#15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
aanm pushed a commit that referenced this issue Oct 18, 2021
@errordeveloper @aanm
datapath: Add a flag to set VXLAN and Geneve ports
0300772 
This change makes it possible for user to set a custom port
for VXLAN or Geneve. In order to anable that, the defaulting
logic was introduced in the agent, so kernel defaults are no
longer relied upon.

Default ports remain unchanged.

Towards: #15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@christarazi christarazi mentioned this issue Oct 26, 2021
Merged
errordeveloper added a commit to errordeveloper/cilium that referenced this issue Nov 17, 2021
@errordeveloper
docs: Remove firewall hack for OKD GSG
69e0a26 
Default tunnel and health ports had been updated in OLM repo now
(see cilium/cilium-olm@5fa5c16).

Towards: cilium#15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@errordeveloper errordeveloper mentioned this issue Nov 17, 2021
Merged
gandro pushed a commit that referenced this issue Nov 18, 2021
@errordeveloper @gandro
docs: Remove firewall hack for OKD GSG
3d759b9 
Default tunnel and health ports had been updated in OLM repo now
(see cilium/cilium-olm@5fa5c16).

Towards: #15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@borkmann
Copy link
Member

TODOs

@errordeveloper Did you follow-up with the small doc change? Just wondering if we can close this blocker issue here.

@errordeveloper
Copy link
Contributor Author

errordeveloper commented Nov 22, 2021 •
edited

[...]

@errordeveloper Did you follow-up with the small doc change? Just wondering if we can close this blocker issue here.

@borkmann no, I haven't yet updated the docs regarding downtime on flag change... Maybe blocking label can be removed now?

@borkmann
Copy link
Member

[...]

@borkmann no, I haven't yet updated the docs regarding downtime on flag change... Maybe blocking label can be removed now?

Ok, given it's just a small paragraph, do you have a chance to send the doc PR? Would be great if we could close/finalize this one here.

@errordeveloper
Copy link
Contributor Author

[...]

@borkmann no, I haven't yet updated the docs regarding downtime on flag change... Maybe blocking label can be removed now?

Ok, given it's just a small paragraph, do you have a chance to send the doc PR? Would be great if we could close/finalize this one here.

Yeah, will try to do it in the next few days!

@borkmann
Copy link
Member

Yeah, will try to do it in the next few days!

Awesome, thanks Ilya!

errordeveloper added a commit to errordeveloper/cilium that referenced this issue Nov 23, 2021
@errordeveloper
docs: Add upgrade note rearding custom ports
be3eeb6 
Towards: cilium#15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@errordeveloper errordeveloper mentioned this issue Nov 23, 2021
Merged
@errordeveloper errordeveloper mentioned this issue Nov 23, 2021
Merged
errordeveloper added a commit to errordeveloper/openshift-release that referenced this issue Nov 23, 2021
@errordeveloper
Upgrade Cilium to 1.11.0-rc1 & add e2e-{aws,gcp}-cilium jobs
b381a5d 
This new version supports custom ports, so that now Cilium can be tested
in any OpenShift cluster (see cilium/cilium#15956 & cilium/cilium-olm@5fa5c167ac5).
errordeveloper added a commit to errordeveloper/cilium that referenced this issue Nov 23, 2021
@errordeveloper
docs: Add upgrade note rearding custom ports
bf77826 
Towards: cilium#15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
borkmann pushed a commit that referenced this issue Nov 23, 2021
@errordeveloper @borkmann
docs: Add upgrade note rearding custom ports
858b5e2 
Towards: #15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
qmonnet pushed a commit to qmonnet/cilium that referenced this issue Nov 29, 2021
@errordeveloper @qmonnet
docs: Add upgrade note rearding custom ports
bc3c0ef 
[ upstream commit 858b5e2 ]

Towards: cilium#15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
aanm pushed a commit that referenced this issue Nov 30, 2021
@errordeveloper @aanm
docs: Add upgrade note rearding custom ports
c44649d 
[ upstream commit 858b5e2 ]

Towards: #15956

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@errordeveloper errordeveloper

Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow area/daemon Impacts operation of the Cilium daemon. area/health Relates to the cilium-health component kind/feature This introduces new functionality. pinned These issues are not marked stale by our issue bot. release-blocker/1.11 This issue will prevent the release of the next version of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Milestone
1.11
Development

Successfully merging a pull request may close this issue.

docs: Add upgrade note regarding custom ports errordeveloper/cilium
3 participants
@brb @errordeveloper @borkmann