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.

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

fix: support ptp networks with ipv4 /31 netmask and ipv6 /127 netmask #4425

Merged
merged 18 commits into from
Sep 9, 2024

Conversation

cnvergence
Copy link
Contributor

@cnvergence cnvergence commented Aug 21, 2024

Pull Request

What type of this PR

Examples of user facing changes:

  • Bug fixes

It would be valuable for kube-ovn to support ptp networks, to create external subnets between kube-ovn and the router, following RFC 3021 and RFC 6164.

Which issue(s) this PR fixes

Fixes

@cnvergence cnvergence changed the title fix: lastIP set to wrong IP with /31 netmask fix: lastIP set to wrong IP with ipv4 /31 netmask and ipv6 /127 netmask Aug 21, 2024
@cnvergence cnvergence changed the title fix: lastIP set to wrong IP with ipv4 /31 netmask and ipv6 /127 netmask fix: support ptp networks with ipv4 /31 netmask and ipv6 /127 netmask Aug 22, 2024
pkg/util/net.go Outdated Show resolved Hide resolved
pkg/util/net.go Outdated Show resolved Hide resolved
pkg/util/net.go Outdated Show resolved Hide resolved
pkg/util/net.go Outdated Show resolved Hide resolved
pkg/util/net.go Outdated Show resolved Hide resolved
@zhangzujian
Copy link
Member

Subnet checks fail for /32 subnets:

status:
  activateGateway: ""
  conditions:
  - lastTransitionTime: "2024-08-29T01:52:46Z"
    lastUpdateTime: "2024-08-29T01:52:46Z"
    message: 'validate gateway 1.1.1.0 for cidr 1.1.1.0/31 failed: 1.1.1.0 is the
      network number ip in cidr 1.1.1.0/31'
    reason: ValidateLogicalSwitchFailed
    status: "True"
    type: Error

We need to fix it.

cnvergence and others added 12 commits September 3, 2024 15:51
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Co-authored-by: 张祖建 <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
@cnvergence
Copy link
Contributor Author

Thanks for the review, I have added validation test cases @zhangzujian

@zhangzujian
Copy link
Member

We need update IPAM to support p2p networks.

Signed-off-by: Karol Szwaj <[email protected]>
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 5, 2024
@cnvergence
Copy link
Contributor Author

hi @zhangzujian, I have added test cases for IPAM in the pkg/ipam
could you verify where we should add the support for ptp in ipam?

@zhangzujian
Copy link
Member

hi @zhangzujian, I have added test cases for IPAM in the pkg/ipam could you verify where we should add the support for ptp in ipam?

I tried the following subnet and deployment but the kube-ovn could not handle the pod creation:

apiVersion: kubeovn.io/v1
kind: ProviderNetwork
metadata:
  name: ethx
spec:
  defaultInterface: ethx
---
apiVersion: kubeovn.io/v1
kind: Vlan
metadata:
  name: vlan0
spec:
  id: 0
  provider: ethx
---
apiVersion: kubeovn.io/v1
kind: Subnet
metadata:
  name: s1
spec:
  protocol: IPv4
  cidrBlock: 1.1.1.0/31
  vlan: vlan0
  disableGatewayCheck: true
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: kubeovn
  namespace: default
spec:
  selector:
    matchLabels:
      app: kubeovn
  replicas: 2
  template:
    metadata:
      labels:
        app: kubeovn
      annotations:
        ovn.kubernetes.io/logical_switch: s1
        ovn.kubernetes.io/ip_address: 1.1.1.0,1.1.1.1
    spec:
      containers:
      - name: pod
        imagePullPolicy: Never
        # image: kubeovn/kube-ovn:v1.13.0
        image: kubeovn/kube-ovn:dev
        securityContext:
          privileged: true
        command:
        - sleep
        - infinity

If this kind of usage is wrong, please tell me how to use /31 subnets correctly.

@cnvergence
Copy link
Contributor Author

cnvergence commented Sep 6, 2024

  I tried the following subnet and deployment but the kube-ovn could not handle the pod creation:

I see, this could cause a problem with trying to assign 1.1.1.0 to a Pod, which might be a problem.
For the Point-to-point network you would use this external subnet you have created with 1.1.1.0/31, excluding 1.1.1.0, as this will be configured as a gateway and have one available IP for your pod 1.1.1.1

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 9, 2024
@zhangzujian zhangzujian merged commit 3653e74 into kubeovn:master Sep 9, 2024
61 of 62 checks passed
liyh-yusur pushed a commit to liyh-yusur/kube-ovn that referenced this pull request Sep 10, 2024
@cnvergence cnvergence deleted the fix-lastip branch September 12, 2024 12:33
bobz965 pushed a commit that referenced this pull request Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants