From: Ville Nuorvala <vnuorval@tcs•hut.fi>
To: Sridhar Samudrala <sri@us•ibm.com>
Cc: "David S. Miller" <davem@davemloft•net>,
YOSHIFUJI Hideaki <yoshfuji@linux-ipv6•org>,
Thomas Graf <tgraf@suug•ch>,
kim.nordlund@nokia•com, lksctp-developers@lists•sourceforge.net,
netdev@vger•kernel.org
Subject: Re: [PATCH 9/13] [SCTP] Merge IPv4 and IPv6 versions of get_saddr() with their corresponding get_dst().
Date: Thu, 02 Nov 2006 14:32:34 +0200 [thread overview]
Message-ID: <4549E562.8030704@tcs.hut.fi> (raw)
In-Reply-To: <1161971255.5820.13.camel@w-sridhar2.beaverton.ibm.com>
On 10/27/06 20:47, Sridhar Samudrala wrote:
> On Tue, 2006-10-17 at 03:19 +0300, Ville Nuorvala wrote:
>> As the IPv6 route lookup now also returns the selected source address
>> there is no need for a separate source address lookup. In fact, the
>> source address selection needs to be moved to get_dst() because the
>> selected IPv6 source address isn't always stored in the route.
>> Sometimes this makes it impossible to guess the correct address later on.
>>
>
> Ville,
>
> Overall the patch looks pretty good. I found only 1 issue in
> sctp_v6_get_dst(). See below.
>
>
> <snip>
>
>
>> +/* Returns the dst cache entry for the given source and destination ip
>> + * addresses.
>> + */
>> +static struct dst_entry *sctp_v6_get_dst(struct sctp_association *asoc,
>> + union sctp_addr *daddr,
>> + union sctp_addr *saddr)
>> +{
>> + struct dst_entry *dst;
>> + struct flowi fl;
>> + struct sctp_bind_addr *bp;
>> + rwlock_t *addr_lock;
>> + struct sctp_sockaddr_entry *laddr;
>> + struct list_head *pos;
>> + struct rt6_info *rt;
>> + union sctp_addr baddr;
>> + sctp_scope_t scope;
>> + __u8 matchlen = 0;
>> + __u8 bmatchlen;
>> +
>> + memset(&fl, 0, sizeof(fl));
>> + ipv6_addr_copy(&fl.fl6_dst, &daddr->v6.sin6_addr);
>> + if (ipv6_addr_type(&daddr->v6.sin6_addr) & IPV6_ADDR_LINKLOCAL)
>> + fl.oif = daddr->v6.sin6_scope_id;
>> +
>> + ipv6_addr_copy(&fl.fl6_src, &saddr->v6.sin6_addr);
>> + SCTP_DEBUG_PRINTK("%s: DST=" NIP6_FMT " SRC=" NIP6_FMT " ",
>> + __FUNCTION__, NIP6(fl.fl6_dst), NIP6(fl.fl6_src));
>> +
>> + dst = ip6_route_output(NULL, &fl);
>> + if (dst->error) {
>> + dst_release(dst);
>> + dst = NULL;
>> + }
>> + if (!ipv6_addr_any(&saddr->v6.sin6_addr))
>> + goto out;
>> + if (!asoc) {
>> + if (dst)
>> + ipv6_addr_copy(&saddr->v6.sin6_addr, &fl.fl6_src);
>> + goto out;
>> + }
>> + bp = &asoc->base.bind_addr;
>> + addr_lock = &asoc->base.addr_lock;
>> +
>> + if (dst) {
>> + /* Walk through the bind address list and look for a bind
>> + * address that matches the source address of the returned rt.
>> + */
>> + sctp_v6_fl_saddr(&baddr, &fl, bp->port);
> Here we are checking if the source address returned in the dst matches one of
> the address in the bind address list of the association. Not the source address
> that is passed to this routine(it could be INADDRY_ANY).
> So this should be changed back to sctp_v6_dst_saddr().
No, see that's the problem. The source address isn't always stored in
the rt6_info. Therefore I copy the address into the flowi, so after
ip6_route_output() fl does indeed contain the selected source address.
Sorry if I didn't cc you all relevant patches from the patch set.
Anyway there are still some unresolved issues with my code, but it's
nice to know I didn't at least mess up SCTP in a big way ;-)
Thanks,
Ville
next prev parent reply other threads:[~2006-11-02 12:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-10-17 0:19 [PATCH 9/13] [SCTP] Merge IPv4 and IPv6 versions of get_saddr() with their corresponding get_dst() Ville Nuorvala
2006-10-17 0:31 ` [PATCH 9/13] [RFC] " Ville Nuorvala
2006-10-17 22:42 ` [PATCH 9/13] " Sridhar Samudrala
2006-10-27 17:47 ` Sridhar Samudrala
2006-11-02 12:32 ` Ville Nuorvala [this message]
2006-11-02 18:07 ` Sridhar Samudrala
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=4549E562.8030704@tcs.hut.fi \
--to=vnuorval@tcs$(echo .)hut.fi \
--cc=davem@davemloft$(echo .)net \
--cc=kim.nordlund@nokia$(echo .)com \
--cc=lksctp-developers@lists$(echo .)sourceforge.net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sri@us$(echo .)ibm.com \
--cc=tgraf@suug$(echo .)ch \
--cc=yoshfuji@linux-ipv6$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox