Send NDP NA message upon assigning egress IP #2196
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2196 +/- ##
==========================================
+ Coverage 60.76% 65.32% +4.55%
==========================================
Files 286 287 +1
Lines 23096 26548 +3452
==========================================
+ Hits 14034 17342 +3308
- Misses 7592 7615 +23
- Partials 1470 1591 +121
Flags with carried forward coverage won't be shown. Click here to find out more.
|
wenqiq
force-pushed
the
egress-ndp
branch
2 times, most recently
from
98d673b
to
e7362d9
Compare
wenqiq
force-pushed
the
egress-ndp
branch
3 times, most recently
from
2745f9b
to
888dc3d
Compare
wenqiq
changed the title
pkg/agent/util/ndp/ndp_linux.go
Outdated
| return nil | ||
| } | ||
|
|
||
| // NeighborAdvertisement sends an NDP Neighbor Advertisement over interface 'iface' from 'srcIP'. |
There was a problem hiding this comment.
| // NeighborAdvertisement sends an NDP Neighbor Advertisement over interface 'iface' from 'srcIP'. | |
| // NeighborAdvertisement sends a NDP Neighbor Advertisement over interface 'iface' from 'srcIP'. |
pkg/agent/util/ndp/ndp_linux.go
Outdated
| return fmt.Errorf("interface address error: %v", err) | ||
| } | ||
|
|
||
| ipAddr := &net.IPAddr{} |
There was a problem hiding this comment.
| ipAddr := &net.IPAddr{} | |
| var ipAddr *net.IPAddr |
Since you create a new instance in the loop.
pkg/agent/util/ndp/ndp_linux.go
Outdated
| } | ||
| } | ||
|
|
||
| ic, err := icmp.ListenPacket("ip6:ipv6-icmp", ipAddr.String()) |
There was a problem hiding this comment.
why it needs to listen incoming icmp packets? I thought it just sends an unsolicited NA?
There was a problem hiding this comment.
done, use syscall.Socket to send a NA message
syscall.Socket(syscall.AF_INET6, syscall.SOCK_RAW, syscall.IPPROTO_ICMPV6)
pkg/agent/util/ndp/ndp_linux.go
Outdated
| } | ||
|
|
||
| cm := &ipv6.ControlMessage{ | ||
| HopLimit: hopLimit, |
There was a problem hiding this comment.
why the hopLimit is set multiple times? I assume only one takes effect.
pkg/agent/util/ndp/ndp_linux.go
Outdated
|
|
||
| cm := &ipv6.ControlMessage{ | ||
| HopLimit: hopLimit, | ||
| Src: ipAddr.IP, |
There was a problem hiding this comment.
If we don't set src, will the OS pick one from the interface?
wenqiq
force-pushed
the
egress-ndp
branch
2 times, most recently
from
1681d9c
to
54171f3
Compare
|
Do you have any comments or considerations about this PR? @tnqn |
| return fmt.Errorf("failed to send gratuitous ARP: %v", err) | ||
| } | ||
| klog.V(2).InfoS("Sent gratuitous ARP", "ip", parsedIP) | ||
| } else { | ||
| klog.ErrorS(ipv6NotSupportErr, "Failed to send Advertisement", "ip", parsedIP) | ||
| } else if addr.IP.To16() != nil { |
There was a problem hiding this comment.
as we have verified the IP, maybe just use "else" to avoid a repeated conversion.
There was a problem hiding this comment.
Done
I assume you have tested this PR in a IPv6 cluster and it worked? Have you checked whether the generated NDP packet can fresh an external Node's ip neighbor cache?
I have tested and I will put some test data here soon.
There was a problem hiding this comment.
The external Node can receive the NDP NA or GratuitousARP packets, but IP neighbor cache didn't fresh.
There was a problem hiding this comment.
IIRC, receiving GARP will only flush the stale entry and won't add a new entry, is this your observation? or the stale entry was not deleted? You mean both NDP NA and GARP didn't work?
There was a problem hiding this comment.
I think both NDP NA and GARP can work, sent message packets successfully. The integration test seems back to normal.
I have tested the basic IPv6 SNAT function and put some info in my repo. PTAL. @tnqn
pkg/agent/util/ndp/ndp_linux_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func mustIPv6(s string) (net.IP, error) { |
There was a problem hiding this comment.
Maybe put it to pkg/util/ip/ip.go like
Lines 174 to 181 in fe4457f
It can be used in other cases too. I think you don't need to check the result since it's only used when we know the IP must be valid.
|
integration test failed: |
|
It seems that the code for IPv4 is also wrong as |
In the test case we use local IP "127.0.0.1" to get IPNetDevice and the interface doesn't have a HardwareAddr which cause testing fail.
|
wenqiq
changed the title
|
Rebased upstream main branch and fixed grammer issues in commit message. |
|
/test-all |
|
/test-ipv6-all |
|
@wenqiq I just notice the tests still don't run on IPv6 clusters. Can you have a following-up PR to enable egress tests on IPv6 and dual-stack clusters? otherwise we wouldn't know when it's broken by accident. |
|
"Go / Check tidy, code generation and manifest (pull_request)" failed, which may be because go 1.17 update. Please update your local go version and regenerate the code. |
Sure, will implement it.
It seems there is no generated code or manifest changes in this PR. |
Implement Neighbor Discovery Protocol(NDP) neighbor advertisement for Egress IPv6 support.Once an IPv6 IP address has been assigned to Node, an unsolicited Neighbor Advertisement ICMPv6 multicast packet will be sent, announcing the IP to all IPv6 nodes as per RFC4861. Signed-off-by: Wenqi Qiu <[email protected]>
|
Rebased upstream/main. |
|
/test-ipv6-all |
|
/test-ipv6-only-all |
|
/test-e2e |
|
It seems |
|
I checked the IPv6 e2e tests failed on go build issue, which may be related to Go version upgrade. Since we haven't enabled egress test on IPv6, it shouldn't be affected in theory. I'm going to merge this. @wenqiq will have a follow-up PR for enabling egress test on IPv6 cluster. |
…antrea-io#2196 Fixed agent start fail nil panic in pure IPv6 cluster. Related antrea-io#2436 Signed-off-by: Wenqi Qiu <[email protected]>
…antrea-io#2196 Fixed agent start fail nil panic in pure IPv6 cluster. Related antrea-io#2436 Signed-off-by: Wenqi Qiu <[email protected]>
…antrea-io#2196 Fixed agent start fail nil panic in pure IPv6 cluster. Related antrea-io#2436 Signed-off-by: Wenqi Qiu <[email protected]>
The nodeConfig.NodeIPv4Addr is nil which would cause panic in agent, when starting agent with Egress feature enabled in pure IPv6 cluster. It also adds Egress IPv6 test cases in dual-stack and pure IPv6 cluster. Related antrea-io#2196 Signed-off-by: Wenqi Qiu <[email protected]>
The nodeConfig.NodeIPv4Addr is nil which would cause panic in agent, when starting agent with Egress feature enabled in pure IPv6 cluster. It also adds Egress IPv6 test cases in dual-stack and pure IPv6 cluster. Related antrea-io#2196 Signed-off-by: Wenqi Qiu <[email protected]>
The nodeConfig.NodeIPv4Addr is nil which would cause panic in agent, when starting agent with Egress feature enabled in pure IPv6 cluster. It also adds Egress IPv6 test cases in dual-stack and pure IPv6 cluster. Related antrea-io#2196 Signed-off-by: Wenqi Qiu <[email protected]>
The nodeConfig.NodeIPv4Addr is nil which would cause panic in agent, when starting agent with Egress feature enabled in pure IPv6 cluster. It also adds Egress IPv6 test cases in dual-stack and pure IPv6 cluster. Related #2196 Signed-off-by: Wenqi Qiu <[email protected]>
The nodeConfig.NodeIPv4Addr is nil which would cause panic in agent, when starting agent with Egress feature enabled in pure IPv6 cluster. It also adds Egress IPv6 test cases in dual-stack and pure IPv6 cluster. Related antrea-io#2196 Signed-off-by: Wenqi Qiu <[email protected]>
The nodeConfig.NodeIPv4Addr is nil which would cause panic in agent, when starting agent with Egress feature enabled in pure IPv6 cluster. It also adds Egress IPv6 test cases in dual-stack and pure IPv6 cluster. Related #2196 Signed-off-by: Wenqi Qiu <[email protected]>
Send NDP NA message upon assigning egress IP
Implement Neighbor Discovery Protocol(NDP) neighbor advertisement for Egress IPv6 support.
Once an IPv6 IP address has been assigned to Node, an unsolicited Neighbor Advertisement ICMPv6
multicast packet will be sent, announcing the IP to all IPv6 nodes as per RFC4861.
Signed-off-by: Wenqi Qiu [email protected]
For #2128