* [PATCH iproute2] ip-address: properly display zero IPv4 peer address
@ 2015-10-22 8:34 Thomas Haller
2015-11-11 11:34 ` Phil Sutter
2015-11-24 0:04 ` Stephen Hemminger
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Haller @ 2015-10-22 8:34 UTC (permalink / raw)
To: netdev; +Cc: Thomas Haller
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(-)
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);
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH iproute2] ip-address: properly display zero IPv4 peer address
2015-10-22 8:34 [PATCH iproute2] ip-address: properly display zero IPv4 peer address Thomas Haller
@ 2015-11-11 11:34 ` Phil Sutter
2015-11-24 0:04 ` Stephen Hemminger
1 sibling, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2015-11-11 11:34 UTC (permalink / raw)
To: Thomas Haller; +Cc: netdev, Stephen Hemminger
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2] ip-address: properly display zero IPv4 peer address
2015-10-22 8:34 [PATCH iproute2] ip-address: properly display zero IPv4 peer address Thomas Haller
2015-11-11 11:34 ` Phil Sutter
@ 2015-11-24 0:04 ` Stephen Hemminger
2015-11-24 11:44 ` Phil Sutter
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2015-11-24 0:04 UTC (permalink / raw)
To: Thomas Haller; +Cc: netdev
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2] ip-address: properly display zero IPv4 peer address
2015-11-24 0:04 ` Stephen Hemminger
@ 2015-11-24 11:44 ` Phil Sutter
2015-11-24 12:14 ` Thomas Haller
0 siblings, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2015-11-24 11:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Thomas Haller, netdev
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2] ip-address: properly display zero IPv4 peer address
2015-11-24 11:44 ` Phil Sutter
@ 2015-11-24 12:14 ` Thomas Haller
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Haller @ 2015-11-24 12:14 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 2547 bytes --]
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;
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-11-24 12:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-22 8:34 [PATCH iproute2] ip-address: properly display zero IPv4 peer address Thomas Haller
2015-11-11 11:34 ` Phil Sutter
2015-11-24 0:04 ` Stephen Hemminger
2015-11-24 11:44 ` Phil Sutter
2015-11-24 12:14 ` Thomas Haller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).