Fixing issue with bit allocation byteoffset calculation

[abhi]

The byteoffset calculation was skewed to double include the offset value calculated. The double calculation happens if the starting ordinal is part of the head sequence block. This error in calculation could result in duplicate bit getting allocated eventually propogating to ipam and vni id allocations

Signed-off-by: Abhinandan Prativadi abhi@docker.com

[aboch]

Thank you for finding this. I think this issue would affect the --ip-range use case.

Please consider this change instead:

diff --git a/bitseq/sequence.go b/bitseq/sequence.go
index 86cf69b..6319f15 100644
--- a/bitseq/sequence.go
+++ b/bitseq/sequence.go
@@ -505,6 +505,7 @@ func getFirstAvailable(head *sequence, start uint64) (uint64, uint64, error) {
                }
                // Moving to next block: Reset bit offset.
                bitOffset = 0
+               byteOffset = 0
                byteOffset += current.count * blockBytes
                current = current.next
        }

By the nature of the rle bitsequence, we will iterate the for loop at most once. Because if current.block == blockMAX, then current.next.block != blockMAX, it cannot be otherwise.

Also, we already have a test function specifically for getFirstAvailable.
Please consider enhancing it (in addition to your new test) controlling the start field in the input array list:

--- a/bitseq/sequence_test.go
+++ b/bitseq/sequence_test.go
@@ -155,44 +155,50 @@ func TestSequenceCopy(t *testing.T) {
 func TestGetFirstAvailable(t *testing.T) {
        input := []struct {
                mask    *sequence
+               start uint64
                bytePos uint64
                bitPos  uint64
        }{

So in addition to the current input slice elements, please add few counterparts for each with start != 0 (and same expected bytePos and bitPos)

[abhi]

Thanks @aboch for taking time to review it. I will incorporate the changes.

[mavenugo]

LGTM