forwarder: update the condition for default route

[genjuro214]

Fixes #1630

The problem is r.Destination.Bits() may return different results if building with different Go versions

r.Destination.Bits() return 0 for go1.20.5, but return -1 for go1.21.1

I tried to change the condition to !r.Destination.IsValid() because it returns consistent results.

The code of IsValid() for go1.20.5:

// IsValid reports whether p.Bits() has a valid range for p.Addr().
// If p.Addr() is the zero Addr, IsValid returns false.
// Note that if p is the zero Prefix, then p.IsValid() == false.
func (p Prefix) IsValid() bool { return !p.ip.isZero() && p.bits >= 0 && int(p.bits) <= p.ip.BitLen() }

The code of IsValid() for go1.21.1:

func (p Prefix) IsValid() bool { return p.bitsPlusOne > 0 }

func PrefixFrom(ip Addr, bits int) Prefix {
	var bitsPlusOne uint8
	if !ip.isZero() && bits >= 0 && bits <= ip.BitLen() {
		bitsPlusOne = uint8(bits) + 1
	}
	return Prefix{
		ip:          ip.withoutZone(),
		bitsPlusOne: bitsPlusOne,
	}
}

I think the purpose of the code here is to judge if the route Destination is the default prefix "0.0.0.0/0". However, the route Destination here is Destination:netip.Prefix{ip:netip.Addr{addr:netip.uint128{hi:0x0, lo:0x0}, z:(*intern.Value)(nil)}, bits:0} for go1.20.5 and Destination:netip.Prefix{ip:netip.Addr{addr:netip.uint128{hi:0x0, lo:0x0}, z:(*intern.Value)(nil)}, bitsPlusOne:0x0} for go1.21.1

Because the z is nil, the Addr.isZero() returns true, and Addr.IsUnspecified() returns false.

The z is nil, because the route Destination is from nil. In https://github.com/confidential-containers/cloud-api-adaptor/blob/main/pkg/util/netops/netops.go#L539, the r.Dst is nil, because in https://github.com/confidential-containers/cloud-api-adaptor/blob/main/pkg/util/netops/netops.go#L458, we just set Dst if dst.Bits() > 0. If change the condition to if dst.Bits() >= 0, ns.handle.RouteListFiltered will return nil. Not sure why, it's the 3rd party code of "github.com/vishvananda/netlink"

[stevenhorsman]

LGTM. Thanks!

[huoqifeng]

LGTM

[huoqifeng]

Just curious why image build/push is triggered in a PR's CI?

[stevenhorsman]

Just curious why image build/push is triggered in a PR's CI?

So I think it is in order to ensure we are testing with the latest image build, however we could potentially do it another way and if the podvm files aren't touched then we fall back to using the latest merged peer pod vm build. I guess there might be issues with rebasing and detecting correctly if a version.yaml file is updated that also updates the peer pod vm though, so that's why we always do it now?

[stevenhorsman]

@genjuro214 - can you rebase your branch and then we can get the libvirt ci tests running to double-check this change. Thanks!

[yoheiueda]

@genjuro214 thank you very much for the investigation!

It looks like the root cause of the problem is my misuse of the zero value of netip.Prefix here.

cloud-api-adaptor/pkg/util/netops/netops.go

Lines 769 to 773 in afe662c

	func toPrefix(ipnet *net.IPNet) netip.Prefix {
	if ipnet == nil {
	return netip.Prefix{}
	}

I think returning the valid "0.0.0.0/0" representation will fix the issue as well.

	if ipnet == nil {
		return netip.PrefixFrom(netip.IPv4Unspecified(), 0)
	}

Bits() returns a consistent value in both go1.20 and go1.21.

https://go.dev/play/p/0seUd4F1OIA

I think changing the toPrefix function seems a more comprehensive fix.. WDYT?

[genjuro214]

@yoheiueda , yes that's more reasonable fix for this issue. I updated the PR according to your suggestion, thanks!

[genjuro214]

rebase the branch to pickup 2 new commits from main

[liudalibj]

please check the fail ut.

--- FAIL: TestWorkerNode (0.01s)
=== RUN   TestPodNode
--- PASS: TestPodNode (0.01s)
=== RUN   TestPluginDetectHostInterface
--- PASS: TestPluginDetectHostInterface (2.00s)
FAIL
	github.com/confidential-containers/cloud-api-adaptor/pkg/podnetwork	coverage: 68.0% of statements
FAIL	github.com/confidential-containers/cloud-api-adaptor/pkg/podnetwork	2.028s

[liudalibj]

maybe need waiting #1634 to fix the e2e related fail.

[yoheiueda]

LGTM. Thank you very much!