Message ID | 4DB0FD45.2050709@cn.fujitsu.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
> > Since the sender MUST NOT use the new IP address as a source for ANY SCTP > packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. > How about this change. If so, you do not need change to sctp_outq_tail(); > > diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c > index 1c88c89..bd6cc9c 100644 > --- a/net/sctp/outqueue.c > +++ b/net/sctp/outqueue.c > @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) > */ > > list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { > + /* RFC 5061, 5.3 > + * F1) This ... > + */ > + if (q->asoc->src_out_of_asoc_ok && > + chunk->chunk_hdr->type != SCTP_CID_ASCONF) SCTP_CID_ASCONF_ACK should be also allowed, the peer may send ASCONF to do the same thing at the same time. > + continue; > + > list_del_init(&chunk->list); > > /* Pick the right transport to use. */ > @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) > } > } > > + if (q->asoc->src_out_of_asoc_ok) > + goto sctp_flush_out; > + > /* Is it OK to send data chunks? */ > switch (asoc->state) { > case SCTP_STATE_COOKIE_ECHOED: > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wei, thanks for your feedback, I'd like to ask about some points that I have a bit of hesitations before fix. Please see in-line. >> >> * ADDIP 3.1.1 Address Configuration Change Chunk (ASCONF) >> * 0 1 2 3 >> @@ -2744,11 +2799,24 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc, >> int addr_param_len = 0; >> int totallen = 0; >> int i; >> + sctp_addip_param_t del_param; /* 8 Bytes (Type 0xC002, Len and CrrID) */ >> + struct sctp_af *del_af; >> + int del_addr_param_len = 0; >> + int del_paramlen = sizeof(sctp_addip_param_t); >> + union sctp_addr_param del_addr_param; /* (v4) 8 Bytes, (v6) 20 Bytes */ >> + int v4 = 0; >> + int v6 = 0; > > you can reuse the af, param_len etc, no need to define new variables. This is for the situation that the application adds a new IPv4 and a new IPv6 address simultaneously, although it never happen in the auto_asconf process. Since the af is overwritten to the last seen address in addrs, defining these variables is useful. (Extracting existence of v4 and v6 parameters is possible, but I think it's overkill. ) So, how about remaining them? >> >> @@ -3361,6 +3486,35 @@ int sctp_process_asconf_ack(struct sctp_association *asoc, >> asconf_len -= length; >> } >> >> + /* When the source address obviously changes to newly added one, we >> + reset the cwnd to re-probe the path condition > > Since we do not do this when the host/peer add/del ip addresses, > remain the peer's cwnd etc. to what it is maybe better. What do you mean "host/peer add/del ip addresses" ? > and this is unnecessary when you just change the address of > the interface. Yes, however, it is mostly impossible to distinguish that situation from change of the physical path. So considering change of source address as change of path could make sense or fine-grain metric to reset congestion control parameter. As one example, in FreeBSD implementation, congestion control parameter is reset when the last remaining address is changed. >> >> @@ -599,6 +597,28 @@ static int sctp_send_asconf_add_ip(struct sock *sk, >> SCTP_ADDR_NEW, GFP_ATOMIC); >> addr_buf += af->sockaddr_len; >> } >> + list_for_each_entry(trans, &asoc->peer.transport_addr_list, >> + transports) { >> + if (asoc->asconf_addr_del_pending != NULL) >> + /* This ADDIP ASCONF piggybacks DELIP for the >> + * last address, so need to select src addr >> + * from the out_of_asoc addrs >> + */ >> + asoc->src_out_of_asoc_ok = 1; >> + /* Clear the source and route cache in the path */ >> + memset(&trans->saddr, 0, sizeof(union sctp_addr)); >> + dst_release(trans->dst); >> + trans->cwnd = min(4*asoc->pathmtu, max_t(__u32, >> + 2*asoc->pathmtu, 4380)); >> + trans->ssthresh = asoc->peer.i.a_rwnd; >> + trans->rto = asoc->rto_initial; >> + trans->rtt = 0; >> + trans->srtt = 0; >> + trans->rttvar = 0; >> + sctp_transport_route(trans, NULL, >> + sctp_sk(asoc->base.sk)); >> + } > > We should and have done update the route after the ASCONF > be ACKed. So it is unnecessary. ASCONF is also set the retransmission timer. Adopting the old RTO value and route cache for this ASCONF doesn't make sense, because it traverses different path. Thanks, - Michio-- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Apr 22, 2011, at 13:10 , Wei Yongjun wrote: > >> >> Since the sender MUST NOT use the new IP address as a source for ANY SCTP >> packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. >> How about this change. If so, you do not need change to sctp_outq_tail(); >> >> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >> index 1c88c89..bd6cc9c 100644 >> --- a/net/sctp/outqueue.c >> +++ b/net/sctp/outqueue.c >> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >> */ >> >> list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { >> + /* RFC 5061, 5.3 >> + * F1) This ... >> + */ >> + if (q->asoc->src_out_of_asoc_ok && >> + chunk->chunk_hdr->type != SCTP_CID_ASCONF) > > SCTP_CID_ASCONF_ACK should be also allowed, the peer may > send ASCONF to do the same thing at the same time. Sorry for my bad understanding, Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"? Or do you mean following situation? 1. the pear sends ADD/DEL ASCONF to me, 2. I receive it, 3. I migrate to the other network and get new address, 4. I send ASCONF-ACK to the peer from the new address > >> + continue; >> + >> list_del_init(&chunk->list); >> >> /* Pick the right transport to use. */ >> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >> } >> } >> >> + if (q->asoc->src_out_of_asoc_ok) >> + goto sctp_flush_out; >> + >> /* Is it OK to send data chunks? */ >> switch (asoc->state) { >> case SCTP_STATE_COOKIE_ECHOED: >> >> >> > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Apr 22, 2011, at 13:10 , Wei Yongjun wrote: > >>> Since the sender MUST NOT use the new IP address as a source for ANY SCTP >>> packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. >>> How about this change. If so, you do not need change to sctp_outq_tail(); >>> >>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>> index 1c88c89..bd6cc9c 100644 >>> --- a/net/sctp/outqueue.c >>> +++ b/net/sctp/outqueue.c >>> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>> */ >>> >>> list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { >>> + /* RFC 5061, 5.3 >>> + * F1) This ... >>> + */ >>> + if (q->asoc->src_out_of_asoc_ok && >>> + chunk->chunk_hdr->type != SCTP_CID_ASCONF) >> SCTP_CID_ASCONF_ACK should be also allowed, the peer may >> send ASCONF to do the same thing at the same time. > Sorry for my bad understanding, > Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"? > Or do you mean following situation? > 1. the pear sends ADD/DEL ASCONF to me, > 2. I receive it, > 3. I migrate to the other network and get new address, > 4. I send ASCONF-ACK to the peer from the new address Yes, If both side send ADD/DEL ASCONF to del the last one address at the same time like this: ASCONF ----- ------ASCONF (ADD/DEL) \ / (ADD/DEL) \/ /\ <----/ \-----> ASCONF-ACK---\ /------ASCONF-ACK \/ /\ <----/ \-----> But I do not test for it. Not sure we need to do this, can you check this before commit your new patchset? >>> + continue; >>> + >>> list_del_init(&chunk->list); >>> >>> /* Pick the right transport to use. */ >>> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>> } >>> } >>> >>> + if (q->asoc->src_out_of_asoc_ok) >>> + goto sctp_flush_out; >>> + >>> /* Is it OK to send data chunks? */ >>> switch (asoc->state) { >>> case SCTP_STATE_COOKIE_ECHOED: >>> >>> >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Hi Wei, thanks for your feedback, I'd like to ask about some points that I have a bit of hesitations before fix. > Please see in-line. > >>> * ADDIP 3.1.1 Address Configuration Change Chunk (ASCONF) >>> * 0 1 2 3 >>> @@ -2744,11 +2799,24 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc, >>> int addr_param_len = 0; >>> int totallen = 0; >>> int i; >>> + sctp_addip_param_t del_param; /* 8 Bytes (Type 0xC002, Len and CrrID) */ >>> + struct sctp_af *del_af; >>> + int del_addr_param_len = 0; >>> + int del_paramlen = sizeof(sctp_addip_param_t); >>> + union sctp_addr_param del_addr_param; /* (v4) 8 Bytes, (v6) 20 Bytes */ >>> + int v4 = 0; >>> + int v6 = 0; >> you can reuse the af, param_len etc, no need to define new variables. > This is for the situation that the application adds a new IPv4 and a new IPv6 address simultaneously, although it never happen in the auto_asconf process. > Since the af is overwritten to the last seen address in addrs, defining these variables is useful. > (Extracting existence of v4 and v6 parameters is possible, but I think it's overkill. ) > So, how about remaining them? I means why your need this check? + if ((asoc->asconf_addr_del_pending->sa.sa_family == AF_INET + && v4) || + (asoc->asconf_addr_del_pending->sa.sa_family == AF_INET6 + && v6)) { Add IPv4 and Del IPv6 is possible if socket is AF_INET6 base. >>> @@ -3361,6 +3486,35 @@ int sctp_process_asconf_ack(struct sctp_association *asoc, >>> asconf_len -= length; >>> } >>> >>> + /* When the source address obviously changes to newly added one, we >>> + reset the cwnd to re-probe the path condition >> Since we do not do this when the host/peer add/del ip addresses, >> remain the peer's cwnd etc. to what it is maybe better. > What do you mean "host/peer add/del ip addresses" ? I means if we delete the other address other than the last one address, we do not reset the peer's information. >> and this is unnecessary when you just change the address of >> the interface. > Yes, however, it is mostly impossible to distinguish that situation from change of the physical path. > So considering change of source address as change of path could make sense or fine-grain metric to reset congestion control parameter. > As one example, in FreeBSD implementation, congestion control parameter is reset when the last remaining address is changed. I got what your said. Can you split the peer reset part to an new patch? This part: +void +sctp_path_check_and_react(struct sctp_association *asoc, struct sockaddr *sa) +{ + struct sctp_transport *trans; + int addrnum, family; + struct sctp_sockaddr_entry *saddr; + struct sctp_bind_addr *bp; + union sctp_addr *tmpaddr; + + family = sa->sa_family; + bp = &asoc->base.bind_addr; + addrnum = 0; + /* count up the number of local addresses in the same family */ + list_for_each_entry(saddr, &bp->address_list, list) { + if (saddr->a.sa.sa_family == family) { + tmpaddr = &saddr->a; + if (family == AF_INET6 && + ipv6_addr_type(&tmpaddr->v6.sin6_addr) & + IPV6_ADDR_LINKLOCAL) { + continue; + } + addrnum++; + } + } the address be added to the asoc should in the scope, refer to sctp_in_scope(). So we may not mix link local with global addresses. If so, we may simply this patch to reset peer only when we recv the asconf-ack and the asoc->src_out_of_asoc_ok == true. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Such operation would not be supported by specification, in Sec.5.3 in RFC 5061: F1) When adding an IP address to an association, the IP address is NOT considered fully added to the association until the ASCONF- ACK arrives. This means that until such time as the ASCONF containing the add is acknowledged, the sender MUST NOT use the new IP address as a source for ANY SCTP packet except on carrying an ASCONF Chunk. I think this means we cannot send ASCONF-ACK from the new address even if it bundles ASCONF... - Michio On Apr 25, 2011, at 9:57 , Wei Yongjun wrote: > >> On Apr 22, 2011, at 13:10 , Wei Yongjun wrote: >> >>>> Since the sender MUST NOT use the new IP address as a source for ANY SCTP >>>> packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. >>>> How about this change. If so, you do not need change to sctp_outq_tail(); >>>> >>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>>> index 1c88c89..bd6cc9c 100644 >>>> --- a/net/sctp/outqueue.c >>>> +++ b/net/sctp/outqueue.c >>>> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>> */ >>>> >>>> list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { >>>> + /* RFC 5061, 5.3 >>>> + * F1) This ... >>>> + */ >>>> + if (q->asoc->src_out_of_asoc_ok && >>>> + chunk->chunk_hdr->type != SCTP_CID_ASCONF) >>> SCTP_CID_ASCONF_ACK should be also allowed, the peer may >>> send ASCONF to do the same thing at the same time. >> Sorry for my bad understanding, >> Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"? >> Or do you mean following situation? >> 1. the pear sends ADD/DEL ASCONF to me, >> 2. I receive it, >> 3. I migrate to the other network and get new address, >> 4. I send ASCONF-ACK to the peer from the new address > > Yes, If both side send ADD/DEL ASCONF to del the last one > address at the same time like this: > > ASCONF ----- ------ASCONF > (ADD/DEL) \ / (ADD/DEL) > \/ > /\ > <----/ \-----> > ASCONF-ACK---\ /------ASCONF-ACK > \/ > /\ > <----/ \-----> > > But I do not test for it. Not sure we need to do this, can you > check this before commit your new patchset? > > >>>> + continue; >>>> + >>>> list_del_init(&chunk->list); >>>> >>>> /* Pick the right transport to use. */ >>>> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>> } >>>> } >>>> >>>> + if (q->asoc->src_out_of_asoc_ok) >>>> + goto sctp_flush_out; >>>> + >>>> /* Is it OK to send data chunks? */ >>>> switch (asoc->state) { >>>> case SCTP_STATE_COOKIE_ECHOED: >>>> >>>> >>>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Hi, > > Such operation would not be supported by specification, in Sec.5.3 in RFC 5061: > F1) When adding an IP address to an association, the IP address is > NOT considered fully added to the association until the ASCONF- > ACK arrives. This means that until such time as the ASCONF > containing the add is acknowledged, the sender MUST NOT use the > new IP address as a source for ANY SCTP packet except on > carrying an ASCONF Chunk. > > I think this means we cannot send ASCONF-ACK from the new address even if it bundles ASCONF... If so, both side do not have valid address to send the such ASCONF-ACK, and can not recv ASCONF-ACK. > - Michio > > On Apr 25, 2011, at 9:57 , Wei Yongjun wrote: > >>> On Apr 22, 2011, at 13:10 , Wei Yongjun wrote: >>> >>>>> Since the sender MUST NOT use the new IP address as a source for ANY SCTP >>>>> packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. >>>>> How about this change. If so, you do not need change to sctp_outq_tail(); >>>>> >>>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>>>> index 1c88c89..bd6cc9c 100644 >>>>> --- a/net/sctp/outqueue.c >>>>> +++ b/net/sctp/outqueue.c >>>>> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>> */ >>>>> >>>>> list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { >>>>> + /* RFC 5061, 5.3 >>>>> + * F1) This ... >>>>> + */ >>>>> + if (q->asoc->src_out_of_asoc_ok && >>>>> + chunk->chunk_hdr->type != SCTP_CID_ASCONF) >>>> SCTP_CID_ASCONF_ACK should be also allowed, the peer may >>>> send ASCONF to do the same thing at the same time. >>> Sorry for my bad understanding, >>> Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"? >>> Or do you mean following situation? >>> 1. the pear sends ADD/DEL ASCONF to me, >>> 2. I receive it, >>> 3. I migrate to the other network and get new address, >>> 4. I send ASCONF-ACK to the peer from the new address >> Yes, If both side send ADD/DEL ASCONF to del the last one >> address at the same time like this: >> >> ASCONF ----- ------ASCONF >> (ADD/DEL) \ / (ADD/DEL) >> \/ >> /\ >> <----/ \-----> >> ASCONF-ACK---\ /------ASCONF-ACK >> \/ >> /\ >> <----/ \-----> >> >> But I do not test for it. Not sure we need to do this, can you >> check this before commit your new patchset? >> >> >>>>> + continue; >>>>> + >>>>> list_del_init(&chunk->list); >>>>> >>>>> /* Pick the right transport to use. */ >>>>> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>> } >>>>> } >>>>> >>>>> + if (q->asoc->src_out_of_asoc_ok) >>>>> + goto sctp_flush_out; >>>>> + >>>>> /* Is it OK to send data chunks? */ >>>>> switch (asoc->state) { >>>>> case SCTP_STATE_COOKIE_ECHOED: >>>>> >>>>> >>>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> -- >>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes, I think the association cannot be kept, if the single-homed ASCONF receiver moves to the new network before sending ASCONF-ACK. Am I missing? Thanks, - Michio On Apr 25, 2011, at 11:02 , Wei Yongjun wrote: > >> Hi, >> >> Such operation would not be supported by specification, in Sec.5.3 in RFC 5061: >> F1) When adding an IP address to an association, the IP address is >> NOT considered fully added to the association until the ASCONF- >> ACK arrives. This means that until such time as the ASCONF >> containing the add is acknowledged, the sender MUST NOT use the >> new IP address as a source for ANY SCTP packet except on >> carrying an ASCONF Chunk. >> >> I think this means we cannot send ASCONF-ACK from the new address even if it bundles ASCONF... > > If so, both side do not have valid address to send the such > ASCONF-ACK, and can not recv ASCONF-ACK. > >> - Michio >> >> On Apr 25, 2011, at 9:57 , Wei Yongjun wrote: >> >>>> On Apr 22, 2011, at 13:10 , Wei Yongjun wrote: >>>> >>>>>> Since the sender MUST NOT use the new IP address as a source for ANY SCTP >>>>>> packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. >>>>>> How about this change. If so, you do not need change to sctp_outq_tail(); >>>>>> >>>>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>>>>> index 1c88c89..bd6cc9c 100644 >>>>>> --- a/net/sctp/outqueue.c >>>>>> +++ b/net/sctp/outqueue.c >>>>>> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>>> */ >>>>>> >>>>>> list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { >>>>>> + /* RFC 5061, 5.3 >>>>>> + * F1) This ... >>>>>> + */ >>>>>> + if (q->asoc->src_out_of_asoc_ok && >>>>>> + chunk->chunk_hdr->type != SCTP_CID_ASCONF) >>>>> SCTP_CID_ASCONF_ACK should be also allowed, the peer may >>>>> send ASCONF to do the same thing at the same time. >>>> Sorry for my bad understanding, >>>> Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"? >>>> Or do you mean following situation? >>>> 1. the pear sends ADD/DEL ASCONF to me, >>>> 2. I receive it, >>>> 3. I migrate to the other network and get new address, >>>> 4. I send ASCONF-ACK to the peer from the new address >>> Yes, If both side send ADD/DEL ASCONF to del the last one >>> address at the same time like this: >>> >>> ASCONF ----- ------ASCONF >>> (ADD/DEL) \ / (ADD/DEL) >>> \/ >>> /\ >>> <----/ \-----> >>> ASCONF-ACK---\ /------ASCONF-ACK >>> \/ >>> /\ >>> <----/ \-----> >>> >>> But I do not test for it. Not sure we need to do this, can you >>> check this before commit your new patchset? >>> >>> >>>>>> + continue; >>>>>> + >>>>>> list_del_init(&chunk->list); >>>>>> >>>>>> /* Pick the right transport to use. */ >>>>>> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>>> } >>>>>> } >>>>>> >>>>>> + if (q->asoc->src_out_of_asoc_ok) >>>>>> + goto sctp_flush_out; >>>>>> + >>>>>> /* Is it OK to send data chunks? */ >>>>>> switch (asoc->state) { >>>>>> case SCTP_STATE_COOKIE_ECHOED: >>>>>> >>>>>> >>>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Yes, I think the association cannot be kept, if the single-homed ASCONF receiver moves to the new network before sending ASCONF-ACK. > Am I missing? Oh, yeah, you are right.:-) > Thanks, > - Michio > > On Apr 25, 2011, at 11:02 , Wei Yongjun wrote: > >>> Hi, >>> >>> Such operation would not be supported by specification, in Sec.5.3 in RFC 5061: >>> F1) When adding an IP address to an association, the IP address is >>> NOT considered fully added to the association until the ASCONF- >>> ACK arrives. This means that until such time as the ASCONF >>> containing the add is acknowledged, the sender MUST NOT use the >>> new IP address as a source for ANY SCTP packet except on >>> carrying an ASCONF Chunk. >>> >>> I think this means we cannot send ASCONF-ACK from the new address even if it bundles ASCONF... >> If so, both side do not have valid address to send the such >> ASCONF-ACK, and can not recv ASCONF-ACK. >> >>> - Michio >>> >>> On Apr 25, 2011, at 9:57 , Wei Yongjun wrote: >>> >>>>> On Apr 22, 2011, at 13:10 , Wei Yongjun wrote: >>>>> >>>>>>> Since the sender MUST NOT use the new IP address as a source for ANY SCTP >>>>>>> packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. >>>>>>> How about this change. If so, you do not need change to sctp_outq_tail(); >>>>>>> >>>>>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>>>>>> index 1c88c89..bd6cc9c 100644 >>>>>>> --- a/net/sctp/outqueue.c >>>>>>> +++ b/net/sctp/outqueue.c >>>>>>> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>>>> */ >>>>>>> >>>>>>> list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { >>>>>>> + /* RFC 5061, 5.3 >>>>>>> + * F1) This ... >>>>>>> + */ >>>>>>> + if (q->asoc->src_out_of_asoc_ok && >>>>>>> + chunk->chunk_hdr->type != SCTP_CID_ASCONF) >>>>>> SCTP_CID_ASCONF_ACK should be also allowed, the peer may >>>>>> send ASCONF to do the same thing at the same time. >>>>> Sorry for my bad understanding, >>>>> Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"? >>>>> Or do you mean following situation? >>>>> 1. the pear sends ADD/DEL ASCONF to me, >>>>> 2. I receive it, >>>>> 3. I migrate to the other network and get new address, >>>>> 4. I send ASCONF-ACK to the peer from the new address >>>> Yes, If both side send ADD/DEL ASCONF to del the last one >>>> address at the same time like this: >>>> >>>> ASCONF ----- ------ASCONF >>>> (ADD/DEL) \ / (ADD/DEL) >>>> \/ >>>> /\ >>>> <----/ \-----> >>>> ASCONF-ACK---\ /------ASCONF-ACK >>>> \/ >>>> /\ >>>> <----/ \-----> >>>> >>>> But I do not test for it. Not sure we need to do this, can you >>>> check this before commit your new patchset? >>>> >>>> >>>>>>> + continue; >>>>>>> + >>>>>>> list_del_init(&chunk->list); >>>>>>> >>>>>>> /* Pick the right transport to use. */ >>>>>>> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>>>> } >>>>>>> } >>>>>>> >>>>>>> + if (q->asoc->src_out_of_asoc_ok) >>>>>>> + goto sctp_flush_out; >>>>>>> + >>>>>>> /* Is it OK to send data chunks? */ >>>>>>> switch (asoc->state) { >>>>>>> case SCTP_STATE_COOKIE_ECHOED: >>>>>>> >>>>>>> >>>>>>> >>>>>> -- >>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>>>> the body of a message to majordomo@vger.kernel.org >>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I just re-submitted cumulative patches. About your suggestion to split the patch that reset route at the reception of ASCONF-ACK, I removed those codes. Because we'd already reset the route just before ASCONF, so not needed after the ASCONF-ACK reception. I believe the other parts follow all your comments. I also cleaned up many parts in single-homed host support patch. Thanks, - Michio On Apr 25, 2011, at 11:45 , Wei Yongjun wrote: > >> Yes, I think the association cannot be kept, if the single-homed ASCONF receiver moves to the new network before sending ASCONF-ACK. >> Am I missing? > > Oh, yeah, you are right.:-) > >> Thanks, >> - Michio >> >> On Apr 25, 2011, at 11:02 , Wei Yongjun wrote: >> >>>> Hi, >>>> >>>> Such operation would not be supported by specification, in Sec.5.3 in RFC 5061: >>>> F1) When adding an IP address to an association, the IP address is >>>> NOT considered fully added to the association until the ASCONF- >>>> ACK arrives. This means that until such time as the ASCONF >>>> containing the add is acknowledged, the sender MUST NOT use the >>>> new IP address as a source for ANY SCTP packet except on >>>> carrying an ASCONF Chunk. >>>> >>>> I think this means we cannot send ASCONF-ACK from the new address even if it bundles ASCONF... >>> If so, both side do not have valid address to send the such >>> ASCONF-ACK, and can not recv ASCONF-ACK. >>> >>>> - Michio >>>> >>>> On Apr 25, 2011, at 9:57 , Wei Yongjun wrote: >>>> >>>>>> On Apr 22, 2011, at 13:10 , Wei Yongjun wrote: >>>>>> >>>>>>>> Since the sender MUST NOT use the new IP address as a source for ANY SCTP >>>>>>>> packet except on carrying an ASCONF Chunk. And ASCONF chunk can be bundled. >>>>>>>> How about this change. If so, you do not need change to sctp_outq_tail(); >>>>>>>> >>>>>>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c >>>>>>>> index 1c88c89..bd6cc9c 100644 >>>>>>>> --- a/net/sctp/outqueue.c >>>>>>>> +++ b/net/sctp/outqueue.c >>>>>>>> @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>>>>> */ >>>>>>>> >>>>>>>> list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { >>>>>>>> + /* RFC 5061, 5.3 >>>>>>>> + * F1) This ... >>>>>>>> + */ >>>>>>>> + if (q->asoc->src_out_of_asoc_ok && >>>>>>>> + chunk->chunk_hdr->type != SCTP_CID_ASCONF) >>>>>>> SCTP_CID_ASCONF_ACK should be also allowed, the peer may >>>>>>> send ASCONF to do the same thing at the same time. >>>>>> Sorry for my bad understanding, >>>>>> Do you mean the situation: "the peer (ASCONF receiver) may send ASCONF-ACK to the unconfirmed destination"? >>>>>> Or do you mean following situation? >>>>>> 1. the pear sends ADD/DEL ASCONF to me, >>>>>> 2. I receive it, >>>>>> 3. I migrate to the other network and get new address, >>>>>> 4. I send ASCONF-ACK to the peer from the new address >>>>> Yes, If both side send ADD/DEL ASCONF to del the last one >>>>> address at the same time like this: >>>>> >>>>> ASCONF ----- ------ASCONF >>>>> (ADD/DEL) \ / (ADD/DEL) >>>>> \/ >>>>> /\ >>>>> <----/ \-----> >>>>> ASCONF-ACK---\ /------ASCONF-ACK >>>>> \/ >>>>> /\ >>>>> <----/ \-----> >>>>> >>>>> But I do not test for it. Not sure we need to do this, can you >>>>> check this before commit your new patchset? >>>>> >>>>> >>>>>>>> + continue; >>>>>>>> + >>>>>>>> list_del_init(&chunk->list); >>>>>>>> >>>>>>>> /* Pick the right transport to use. */ >>>>>>>> @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> + if (q->asoc->src_out_of_asoc_ok) >>>>>>>> + goto sctp_flush_out; >>>>>>>> + >>>>>>>> /* Is it OK to send data chunks? */ >>>>>>>> switch (asoc->state) { >>>>>>>> case SCTP_STATE_COOKIE_ECHOED: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> -- >>>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>>>>> the body of a message to majordomo@vger.kernel.org >>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> -- >>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>>>> the body of a message to majordomo@vger.kernel.org >>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c index 1c88c89..bd6cc9c 100644 --- a/net/sctp/outqueue.c +++ b/net/sctp/outqueue.c @@ -754,6 +754,13 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) */ list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) { + /* RFC 5061, 5.3 + * F1) This ... + */ + if (q->asoc->src_out_of_asoc_ok && + chunk->chunk_hdr->type != SCTP_CID_ASCONF) + continue; + list_del_init(&chunk->list); /* Pick the right transport to use. */ @@ -881,6 +888,9 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout) } } + if (q->asoc->src_out_of_asoc_ok) + goto sctp_flush_out; + /* Is it OK to send data chunks? */ switch (asoc->state) { case SCTP_STATE_COOKIE_ECHOED: