)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f27e1e06a2547f9f9a961443f4a53f0131ad33aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"743f2b1f_fe86f9f3","updated":"2021-12-13 16:10:06.000000000","message":"@Leo Could you test this on a GCP machine? It has integration tests, but a quick test on actual GCP would be nice to validate everything.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000000,"name":"Leopold Schabel","display_name":"Leo","email":"leo@monogon.tech","username":"leo","avatars":[{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"52f88c524d75a53c1eefe54a7fe356bb812e89b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"46bb6864_6304a6e6","in_reply_to":"461216cf_5f4af564","updated":"2021-12-16 11:00:21.000000000","message":"Still works 🎉","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000000,"name":"Leopold Schabel","display_name":"Leo","email":"leo@monogon.tech","username":"leo","avatars":[{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"88019d7468c9740af8654e2d99c7ae5f22a4837a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"461216cf_5f4af564","in_reply_to":"743f2b1f_fe86f9f3","updated":"2021-12-14 12:31:33.000000000","message":"Works 🎉","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"}],"metropolis/node/core/network/dhcp4c/callback/callback.go":[{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":142,"context_line":"\t\t\t\t}"},{"line_number":143,"context_line":"\t\t\t}"},{"line_number":144,"context_line":"\t\t\tif !found {"},{"line_number":145,"context_line":"\t\t\t\terr :\u003d netlink.RouteDel(\u0026route)"},{"line_number":146,"context_line":"\t\t\t\tif !os.IsNotExist(err) \u0026\u0026 err !\u003d nil {"},{"line_number":147,"context_line":"\t\t\t\t\treturn fmt.Errorf(\"failed to delete DHCP route: %w\", err)"},{"line_number":148,"context_line":"\t\t\t\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"c7e6df49_5d93fdb5","line":145,"range":{"start_line":145,"start_character":4,"end_line":145,"end_character":35},"updated":"2021-12-13 16:20:06.000000000","message":"This can have a large blast radius if we first delete a bunch of routes and then just fail to add one new route (because of a programming bug or transient error).\n\nThis could instead collect a list of new routes to add/replace with an optional reference to an existing route that must be deleted beforehand, and only work on one route at a time (instead of all at once). You could also \u0027transpose\u0027 this for loop to instead look at existing routes whenever one is about to be added/replaced and remove any right beforehand, instead of ahead of time here.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"dd5e2072673a536a4f0c3ebef7d2b28bc4c639b7","unresolved":false,"context_lines":[{"line_number":142,"context_line":"\t\t\t\t}"},{"line_number":143,"context_line":"\t\t\t}"},{"line_number":144,"context_line":"\t\t\tif !found {"},{"line_number":145,"context_line":"\t\t\t\terr :\u003d netlink.RouteDel(\u0026route)"},{"line_number":146,"context_line":"\t\t\t\tif !os.IsNotExist(err) \u0026\u0026 err !\u003d nil {"},{"line_number":147,"context_line":"\t\t\t\t\treturn fmt.Errorf(\"failed to delete DHCP route: %w\", err)"},{"line_number":148,"context_line":"\t\t\t\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"ca406907_a1a36d2f","line":145,"range":{"start_line":145,"start_character":4,"end_line":145,"end_character":35},"in_reply_to":"c7e6df49_5d93fdb5","updated":"2021-12-13 16:23:21.000000000","message":"Ah, no, I misunderstood the code. I don\u0027t think we can do better than this, as this really will only remove routes that have no corresponding routes that are added afterwards.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"}],"metropolis/node/core/network/dhcp4c/lease.go":[{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":65,"context_line":"// Router returns the first valid router from the DHCP router option or nil if"},{"line_number":66,"context_line":"// none such exists."},{"line_number":67,"context_line":"// It returns nil if the lease is nil."},{"line_number":68,"context_line":"func (l *Lease) Router() net.IP {"},{"line_number":69,"context_line":"\tif l \u003d\u003d nil {"},{"line_number":70,"context_line":"\t\treturn nil"},{"line_number":71,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"8dd4c56d_8de63848","line":68,"range":{"start_line":68,"start_character":16,"end_line":68,"end_character":22},"updated":"2021-12-13 16:20:06.000000000","message":"I\u0027d maybe add a warning that this might not contain the default route contained in in the static routes option, and that users are encouraged to use Routes() instead. Or maybe even unexport it? I mean, it does say that this is about the DHCP option, but maybe we should more storngly discourage the accidental use of this.\n\nNo strong feelings on this, though.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":false,"context_lines":[{"line_number":65,"context_line":"// Router returns the first valid router from the DHCP router option or nil if"},{"line_number":66,"context_line":"// none such exists."},{"line_number":67,"context_line":"// It returns nil if the lease is nil."},{"line_number":68,"context_line":"func (l *Lease) Router() net.IP {"},{"line_number":69,"context_line":"\tif l \u003d\u003d nil {"},{"line_number":70,"context_line":"\t\treturn nil"},{"line_number":71,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"d5247c3b_0b598ab9","line":68,"range":{"start_line":68,"start_character":16,"end_line":68,"end_character":22},"in_reply_to":"8dd4c56d_8de63848","updated":"2021-12-13 19:05:58.000000000","message":"I just inlined the Router definition as there are apparently no users of this left anyways.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":90,"context_line":"\tvar routes dhcpv4.Routes"},{"line_number":91,"context_line":"\trawCIDRRoutes :\u003d l.Options.Get(dhcpv4.OptionClasslessStaticRoute)"},{"line_number":92,"context_line":"\tif rawCIDRRoutes !\u003d nil {"},{"line_number":93,"context_line":"\t\t// Ignore errors intentionally and just return what has been parsed"},{"line_number":94,"context_line":"\t\t_ \u003d routes.FromBytes(rawCIDRRoutes)"},{"line_number":95,"context_line":"\t\t// RFC 3442 Section DHCP Client Behavior:"},{"line_number":96,"context_line":"\t\t// If the DHCP server returns both a Classless Static Routes option and"}],"source_content_type":"text/x-go","patch_set":1,"id":"771af671_e769f174","line":93,"range":{"start_line":93,"start_character":2,"end_line":93,"end_character":69},"updated":"2021-12-13 16:20:06.000000000","message":"Should we at least log this? It might help in debugging scenarios where Metropolis operators have to deal with very broken DHCP servers.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"b935483a6f1f3fe3aa4928933062ddbf040da930","unresolved":false,"context_lines":[{"line_number":90,"context_line":"\tvar routes dhcpv4.Routes"},{"line_number":91,"context_line":"\trawCIDRRoutes :\u003d l.Options.Get(dhcpv4.OptionClasslessStaticRoute)"},{"line_number":92,"context_line":"\tif rawCIDRRoutes !\u003d nil {"},{"line_number":93,"context_line":"\t\t// Ignore errors intentionally and just return what has been parsed"},{"line_number":94,"context_line":"\t\t_ \u003d routes.FromBytes(rawCIDRRoutes)"},{"line_number":95,"context_line":"\t\t// RFC 3442 Section DHCP Client Behavior:"},{"line_number":96,"context_line":"\t\t// If the DHCP server returns both a Classless Static Routes option and"}],"source_content_type":"text/x-go","patch_set":1,"id":"ab015431_4860bb00","line":93,"range":{"start_line":93,"start_character":2,"end_line":93,"end_character":69},"in_reply_to":"253ccf9f_7789587a","updated":"2021-12-16 08:45:34.000000000","message":"Ack","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000000,"name":"Leopold Schabel","display_name":"Leo","email":"leo@monogon.tech","username":"leo","avatars":[{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/98f8f79a6bb45adef37defa7ead8f3d2.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"12365ed3b645c3fbdb7c4f6a90fbb038110b1fc4","unresolved":true,"context_lines":[{"line_number":90,"context_line":"\tvar routes dhcpv4.Routes"},{"line_number":91,"context_line":"\trawCIDRRoutes :\u003d l.Options.Get(dhcpv4.OptionClasslessStaticRoute)"},{"line_number":92,"context_line":"\tif rawCIDRRoutes !\u003d nil {"},{"line_number":93,"context_line":"\t\t// Ignore errors intentionally and just return what has been parsed"},{"line_number":94,"context_line":"\t\t_ \u003d routes.FromBytes(rawCIDRRoutes)"},{"line_number":95,"context_line":"\t\t// RFC 3442 Section DHCP Client Behavior:"},{"line_number":96,"context_line":"\t\t// If the DHCP server returns both a Classless Static Routes option and"}],"source_content_type":"text/x-go","patch_set":1,"id":"253ccf9f_7789587a","line":93,"range":{"start_line":93,"start_character":2,"end_line":93,"end_character":69},"in_reply_to":"265cff55_6d6f07c7","updated":"2021-12-14 11:45:38.000000000","message":"Just to add to this, I ended up adding a bunch of print statements to the DHCP client when debugging it - some level-ed debugging output would certainly be useful for future debugging endeavors in customer environments :-)\n\n-\u003e https://github.com/monogon-dev/monogon/issues/96","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":true,"context_lines":[{"line_number":90,"context_line":"\tvar routes dhcpv4.Routes"},{"line_number":91,"context_line":"\trawCIDRRoutes :\u003d l.Options.Get(dhcpv4.OptionClasslessStaticRoute)"},{"line_number":92,"context_line":"\tif rawCIDRRoutes !\u003d nil {"},{"line_number":93,"context_line":"\t\t// Ignore errors intentionally and just return what has been parsed"},{"line_number":94,"context_line":"\t\t_ \u003d routes.FromBytes(rawCIDRRoutes)"},{"line_number":95,"context_line":"\t\t// RFC 3442 Section DHCP Client Behavior:"},{"line_number":96,"context_line":"\t\t// If the DHCP server returns both a Classless Static Routes option and"}],"source_content_type":"text/x-go","patch_set":1,"id":"adb51c28_c29f2fe8","line":93,"range":{"start_line":93,"start_character":2,"end_line":93,"end_character":69},"in_reply_to":"771af671_e769f174","updated":"2021-12-13 19:05:58.000000000","message":"This function has no good way of logging things. It just returns a list of routes. I\u0027m open to suggestions how to best handle this.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"80141189864db8f7ef4fa9686486a3577ffcbbe1","unresolved":true,"context_lines":[{"line_number":90,"context_line":"\tvar routes dhcpv4.Routes"},{"line_number":91,"context_line":"\trawCIDRRoutes :\u003d l.Options.Get(dhcpv4.OptionClasslessStaticRoute)"},{"line_number":92,"context_line":"\tif rawCIDRRoutes !\u003d nil {"},{"line_number":93,"context_line":"\t\t// Ignore errors intentionally and just return what has been parsed"},{"line_number":94,"context_line":"\t\t_ \u003d routes.FromBytes(rawCIDRRoutes)"},{"line_number":95,"context_line":"\t\t// RFC 3442 Section DHCP Client Behavior:"},{"line_number":96,"context_line":"\t\t// If the DHCP server returns both a Classless Static Routes option and"}],"source_content_type":"text/x-go","patch_set":1,"id":"265cff55_6d6f07c7","line":93,"range":{"start_line":93,"start_character":2,"end_line":93,"end_character":69},"in_reply_to":"adb51c28_c29f2fe8","updated":"2021-12-14 10:56:19.000000000","message":"Ah, this is a fairly thin datatype. Yeah, we\u0027re missing some clear way of doing \u0027library\u0027 logging in Metropolis right now.\n\nOh well, let\u0027s leave it as is. Maybe add some TODOs at least, pointing to an issue about logging in libraries? Feel free to mark all of the logging comments as resolved.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":93,"context_line":"\t\t// Ignore errors intentionally and just return what has been parsed"},{"line_number":94,"context_line":"\t\t_ \u003d routes.FromBytes(rawCIDRRoutes)"},{"line_number":95,"context_line":"\t\t// RFC 3442 Section DHCP Client Behavior:"},{"line_number":96,"context_line":"\t\t// If the DHCP server returns both a Classless Static Routes option and"},{"line_number":97,"context_line":"\t\t// a Router option, the DHCP client MUST ignore the Router option."},{"line_number":98,"context_line":"\t\t// Similarly, if the DHCP server returns both a Classless Static Routes"},{"line_number":99,"context_line":"\t\t// option and a Static Routes option, the DHCP client MUST ignore the"}],"source_content_type":"text/x-go","patch_set":1,"id":"0a0a8196_48d0ea3d","line":96,"range":{"start_line":96,"start_character":2,"end_line":96,"end_character":73},"updated":"2021-12-13 16:20:06.000000000","message":"It took me a while to understand why we return early here, perhaps move this comment above the if.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":false,"context_lines":[{"line_number":93,"context_line":"\t\t// Ignore errors intentionally and just return what has been parsed"},{"line_number":94,"context_line":"\t\t_ \u003d routes.FromBytes(rawCIDRRoutes)"},{"line_number":95,"context_line":"\t\t// RFC 3442 Section DHCP Client Behavior:"},{"line_number":96,"context_line":"\t\t// If the DHCP server returns both a Classless Static Routes option and"},{"line_number":97,"context_line":"\t\t// a Router option, the DHCP client MUST ignore the Router option."},{"line_number":98,"context_line":"\t\t// Similarly, if the DHCP server returns both a Classless Static Routes"},{"line_number":99,"context_line":"\t\t// option and a Static Routes option, the DHCP client MUST ignore the"}],"source_content_type":"text/x-go","patch_set":1,"id":"bbbb22fa_f7076af0","line":96,"range":{"start_line":96,"start_character":2,"end_line":96,"end_character":73},"in_reply_to":"0a0a8196_48d0ea3d","updated":"2021-12-13 19:05:58.000000000","message":"Done","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":100,"context_line":"\t\t// Static Routes option."},{"line_number":101,"context_line":"\t\treturn routes"},{"line_number":102,"context_line":"\t}"},{"line_number":103,"context_line":"\t// The Static Route option is serialized as a list of IPs belongin in pairs"},{"line_number":104,"context_line":"\t// in which the first one is the destination network and the second one is"},{"line_number":105,"context_line":"\t// the the router IP. See RFC 2132 Section 5.8 for further details."},{"line_number":106,"context_line":"\tlegacyRouteIPs :\u003d dhcpv4.GetIPs(dhcpv4.OptionStaticRoutingTable, l.Options)"}],"source_content_type":"text/x-go","patch_set":1,"id":"43b1228e_4968ee45","line":103,"range":{"start_line":103,"start_character":59,"end_line":103,"end_character":67},"updated":"2021-12-13 16:20:06.000000000","message":"As per Monogon style, this should either be belonging or belongin\u0027. 😊\n\nBut generaly the grammar of this sentence is odd, I assume it\u0027s a leftover from a refactor.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":false,"context_lines":[{"line_number":100,"context_line":"\t\t// Static Routes option."},{"line_number":101,"context_line":"\t\treturn routes"},{"line_number":102,"context_line":"\t}"},{"line_number":103,"context_line":"\t// The Static Route option is serialized as a list of IPs belongin in pairs"},{"line_number":104,"context_line":"\t// in which the first one is the destination network and the second one is"},{"line_number":105,"context_line":"\t// the the router IP. See RFC 2132 Section 5.8 for further details."},{"line_number":106,"context_line":"\tlegacyRouteIPs :\u003d dhcpv4.GetIPs(dhcpv4.OptionStaticRoutingTable, l.Options)"}],"source_content_type":"text/x-go","patch_set":1,"id":"e05431fb_f4fd2e5c","line":103,"range":{"start_line":103,"start_character":59,"end_line":103,"end_character":67},"in_reply_to":"43b1228e_4968ee45","updated":"2021-12-13 19:05:58.000000000","message":"Done","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":104,"context_line":"\t// in which the first one is the destination network and the second one is"},{"line_number":105,"context_line":"\t// the the router IP. See RFC 2132 Section 5.8 for further details."},{"line_number":106,"context_line":"\tlegacyRouteIPs :\u003d dhcpv4.GetIPs(dhcpv4.OptionStaticRoutingTable, l.Options)"},{"line_number":107,"context_line":"\t// Routes are only valid in pairs, cut the last one off if necessary"},{"line_number":108,"context_line":"\tif len(legacyRouteIPs)%2 !\u003d 0 {"},{"line_number":109,"context_line":"\t\tlegacyRouteIPs \u003d legacyRouteIPs[:len(legacyRouteIPs)-1]"},{"line_number":110,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"b714958b_0f618277","line":107,"range":{"start_line":107,"start_character":1,"end_line":107,"end_character":69},"updated":"2021-12-13 16:20:06.000000000","message":"I\u0027d probably also log this just in case of DHCP server weirdness (also the 2132 5.8 check below).","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"b935483a6f1f3fe3aa4928933062ddbf040da930","unresolved":false,"context_lines":[{"line_number":104,"context_line":"\t// in which the first one is the destination network and the second one is"},{"line_number":105,"context_line":"\t// the the router IP. See RFC 2132 Section 5.8 for further details."},{"line_number":106,"context_line":"\tlegacyRouteIPs :\u003d dhcpv4.GetIPs(dhcpv4.OptionStaticRoutingTable, l.Options)"},{"line_number":107,"context_line":"\t// Routes are only valid in pairs, cut the last one off if necessary"},{"line_number":108,"context_line":"\tif len(legacyRouteIPs)%2 !\u003d 0 {"},{"line_number":109,"context_line":"\t\tlegacyRouteIPs \u003d legacyRouteIPs[:len(legacyRouteIPs)-1]"},{"line_number":110,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"108e0720_c88ba59f","line":107,"range":{"start_line":107,"start_character":1,"end_line":107,"end_character":69},"in_reply_to":"b714958b_0f618277","updated":"2021-12-16 08:45:34.000000000","message":"Ack","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":109,"context_line":"\t\tlegacyRouteIPs \u003d legacyRouteIPs[:len(legacyRouteIPs)-1]"},{"line_number":110,"context_line":"\t}"},{"line_number":111,"context_line":"\tfor i :\u003d 0; i \u003c len(legacyRouteIPs)/2; i++ {"},{"line_number":112,"context_line":"\t\tdestNetClassfulIP :\u003d legacyRouteIPs[i*2]"},{"line_number":113,"context_line":"\t\tif destNetClassfulIP.IsUnspecified() {"},{"line_number":114,"context_line":"\t\t\t// RFC 2132 Section 5.8:"},{"line_number":115,"context_line":"\t\t\t// The default route (0.0.0.0) is an illegal destination for a"}],"source_content_type":"text/x-go","patch_set":1,"id":"0769ccc6_5b78ffed","line":112,"range":{"start_line":112,"start_character":2,"end_line":112,"end_character":19},"updated":"2021-12-13 16:20:06.000000000","message":"go style nit: `dest`","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":false,"context_lines":[{"line_number":109,"context_line":"\t\tlegacyRouteIPs \u003d legacyRouteIPs[:len(legacyRouteIPs)-1]"},{"line_number":110,"context_line":"\t}"},{"line_number":111,"context_line":"\tfor i :\u003d 0; i \u003c len(legacyRouteIPs)/2; i++ {"},{"line_number":112,"context_line":"\t\tdestNetClassfulIP :\u003d legacyRouteIPs[i*2]"},{"line_number":113,"context_line":"\t\tif destNetClassfulIP.IsUnspecified() {"},{"line_number":114,"context_line":"\t\t\t// RFC 2132 Section 5.8:"},{"line_number":115,"context_line":"\t\t\t// The default route (0.0.0.0) is an illegal destination for a"}],"source_content_type":"text/x-go","patch_set":1,"id":"f967a111_31824545","line":112,"range":{"start_line":112,"start_character":2,"end_line":112,"end_character":19},"in_reply_to":"0769ccc6_5b78ffed","updated":"2021-12-13 19:05:58.000000000","message":"Done","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":116,"context_line":"\t\t\t// static route."},{"line_number":117,"context_line":"\t\t\tcontinue"},{"line_number":118,"context_line":"\t\t}"},{"line_number":119,"context_line":"\t\trouterIP :\u003d legacyRouteIPs[i*2+1]"},{"line_number":120,"context_line":"\t\tdestNet :\u003d net.IPNet{"},{"line_number":121,"context_line":"\t\t\t// Apply the default mask just to make sure this is a valid route"},{"line_number":122,"context_line":"\t\t\tIP:   destNetClassfulIP.Mask(destNetClassfulIP.DefaultMask()),"}],"source_content_type":"text/x-go","patch_set":1,"id":"6a62a91c_a69959cf","line":119,"range":{"start_line":119,"start_character":2,"end_line":119,"end_character":10},"updated":"2021-12-13 16:20:06.000000000","message":"go style nit: `via` (or router)","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":false,"context_lines":[{"line_number":116,"context_line":"\t\t\t// static route."},{"line_number":117,"context_line":"\t\t\tcontinue"},{"line_number":118,"context_line":"\t\t}"},{"line_number":119,"context_line":"\t\trouterIP :\u003d legacyRouteIPs[i*2+1]"},{"line_number":120,"context_line":"\t\tdestNet :\u003d net.IPNet{"},{"line_number":121,"context_line":"\t\t\t// Apply the default mask just to make sure this is a valid route"},{"line_number":122,"context_line":"\t\t\tIP:   destNetClassfulIP.Mask(destNetClassfulIP.DefaultMask()),"}],"source_content_type":"text/x-go","patch_set":1,"id":"df4c380e_1cd1261d","line":119,"range":{"start_line":119,"start_character":2,"end_line":119,"end_character":10},"in_reply_to":"6a62a91c_a69959cf","updated":"2021-12-13 19:05:58.000000000","message":"Done","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":118,"context_line":"\t\t}"},{"line_number":119,"context_line":"\t\trouterIP :\u003d legacyRouteIPs[i*2+1]"},{"line_number":120,"context_line":"\t\tdestNet :\u003d net.IPNet{"},{"line_number":121,"context_line":"\t\t\t// Apply the default mask just to make sure this is a valid route"},{"line_number":122,"context_line":"\t\t\tIP:   destNetClassfulIP.Mask(destNetClassfulIP.DefaultMask()),"},{"line_number":123,"context_line":"\t\t\tMask: destNetClassfulIP.DefaultMask(),"},{"line_number":124,"context_line":"\t\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"accc01ac_43d9a75c","line":121,"range":{"start_line":121,"start_character":3,"end_line":121,"end_character":68},"updated":"2021-12-13 16:20:06.000000000","message":"Isn\u0027t the mask by already conjured out of thin air (with a whiff of yellowed dot matrix RFC printouts) by the DHCP client library we use - in the exact same way as here?\n\nIf the DHCP library does something wrong/different, then we should probably fix it to not that, instead of doing its job for it here.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"80141189864db8f7ef4fa9686486a3577ffcbbe1","unresolved":false,"context_lines":[{"line_number":118,"context_line":"\t\t}"},{"line_number":119,"context_line":"\t\trouterIP :\u003d legacyRouteIPs[i*2+1]"},{"line_number":120,"context_line":"\t\tdestNet :\u003d net.IPNet{"},{"line_number":121,"context_line":"\t\t\t// Apply the default mask just to make sure this is a valid route"},{"line_number":122,"context_line":"\t\t\tIP:   destNetClassfulIP.Mask(destNetClassfulIP.DefaultMask()),"},{"line_number":123,"context_line":"\t\t\tMask: destNetClassfulIP.DefaultMask(),"},{"line_number":124,"context_line":"\t\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"8f0727df_50077dd9","line":121,"range":{"start_line":121,"start_character":3,"end_line":121,"end_character":68},"in_reply_to":"4bcb4d27_6708e059","updated":"2021-12-14 10:56:19.000000000","message":"Ah, I see.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":true,"context_lines":[{"line_number":118,"context_line":"\t\t}"},{"line_number":119,"context_line":"\t\trouterIP :\u003d legacyRouteIPs[i*2+1]"},{"line_number":120,"context_line":"\t\tdestNet :\u003d net.IPNet{"},{"line_number":121,"context_line":"\t\t\t// Apply the default mask just to make sure this is a valid route"},{"line_number":122,"context_line":"\t\t\tIP:   destNetClassfulIP.Mask(destNetClassfulIP.DefaultMask()),"},{"line_number":123,"context_line":"\t\t\tMask: destNetClassfulIP.DefaultMask(),"},{"line_number":124,"context_line":"\t\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"4bcb4d27_6708e059","line":121,"range":{"start_line":121,"start_character":3,"end_line":121,"end_character":68},"in_reply_to":"accc01ac_43d9a75c","updated":"2021-12-13 19:05:58.000000000","message":"Nope. RFC 2132 specifies an encoding for a list of IPv4 addresses. The DHCP protocol library implements a decoder for that, returning []net.IP. Classful routes are defined as IP pairs, the first one being the network (as the mask can be derived from that by checking in which class said IP is) and the second one being the gateway/router. But people have historically done dumb things like put the node IP instead of the first IP of the network in there. This just ensures that the resulting route is valid.","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c0bb9a2b4e205705f3aca521cfda063a870cdb51","unresolved":true,"context_lines":[{"line_number":131,"context_line":"\t\t\tRouter: routerOptIP,"},{"line_number":132,"context_line":"\t\t})"},{"line_number":133,"context_line":"\t}"},{"line_number":134,"context_line":"\treturn routes"},{"line_number":135,"context_line":"}"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"// DNSServers represents an ordered collection of DNS servers"}],"source_content_type":"text/x-go","patch_set":1,"id":"83492dad_d2438466","line":134,"range":{"start_line":134,"start_character":8,"end_line":134,"end_character":14},"updated":"2021-12-13 16:20:06.000000000","message":"I think we should make an effort to sanitize the routes here (and the early return classless routes, too) - eg. to remove any potential duplicates. Some wonky DHCP server might return something that our route wrangler might disapprove of (and break networking instead of failing gracefully).","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"80141189864db8f7ef4fa9686486a3577ffcbbe1","unresolved":true,"context_lines":[{"line_number":131,"context_line":"\t\t\tRouter: routerOptIP,"},{"line_number":132,"context_line":"\t\t})"},{"line_number":133,"context_line":"\t}"},{"line_number":134,"context_line":"\treturn routes"},{"line_number":135,"context_line":"}"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"// DNSServers represents an ordered collection of DNS servers"}],"source_content_type":"text/x-go","patch_set":1,"id":"8c4fb83a_e6eef569","line":134,"range":{"start_line":134,"start_character":8,"end_line":134,"end_character":14},"in_reply_to":"7066a9cc_ce226a2d","updated":"2021-12-14 10:56:19.000000000","message":"I think we should focus on stuff that will make netlink barf at us, or cause us to be stuck in reconciliation loops (or other logic issues).\n\nI don\u0027t think there\u0027s a problem with b) in your example - in fact, having something like a IPv6 /48 on an interface and different chunks of that same /48 in the form of /64 point into addresses within that /48 is a production deployment I use quite regularly (granted, it\u0027s IPv6, but I\u0027ve also seen similar things with IPv4).\n\nFor c), we should at least limit things like IPv4 multicast space. Not sure what else could cause problems.\n\nPerhaps it would be best to look at what we should explicitly allow, and prohibit everything else? In very handwavy pseudocode:\n\n  For every route in intent:\n    1. If route prefix ~\u003d ip.GlobalMulticast() || ip.IsPrivate():\n       1. Ensure no existing route intent for same prefix but different gateway/target, discard subsequent.\n    2. If Route prefix \u003d\u003d 0/0:\n       1. Ensure only one route intent exists, discard subsequent.\n    3. Ensure route hop is already reachable by already parsed intent or by interface address.\n\nThese seem like a good starting point? What do you think?","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d53b84579d08340d1faf171fe58130978a256d10","unresolved":true,"context_lines":[{"line_number":131,"context_line":"\t\t\tRouter: routerOptIP,"},{"line_number":132,"context_line":"\t\t})"},{"line_number":133,"context_line":"\t}"},{"line_number":134,"context_line":"\treturn routes"},{"line_number":135,"context_line":"}"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"// DNSServers represents an ordered collection of DNS servers"}],"source_content_type":"text/x-go","patch_set":1,"id":"7066a9cc_ce226a2d","line":134,"range":{"start_line":134,"start_character":8,"end_line":134,"end_character":14},"in_reply_to":"83492dad_d2438466","updated":"2021-12-13 19:05:58.000000000","message":"In general I agree, but I\u0027m not sure what sort of scope this validation should have. We could validate a few things:\na) Internal Consistency: there\u0027s no L3 route pointing into unreachable space\nb) Invalid overlapping: duplicate routes, L3 routes shadowing interface routes (but I *think* this might be allowed to some degree), ...\nc) Invalid IP space: routes for multicast or broadcast space (but that gets sketchy with default routes), routes for IP space used within Metropolis (but that can\u0027t live here for obvious reasons), ...","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"b935483a6f1f3fe3aa4928933062ddbf040da930","unresolved":false,"context_lines":[{"line_number":131,"context_line":"\t\t\tRouter: routerOptIP,"},{"line_number":132,"context_line":"\t\t})"},{"line_number":133,"context_line":"\t}"},{"line_number":134,"context_line":"\treturn routes"},{"line_number":135,"context_line":"}"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"// DNSServers represents an ordered collection of DNS servers"}],"source_content_type":"text/x-go","patch_set":1,"id":"9d12ea2f_e68f9cb6","line":134,"range":{"start_line":134,"start_character":8,"end_line":134,"end_character":14},"in_reply_to":"8c4fb83a_e6eef569","updated":"2021-12-16 08:45:34.000000000","message":"Done","commit_id":"eaf68bf16cdb93e622db417c239126052c0fc826"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f7de8ee9285414f20c320bbd863a8cef89a67e5a","unresolved":true,"context_lines":[{"line_number":170,"context_line":"\t\tif route.Dest.IP.IsLoopback() \u0026\u0026 ones \u003e\u003d 8 {"},{"line_number":171,"context_line":"\t\t\tcontinue"},{"line_number":172,"context_line":"\t\t}"},{"line_number":173,"context_line":"\t\t// Ignore routes that would shadow the implicit interface route"},{"line_number":174,"context_line":"\t\tassignedOnes, _ :\u003d assignedNet.Mask.Size()"},{"line_number":175,"context_line":"\t\tif assignedNet.IP.Equal(route.Dest.IP) \u0026\u0026 assignedOnes \u003d\u003d ones {"},{"line_number":176,"context_line":"\t\t\tcontinue"}],"source_content_type":"text/x-go","patch_set":4,"id":"242b9ccf_d5ca08bc","line":173,"range":{"start_line":173,"start_character":2,"end_line":173,"end_character":65},"updated":"2021-12-16 11:27:28.000000000","message":"Do we actually care about this? Aren\u0027t implicit interface routes higher priority than installed routes so they will always win in this case, anyway?","commit_id":"64073d7a077033964ab385a6846fb6aea4ce8ec1"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"a8e34eb5ec2bf1711e18fd92afa7a77603e0c9b2","unresolved":true,"context_lines":[{"line_number":170,"context_line":"\t\tif route.Dest.IP.IsLoopback() \u0026\u0026 ones \u003e\u003d 8 {"},{"line_number":171,"context_line":"\t\t\tcontinue"},{"line_number":172,"context_line":"\t\t}"},{"line_number":173,"context_line":"\t\t// Ignore routes that would shadow the implicit interface route"},{"line_number":174,"context_line":"\t\tassignedOnes, _ :\u003d assignedNet.Mask.Size()"},{"line_number":175,"context_line":"\t\tif assignedNet.IP.Equal(route.Dest.IP) \u0026\u0026 assignedOnes \u003d\u003d ones {"},{"line_number":176,"context_line":"\t\t\tcontinue"}],"source_content_type":"text/x-go","patch_set":4,"id":"8abf6a37_f9967e10","line":173,"range":{"start_line":173,"start_character":2,"end_line":173,"end_character":65},"in_reply_to":"242b9ccf_d5ca08bc","updated":"2021-12-16 11:51:17.000000000","message":"I do think so, but IMO we still shouldn\u0027t return obviously wrong routes even if they do not cause any damage as there might be consumers of this that do not have the same precedence rules.","commit_id":"64073d7a077033964ab385a6846fb6aea4ce8ec1"},{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"627414115fd1f213b53e8808a6ff41f2c04e6a64","unresolved":false,"context_lines":[{"line_number":170,"context_line":"\t\tif route.Dest.IP.IsLoopback() \u0026\u0026 ones \u003e\u003d 8 {"},{"line_number":171,"context_line":"\t\t\tcontinue"},{"line_number":172,"context_line":"\t\t}"},{"line_number":173,"context_line":"\t\t// Ignore routes that would shadow the implicit interface route"},{"line_number":174,"context_line":"\t\tassignedOnes, _ :\u003d assignedNet.Mask.Size()"},{"line_number":175,"context_line":"\t\tif assignedNet.IP.Equal(route.Dest.IP) \u0026\u0026 assignedOnes \u003d\u003d ones {"},{"line_number":176,"context_line":"\t\t\tcontinue"}],"source_content_type":"text/x-go","patch_set":4,"id":"cdd496a4_801c3202","line":173,"range":{"start_line":173,"start_character":2,"end_line":173,"end_character":65},"in_reply_to":"8abf6a37_f9967e10","updated":"2021-12-16 11:57:40.000000000","message":"Ack","commit_id":"64073d7a077033964ab385a6846fb6aea4ce8ec1"}]}
