search

ovn-org / ovn-kubernetes

A robust Kubernetes networking platform
https://ovn-kubernetes.io/
Apache License 2.0
767 stars 333 forks source link

node: Use mdlayher/arp at link_network_manager #4433

Open qinqon opened 2 weeks ago

qinqon commented 2 weeks ago

What this PR does and why is it needed

This PR replace the use of forked golang arping library github.com/JacobTanenbaum/arping with github.com/mdlayher for both GARPs and ip to mac resolving, the new library do not has the issue originally fixed related to use of socke file descriptor at different golang routines.

Commit introducing the fork to fix the issue:

The use of this library was discovering during live migration issue fix where we have to send a GARP with operator type reply, that it's not possible with arping library so the low level mdlayher/arp is used, this is the PR that generated the GARP with operator type reply:

This is a code snippet where we can change the operation type

p, err := arp.NewPacket(arp.OperationReply, arpProxyHardwareAddr, addr, net.HardwareAddr{0, 0, 0, 0, 0, 0}, addr)
            if err != nil {
                return fmt.Errorf("failed create GARP: %w", err)
            }
Use `mdlayher/arp`.
coveralls commented 1 week ago

Coverage Status

coverage: 52.675% (-0.04%) from 52.716% when pulling 04692a052efd2ef16e438ff8009d12285b26458d on qinqon:replace-arping into 701b8e077c6d5639c45ad5cc41fbd4d8759ddc56 on ovn-org:master.

qinqon commented 1 week ago

I see some external-gateway lane failing sometimes, it may be related.

UPDATE: Is not related there is a known flake at the very same test that is failing

https://github.com/ovn-org/ovn-kubernetes/issues/4432

coveralls commented 1 week ago

Coverage Status

Changes unknown when pulling dc918e68d2c41fd196cc4f39c08f9815adb15c1a on qinqon:replace-arping into on ovn-org:master.

coveralls commented 1 week ago

Coverage Status

Changes unknown when pulling be1deea2c066a4398042f1779ccacc44aaecaff6 on qinqon:replace-arping into on ovn-org:master.

qinqon commented 3 days ago

/cc @trozet

Extended PR description with live-migration fix

coveralls commented 2 days ago

Coverage Status

coverage: 52.7% (-0.03%) from 52.734% when pulling c9af102107b681b91b0932b3982f1076ebbc00cf on qinqon:replace-arping into 798eb141b427aaba1c909124a9c47acc38a3f1b4 on ovn-org:master.