Support unnumbered BGP peering #212

karampok · 2024-10-22T13:31:11Z

/kind feature

What this PR does / why we need it:
As the result of this design proposal
https://github.com/metallb/metallb/blob/main/design/unnumbered-bgp.md

Special notes for your reviewer:
Currently depends on this PR that adds the test functions needed
metallb/metallb#2595,

Release note:

Support unnumbered BGP peering

oribon · 2024-11-17T10:22:30Z

api/v1beta1/frrconfiguration_types.go

@@ -117,7 +117,15 @@ type Neighbor struct {
 	SourceAddress string `json:"sourceaddress,omitempty"`

 	// Address is the IP address to establish the session with.


nit: can you add the // +optional tag as well?

oribon · 2024-11-17T10:25:36Z

internal/controller/api_to_config.go

+	if n.Address == "" && n.Interface == "" {
+		return nil, fmt.Errorf("neighbor %s has no Address or Interface specified", neighborName(n))
+	}
+
+	if n.Address != "" && n.Interface != "" {
+		return nil, fmt.Errorf("neighbor %s has both Address and Interface specified", neighborName(n))
+	}
+


let's put these first

oribon · 2024-11-17T10:26:27Z

internal/controller/api_to_config.go

+	neighborFamily := ipfamily.Unknown
+	if n.Address != "" && n.Interface == "" {
+		neigh, err := ipfamily.ForAddresses(n.Address)
+		if err != nil {
+			return nil, fmt.Errorf("failed to find ipfamily for %s, %w", n.Address, err)
+		}
+		neighborFamily = neigh
 	}


when this is below the validations, this can be if n.Address != ""

oribon · 2024-11-17T10:29:27Z

internal/controller/api_to_config.go

+	if n.Address == "" && n.Interface != "" {
+		res.Unnumbered = true
+		res.Addr = n.Interface
+		res.IPFamily = ipfamily.DualStack
+	}


let's put this above the res, and change the if to n.Interface != "" (or something like defaulting these with vars and only have the n.Address != "" condition, overwriting them)

I think done, but I am not sure I understood exactly what you suggested.

oribon · 2024-11-17T10:29:42Z

internal/controller/api_to_config.go

-		return nil, fmt.Errorf("failed to find ipfamily for %s, %w", n.Address, err)
+	neighborFamily := ipfamily.Unknown
+	if n.Address != "" && n.Interface == "" {
+		neigh, err := ipfamily.ForAddresses(n.Address)


nit: neigh -> f

oribon · 2024-11-17T11:30:00Z

e2etests/tests/unnumbered.go

+			ginkgo.DeferCleanup(func() {
+				err := updater.CleanFRRConfiguration(cr)
+				Expect(err).NotTo(HaveOccurred())
+			})


nit: move this below the create

oribon · 2024-11-17T11:34:27Z

e2etests/tests/unnumbered.go

+	})
+})
+
+func validate(peer *frrcontainer.FRR, prefixes []string) {


same, more descriptive name

oribon · 2024-11-17T11:35:03Z

e2etests/tests/unnumbered.go

+		Expect(err).NotTo(HaveOccurred())
+		for _, n := range neighbors {


I think we should also verify that there are actual neighbors (len > 0 or something)

good catch, true, done

oribon · 2024-11-17T11:38:04Z

e2etests/tests/unnumbered.go

+						Routers: []frrk8sv1beta1.Router{
+							{
+								ASN: infra.FRRK8sASN,
+								Neighbors: []frrk8sv1beta1.Neighbor{


can we also add a simple bfd here just to be sure it works?

sounds good idea, not done, todo

oribon · 2024-11-17T11:39:51Z

internal/frr/config.go

@@ -68,6 +68,7 @@ type NeighborConfig struct {
 	ASN             string
 	SrcAddr         string
 	Addr            string


not sure I like that Addr can now be both interface or address, but for now can you add a comment about it please?

comment added, but we can sure discuss what implementation you would like. Any suggestions?

karampok

Thanks for the feedback, all done except the addition of BFD

karampok · 2024-11-18T08:21:28Z

e2etests/tests/unnumbered.go

+// addresses in the interface.
+var _ = ginkgo.Describe("Unnumbered configure BGP peering", func() {
+	var (
+		node    corev1.Node


karampok · 2024-11-18T08:21:31Z

e2etests/tests/unnumbered.go

+	var (
+		node    corev1.Node
+		updater *config.Updater
+		peer    *frrcontainer.FRR


done (but if you have better name suggestion let me know)

karampok · 2024-11-18T08:21:42Z

e2etests/pkg/config/config.go

@@ -80,3 +80,12 @@ func (u *Updater) Clean() error {

 	return nil
 }
+
+func (u *Updater) CleanFRRConfiguration(cr frrk8sv1beta1.FRRConfiguration) error {


karampok · 2024-11-18T08:23:39Z

e2etests/tests/unnumbered.go

+		updater, err = config.NewUpdater()
+		Expect(err).NotTo(HaveOccurred())
+
+		nodes, err := k8s.Nodes(k8sclient.New())


karampok · 2024-11-18T08:24:09Z

e2etests/tests/unnumbered.go

+						Routers: []frrk8sv1beta1.Router{
+							{
+								ASN: infra.FRRK8sASN,
+								Neighbors: []frrk8sv1beta1.Neighbor{


sounds good idea, not done, todo

karampok · 2024-11-18T08:55:58Z

internal/controller/api_to_config.go

+	if n.Address == "" && n.Interface == "" {
+		return nil, fmt.Errorf("neighbor %s has no Address or Interface specified", neighborName(n))
+	}
+
+	if n.Address != "" && n.Interface != "" {
+		return nil, fmt.Errorf("neighbor %s has both Address and Interface specified", neighborName(n))
+	}
+


karampok · 2024-11-18T08:57:27Z

internal/controller/api_to_config.go

+	neighborFamily := ipfamily.Unknown
+	if n.Address != "" && n.Interface == "" {
+		neigh, err := ipfamily.ForAddresses(n.Address)
+		if err != nil {
+			return nil, fmt.Errorf("failed to find ipfamily for %s, %w", n.Address, err)
+		}
+		neighborFamily = neigh
 	}


karampok · 2024-11-18T09:06:25Z

internal/controller/api_to_config.go

+	if n.Address == "" && n.Interface != "" {
+		res.Unnumbered = true
+		res.Addr = n.Interface
+		res.IPFamily = ipfamily.DualStack
+	}


I think done, but I am not sure I understood exactly what you suggested.

karampok · 2024-11-18T09:06:58Z

internal/controller/api_to_config_test.go

+			err:     nil,
+		},
+		{
+			name: "Neighbor with without Interface and without Address",


karampok · 2024-11-18T09:09:29Z

internal/frr/config.go

@@ -68,6 +68,7 @@ type NeighborConfig struct {
 	ASN             string
 	SrcAddr         string
 	Addr            string


comment added, but we can sure discuss what implementation you would like. Any suggestions?

karampok · 2024-11-18T10:51:33Z

Added one commit that I will squash, now I am testing with BFD the session.

oribon · 2024-12-04T11:19:10Z

e2etests/tests/unnumbered.go

+
+		externalP2PContainer, err := frrcontainer.CreateP2PPeerFor(nodeWithP2PConnection.Name, iface, FRRImage)
+		Expect(err).NotTo(HaveOccurred())
+		ginkgo.By(fmt.Sprintf("create p2p %s:%s -- %s:%s)", nodeWithP2PConnection.Name, iface, iface, externalP2PContainer.Name))


create -> creating + move this above the CreateP2P

done ( need to skip the real name though)

oribon · 2024-12-04T11:20:20Z

e2etests/tests/unnumbered.go

+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		ginkgo.By(fmt.Sprintf("load frrconfig to %s", externalP2PContainer.Name))


nit: -> updating frr config for container %s

oribon · 2024-12-04T11:22:05Z

e2etests/tests/unnumbered.go

+		err = externalP2PContainer.UpdateBGPConfigFile(c)
+		Expect(err).NotTo(HaveOccurred())
+
+		ginkgo.By(fmt.Sprintf("apply frrconfiguration %s", "for-"+externalP2PContainer.Name))


-> peering node %s with its p2p container %s

oribon · 2024-12-04T11:23:07Z

e2etests/tests/unnumbered.go

+	ginkgo.DescribeTable("BGP session is established and routes are verified", func(rc frrconfig.RouterConfigUnnumbered) {
+		iface := rc.Interface
+		remoteASN := rc.ASNLocal
+		prefixSend := []string{"5.5.5.0/24", "5555::/64"}


put this closer to where it's needed

I added on top, I think it fits better

oribon · 2024-12-04T11:23:47Z

e2etests/tests/unnumbered.go

+		Expect(err).NotTo(HaveOccurred())
+		peerLLA, err := container.GetIPv6Link(externalP2PContainer.Name, iface)
+		Expect(err).NotTo(HaveOccurred())
+		ginkgo.By(fmt.Sprintf("%s/%s/%s --- %s/%s/%s",


By("validating the node and p2p container peered")

oribon · 2024-12-04T11:27:20Z

e2etests/tests/unnumbered.go

+		// We can also use existing functions if fix the frr.Ipv4 to be LLA
+		pod, err := k8s.FRRK8sPodOnNode(cs, nodeWithP2PConnection.Name)
+		Expect(err).NotTo(HaveOccurred())
+		ValidateNodesHaveRoutes([]*corev1.Pod{pod}, *externalP2PContainer, rc.ToAdvertiseV4, rc.ToAdvertiseV6)


what does this add?

nothing I let it there to let you know that the usual function can work also in case you wanted to use that. Removed

oribon · 2024-12-04T11:29:37Z

e2etests/tests/unnumbered.go

+	})
+})
+
+func validateBGPPeering(peer *frrcontainer.FRR, lla string) {


nit: validateBGPPeering -> validateUnnumberedBGPPeering ? also lla -> nodeLLA ?

oribon · 2024-12-04T11:30:50Z

e2etests/tests/unnumbered.go

+				return nil
+			}
+		}
+		return fmt.Errorf("no connected neighbor was found")


modify this to include the node's lla

oribon · 2024-12-04T11:32:10Z

e2etests/tests/unnumbered.go

+}
+
+func validatePrefixViaFor(peer executor.Executor, dev, nextHop string, prefixes ...string) {
+	ginkgo.By(fmt.Sprintf("validate prefix %s via %s dev %s", prefixes, nextHop, dev))


validate -> validating

oribon · 2024-12-04T11:34:50Z

e2etests/tests/unnumbered.go

+})
+
+func validateBGPPeering(peer *frrcontainer.FRR, lla string) {
+	ginkgo.By(fmt.Sprintf("validate BGP peering to %s", peer.Name))


validate -> validating, from %s to %s

karampok

Hope I did not miss something, thanks for the feedback @oribon

karampok · 2024-12-04T13:03:37Z

e2etests/tests/unnumbered.go

+		// we cannot use
+		// ValidatePrefixesForNeighbor(*externalP2PContainer, []corev1.Node{nodeWithP2PConnection}, prefixSend...)
+		// because deep down the LLA ip cannot be found as part of the node


i think done

karampok · 2024-12-04T13:04:48Z

e2etests/tests/unnumbered.go

+		// We can also use existing functions if fix the frr.Ipv4 to be LLA
+		pod, err := k8s.FRRK8sPodOnNode(cs, nodeWithP2PConnection.Name)
+		Expect(err).NotTo(HaveOccurred())
+		ValidateNodesHaveRoutes([]*corev1.Pod{pod}, *externalP2PContainer, rc.ToAdvertiseV4, rc.ToAdvertiseV6)


nothing I let it there to let you know that the usual function can work also in case you wanted to use that. Removed

karampok · 2024-12-04T13:14:44Z

e2etests/tests/unnumbered.go

+}
+
+func validatePrefixViaFor(peer executor.Executor, dev, nextHop string, prefixes ...string) {
+	ginkgo.By(fmt.Sprintf("validate prefix %s via %s dev %s", prefixes, nextHop, dev))


karampok · 2024-12-04T13:17:25Z

e2etests/tests/unnumbered.go

+	ginkgo.DescribeTable("BGP session is established and routes are verified", func(rc frrconfig.RouterConfigUnnumbered) {
+		iface := rc.Interface
+		remoteASN := rc.ASNLocal
+		prefixSend := []string{"5.5.5.0/24", "5555::/64"}


I added on top, I think it fits better

karampok · 2024-12-04T13:17:57Z

e2etests/tests/unnumbered.go

+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		ginkgo.By(fmt.Sprintf("load frrconfig to %s", externalP2PContainer.Name))


karampok · 2024-12-04T13:32:20Z

e2etests/tests/unnumbered.go

+
+		validateBGPPeering(externalP2PContainer, nodeLLA)
+
+		ginkgo.By(fmt.Sprintf("validate routing table on %s (prefixSend)", externalP2PContainer.Name))


karampok · 2024-12-04T13:33:10Z

e2etests/tests/unnumbered.go

+		// ValidatePrefixesForNeighbor(*externalP2PContainer, []corev1.Node{nodeWithP2PConnection}, prefixSend...)
+		// because deep down the LLA ip cannot be found as part of the node
+
+		ginkgo.By(fmt.Sprintf("validate routing table on %s (prefixReceive)", nodeWithP2PConnection.Name))


karampok · 2024-12-04T13:35:30Z

e2etests/tests/unnumbered.go

+	})
+})
+
+func validateBGPPeering(peer *frrcontainer.FRR, lla string) {


karampok · 2024-12-04T13:35:37Z

e2etests/tests/unnumbered.go

+})
+
+func validateBGPPeering(peer *frrcontainer.FRR, lla string) {
+	ginkgo.By(fmt.Sprintf("validate BGP peering to %s", peer.Name))


karampok · 2024-12-04T13:37:13Z

e2etests/tests/unnumbered.go

+				return nil
+			}
+		}
+		return fmt.Errorf("no connected neighbor was found")


fedepaol · 2024-12-12T21:27:52Z

internal/controller/api_to_config.go

-	if err != nil {
-		return nil, fmt.Errorf("failed to find ipfamily for %s, %w", n.Address, err)
+	if n.Address == "" && n.Interface == "" {
+		return nil, fmt.Errorf("there is a neighbor with no interface and address")


nit: let's say which neighbor

how? both interface and address is empty, which field should I print ASN?

yep, that for example

I can do that . Fyi @oribon if I remember that error message was your feedback

fedepaol · 2024-12-12T21:29:42Z

internal/frr/parse_test.go

@@ -530,6 +531,215 @@ func TestNeighbours(t *testing.T) {
 	}
 }

+const unnumberedNeihbours = `


typo in neighbors

fedepaol · 2024-12-12T21:30:45Z

internal/controller/api_to_config.go

 	res := &frr.NeighborConfig{
 		Name:            neighborName(n),
 		ASN:             asnFor(n),
 		SrcAddr:         n.SourceAddress,
-		Addr:            n.Address,
+		Addr:            address,


I don't like the fact that we hide the interface under Addr here. Can we have an explicit field?

I agree that name Addr and the value looks not ideal.

Given that FRR itself is using the word PEER to indicate either Addr or Interface name neighbor PEER interface remote-as <internal|external|auto|ASN

What do you think about renaming .Addr to .Peer? Or you prefer to have .Interface field?

I am not in favor of two field because it means everytime we are using .Addr (e.g. error) we should add logic if .Addr empty use the .Interface (but is not that we currently reference that a lot, is more about the future, so not strong argument)

(https://docs.frrouting.org/en/latest/bgp.html#clicmd-neighbor-PEER-interface-remote-as-internal-external-auto-ASN)

I would've agreed if we hadn't to make logic on the fact that it is unnumbered or not, but we are doing it:

neighbor {{.neighbor.Addr}}{{ if .neighbor.Unnumbered }} interface{{ end }} remote-as {{.neighbor.ASN}}

if that's the case I'd prefer having a clear api where fields named explicitly and if we need to call out errors, have a function that returns the proper string (as we do elsewhere).
Names and fields are (almost) free, and we should strive for clarity first.

not done, TODO

fedepaol · 2024-12-12T21:33:27Z

e2etests/tests/unnumbered.go

+	)
+
+	ginkgo.BeforeEach(func() {
+		if _, found := os.LookupEnv("RUN_FRR_CONTAINER_ON_HOST_NETWORK"); found {


let's leave this to the caller. I'd rather have the runner willingly opt out instead of having discovery logic in the tests themselves

Sure I can remove

fedepaol · 2024-12-13T11:02:08Z

e2etests/tests/unnumbered.go

+		err = updater.Update([]corev1.Secret{}, cr)
+		Expect(err).NotTo(HaveOccurred())
+		ginkgo.DeferCleanup(func() {
+			err := updater.DeleteFRRConfiguration(cr)


why do we need this and the clean we are doing in beforeEach is not enough?

the clean in beforeEach is generic clean all so is enough and currently required because most tests do not cleanup their CRs. Nevertheless from gikngko perspective it is recommended each test to cleanup the resources, for example if we want to move towards parallel test that is a small contribution.

Given you comment I will remove it also.

What's bugging me is that all the tests clean the configuration to start with a clean slate, then I land on this and have to spend extra energy to understand why it's different, just to find out that it's unnecessary.
So yes, let's keep the structure consistent with the other tests.

removed for consistency

(but for the record is bugging me the opposite, why tests are not take care of the cleanup)

fedepaol · 2024-12-13T11:05:10Z

e2etests/tests/unnumbered.go

+	)
+
+	// This spec is added to verify that the point-to-point setup works
+	ginkgo.When("raw config", func() {


mind explaining why using When instead of "context" as per all the other tests?

No real reason, I suppose to avoid having the "when" in the description. Not sure if there is any guidance when When should be used instead of Context.

I will correct it so test looks similar

There is no guidance, and I even did not know about when. I'd rather keep consistency if there is no real need.

done (test was removed)

fedepaol · 2024-12-13T11:06:26Z

e2etests/tests/unnumbered.go

+		}),
+	)
+
+	// This spec is added to verify that the point-to-point setup works


Not sure I understood. What's the difference with the other test (apart from using the raw config?)

This test is actually checking that the create container, the p2p wire function , and the unnumber peer config works, without depending on actual code.

E.g. if that test is failing we should be looking whether test code change, external peer config, docker version, frr version introduced something.

Do you prefer that I remove it or update the comment?

Let's remove it. I understand what you mean, but the point of having a CI is to run it incrementally every change and have vision of what's breaking and when. So ideally, if changing a test or frr or docker version is breaking, we'll find out when we apply that change.

Under this logic we should duplicate all our tests to use raw config too.

done

Under this logic we should duplicate all our tests to use raw config too.

No other test is setting up neighbor in a unique way so I do not agree fully with that but no strong opinion to keep it also.

fedepaol · 2024-12-13T11:11:56Z

internal/controller/api_to_config.go

 	res := &frr.NeighborConfig{
 		Name:            neighborName(n),
 		ASN:             asnFor(n),
 		SrcAddr:         n.SourceAddress,
-		Addr:            n.Address,
+		Addr:            address,


I would've agreed if we hadn't to make logic on the fact that it is unnumbered or not, but we are doing it:

neighbor {{.neighbor.Addr}}{{ if .neighbor.Unnumbered }} interface{{ end }} remote-as {{.neighbor.ASN}}

if that's the case I'd prefer having a clear api where fields named explicitly and if we need to call out errors, have a function that returns the proper string (as we do elsewhere).
Names and fields are (almost) free, and we should strive for clarity first.

The API changes ``` type Neighbor struct { Address string `json:address` Interface string `json:interface` } ``` For metrics when we have unnumbered peering the `vtysh show neigh` return a key that is not a valid IP (is `net0`), code was modified accordingly ``` frrk8s_bgp_announced_prefixes_total{peer="net1",vrf="default"} 0 frrk8s_bgp_keepalives_received{peer="net1",vrf="default"} 0 frrk8s_bgp_keepalives_sent{peer="net1",vrf="default"} 0 frrk8s_bgp_notifications_sent{peer="net1",vrf="default"} 0 frrk8s_bgp_opens_received{peer="net1",vrf="default"} 0 ``` Signed-off-by: karampok <karampok@gmail.com>

With the new version we need to change the .IP of the neighbor to .ID. The struct in metallb/e2etest looks as follows: ``` type Neighbor struct { // ID is the key that vtysh returns for the neighbor, // it can be either IP or interface name if unnumbered. ID string VRF string ``` Signed-off-by: karampok <karampok@gmail.com>

The test requires point-to-point connection so we cannot use any existing infra peer. Therefore a new peer (docker container) runs with no network, and point-to-point connections are created using `sudo ip` commands. There are three specs ``` Unnumbered BGP session is established and routes are verified iBGP Unnumbered BGP session is established and routes are verified eBGP ``` Signed-off-by: karampok <karampok@gmail.com>

karampok · 2024-12-16T12:43:52Z

@oribon @fedepaol I think I have covered all you points. Thanks

fedepaol · 2024-12-17T09:45:26Z

LGTM, sending to merge queue

github-actions bot added the kind/feature label Oct 22, 2024

karampok force-pushed the unnumbered branch 7 times, most recently from 104a2fc to d1a45ba Compare October 29, 2024 09:33

karampok force-pushed the unnumbered branch from b9a456a to d1a45ba Compare October 31, 2024 08:45

karampok force-pushed the unnumbered branch 7 times, most recently from ecdded6 to 4d465b0 Compare November 12, 2024 11:26

github-actions bot added kind/feature and removed kind/feature labels Nov 12, 2024

karampok marked this pull request as ready for review November 12, 2024 12:24

github-actions bot added kind/feature and removed kind/feature labels Nov 13, 2024

oribon reviewed Nov 17, 2024

View reviewed changes

karampok force-pushed the unnumbered branch from 4d465b0 to 9b0b953 Compare November 18, 2024 09:38

karampok commented Nov 18, 2024

View reviewed changes

karampok force-pushed the unnumbered branch 3 times, most recently from 500f5f7 to de8afe2 Compare November 18, 2024 15:18

oribon reviewed Dec 4, 2024

View reviewed changes

karampok force-pushed the unnumbered branch from cbb4ae3 to d18fcf1 Compare December 4, 2024 13:47

karampok commented Dec 4, 2024

View reviewed changes

karampok force-pushed the unnumbered branch 13 times, most recently from c6d870b to 62264b7 Compare December 12, 2024 11:40

fedepaol reviewed Dec 12, 2024

View reviewed changes

fedepaol reviewed Dec 13, 2024

View reviewed changes

karampok force-pushed the unnumbered branch from cec2b43 to 488fcb9 Compare December 13, 2024 14:57

karampok mentioned this pull request Dec 16, 2024

E2E: bump e2etest version and use new field #239

Closed

karampok force-pushed the unnumbered branch from 976f960 to f4ad110 Compare December 16, 2024 11:21

karampok added 3 commits December 16, 2024 13:18

karampok force-pushed the unnumbered branch from f4ad110 to 79f4f21 Compare December 16, 2024 12:41

fedepaol added this pull request to the merge queue Dec 17, 2024

Merged via the queue into metallb:main with commit 80d4b7a Dec 17, 2024
11 checks passed

		@@ -117,7 +117,15 @@ type Neighbor struct {
		SourceAddress string `json:"sourceaddress,omitempty"`

		// Address is the IP address to establish the session with.

		Expect(err).NotTo(HaveOccurred())
		for _, n := range neighbors {


		validateBGPPeering(externalP2PContainer, nodeLLA)

		ginkgo.By(fmt.Sprintf("validate routing table on %s (prefixSend)", externalP2PContainer.Name))

Support unnumbered BGP peering #212

Support unnumbered BGP peering #212

Conversation

karampok commented Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karampok left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karampok commented Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karampok left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karampok commented Oct 22, 2024 •

edited

Loading

karampok commented Nov 18, 2024 •

edited

Loading