getifaddrs(3) returns 2 AF_LINK instances for the same interface

[Roy Marples]

This happens in NetBSD-5 -> -current.
NetBSD-4 and FreeBSD-7 aren't affected.

#include <net/if.h>
#include <ifaddrs.h>
#include <stdio.h>
#include <stdlib.h>

int main(void)
{
struct ifaddrs *ifaddrs, *ifa;

if (getifaddrs(&ifaddrs) == -1)
exit(EXIT_FAILURE);

for (ifa = ifaddrs; ifa; ifa = ifa->ifa_next) {
if (ifa->ifa_addr->sa_family != AF_LINK)
continue;
printf ("%s %p\n", ifa->ifa_name, ifa);
}
freeifaddrs(ifaddrs);
exit(0);
}

uberlaptop$ ./ifaddrs
bge0 0xbb902000
bge0 0xbb90201c
fwip0 0xbb902070
fwip0 0xbb90208c
iwi0 0xbb9020c4
iwi0 0xbb9020e0
lo0 0xbb902134
lo0 0xbb902150

Is this a bug or do I need to check something else?

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Martin Husemann]

Post by Roy Marples
Is this a bug or do I need to check something else?

At least the copies are mostly identical ;-)
The only difference is the netmask pointer, fist copy has it as NULL, the
second copy points to a all zero netmask.

I'd suspect the libc code...

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Roy Marples]

Post by Martin Husemann

Post by Roy Marples
Is this a bug or do I need to check something else?

At least the copies are mostly identical ;-)
The only difference is the netmask pointer, fist copy has it as NULL, the
second copy points to a all zero netmask.

Good catch!
Makes my work around a little easier.

Post by Martin Husemann
I'd suspect the libc code...

The libc code for getifaddrs.c hasn't changed much since 4.0, and not
for causing this error.

I'm guessing it's something in the kernel as it gets info from sysctl...

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Roy Marples]

Post by Roy Marples

Post by Martin Husemann
I'd suspect the libc code...

The libc code for getifaddrs.c hasn't changed much since 4.0, and not
for causing this error.
I'm guessing it's something in the kernel as it gets info from sysctl...

