COLIBRI backend continuation

[juagargi]

• getter DB functions
• UTs
• Rebase after Colibri store backend #3778 (first proper commit is a30df "refactor, move private function to bottom")

---

This change is 

[oncilla]

Reviewed 14 of 15 files at r1.
Reviewable status: 14 of 15 files reviewed, 4 unresolved discussions (waiting on @juagargi)

---

go/cs/reservation/segment/index.go, line 103 at r1 (raw file):

func (idxs Indices) GetExpiration(i int) time.Time                { return idxs[i].Expiration }

// Sort sorts these Indices.

Please add a description of this is sorted.
What happens in the case when there is no discontinuity?

---

go/cs/reservation/segment/index.go, line 104 at r1 (raw file):

// Sort sorts these Indices.
func (idxs *Indices) Sort() {

What relies on this slice being sorted?

It is hard to judge if this sorts them in the right order.
I.e. how do you decide what the first index is, or does it not matter?

---

go/cs/reservation/segment/reservation.go, line 98 at r1 (raw file):

// NewIndex creates a new index in this reservation and returns a pointer to it.
// Parameters of this index can be changed using the pointer, except for the state.
func (r *Reservation) NewIndex(expTime time.Time, token reservation.Token) (

That is a bit surprising.
How does the token already exist when the index is created?

Also, on-path ASes do not really have access to the token, right?

---

go/cs/reservation/segment/reservation.go, line 153 at r1 (raw file):

		return nil // already active
	}
	if r.Indices[sliceIndex].state != IndexPending && r.Indices[sliceIndex].state != IndexActive {

Wait, isn't this method supposed to set the state to IndexActive?
How can this already be active?

[juagargi]

Reviewable status: 14 of 15 files reviewed, 4 unresolved discussions (waiting on @oncilla)

---

go/cs/reservation/segment/index.go, line 103 at r1 (raw file):

Previously, Oncilla wrote…

Please add a description of this is sorted.
What happens in the case when there is no discontinuity?

Done. There is always a discontinuity if len <16. If len=16, any rotation of a sorted sequence is also a sorted sequence mod 16.

---

go/cs/reservation/segment/index.go, line 104 at r1 (raw file):

Previously, Oncilla wrote…

What relies on this slice being sorted?

It is hard to judge if this sorts them in the right order.
I.e. how do you decide what the first index is, or does it not matter?

Indices must be restored in the same order they where originally in the slice. Several things in the reservation rely on the indices being sorted, such as validation (indices must be consecutive) or the activeIndex reference (identifies the index by position in the slice.
I just realized there is a bug related to this: when we have 16 indices, Sort cannot differentiate between rotations of the slice, but only one configuration is actually valid, the one with the expiration times in increasing order and sorted according to index number modulo 16. I'm fixing this and adding a test.

---

go/cs/reservation/segment/reservation.go, line 98 at r1 (raw file):

Previously, Oncilla wrote…

That is a bit surprising.
How does the token already exist when the index is created?

Also, on-path ASes do not really have access to the token, right?

I could be mistaken here, this is my interpretation: the token is a partial token consisting only of the InfoField header; when the response to a setup/renewal comes back, the on-path ASes will see a partial token, but the final token is not visible until it's built at the request originator AS.
Because an index is associated with a (partial) token, and just the InfoField is also a partial token, one is needed to create an index.
Maybe the signature should accept an InfoField instead, to clearly indicate the minimum that is needed to build an Index. Tell me if I should change it.

---

go/cs/reservation/segment/reservation.go, line 153 at r1 (raw file):

Previously, Oncilla wrote…

Wait, isn't this method supposed to set the state to IndexActive?
How can this already be active?

We can construct a segment.Reservation with indexActive==-1 but one of their indices already active e.g. Indices[0].status==IndexActive. This happens in the DB code.
I saw two ways to go around this:

• Allow going active even if the state is active
• In the caller, do not set state to active, but to the previous step, which is IndexConfirmed.

Both solutions are quite coupled with the inner behavior of the reservation and index, and I like none of them sad-:panda_face: . If you find a better one, or if you prefer the other, tell me and I'll gladly change this :smiley:

[oncilla]

Reviewed 1 of 3 files at r2.
Reviewable status: 12 of 15 files reviewed, 3 unresolved discussions (waiting on @juagargi and @oncilla)

---

go/cs/reservation/segment/index.go, line 104 at r1 (raw file):

Previously, juagargi (Juan A. Garcia Pardo) wrote…

Indices must be restored in the same order they where originally in the slice. Several things in the reservation rely on the indices being sorted, such as validation (indices must be consecutive) or the activeIndex reference (identifies the index by position in the slice.
I just realized there is a bug related to this: when we have 16 indices, Sort cannot differentiate between rotations of the slice, but only one configuration is actually valid, the one with the expiration times in increasing order and sorted according to index number modulo 16. I'm fixing this and adding a test.

This requires that expiration times are increase monotonically.
TBH, I don't remember if that is in the spec or not.

---

go/cs/reservation/segment/reservation.go, line 98 at r1 (raw file):

Previously, juagargi (Juan A. Garcia Pardo) wrote…

I could be mistaken here, this is my interpretation: the token is a partial token consisting only of the InfoField header; when the response to a setup/renewal comes back, the on-path ASes will see a partial token, but the final token is not visible until it's built at the request originator AS.
Because an index is associated with a (partial) token, and just the InfoField is also a partial token, one is needed to create an index.
Maybe the signature should accept an InfoField instead, to clearly indicate the minimum that is needed to build an Index. Tell me if I should change it.

I'm always in favor of only taking the information you actually need as argument.
So, it would be good to distill what information you need from the info field.
My hunch is that you do not need most of the info that is there.

---

go/cs/reservation/segment/reservation.go, line 153 at r1 (raw file):

Previously, juagargi (Juan A. Garcia Pardo) wrote…

We can construct a segment.Reservation with indexActive==-1 but one of their indices already active e.g. Indices[0].status==IndexActive. This happens in the DB code.
I saw two ways to go around this:

• Allow going active even if the state is active
• In the caller, do not set state to active, but to the previous step, which is IndexConfirmed.

Both solutions are quite coupled with the inner behavior of the reservation and index, and I like none of them sad-:panda_face: . If you find a better one, or if you prefer the other, tell me and I'll gladly change this :smiley:

Hm, okay.

Please add a comment describing why we need to check for != IndexActive above. As it is not that obvious.

[juagargi]

Reviewable status: 11 of 15 files reviewed, 3 unresolved discussions (waiting on @juagargi and @oncilla)

---

go/cs/reservation/segment/index.go, line 104 at r1 (raw file):

Previously, Oncilla wrote…

This requires that expiration times are increase monotonically.
TBH, I don't remember if that is in the spec or not.

These semantics are embedded in the segment.Reservation.Validate. Let me check the bible (aka your thesis).

---

go/cs/reservation/segment/reservation.go, line 153 at r1 (raw file):

Previously, Oncilla wrote…

Hm, okay.

Please add a comment describing why we need to check for != IndexActive above. As it is not that obvious.

Done.

[juagargi]

Reviewable status: 11 of 15 files reviewed, 3 unresolved discussions (waiting on @juagargi and @oncilla)

---

go/cs/reservation/segment/index.go, line 104 at r1 (raw file):

Previously, juagargi (Juan A. Garcia Pardo) wrote…

These semantics are embedded in the segment.Reservation.Validate. Let me check the bible (aka your thesis).

3.2.7 Reservation Indices:
"Also,
the expiration time of the active indices must be monotonically increasing."

It mentions active indices, not indices in general. Now, if we allow this reservations: t1,t2,t1,t2,..., the second t1 cannot be active if we ever activate the first t2. There might be a strategy where it makes sense to try to activate the second t1 and second t2, and rollback to their first versions if there was a problem, but I fail to see the usefulness of it ATM.
Question is: should I change semantics to allow for such reservations? or should I document this as a property of the reservations?

[juagargi]

Reviewable status: 11 of 15 files reviewed, 3 unresolved discussions (waiting on @oncilla)

---

go/cs/reservation/segment/reservation.go, line 98 at r1 (raw file):

Previously, Oncilla wrote…

I'm always in favor of only taking the information you actually need as argument.
So, it would be good to distill what information you need from the info field.
My hunch is that you do not need most of the info that is there.

Wait, the response must include the InfoField, and there is an InfoField arriving in the request. The natural thing to do is to store it right when we create the index, no? I can replace the Token parameter with an InfoField one. Is that what you refer to?

[oncilla]

Reviewable status: 11 of 15 files reviewed, 2 unresolved discussions (waiting on @juagargi and @oncilla)

---

go/cs/reservation/segment/index.go, line 104 at r1 (raw file):

Previously, juagargi (Juan A. Garcia Pardo) wrote…

3.2.7 Reservation Indices:
"Also,
the expiration time of the active indices must be monotonically increasing."

It mentions active indices, not indices in general. Now, if we allow this reservations: t1,t2,t1,t2,..., the second t1 cannot be active if we ever activate the first t2. There might be a strategy where it makes sense to try to activate the second t1 and second t2, and rollback to their first versions if there was a problem, but I fail to see the usefulness of it ATM.
Question is: should I change semantics to allow for such reservations? or should I document this as a property of the reservations?

Hm, I think just document it should be fine.
It is less flexible, but leads to less corner cases, and there are plenty already.

---

go/cs/reservation/segment/reservation.go, line 98 at r1 (raw file):

Previously, juagargi (Juan A. Garcia Pardo) wrote…

Wait, the response must include the InfoField, and there is an InfoField arriving in the request. The natural thing to do is to store it right when we create the index, no? I can replace the Token parameter with an InfoField one. Is that what you refer to?

The info field is the data plane representation, right?

type InfoField struct {
	ExpirationTick Tick
	BWCls          BWCls
	RLC            RLC
	Idx            IndexNumber
	PathType       PathType
}

so you have expiration tick, idx, and path type that are just duplicate information, no?

[juagargi]

Reviewable status: 11 of 15 files reviewed, 2 unresolved discussions (waiting on @juagargi and @oncilla)

---

go/cs/reservation/segment/reservation.go, line 98 at r1 (raw file):

Previously, Oncilla wrote…

The info field is the data plane representation, right?

type InfoField struct {
	ExpirationTick Tick
	BWCls          BWCls
	RLC            RLC
	Idx            IndexNumber
	PathType       PathType
}

so you have expiration tick, idx, and path type that are just duplicate information, no?

This is the proposed change:

• keep the InfoField in as part of the Token, and all with the semantics of a []byte. Black box it is.
• the InfoField can be empty. If it's not empty, it gets validated along with the other values.
• there are two ways of creating the index: as a source AS or as an on-path AS.
  • the first one takes expiration time, etc as parameters, to build both the index and InfoField
  • the second takes an existing InfoField as parameter and whatever piece of information not already present in the InfoField.
• N.B. whenever I wrote InfoField, the type will be a Token with 0 or more hop colibri opaque fields

[juagargi]

Reviewable status: 11 of 15 files reviewed, 2 unresolved discussions (waiting on @oncilla)

---

go/cs/reservation/segment/index.go, line 104 at r1 (raw file):

Previously, Oncilla wrote…

Hm, I think just document it should be fine.
It is less flexible, but leads to less corner cases, and there are plenty already.

Done.

[oncilla]

Reviewed 1 of 12 files at r3.
Reviewable status: 8 of 19 files reviewed, 2 unresolved discussions (waiting on @oncilla)

[oncilla]

Reviewed 11 of 12 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved

[oncilla]

Reviewable status: complete! all files reviewed, all discussions resolved