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 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html