mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [PATCHv2] properly handle point-to-point interfaces in getifaddrs()
Date: Sat, 21 Nov 2015 14:20:35 -0500	[thread overview]
Message-ID: <20151121192035.GC3818@brightrain.aerifal.cx> (raw)
In-Reply-To: <20151121111446.6d23921c@vostro>

On Sat, Nov 21, 2015 at 11:14:46AM +0200, Timo Teras wrote:
> On Thu, 19 Nov 2015 21:43:10 +0100
> Jo-Philipp Wich <jow@openwrt.org> wrote:
> 
> > With point-to-point interfaces, the IFA_ADDRESS netlink attribute
> > contains the peer address while an extra attribute IFA_LOCAL carries
> > the actual local interface address.
> > 
> > Both the glibc and uclibc implementations of getifaddrs() handle this
> > case by moving the ifa_addr contents to the broadcast/remote address
> > union and overwriting ifa_addr upon receipt of an IFA_LOCAL attribute.
> > 
> > This patch adds the same special treatment logic of IFA_LOCAL to
> > musl's implementation of getifaddrs() in order to align its behaviour
> > with that of uclibc and musl.
> > 
> > Signed-off-by: Jo-Philipp Wich <jow@openwrt.org>
> > ---
> > Changelog v2:
> >  * Handle IFA_LOCAL, IFA_ADDRESS in arbritary order
> >  * Remove misleading comment for IFA_BROADCAST, no such attribute on
> > ptp links ---
> >  src/network/getifaddrs.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> I wrote the code before looking into how ptp links are reported, and
> just assumed it'd be somehow consistent. But IFA_ADDRESS indeed is
> the peer for ptp links. How nicely inconsistent from kernel side ;)
> 
> Seems iproute2 basically does:
>  1. If IFA_LOCAL not set, copy IFA_ADDRESS to it
>  2. If IFA_ADDRESS is not set, copy IFA_LOCAL to it
>  3. Print IFA_LOCAL as local address
>  4. Print IFA_ADDRESS as peer address if it's not equal to IFA_LOCAL
> 
> So this looks right to me.

Are you sure? The new patch seems to have exactly the same issue with
depending on the order of the records as the old patch had. To solve
it I think both need to be stored in temp storage during the loop,
then code after the loop has to resolve which to use. Am I missing
something?

Rich


  reply	other threads:[~2015-11-21 19:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-17 10:33 Fix handling of peer-to-peer " Jo-Philipp Wich
2015-11-17 10:33 ` [PATCH] properly handle point-to-point " Jo-Philipp Wich
2015-11-19 20:43   ` [PATCHv2] " Jo-Philipp Wich
2015-11-21  9:14     ` Timo Teras
2015-11-21 19:20       ` Rich Felker [this message]
2015-11-23  9:12         ` Timo Teras
2015-11-23 17:01           ` Rich Felker
2015-11-30 20:01     ` Rich Felker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151121192035.GC3818@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

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).