GetKeys() parsing IPv6 addresses incorrectly

[jeff-yin]

When we get the IPv6 address from redis-db for an interface, say a::b/64, and the api used is GetKeys(), I am getting key[] as

Comp: []string len: 4, cap: 4, [
                       "Ethernet12",
                       "a",
                       "",
                       "b/64",
               ],},

It's supposed to come as

Comp: []string len: 2, cap: 2, [
                       "Ethernet12",
                       "a::b/64",
               ],},

Since the separator is ':', its parsing it wrongly. Works fine for IPv4 addresses.

[jeff-yin]

@justinejose91 -- can you provide the dump of redis KEYS output for the table?

[a-barboza]

Looks like the AppDB has ":" key separator. If the key itself has ":" then how to tell if it is a separator or part of the key ? If the ":" can only appear in the last key, then can we pass the # of keys in the TableSpec parameter ?

[justinejose91]

root@sonic:~# redis-cli
127.0.0.1:6379> KEYS INTF

1. "INTF_TABLE:Ethernet20:10.0.0.10/31"
2. "INTF_TABLE:Ethernet60:10.0.0.30/31"
3. "INTF_TABLE:Ethernet36:10.0.0.18/31"
4. "INTF_TABLE:Ethernet88:10.0.0.44/31"
5. "INTF_TABLE:Ethernet8:10.0.0.4/31"
6. "INTF_TABLE:Ethernet120:10.0.0.60/31"
7. "INTF_TABLE:Ethernet32:10.0.0.16/31"
8. "INTF_TABLE:Ethernet56:10.0.0.28/31"
9. "INTF_TABLE:Ethernet80:10.0.0.40/31"
10. "INTF_TABLE:Ethernet112:10.0.0.56/31"
11. "INTF_TABLE:Ethernet92:10.0.0.46/31"
12. "INTF_TABLE:Ethernet48:10.0.0.24/31"
13. "INTF_TABLE:Ethernet104:10.0.0.52/31"
14. "INTF_TABLE:Ethernet4:10.0.0.2/31"
15. "INTF_TABLE:Ethernet24:10.0.0.12/31"
16. "INTF_TABLE:Ethernet116:10.0.0.58/31"
17. "INTF_TABLE:lo:10.1.0.1/32"
18. "INTF_TABLE:Ethernet44:10.0.0.22/31"
19. "INTF_TABLE:Ethernet28:10.0.0.14/31"
20. "INTF_TABLE:Ethernet64:10.0.0.32/31"
21. "INTF_TABLE:Ethernet12:10.0.0.6/31"
22. "INTF_TABLE:Ethernet16:10.0.0.8/31"
23. "INTF_TABLE:Ethernet24:a::c/64"
24. "INTF_TABLE:Ethernet124:10.0.0.62/31"
25. "INTF_TABLE:Ethernet76:10.0.0.38/31"
26. "INTF_TABLE:Ethernet84:10.0.0.42/31"
27. "INTF_TABLE:Ethernet108:10.0.0.54/31"
28. "INTF_TABLE:Ethernet68:10.0.0.34/31"
29. "INTF_TABLE:Ethernet72:10.0.0.36/31"
30. "INTF_TABLE:Ethernet0:10.0.0.0/31"
31. "INTF_TABLE:Ethernet100:10.0.0.50/31"

[justinejose91]

@a-barboza could you give more information on how to solve this issue?

[a-barboza]

I talked to some users of AppDB here, and they mentioned that each application knows how to distinguish the ":" as a key separator or part of the key based on some knowledge. So, my proposal is to add additional field in TableSpec. Eg: TableSpec { Name: "INTF_TABLE", CompCt: 2 }.
Eg:
d.GetKeys ( &TableSpec { Name: "INTF_TABLE", CompCt: 2 })

Now, the db has some additional information to allow the entry number 23 to be returned correctly as
Key { Comp: []string { "Ethernet24", "a::b/64" }}

Should we try this approach ?

[justinejose91]

Yes, I could try, as per our conversation. I will update you soon. Thanks for the info.

[justinejose91]

Hey @a-barboza , I checked it and modified some code in db.go, which works fine.
func (d *DB) redis2key(ts *TableSpec, redisKey string) Key {

    splitTable := strings.SplitN(redisKey, d.Opts.TableNameSeparator, 2)
    **return Key{strings.SplitN(splitTable[1], d.Opts.KeySeparator, 2)}**

}
Please do check the bold line, which got changed. This is where, we do separate the keys. Since, this being a generic function, it's better to have a generic variable which displays how much splits should happen, and not to go with the above change.

[a-barboza]

Hi Justine, I believe this is on the right track. As you suspected, since this is a generic function, the line should probably change to (in the style of C ternary operator)
return (ts.CompCt) ? Key{string.SplitN(splitTable[1], d.Opts.KeySeparator, ts.ComptCt) : Key{strings.Split(splitTable[1], d.Opts.KeySeparator)}

or whatever the Go equivalent is. I will put these changes in tomorrow, and attempt to push them.

thanks for finding the issue.

[justinejose91]

Yes @a-barboza , this works. Please do let me know once your fix is in. Thank you :)

[a-barboza]

@justinejose91 . Ok, the fix is in. Will wait till tomorrow for the push to project-arlo master

[jeff-yin]

@a-barboza -- has the fix been pushed to master? If so, let's tag the commit ID in this thread and then close the issue.

[justinejose91]

@jeff-yin , fixes are in brcm_poc branch. He will push it to master today. This is what @a-barboza updated yesterday.

[justinejose91]

Changes are merged. Thanks @a-barboza