| Message ID | 1445502868-29474-1-git-send-email-thaller@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Thu, Oct 22, 2015 at 10:34:28AM +0200, Thomas Haller wrote: > Kernel allows for zero IPv4 peer addresses (IFA_ADDRESS): > > ip address add 192.168.5.1 peer 0.0.0.0/24 dev dummy > > which is distinct from a usual address like: > > ip address add 192.168.5.1/24 dev dummy > ip address add 192.168.5.1 peer 192.168.5.1/24 dev dummy > > For IPv4, a missing IFA_ADDRESS attribute means that the peer > is 0.0.0.0. See inet_fill_ifaddr(), which does: > > if ((ifa->ifa_address && > nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) || > > Signed-off-by: Thomas Haller <thaller@redhat.com> Acked-by: Phil Sutter <phil@nwl.cc> I wonder why this is not the case for IPv6. What is the functional effect of setting a zero peer with non-zero netmask? Cheers, Phil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 22 Oct 2015 10:34:28 +0200 Thomas Haller <thaller@redhat.com> wrote: > Kernel allows for zero IPv4 peer addresses (IFA_ADDRESS): > > ip address add 192.168.5.1 peer 0.0.0.0/24 dev dummy > > which is distinct from a usual address like: > > ip address add 192.168.5.1/24 dev dummy > ip address add 192.168.5.1 peer 192.168.5.1/24 dev dummy > > For IPv4, a missing IFA_ADDRESS attribute means that the peer > is 0.0.0.0. See inet_fill_ifaddr(), which does: > > if ((ifa->ifa_address && > nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) || > > Signed-off-by: Thomas Haller <thaller@redhat.com> I would prefer that this apply to both IPv4 and IPv6. If the kernel sends back an address, then display it. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 23, 2015 at 04:04:50PM -0800, Stephen Hemminger wrote: > On Thu, 22 Oct 2015 10:34:28 +0200 > Thomas Haller <thaller@redhat.com> wrote: > > > Kernel allows for zero IPv4 peer addresses (IFA_ADDRESS): > > > > ip address add 192.168.5.1 peer 0.0.0.0/24 dev dummy > > > > which is distinct from a usual address like: > > > > ip address add 192.168.5.1/24 dev dummy > > ip address add 192.168.5.1 peer 192.168.5.1/24 dev dummy > > > > For IPv4, a missing IFA_ADDRESS attribute means that the peer > > is 0.0.0.0. See inet_fill_ifaddr(), which does: > > > > if ((ifa->ifa_address && > > nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) || > > > > Signed-off-by: Thomas Haller <thaller@redhat.com> > > I would prefer that this apply to both IPv4 and IPv6. The case that patch handles does not happen in IPv6. > If the kernel sends back an address, then display it. It's rather "if the kernel *does not* send back an address ...". When reviewing this patch, I tried to find an easier (and less ugly) solution, but failed. Here's the result from testing all variants: 1) ip a a 192.168.1.1/24 dev test0 2) ip a a 192.168.2.1 peer 192.168.2.1/24 dev test0 3) ip a a 192.168.3.1 peer 0.0.0.0/24 dev test0 4) ip a a 192.168.4.1 peer 192.168.4.2 dev test0 5) ip a a feed:babe::1:1/112 dev test0 6) ip a a feed:babe::2:1 peer feed:babe::2:1/112 dev test0 7) ip a a feed:babe::3:1 peer ::/112 dev test0 8) ip a a feed:babe::4:1 peer feed:babe::4:2 dev test0 cmd ifa_local ifa_address --------------------------------- 1) 192.168.1.1 192.168.1.1 2) 192.168.2.1 192.168.2.1 3) 192.168.3.1 unset 4) 192.168.4.1 192.168.4.2 5) unset feed:babe::1:1 6) unset feed:babe::2:1 7) unset feed:babe::3:1 8) feed:babe::4:1 feed:babe::4:2 No idea how this looks for decnet and ipx. Looking only at IPv6 though, the patch's check for !AF_INET before setting rta_tb[IFA_ADDRESS] = rta_tb[IFA_LOCAL] could indeed be skipped. On a side note, I'm pretty sure the later memcmp() could be skipped in many cases, at least by comparing the pointer values of rta_tb[IFA_ADDRESS] and rta_tb[IFA_LOCAL]. Cheers, Phil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-11-24 at 12:44 +0100, Phil Sutter wrote: > On Mon, Nov 23, 2015 at 04:04:50PM -0800, Stephen Hemminger wrote: > > On Thu, 22 Oct 2015 10:34:28 +0200 > > Thomas Haller <thaller@redhat.com> wrote: > > > > > Kernel allows for zero IPv4 peer addresses (IFA_ADDRESS): > > > > > > ip address add 192.168.5.1 peer 0.0.0.0/24 dev dummy > > > > > > which is distinct from a usual address like: > > > > > > ip address add 192.168.5.1/24 dev dummy > > > ip address add 192.168.5.1 peer 192.168.5.1/24 dev dummy > > > > > > For IPv4, a missing IFA_ADDRESS attribute means that the peer > > > is 0.0.0.0. See inet_fill_ifaddr(), which does: > > > > > > if ((ifa->ifa_address && > > > nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) || > > > > > > Signed-off-by: Thomas Haller <thaller@redhat.com> > > > > I would prefer that this apply to both IPv4 and IPv6. > > The case that patch handles does not happen in IPv6. > > > If the kernel sends back an address, then display it. > > It's rather "if the kernel *does not* send back an address ...". > > When reviewing this patch, I tried to find an easier (and less ugly) > solution, but failed. Here's the result from testing all variants: Thank you Phil for evaluating this. I also tried to change the patch to come up with a cleaner/unified way, but the result wasn't subjectively better. Thomas For reference, the relevant lines of code from kernel: ipv4: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ipv4/devinet.c?id=6a13feb9c82803e2b815eca72fa7a9f5561d7861#n1542 if ((ifa->ifa_address && nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) || (ifa->ifa_local && nla_put_in_addr(skb, IFA_LOCAL, ifa->ifa_local)) || ipv6: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ipv6/addrconf.c?id=6a13feb9c82803e2b815eca72fa7a9f5561d7861#n4310 if (!ipv6_addr_any(&ifa->peer_addr)) { if (nla_put_in6_addr(skb, IFA_LOCAL, &ifa->addr) < 0 || nla_put_in6_addr(skb, IFA_ADDRESS, &ifa->peer_addr) < 0) goto error; } else if (nla_put_in6_addr(skb, IFA_ADDRESS, &ifa->addr) < 0) goto error;
diff --git a/ip/ipaddress.c b/ip/ipaddress.c index f290205..6fc4520 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -949,7 +949,8 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n, if (!rta_tb[IFA_LOCAL]) rta_tb[IFA_LOCAL] = rta_tb[IFA_ADDRESS]; - if (!rta_tb[IFA_ADDRESS]) + if (!rta_tb[IFA_ADDRESS] && + ifa->ifa_family != AF_INET) rta_tb[IFA_ADDRESS] = rta_tb[IFA_LOCAL]; if (filter.ifindex && filter.ifindex != ifa->ifa_index) @@ -1034,16 +1035,18 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n, RTA_DATA(rta_tb[IFA_LOCAL]), abuf, sizeof(abuf))); - if (rta_tb[IFA_ADDRESS] == NULL || + if (rta_tb[IFA_ADDRESS] != NULL && memcmp(RTA_DATA(rta_tb[IFA_ADDRESS]), RTA_DATA(rta_tb[IFA_LOCAL]), ifa->ifa_family == AF_INET ? 4 : 16) == 0) { fprintf(fp, "/%d ", ifa->ifa_prefixlen); } else { fprintf(fp, " peer %s/%d ", - format_host(ifa->ifa_family, - RTA_PAYLOAD(rta_tb[IFA_ADDRESS]), - RTA_DATA(rta_tb[IFA_ADDRESS]), - abuf, sizeof(abuf)), + rta_tb[IFA_ADDRESS] + ? format_host(ifa->ifa_family, + RTA_PAYLOAD(rta_tb[IFA_ADDRESS]), + RTA_DATA(rta_tb[IFA_ADDRESS]), + abuf, sizeof(abuf)) + : "0.0.0.0", ifa->ifa_prefixlen); } }
Kernel allows for zero IPv4 peer addresses (IFA_ADDRESS): ip address add 192.168.5.1 peer 0.0.0.0/24 dev dummy which is distinct from a usual address like: ip address add 192.168.5.1/24 dev dummy ip address add 192.168.5.1 peer 192.168.5.1/24 dev dummy For IPv4, a missing IFA_ADDRESS attribute means that the peer is 0.0.0.0. See inet_fill_ifaddr(), which does: if ((ifa->ifa_address && nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) || Signed-off-by: Thomas Haller <thaller@redhat.com> --- ip/ipaddress.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)