Re: [PATCH 6/6 v8] sctp: Add ASCONF operation on the single-homed host

Michio Honda
2011-04-28T08:04:21Z

>>
>>>> + /* reuse the parameter length from the same scope one */
>>>> + totallen += paramlen;
>>>> + totallen += addr_param_len;
>>>> + del_pickup = 1;
>>>> + asoc->src_out_of_asoc_ok = 1;
>>> src_out_of_asoc_ok should be marked when the last address is
>>> assigned to asoc->asconf_addr_del_pending?
>> I thought marking src_out_of_asoc_ok should be set when the candidate new address appears, rather than when the last address is being deleted.
>> Because until such address appears, there is no situation to send any chunk from the address not in the association.
>
> The last address have been marked as DEL, will never using
> for sending chunks. At this time, there is no valid address in the
> host, chunk can not be send out by host.
I understand that, marking out_of_asoc_ok at the last address being deleted does same thing.
However, out_of_asoc_ok state is not regular, so I think we should shorten that duration as much as possible.
But this is my personal opinion, so if you don't think so, I will mark it when the last address being deleted.
>
>>>> + SCTP_DEBUG_PRINTK("mkasconf_update_ip: picked same-scope del_pending addr, totallen for all addresses is %d\n", totallen);
>>>> + }
>>>> }
>>>>
>>>> /* Create an asconf chunk with the required length. */
>>>> @@ -2802,6 +2819,19 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc,
>>>>
>>>> addr_buf += af->sockaddr_len;
>>>> }
>>>> + if (flags == SCTP_PARAM_ADD_IP && del_pickup) {
>>>> + addr = asoc->asconf_addr_del_pending;
>>>> + del_af = sctp_get_af_specific(addr->v4.sin_family);
>>>> + del_addr_param_len = del_af->to_addr_param(addr,
>>>> + &del_addr_param);
>>>> + del_param.param_hdr.type = SCTP_PARAM_DEL_IP;
>>>> + del_param.param_hdr.length = htons(del_paramlen +
>>>> + del_addr_param_len);
>>>> + del_param.crr_id = i;
>>>> +
>>>> + sctp_addto_chunk(retval, del_paramlen, &del_param);
>>>> + sctp_addto_chunk(retval, del_addr_param_len, &del_addr_param);
>>>> + }
>>>> return retval;
>>>> }
>>> How about clean up this part as:
>>>
>>> + if (...) {
>>> + addr = asoc->asconf_addr_del_pending;
>>> + af = sctp_get_af_specific(addr->v4.sin_family);
>>> + addr_param_len = af->to_addr_param(addr, &addr_param);
>>> + totallen += paramlen;
>>> + totallen += addr_param_len;
>>> + ...
>>> + }
>>> +
>>> /* Create an asconf chunk with the required length. */
>>> retval = sctp_make_asconf(asoc, laddr, totallen);
>>> if (!retval)
>>> @@ -2802,6 +2812,18 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc,
>>>
>>> addr_buf += af->sockaddr_len;
>>> }
>>> +
>>> + if (...) {
>>> + addr = asoc->asconf_addr_del_pending;
>>> + af = sctp_get_af_specific(addr->v4.sin_family);
>>> + addr_param_len = af->to_addr_param(addr, &addr_param);
>>> + param.param_hdr.type = SCTP_PARAM_DEL_IP;
>>> + param.param_hdr.length = htons(paramlen + addr_param_len);
>>> + param.crr_id = i;
>>> +
>>> + sctp_addto_chunk(retval, paramlen, &param);
>>> + sctp_addto_chunk(retval, addr_param_len, &addr_param);
>>> + }
>> agreed with reusing af instead of defining del_af.
>> --
>> 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
>>