geneve: fix IPv6 remote address reporting
Subsystems (2)
PATCH 1/1 geneve: fix IPv6 remote address reporting
Since we can only configure unicast, we probably want to be able to display unicast, rather than multicast.
---
I'm assuming this is what was intended, but tbh I don't know why we need to check for multicast on the display side at all, rather than just displaying whatever the kernel gives us.
ip/iplink_geneve.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ip/iplink_geneve.c +1 -1
@@ -204,7 +204,7 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])memcpy (&addr,RTA_DATA (tb[IFLA_GENEVE_REMOTE6 ]),sizeof (struct in6_addr ));if (memcmp (&addr, &in6addr_any,sizeof (addr)) !=0 ) { -if (IN6_IS_ADDR_MULTICAST (&addr)) +if (!IN6_IS_ADDR_MULTICAST (&addr))fprintf (f,"remote %s " ,format_host (AF_INET6 ,sizeof (struct in6_addr ), &addr)); }
Signature · 1 line
2.4.3
Quoted 2 lines · Edward Cree
Since we can only configure unicast, we probably want to be able to display unicast, rather than multicast.
Furthermore, the kernel even rejects multicast peer addresses.
Quoted 3 lines
I'm assuming this is what was intended, but tbh I don't know why we need to check for multicast on the display side at all, rather than just displaying whatever the kernel gives us.
Why do you then propose a dubious fix to a dubious check instead of getting rid of it in the first place? Reminds me a bit of this here (no offense intended):
http://geekandpoke.typepad.com/geekandpoke/2011/07/good-coders.html
Cheers, Phil
Quoted 4 lines · Phil Sutter
Quoted 2 lines · Edward Cree
Since we can only configure unicast, we probably want to be able to display unicast, rather than multicast.
Furthermore, the kernel even rejects multicast peer addresses.
Yes, but a future kernel might not, and iproute2 is meant to be forward- compatible.
Quoted 2 lines
Why do you then propose a dubious fix to a dubious check instead of getting rid of it in the first place?
Because John Linville clearly had some reason for putting a check there, and he probably knows better than me. Chesterton's fence.
-Ed
Quoted 7 lines · Edward Cree
Quoted 4 lines · Phil Sutter
Quoted 2 lines · Edward Cree
Since we can only configure unicast, we probably want to be able to display unicast, rather than multicast.
Furthermore, the kernel even rejects multicast peer addresses.
Yes, but a future kernel might not, and iproute2 is meant to be forward- compatible.
Sorry, but I fail to see how this might break forward compatibility. Quite the contrary, suppose geneve in future supported multicast peers, current iproute2 would fail to recognize it's existence. What am I missing here?
Quoted 4 lines
Quoted 2 lines
Why do you then propose a dubious fix to a dubious check instead of getting rid of it in the first place?
Because John Linville clearly had some reason for putting a check there, and he probably knows better than me. Chesterton's fence.
A valid point, indeed. In my opinion the same applies to your patch as well, as instead of removing the fence you're moving it to the other lane. :)
Cheers, Phil
Quoted 5 lines · On Fri, 6 May 2016 15:28:25 +0100 Edward Cree <[email protected]> wrote:
Since we can only configure unicast, we probably want to be able to display unicast, rather than multicast.
Fixes 906ac5437ab8 ("geneve: add support for IPv6 link partners")Signed-off-by Edward Cree <[email protected]>
Quoted 7 lines
---
I'm assuming this is what was intended, but tbh I don't know why we need to check for multicast on the display side at all, rather than just displaying whatever the kernel gives us.ip/iplink_geneve.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied thanks