* [PATCH 1/4] sctp: merge two if statements to one
2013-10-25 1:50 [PATCH 0/4] sctp: do some clean up and fix comments Wang Weidong
@ 2013-10-25 1:50 ` Wang Weidong
2013-10-25 13:04 ` Sergei Shtylyov
2013-10-25 23:40 ` Vlad Yasevich
2013-10-25 1:50 ` [PATCH 2/4] sctp: remove the repeat initialize with 0 Wang Weidong
` (2 subsequent siblings)
3 siblings, 2 replies; 12+ messages in thread
From: Wang Weidong @ 2013-10-25 1:50 UTC (permalink / raw)
To: davem, nhorman, vyasevich; +Cc: dingtianhong, linux-sctp, netdev
Two if statements do the same work, maybe we can merge them to
one. There is just code simplification, no functional changes.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/auth.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/net/sctp/auth.c b/net/sctp/auth.c
index 8c4fa5d..19fb0ae 100644
--- a/net/sctp/auth.c
+++ b/net/sctp/auth.c
@@ -539,18 +539,14 @@ struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
for (i = 0; i < n_elt; i++) {
id = ntohs(hmacs->hmac_ids[i]);
- /* Check the id is in the supported range */
- if (id > SCTP_AUTH_HMAC_ID_MAX) {
- id = 0;
- continue;
- }
-
- /* See is we support the id. Supported IDs have name and
+ /* Check the id is in the supported range. And
+ * see is we support the id. Supported IDs have name and
* length fields set, so that we can allocated and use
* them. We can safely just check for name, for without the
* name, we can't allocate the TFM.
*/
- if (!sctp_hmac_list[id].hmac_name) {
+ if (id > SCTP_AUTH_HMAC_ID_MAX ||
+ !sctp_hmac_list[id].hmac_name) {
id = 0;
continue;
}
--
1.7.12
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] sctp: merge two if statements to one
2013-10-25 1:50 ` [PATCH 1/4] sctp: merge two if statements to one Wang Weidong
@ 2013-10-25 13:04 ` Sergei Shtylyov
2013-10-26 2:55 ` wangweidong
2013-10-25 23:40 ` Vlad Yasevich
1 sibling, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2013-10-25 13:04 UTC (permalink / raw)
To: Wang Weidong, davem, nhorman, vyasevich; +Cc: dingtianhong, linux-sctp, netdev
Hello.
On 25-10-2013 5:50, Wang Weidong wrote:
> Two if statements do the same work, maybe we can merge them to
> one. There is just code simplification, no functional changes.
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> net/sctp/auth.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
I understand what I noticed below is not your typos but maybe it's time to
fix them?
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index 8c4fa5d..19fb0ae 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -539,18 +539,14 @@ struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
> for (i = 0; i < n_elt; i++) {
> id = ntohs(hmacs->hmac_ids[i]);
>
> - /* Check the id is in the supported range */
> - if (id > SCTP_AUTH_HMAC_ID_MAX) {
> - id = 0;
> - continue;
> - }
> -
> - /* See is we support the id. Supported IDs have name and
> + /* Check the id is in the supported range. And
> + * see is we support the id. Supported IDs have name and
s/is/if/.
> * length fields set, so that we can allocated and use
s/allocated/allocate/.
WBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] sctp: merge two if statements to one
2013-10-25 13:04 ` Sergei Shtylyov
@ 2013-10-26 2:55 ` wangweidong
0 siblings, 0 replies; 12+ messages in thread
From: wangweidong @ 2013-10-26 2:55 UTC (permalink / raw)
To: Sergei Shtylyov, davem, nhorman, vyasevich
Cc: dingtianhong, linux-sctp, netdev
On 2013/10/25 21:04, Sergei Shtylyov wrote:
> Hello.
>
> On 25-10-2013 5:50, Wang Weidong wrote:
>
>> Two if statements do the same work, maybe we can merge them to
>> one. There is just code simplification, no functional changes.
>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>> net/sctp/auth.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> I understand what I noticed below is not your typos but maybe it's time to fix them?
Yeah, I will fix them.
Thanks.
>
>> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
>> index 8c4fa5d..19fb0ae 100644
>> --- a/net/sctp/auth.c
>> +++ b/net/sctp/auth.c
>> @@ -539,18 +539,14 @@ struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
>> for (i = 0; i < n_elt; i++) {
>> id = ntohs(hmacs->hmac_ids[i]);
>>
>> - /* Check the id is in the supported range */
>> - if (id > SCTP_AUTH_HMAC_ID_MAX) {
>> - id = 0;
>> - continue;
>> - }
>> -
>> - /* See is we support the id. Supported IDs have name and
>> + /* Check the id is in the supported range. And
>> + * see is we support the id. Supported IDs have name and
>
> s/is/if/.
>
>> * length fields set, so that we can allocated and use
>
> s/allocated/allocate/.
>
> WBR, Sergei
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] sctp: merge two if statements to one
2013-10-25 1:50 ` [PATCH 1/4] sctp: merge two if statements to one Wang Weidong
2013-10-25 13:04 ` Sergei Shtylyov
@ 2013-10-25 23:40 ` Vlad Yasevich
2013-10-26 2:59 ` wangweidong
1 sibling, 1 reply; 12+ messages in thread
From: Vlad Yasevich @ 2013-10-25 23:40 UTC (permalink / raw)
To: Wang Weidong, davem, nhorman; +Cc: dingtianhong, linux-sctp, netdev
On 10/24/2013 09:50 PM, Wang Weidong wrote:
> Two if statements do the same work, maybe we can merge them to
> one. There is just code simplification, no functional changes.
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> net/sctp/auth.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index 8c4fa5d..19fb0ae 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -539,18 +539,14 @@ struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
> for (i = 0; i < n_elt; i++) {
> id = ntohs(hmacs->hmac_ids[i]);
>
> - /* Check the id is in the supported range */
> - if (id > SCTP_AUTH_HMAC_ID_MAX) {
> - id = 0;
> - continue;
> - }
> -
> - /* See is we support the id. Supported IDs have name and
> + /* Check the id is in the supported range. And
> + * see is we support the id. Supported IDs have name and
> * length fields set, so that we can allocated and use
> * them. We can safely just check for name, for without the
> * name, we can't allocate the TFM.
> */
> - if (!sctp_hmac_list[id].hmac_name) {
> + if (id > SCTP_AUTH_HMAC_ID_MAX ||
> + !sctp_hmac_list[id].hmac_name) {
Can you please make the 2 parts of the 'if' statement above line up
with each other instead of the code below. I makes it easy to see what
the whole 'if conditional' is.
Thanks
-vlad
> id = 0;
> continue;
> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] sctp: merge two if statements to one
2013-10-25 23:40 ` Vlad Yasevich
@ 2013-10-26 2:59 ` wangweidong
0 siblings, 0 replies; 12+ messages in thread
From: wangweidong @ 2013-10-26 2:59 UTC (permalink / raw)
To: Vlad Yasevich, davem, nhorman; +Cc: dingtianhong, linux-sctp, netdev
On 2013/10/26 7:40, Vlad Yasevich wrote:
> On 10/24/2013 09:50 PM, Wang Weidong wrote:
>> Two if statements do the same work, maybe we can merge them to
>> one. There is just code simplification, no functional changes.
>>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>> net/sctp/auth.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
>> index 8c4fa5d..19fb0ae 100644
>> --- a/net/sctp/auth.c
>> +++ b/net/sctp/auth.c
>> @@ -539,18 +539,14 @@ struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
>> for (i = 0; i < n_elt; i++) {
>> id = ntohs(hmacs->hmac_ids[i]);
>>
>> - /* Check the id is in the supported range */
>> - if (id > SCTP_AUTH_HMAC_ID_MAX) {
>> - id = 0;
>> - continue;
>> - }
>> -
>> - /* See is we support the id. Supported IDs have name and
>> + /* Check the id is in the supported range. And
>> + * see is we support the id. Supported IDs have name and
>> * length fields set, so that we can allocated and use
>> * them. We can safely just check for name, for without the
>> * name, we can't allocate the TFM.
>> */
>> - if (!sctp_hmac_list[id].hmac_name) {
>> + if (id > SCTP_AUTH_HMAC_ID_MAX ||
>> + !sctp_hmac_list[id].hmac_name) {
>
> Can you please make the 2 parts of the 'if' statement above line up
> with each other instead of the code below. I makes it easy to see what
> the whole 'if conditional' is.
>
> Thanks
> -vlad
>
Ok, I will resend a new version.
Thanks.
>> id = 0;
>> continue;
>> }
>>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] sctp: remove the repeat initialize with 0
2013-10-25 1:50 [PATCH 0/4] sctp: do some clean up and fix comments Wang Weidong
2013-10-25 1:50 ` [PATCH 1/4] sctp: merge two if statements to one Wang Weidong
@ 2013-10-25 1:50 ` Wang Weidong
2013-10-25 23:42 ` Vlad Yasevich
2013-10-25 1:50 ` [PATCH 3/4] sctp: fix some comments in associola.c Wang Weidong
2013-10-25 1:50 ` [PATCH 4/4] sctp: fix comment in chunk.c Wang Weidong
3 siblings, 1 reply; 12+ messages in thread
From: Wang Weidong @ 2013-10-25 1:50 UTC (permalink / raw)
To: davem, nhorman, vyasevich; +Cc: dingtianhong, linux-sctp, netdev
kmem_cache_zalloc had set the allocated memory to zero. I think no need
to initialize with 0. And move the comments to the function begin.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/sm_make_chunk.c | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index d244a23..fe69032 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1297,6 +1297,13 @@ struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc)
/* Turn an skb into a chunk.
* FIXME: Eventually move the structure directly inside the skb->cb[].
+ *
+ * sctpimpguide-05.txt Section 2.8.2
+ * M1) Each time a new DATA chunk is transmitted
+ * set the 'TSN.Missing.Report' count for that TSN to 0. The
+ * 'TSN.Missing.Report' count will be used to determine missing chunks
+ * and when to fast retransmit.
+ *
*/
struct sctp_chunk *sctp_chunkify(struct sk_buff *skb,
const struct sctp_association *asoc,
@@ -1314,29 +1321,9 @@ struct sctp_chunk *sctp_chunkify(struct sk_buff *skb,
INIT_LIST_HEAD(&retval->list);
retval->skb = skb;
retval->asoc = (struct sctp_association *)asoc;
- retval->has_tsn = 0;
- retval->has_ssn = 0;
- retval->rtt_in_progress = 0;
- retval->sent_at = 0;
retval->singleton = 1;
- retval->end_of_packet = 0;
- retval->ecn_ce_done = 0;
- retval->pdiscard = 0;
-
- /* sctpimpguide-05.txt Section 2.8.2
- * M1) Each time a new DATA chunk is transmitted
- * set the 'TSN.Missing.Report' count for that TSN to 0. The
- * 'TSN.Missing.Report' count will be used to determine missing chunks
- * and when to fast retransmit.
- */
- retval->tsn_missing_report = 0;
- retval->tsn_gap_acked = 0;
- retval->fast_retransmit = SCTP_CAN_FRTX;
- /* If this is a fragmented message, track all fragments
- * of the message (for SEND_FAILED).
- */
- retval->msg = NULL;
+ retval->fast_retransmit = SCTP_CAN_FRTX;
/* Polish the bead hole. */
INIT_LIST_HEAD(&retval->transmitted_list);
--
1.7.12
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] sctp: remove the repeat initialize with 0
2013-10-25 1:50 ` [PATCH 2/4] sctp: remove the repeat initialize with 0 Wang Weidong
@ 2013-10-25 23:42 ` Vlad Yasevich
0 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-10-25 23:42 UTC (permalink / raw)
To: Wang Weidong, davem, nhorman; +Cc: dingtianhong, linux-sctp, netdev
On 10/24/2013 09:50 PM, Wang Weidong wrote:
> kmem_cache_zalloc had set the allocated memory to zero. I think no need
> to initialize with 0. And move the comments to the function begin.
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
Yes, thank you.
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/sm_make_chunk.c | 29 ++++++++---------------------
> 1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index d244a23..fe69032 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1297,6 +1297,13 @@ struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc)
>
> /* Turn an skb into a chunk.
> * FIXME: Eventually move the structure directly inside the skb->cb[].
> + *
> + * sctpimpguide-05.txt Section 2.8.2
> + * M1) Each time a new DATA chunk is transmitted
> + * set the 'TSN.Missing.Report' count for that TSN to 0. The
> + * 'TSN.Missing.Report' count will be used to determine missing chunks
> + * and when to fast retransmit.
> + *
> */
> struct sctp_chunk *sctp_chunkify(struct sk_buff *skb,
> const struct sctp_association *asoc,
> @@ -1314,29 +1321,9 @@ struct sctp_chunk *sctp_chunkify(struct sk_buff *skb,
> INIT_LIST_HEAD(&retval->list);
> retval->skb = skb;
> retval->asoc = (struct sctp_association *)asoc;
> - retval->has_tsn = 0;
> - retval->has_ssn = 0;
> - retval->rtt_in_progress = 0;
> - retval->sent_at = 0;
> retval->singleton = 1;
> - retval->end_of_packet = 0;
> - retval->ecn_ce_done = 0;
> - retval->pdiscard = 0;
> -
> - /* sctpimpguide-05.txt Section 2.8.2
> - * M1) Each time a new DATA chunk is transmitted
> - * set the 'TSN.Missing.Report' count for that TSN to 0. The
> - * 'TSN.Missing.Report' count will be used to determine missing chunks
> - * and when to fast retransmit.
> - */
> - retval->tsn_missing_report = 0;
> - retval->tsn_gap_acked = 0;
> - retval->fast_retransmit = SCTP_CAN_FRTX;
>
> - /* If this is a fragmented message, track all fragments
> - * of the message (for SEND_FAILED).
> - */
> - retval->msg = NULL;
> + retval->fast_retransmit = SCTP_CAN_FRTX;
>
> /* Polish the bead hole. */
> INIT_LIST_HEAD(&retval->transmitted_list);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] sctp: fix some comments in associola.c
2013-10-25 1:50 [PATCH 0/4] sctp: do some clean up and fix comments Wang Weidong
2013-10-25 1:50 ` [PATCH 1/4] sctp: merge two if statements to one Wang Weidong
2013-10-25 1:50 ` [PATCH 2/4] sctp: remove the repeat initialize with 0 Wang Weidong
@ 2013-10-25 1:50 ` Wang Weidong
2013-10-25 23:43 ` Vlad Yasevich
2013-10-25 1:50 ` [PATCH 4/4] sctp: fix comment in chunk.c Wang Weidong
3 siblings, 1 reply; 12+ messages in thread
From: Wang Weidong @ 2013-10-25 1:50 UTC (permalink / raw)
To: davem, nhorman, vyasevich; +Cc: dingtianhong, linux-sctp, netdev
fix some spellings
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/associola.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index cef5099..c9b91cb 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -602,7 +602,7 @@ void sctp_assoc_rm_peer(struct sctp_association *asoc,
/* Start a T3 timer here in case it wasn't running so
* that these migrated packets have a chance to get
- * retrnasmitted.
+ * retransmitted.
*/
if (!timer_pending(&active->T3_rtx_timer))
if (!mod_timer(&active->T3_rtx_timer,
@@ -665,7 +665,7 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
/* Set the path max_retrans. */
peer->pathmaxrxt = asoc->pathmaxrxt;
- /* And the partial failure retrnas threshold */
+ /* And the partial failure retrans threshold */
peer->pf_retrans = asoc->pf_retrans;
/* Initialize the peer's SACK delay timeout based on the
--
1.7.12
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] sctp: fix some comments in associola.c
2013-10-25 1:50 ` [PATCH 3/4] sctp: fix some comments in associola.c Wang Weidong
@ 2013-10-25 23:43 ` Vlad Yasevich
0 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-10-25 23:43 UTC (permalink / raw)
To: Wang Weidong, davem, nhorman; +Cc: dingtianhong, linux-sctp, netdev
On 10/24/2013 09:50 PM, Wang Weidong wrote:
> fix some spellings
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/associola.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index cef5099..c9b91cb 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -602,7 +602,7 @@ void sctp_assoc_rm_peer(struct sctp_association *asoc,
>
> /* Start a T3 timer here in case it wasn't running so
> * that these migrated packets have a chance to get
> - * retrnasmitted.
> + * retransmitted.
> */
> if (!timer_pending(&active->T3_rtx_timer))
> if (!mod_timer(&active->T3_rtx_timer,
> @@ -665,7 +665,7 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
> /* Set the path max_retrans. */
> peer->pathmaxrxt = asoc->pathmaxrxt;
>
> - /* And the partial failure retrnas threshold */
> + /* And the partial failure retrans threshold */
> peer->pf_retrans = asoc->pf_retrans;
>
> /* Initialize the peer's SACK delay timeout based on the
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] sctp: fix comment in chunk.c
2013-10-25 1:50 [PATCH 0/4] sctp: do some clean up and fix comments Wang Weidong
` (2 preceding siblings ...)
2013-10-25 1:50 ` [PATCH 3/4] sctp: fix some comments in associola.c Wang Weidong
@ 2013-10-25 1:50 ` Wang Weidong
2013-10-25 23:45 ` Vlad Yasevich
3 siblings, 1 reply; 12+ messages in thread
From: Wang Weidong @ 2013-10-25 1:50 UTC (permalink / raw)
To: davem, nhorman, vyasevich; +Cc: dingtianhong, linux-sctp, netdev
fix a spelling
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/chunk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 7bd5ed4..f2044fc 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -201,7 +201,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
max = asoc->frag_point;
/* If the the peer requested that we authenticate DATA chunks
- * we need to accound for bundling of the AUTH chunks along with
+ * we need to account for bundling of the AUTH chunks along with
* DATA.
*/
if (sctp_auth_send_cid(SCTP_CID_DATA, asoc)) {
--
1.7.12
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/4] sctp: fix comment in chunk.c
2013-10-25 1:50 ` [PATCH 4/4] sctp: fix comment in chunk.c Wang Weidong
@ 2013-10-25 23:45 ` Vlad Yasevich
0 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-10-25 23:45 UTC (permalink / raw)
To: Wang Weidong, davem, nhorman; +Cc: dingtianhong, linux-sctp, netdev
On 10/24/2013 09:50 PM, Wang Weidong wrote:
> fix a spelling
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
You could have combined these spelling fixes into 1 patch.
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/chunk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index 7bd5ed4..f2044fc 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -201,7 +201,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
>
> max = asoc->frag_point;
> /* If the the peer requested that we authenticate DATA chunks
> - * we need to accound for bundling of the AUTH chunks along with
> + * we need to account for bundling of the AUTH chunks along with
> * DATA.
> */
> if (sctp_auth_send_cid(SCTP_CID_DATA, asoc)) {
>
^ permalink raw reply [flat|nested] 12+ messages in thread