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

[Wei Yongjun <yjwei@cn.fujitsu.com>]

> Hi, 
>
>>>
>>> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
>>> index 3740603..6363c46 100644
>>> --- a/net/sctp/sm_make_chunk.c
>>> +++ b/net/sctp/sm_make_chunk.c
>>> @@ -2768,6 +2768,12 @@ 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			del_pickup = 0;
>>>
>>> 	/* Get total length of all the address parameters. */
>>> 	addr_buf = addrs;
>>> @@ -2780,6 +2786,17 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *asoc,
>>> 		totallen += addr_param_len;
>>>
>>> 		addr_buf += af->sockaddr_len;
>>> +		if (asoc->asconf_addr_del_pending && !del_pickup) {
>>> +			if (!sctp_in_scope(asoc->asconf_addr_del_pending,
>>> +			    sctp_scope(addr)))
>>> +				continue;
>> scope should olny be check before address is added to the asoc.
>> delete does not need this, since we have checked that the address
>> be deleted is existed in the asoc.
> Exactly, I remove this check and add the code to check scope for adding address in sctp_asconf_mgmt().  
>> You can test with the last test commands I send to your, instead,
>> ifdown/up the lo device.
>> $ ifdown lo && ifup lo
> Ah, I see, I try.  
>>> +			/* 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.

>>> +			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
>