bump netlink&cni&golang version and support add ecmp route

[cyclinder]

Support add ECMP default route:

When I add a sriov-cni NIC to the pod by using multus( The first NIC in the pod is assigned by calico ). I want the default route for the second NIC (net1) to be created correctly, but there is actually only one default route for calico.

root@172-17-8-110:/var/lib/cni/sriov# kubectl exec -it sriov-overlay-8475768cfd-r6slw sh
kubectl exec [POD] [COMMAND] is DEPRECATED and will be removed in a future version. Use kubectl exec [POD] -- [COMMAND] instead.
/ # ip r
default via 169.254.1.1 dev eth0
169.254.1.1 dev eth0 scope link
172.17.8.0/21 dev net1 scope link  src 172.17.8.191

This should actually have two default routes:

default via 169.254.1.1 dev eth0
default via 172.17.8.1 dev net1  # this is the default route for net1
169.254.1.1 dev eth0 scope link
172.17.8.0/21 dev net1 scope link  src 172.17.8.191

In the calico + macvlan scenario, where two default routes are created. I checked the source code of sriov-cni and found that the version of CNI it references is too old to cause this problem.

github.com/containernetworking/cni: v0.8.1

github.com/containernetworking/cni: v1.1.1

In summary, I solved this problem by upgrading CNI to v1.1.1, and I tested it in my local environment, It works well. As an add-on, we also have to upgrade the netlink and golang versions.

That's all this PR is doing.

Signed-off-by: cyclinder qifeng.guo@daocloud.io

[zeeke]

hi @cyclinder , can you provide a little more context about this change?

[cyclinder]

Hi @zeeke , I have just updated the context of this change, please review it. Thanks!

[cyclinder]

I think this change makes sense :) , could it be merged into the master branch if appropriate?

[zeeke]

@cyclinder thanks for explaining.

nit: is the bump to go1.19 strictly needed? if not, can you at least split it in a different commit?

other than that,
LGTM

Thanks

[cyclinder]

@cyclinder thanks for explaining.

nit: is the bump to go1.19 strictly needed? if not, can you at least split it in a different commit?

other than that, LGTM

Thanks

@zeeke Yeah, bumping to go1.19 is strictly needed. and I could raise another PR to bump to go1.19,After waiting for #PR 228 to merge to master brach, This PR is ready to merged, WDYT?

[coveralls]

Pull Request Test Coverage Report for Build 3204643668

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 34.347%

---

Totals
Change from base Build 3015757156:	0.0%
Covered Lines:	339
Relevant Lines:	987

---

💛 - Coveralls

[zeeke]

@Eoghan1232 please, have a look at this.

[Eoghan1232]

overall lgtm, once we clean up the pr to be in line with #228

we can look to merge both

changes needed.

[cyclinder]

Hi @Eoghan1232 Can you take a look ? thanks

[Eoghan1232]

lgtm!
@adrianchiris can you take a look?

[adrianchiris]

Greeting !

while i have no objections with the changes introduced. its a bit unclear to me what fixed ecmp route issue you described.

also updating commit message with a bit more info on the fix would be great !

[adrianchiris]

is netlink version bump needed ? if so do you know what was added/fixed ?

[cyclinder]

Yes, In the latest release of netlink, they have added the RouteAddEcmp function, which is the most important part.

[adrianchiris]

OK here is what i found:

support for calling RouteAddEcmp was added to containernetworking/plugins in v1.0.0 in this commit

in sriov-cni code we call ConfigureInterface from containernetworking plugins ipam package.

so we need to bump containernetworking/plugins verison.

in v1.1.1 an older version of netlink is used
which apparently is good enough, as bumping netlink version here will not affect the netlink version set in plugins go.mod (we are using go modules to build so each package provides its own dependencies)

still, i do not think netlink version should be bumped as there is no reason to bump it.
am i missing anything ?

[adrianchiris]

ah i see latest PR update now uses netlink v1.1.1 and not 1.2.0 so disregard my comment RE netlink version :)

[cyclinder]

Thanks @adrianchiris

Do you have plans to publish a new release?

[adrianchiris]

ATM, release is on an "on-demand" basis. can you open an issue requesting a new release ?

[cyclinder]

Yes! I could open an issue for this, I think it's meaningful.