Yes, a simple off by one error, caused by r1.96 by dyoung

Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.125
diff -u -p -r1.125 rtsock.c
--- sys/net/rtsock.c 2 Apr 2009 21:02:06 -0000 1.125
+++ sys/net/rtsock.c 17 Apr 2009 19:00:02 -0000
@@ -1060,7 +1060,12 @@ sysctl_iflist(int af, struct rt_walkarg
panic("sysctl_iflist(2)");
}
}
- IFADDR_FOREACH(ifa, ifp) {
+ /* We can't use IFADDR_FOREACH because the first address
+ is reported above */
+ for (ifa = IFADDR_NEXT(IFADDR_FIRST(ifp));
+ ifa != NULL;
+ ifa = IFADDR_NEXT(ifa))
+ {
if (af && af != ifa->ifa_addr->sa_family)
continue;
info.rti_info[RTAX_IFA] = ifa->ifa_addr;

OK to commit?

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[David Young]

Post by Roy Marples

Post by Roy Marples

Post by Martin Husemann
I'd suspect the libc code...

The libc code for getifaddrs.c hasn't changed much since 4.0, and not
for causing this error.
I'm guessing it's something in the kernel as it gets info from sysctl...

Yes, a simple off by one error, caused by r1.96 by dyoung

I think that the kernel is doing the right thing.

In your patch, maybe you have equated IFADDR_FIRST(ifp) with ifp->if_dl?
That hasn't been true for a long time.

Post by Roy Marples
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.125
diff -u -p -r1.125 rtsock.c
--- sys/net/rtsock.c 2 Apr 2009 21:02:06 -0000 1.125
+++ sys/net/rtsock.c 17 Apr 2009 19:00:02 -0000
@@ -1060,7 +1060,12 @@ sysctl_iflist(int af, struct rt_walkarg
panic("sysctl_iflist(2)");
}
}
- IFADDR_FOREACH(ifa, ifp) {
+ /* We can't use IFADDR_FOREACH because the first address
+ is reported above */
+ for (ifa = IFADDR_NEXT(IFADDR_FIRST(ifp));
+ ifa != NULL;
+ ifa = IFADDR_NEXT(ifa))
+ {
if (af && af != ifa->ifa_addr->sa_family)
continue;
info.rti_info[RTAX_IFA] = ifa->ifa_addr;

Dave

[Roy Marples]

Post by David Young
I think that the kernel is doing the right thing.
In your patch, maybe you have equated IFADDR_FIRST(ifp) with ifp->if_dl?
That hasn't been true for a long time.

So something like this?

Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.125
diff -u -p -r1.125 rtsock.c
--- sys/net/rtsock.c 2 Apr 2009 21:02:06 -0000 1.125
+++ sys/net/rtsock.c 17 Apr 2009 19:51:33 -0000
@@ -1061,7 +1061,8 @@ sysctl_iflist(int af, struct rt_walkarg
}
}
IFADDR_FOREACH(ifa, ifp) {
- if (af && af != ifa->ifa_addr->sa_family)
+ if (ifa == ifp->if_dl ||
+ (af && af != ifa->ifa_addr->sa_family))
continue;
info.rti_info[RTAX_IFA] = ifa->ifa_addr;
info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[David Young]

Post by Roy Marples

Post by David Young
I think that the kernel is doing the right thing.
In your patch, maybe you have equated IFADDR_FIRST(ifp) with ifp->if_dl?
That hasn't been true for a long time.

So something like this?
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.125
diff -u -p -r1.125 rtsock.c
--- sys/net/rtsock.c 2 Apr 2009 21:02:06 -0000 1.125
+++ sys/net/rtsock.c 17 Apr 2009 19:51:33 -0000
@@ -1061,7 +1061,8 @@ sysctl_iflist(int af, struct rt_walkarg
}
}
IFADDR_FOREACH(ifa, ifp) {
- if (af && af != ifa->ifa_addr->sa_family)
+ if (ifa == ifp->if_dl ||
+ (af && af != ifa->ifa_addr->sa_family))
continue;
info.rti_info[RTAX_IFA] = ifa->ifa_addr;
info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;

The kernel copies out the interface information in an RTM_IFINFO
message, then each of the addresses in an RTM_NEWADDR message. There
is overlap between the _IFINFO and _NEWADDR information, but it's not
a *total* overlap, so you really do have to copy out one _NEWADDR per
ifaddr.

Have another look at the doco and source for getifaddrs(3). I think
that if there is a bug, you will find it there.

Dave

[Roy Marples]

Post by David Young

Post by Roy Marples
So something like this?
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.125
diff -u -p -r1.125 rtsock.c
--- sys/net/rtsock.c 2 Apr 2009 21:02:06 -0000 1.125
+++ sys/net/rtsock.c 17 Apr 2009 19:51:33 -0000
@@ -1061,7 +1061,8 @@ sysctl_iflist(int af, struct rt_walkarg
}
}
IFADDR_FOREACH(ifa, ifp) {
- if (af && af != ifa->ifa_addr->sa_family)
+ if (ifa == ifp->if_dl ||
+ (af && af != ifa->ifa_addr->sa_family))
continue;
info.rti_info[RTAX_IFA] = ifa->ifa_addr;
info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;

The kernel copies out the interface information in an RTM_IFINFO
message, then each of the addresses in an RTM_NEWADDR message. There
is overlap between the _IFINFO and _NEWADDR information, but it's not
a *total* overlap, so you really do have to copy out one _NEWADDR per
ifaddr.

So what info is present in _NEWADDR that isn't in _IFINFO for ifp->if_dl?
How is userland supposed to pick the right AF_LINK or are they both right?

Post by David Young
Have another look at the doco and source for getifaddrs(3). I think
that if there is a bug, you will find it there.

I have checked and the last change of note was over 7 years ago.
So are you suggesting that getifaddrs(3) should ignore the _IFINFO
message or somehow merge it into the _NEWADDR one - if it's present?

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[David Young]

Post by Roy Marples

Post by David Young

Post by Roy Marples
So something like this?
Index: sys/net/rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.125
diff -u -p -r1.125 rtsock.c
--- sys/net/rtsock.c 2 Apr 2009 21:02:06 -0000 1.125
+++ sys/net/rtsock.c 17 Apr 2009 19:51:33 -0000
@@ -1061,7 +1061,8 @@ sysctl_iflist(int af, struct rt_walkarg
}
}
IFADDR_FOREACH(ifa, ifp) {
- if (af && af != ifa->ifa_addr->sa_family)
+ if (ifa == ifp->if_dl ||
+ (af && af != ifa->ifa_addr->sa_family))
continue;
info.rti_info[RTAX_IFA] = ifa->ifa_addr;
info.rti_info[RTAX_NETMASK] = ifa->ifa_netmask;

The kernel copies out the interface information in an RTM_IFINFO
message, then each of the addresses in an RTM_NEWADDR message. There
is overlap between the _IFINFO and _NEWADDR information, but it's not
a *total* overlap, so you really do have to copy out one _NEWADDR per
ifaddr.

So what info is present in _NEWADDR that isn't in _IFINFO for ifp->if_dl?

I don't know. You will have to find out. IIRC, one of the AF_LINK
records produced by getifaddrs(3) has the wrong netmask (should be
all-1s), and the other has the struct if_data filled out. I don't know
remember what other differences there are.

Post by Roy Marples
How is userland supposed to pick the right AF_LINK or are they both right?

I guess it depends on your purposes which is right. There may be more
than two AF_LINK ifaddrs on an interface, BTW.

Post by Roy Marples

Post by David Young
Have another look at the doco and source for getifaddrs(3). I think
that if there is a bug, you will find it there.

I have checked and the last change of note was over 7 years ago.
So are you suggesting that getifaddrs(3) should ignore the _IFINFO
message or somehow merge it into the _NEWADDR one - if it's present?

Based on my recollections, above, my guess is that getifaddrs(3) should
merge the messages.

Dave

[Roy Marples]

Post by David Young
I don't know. You will have to find out. IIRC, one of the AF_LINK
records produced by getifaddrs(3) has the wrong netmask (should be
all-1s), and the other has the struct if_data filled out. I don't know
remember what other differences there are.

Do you mean the AF_LINK with the netmask is wrong?

Post by David Young

Post by Roy Marples
How is userland supposed to pick the right AF_LINK or are they both right?

I guess it depends on your purposes which is right. There may be more
than two AF_LINK ifaddrs on an interface, BTW.

You mean two different if_data structures?
In simple terms, >1 hardware address? Interesting. So I need to check
for dups regardless of this outcome then.

Post by David Young

Post by Roy Marples

Post by David Young
Have another look at the doco and source for getifaddrs(3). I think
that if there is a bug, you will find it there.

I have checked and the last change of note was over 7 years ago.
So are you suggesting that getifaddrs(3) should ignore the _IFINFO
message or somehow merge it into the _NEWADDR one - if it's present?

Based on my recollections, above, my guess is that getifaddrs(3) should
merge the messages.

If we can have different AF_LINK addrs on the same interface then I how
would I know which AF_LINK addr was in _IFINFO so I could merge in the
correct data?

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Joerg Sonnenberger]

Post by David Young

Post by Roy Marples
I have checked and the last change of note was over 7 years ago.
So are you suggesting that getifaddrs(3) should ignore the _IFINFO
message or somehow merge it into the _NEWADDR one - if it's present?

Based on my recollections, above, my guess is that getifaddrs(3) should
merge the messages.

Doesn't that break the interface for old libc with new kernel?

Joerg

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[David Young]

Post by Roy Marples

Post by David Young
I don't know. You will have to find out. IIRC, one of the AF_LINK
records produced by getifaddrs(3) has the wrong netmask (should be
all-1s), and the other has the struct if_data filled out. I don't know
remember what other differences there are.

Do you mean the AF_LINK with the netmask is wrong?

The AF_LINK with the right netmask is right.
I think that I mis-spoke when I said that it should be all-1s.

Post by Roy Marples

Post by David Young

Post by Roy Marples
How is userland supposed to pick the right AF_LINK or are they both right?

I guess it depends on your purposes which is right. There may be more
than two AF_LINK ifaddrs on an interface, BTW.

You mean two different if_data structures?
In simple terms, >1 hardware address? Interesting. So I need to check
for dups regardless of this outcome then.

It depends on your particular application what is a duplicate, or
whether there is a duplicate at all.

Post by Roy Marples

Post by David Young

Post by Roy Marples

Post by David Young
Have another look at the doco and source for getifaddrs(3). I think
that if there is a bug, you will find it there.

I have checked and the last change of note was over 7 years ago.
So are you suggesting that getifaddrs(3) should ignore the _IFINFO
message or somehow merge it into the _NEWADDR one - if it's present?

Based on my recollections, above, my guess is that getifaddrs(3) should
merge the messages.

If we can have different AF_LINK addrs on the same interface then I how
would I know which AF_LINK addr was in _IFINFO so I could merge in the
correct data?

Compare the address in the _IFINFO message with the _NEWADDR message.

If you tell me what you're trying to do, I may be able to help you
better. :-)

Dave

[David Young]

Post by Joerg Sonnenberger

Post by David Young

Post by Roy Marples
I have checked and the last change of note was over 7 years ago.
So are you suggesting that getifaddrs(3) should ignore the _IFINFO
message or somehow merge it into the _NEWADDR one - if it's present?

Based on my recollections, above, my guess is that getifaddrs(3) should
merge the messages.

Doesn't that break the interface for old libc with new kernel?

Do you mean, does old getifaddrs(3) do the right thing with the messages
in the new kernel interface? The messages are the same in form,
but different in number, so I reckon it depends on an application's
expectations more than anything else.

Dave

[Joerg Sonnenberger]

Post by David Young

Post by Joerg Sonnenberger

Post by David Young

Post by Roy Marples
I have checked and the last change of note was over 7 years ago.
So are you suggesting that getifaddrs(3) should ignore the _IFINFO
message or somehow merge it into the _NEWADDR one - if it's present?

Based on my recollections, above, my guess is that getifaddrs(3) should
merge the messages.

Doesn't that break the interface for old libc with new kernel?

Do you mean, does old getifaddrs(3) do the right thing with the messages
in the new kernel interface? The messages are the same in form,
but different in number, so I reckon it depends on an application's
expectations more than anything else.

I mean if the answer is "getifaddrs need to change" to accomodate for
the kernel changes, that does change the visible result for older
programs, doesn't it? I don't know how many users (if at all) actually
care about AF_LINK, but it is something to keep in mind for this
discussion.

Joerg

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Roy Marples]

Post by Joerg Sonnenberger

Post by David Young
Do you mean, does old getifaddrs(3) do the right thing with the messages
in the new kernel interface? The messages are the same in form,
but different in number, so I reckon it depends on an application's
expectations more than anything else.

I mean if the answer is "getifaddrs need to change" to accomodate for
the kernel changes, that does change the visible result for older
programs, doesn't it? I don't know how many users (if at all) actually
care about AF_LINK, but it is something to keep in mind for this
discussion.

I would argue that regardless of the outcome of this discussion,
getifaddrs(3) needs to report each address once for NetBSD-5.
An analogy would be readdir(3) reporting "." twice.

Thanks

Roy

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de