[PATCH v2 1/2] tipc: tipc_bcbearer_send(): simplify bearer selection

[PATCH v2 1/2] tipc: tipc_bcbearer_send(): simplify bearer selection

[Gerlando Falauto]

Also some minor cosmetic improvements and comment style changes

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 net/tipc/bcast.c |   29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 2655c9f..be1f945 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -584,8 +584,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 {
 	int bp_index;
 
-	/*
-	 * Prepare broadcast link message for reliable transmission,
+	/* Prepare broadcast link message for reliable transmission,
 	 * if first time trying to send it;
 	 * preparation is skipped for broadcast link protocol messages
 	 * since they are sent in an unreliable manner and don't need it
@@ -610,31 +609,33 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 
 	for (bp_index = 0; bp_index < MAX_BEARERS; bp_index++) {
 		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
+		struct tipc_bearer *b = p;
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
 
 		if (!p)
-			break;	/* no more bearers to try */
+			break;	/* No more bearers to try */
+
+		if (tipc_bearer_blocked(p)) {
+			if (!s || tipc_bearer_blocked(s))
+				continue; /* Can't use either bearer */
+			b = s;
+		}
 
-		tipc_nmap_diff(&bcbearer->remains, &p->nodes, &bcbearer->remains_new);
+		tipc_nmap_diff(&bcbearer->remains, &b->nodes,
+			&bcbearer->remains_new);
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
-			continue;	/* bearer pair doesn't add anything */
+			continue;  /* Nothing added by bearer pair */
 
-		if (!tipc_bearer_blocked(p))
-			tipc_bearer_send(p, buf, &p->media->bcast_addr);
-		else if (s && !tipc_bearer_blocked(s))
-			/* unable to send on primary bearer */
-			tipc_bearer_send(s, buf, &s->media->bcast_addr);
-		else
-			/* unable to send on either bearer */
-			continue;
+		tipc_bearer_send(b, buf, &s->media->bcast_addr);
 
+		/* Swap bearers for next packet */
 		if (s) {
 			bcbearer->bpairs[bp_index].primary = s;
 			bcbearer->bpairs[bp_index].secondary = p;
 		}
 
 		if (bcbearer->remains_new.count == 0)
-			break;	/* all targets reached */
+			break;	/* All targets reached */
 
 		bcbearer->remains = bcbearer->remains_new;
 	}
-- 
1.7.10.1

[PATCH v2 2/2] tipc: pskb_copy() buffers when sending on more than one bearer

[Gerlando Falauto]

When sending packets, TIPC bearers use skb_clone() before writing their
hardware header. This will however NOT copy the data buffer.
So when the same packet is sent over multiple bearers (to reach multiple
nodes), the same socket buffer data will be treated by multiple
tipc_media drivers which will write their own hardware header through
dev_hard_header().
Most of the time this is not a problem, because by the time the
packet is processed by the second media, it has already been sent over
the first one. However, when the first transmission is delayed (e.g.
because of insufficient bandwidth or through a shaper), the next bearer
will overwrite the hardware header, resulting in the packet being sent:
a) with the wrong source address, when bearers of the same type,
e.g. ethernet, are involved
b) with a completely corrupt header, or even dropped, when bearers of
different types are involved.

So when the same socket buffer is to be sent multiple times, send a
pskb_copy() instead (from the second instance on), and release it
afterwards (the bearer will skb_clone() it anyway).

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 net/tipc/bcast.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index be1f945..7f57f76 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -611,6 +611,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
 		struct tipc_bearer *b = p;
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
+		struct sk_buff *tbuf;
 
 		if (!p)
 			break;	/* No more bearers to try */
@@ -626,7 +627,17 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
 			continue;  /* Nothing added by bearer pair */
 
-		tipc_bearer_send(b, buf, &s->media->bcast_addr);
+		if (bp_index == 0) {
+			/* Use original buffer for first bearer */
+			tipc_bearer_send(b, buf, &b->media->bcast_addr);
+		} else {
+			/* Avoid concurrent buffer access */
+			tbuf = pskb_copy(buf, GFP_ATOMIC);
+			if (!tbuf)
+				break;
+			tipc_bearer_send(b, tbuf, &b->media->bcast_addr);
+			kfree_skb(tbuf); /* Bearer keeps a clone */
+		}
 
 		/* Swap bearers for next packet */
 		if (s) {
-- 
1.7.10.1

Re: [PATCH v2 2/2] tipc: pskb_copy() buffers when sending on more than one bearer

[David Miller]

You'll need to respin and resubmit this when you fix the problems
in the first patch.

Re: [PATCH v2 1/2] tipc: tipc_bcbearer_send(): simplify bearer selection

[David Miller]

From: Gerlando Falauto <gerlando.falauto@keymile.com>
Date: Mon, 29 Apr 2013 16:13:39 +0200

> Also some minor cosmetic improvements and comment style changes
> 
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>

You're mixing too many things, and actually adding style problems.

>  		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
> +		struct tipc_bearer *b = p;
>  		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;

Local variable declarations should be ordered from longest line
to shortest line.

> -		tipc_nmap_diff(&bcbearer->remains, &p->nodes, &bcbearer->remains_new);
> +		tipc_nmap_diff(&bcbearer->remains, &b->nodes,
> +			&bcbearer->remains_new);

This second line is indented incorrectly, do it like this:

	func(arg1, arg2,
	     arg3, arg4);

Specifically arg3 is aligned, using the appropriate mixture of TAB
and space characters, with the first column after the openning
parenthesis of the func() call.

>  		if (bcbearer->remains_new.count == bcbearer->remains.count)
> -			continue;	/* bearer pair doesn't add anything */
> +			continue;  /* Nothing added by bearer pair */

Why is two spaces better than a TAB, make it one space if you're going
to change this.

>  		if (bcbearer->remains_new.count == 0)
> -			break;	/* all targets reached */
> +			break;	/* All targets reached */

Untab this gap between break; and the comment into a space too.

[PATCH v3 1/3] tipc: cosmetic: clean up comments

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
Changes from v2:
* Split cosmetic from functional changes

 net/tipc/bcast.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 2655c9f..8438e01 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -584,8 +584,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 {
 	int bp_index;
 
-	/*
-	 * Prepare broadcast link message for reliable transmission,
+	/* Prepare broadcast link message for reliable transmission,
 	 * if first time trying to send it;
 	 * preparation is skipped for broadcast link protocol messages
 	 * since they are sent in an unreliable manner and don't need it
@@ -613,11 +612,12 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
 
 		if (!p)
-			break;	/* no more bearers to try */
+			break; /* No more bearers to try */
 
-		tipc_nmap_diff(&bcbearer->remains, &p->nodes, &bcbearer->remains_new);
+		tipc_nmap_diff(&bcbearer->remains, &p->nodes,
+			       &bcbearer->remains_new);
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
-			continue;	/* bearer pair doesn't add anything */
+			continue; /* Nothing added by bearer pair */
 
 		if (!tipc_bearer_blocked(p))
 			tipc_bearer_send(p, buf, &p->media->bcast_addr);
@@ -628,13 +628,14 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 			/* unable to send on either bearer */
 			continue;
 
+		/* Swap bearers for next packet */
 		if (s) {
 			bcbearer->bpairs[bp_index].primary = s;
 			bcbearer->bpairs[bp_index].secondary = p;
 		}
 
 		if (bcbearer->remains_new.count == 0)
-			break;	/* all targets reached */
+			break; /* All targets reached */
 
 		bcbearer->remains = bcbearer->remains_new;
 	}
-- 
1.7.10.1

[PATCH v3 2/3] tipc: tipc_bcbearer_send(): simplify bearer selection

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
Changes from v2:
* Fixed non-bisectability; was tipc_bearer_send(b, buf, &s->media...)

 net/tipc/bcast.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 8438e01..c8f572f 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -610,23 +610,23 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 	for (bp_index = 0; bp_index < MAX_BEARERS; bp_index++) {
 		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
+		struct tipc_bearer *b = p;
 
 		if (!p)
 			break; /* No more bearers to try */
 
-		tipc_nmap_diff(&bcbearer->remains, &p->nodes,
+		if (tipc_bearer_blocked(p)) {
+			if (!s || tipc_bearer_blocked(s))
+				continue; /* Can't use either bearer */
+			b = s;
+		}
+
+		tipc_nmap_diff(&bcbearer->remains, &b->nodes,
 			       &bcbearer->remains_new);
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
 			continue; /* Nothing added by bearer pair */
 
-		if (!tipc_bearer_blocked(p))
-			tipc_bearer_send(p, buf, &p->media->bcast_addr);
-		else if (s && !tipc_bearer_blocked(s))
-			/* unable to send on primary bearer */
-			tipc_bearer_send(s, buf, &s->media->bcast_addr);
-		else
-			/* unable to send on either bearer */
-			continue;
+		tipc_bearer_send(b, buf, &b->media->bcast_addr);
 
 		/* Swap bearers for next packet */
 		if (s) {
-- 
1.7.10.1

[PATCH v3 3/3] tipc: pskb_copy() buffers when sending on more than one bearer

[Gerlando Falauto]

When sending packets, TIPC bearers use skb_clone() before writing their
hardware header. This will however NOT copy the data buffer.
So when the same packet is sent over multiple bearers (to reach multiple
nodes), the same socket buffer data will be treated by multiple
tipc_media drivers which will write their own hardware header through
dev_hard_header().
Most of the time this is not a problem, because by the time the
packet is processed by the second media, it has already been sent over
the first one. However, when the first transmission is delayed (e.g.
because of insufficient bandwidth or through a shaper), the next bearer
will overwrite the hardware header, resulting in the packet being sent:
a) with the wrong source address, when bearers of the same type,
e.g. ethernet, are involved
b) with a completely corrupt header, or even dropped, when bearers of
different types are involved.

So when the same socket buffer is to be sent multiple times, send a
pskb_copy() instead (from the second instance on), and release it
afterwards (the bearer will skb_clone() it anyway).

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
Changes from v2:
* Rebased

 net/tipc/bcast.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index c8f572f..a8e574d 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -611,6 +611,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
 		struct tipc_bearer *b = p;
+		struct sk_buff *tbuf;
 
 		if (!p)
 			break; /* No more bearers to try */
@@ -626,7 +627,17 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
 			continue; /* Nothing added by bearer pair */
 
-		tipc_bearer_send(b, buf, &b->media->bcast_addr);
+		if (bp_index == 0) {
+			/* Use original buffer for first bearer */
+			tipc_bearer_send(b, buf, &b->media->bcast_addr);
+		} else {
+			/* Avoid concurrent buffer access */
+			tbuf = pskb_copy(buf, GFP_ATOMIC);
+			if (!tbuf)
+				break;
+			tipc_bearer_send(b, tbuf, &b->media->bcast_addr);
+			kfree_skb(tbuf); /* Bearer keeps a clone */
+		}
 
 		/* Swap bearers for next packet */
 		if (s) {
-- 
1.7.10.1

Re: [PATCH v3 1/3] tipc: cosmetic: clean up comments

[Sergei Shtylyov]

Hello.

On 30-04-2013 11:33, Gerlando Falauto wrote:

> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
> Changes from v2:
> * Split cosmetic from functional changes

>   net/tipc/bcast.c |   13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)

> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
> index 2655c9f..8438e01 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -584,8 +584,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
>   {
>   	int bp_index;
>
> -	/*
> -	 * Prepare broadcast link message for reliable transmission,
> +	/* Prepare broadcast link message for reliable transmission,
>   	 * if first time trying to send it;
>   	 * preparation is skipped for broadcast link protocol messages
>   	 * since they are sent in an unreliable manner and don't need it
> @@ -613,11 +612,12 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
>   		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
>
>   		if (!p)
> -			break;	/* no more bearers to try */
> +			break; /* No more bearers to try */
>
> -		tipc_nmap_diff(&bcbearer->remains, &p->nodes, &bcbearer->remains_new);
> +		tipc_nmap_diff(&bcbearer->remains, &p->nodes,
> +			       &bcbearer->remains_new);

    It apeears you nopt only cleaning up comments in this patch but you don't 
mention this change in the changelog...

WBR, Sergei

Re: [PATCH v3 1/3] tipc: cosmetic: clean up comments

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Tue, 30 Apr 2013 15:54:01 +0400

>    It apeears you nopt only cleaning up comments in this patch but you
>    don't mention this change in the changelog...

Agreed.

Re: [PATCH v3 1/3] tipc: cosmetic: clean up comments

[Gerlando Falauto]

Hi Sergei,

On 04/30/2013 01:54 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 30-04-2013 11:33, Gerlando Falauto wrote:
>
>> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
>> ---
>> Changes from v2:
>> * Split cosmetic from functional changes
>
>>    net/tipc/bcast.c |   13 +++++++------
>>    1 file changed, 7 insertions(+), 6 deletions(-)
>
>> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
>> index 2655c9f..8438e01 100644
>> --- a/net/tipc/bcast.c
>> +++ b/net/tipc/bcast.c
>> @@ -584,8 +584,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
>>    {
>>    	int bp_index;
>>
>> -	/*
>> -	 * Prepare broadcast link message for reliable transmission,
>> +	/* Prepare broadcast link message for reliable transmission,
>>    	 * if first time trying to send it;
>>    	 * preparation is skipped for broadcast link protocol messages
>>    	 * since they are sent in an unreliable manner and don't need it
>> @@ -613,11 +612,12 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
>>    		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
>>
>>    		if (!p)
>> -			break;	/* no more bearers to try */
>> +			break; /* No more bearers to try */
>>
>> -		tipc_nmap_diff(&bcbearer->remains, &p->nodes, &bcbearer->remains_new);
>> +		tipc_nmap_diff(&bcbearer->remains, &p->nodes,
>> +			       &bcbearer->remains_new);
>
>      It apeears you nopt only cleaning up comments in this patch but you don't
> mention this change in the changelog...

That's just adding a line break to split a long line.
Come on, seriously, is that *really* worth mentioning?
How about "tipc: cosmetic: clean up comments and break a long line"?

Thanks,
Gerlando

Re: [PATCH v3 1/3] tipc: cosmetic: clean up comments

[Sergei Shtylyov]

Hello.

On 01-05-2013 11:37, Gerlando Falauto wrote:

>>> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
>>> ---
>>> Changes from v2:
>>> * Split cosmetic from functional changes

>>>    net/tipc/bcast.c |   13 +++++++------
>>>    1 file changed, 7 insertions(+), 6 deletions(-)

>>> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
>>> index 2655c9f..8438e01 100644
>>> --- a/net/tipc/bcast.c
>>> +++ b/net/tipc/bcast.c
>>> @@ -584,8 +584,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
>>>    {
>>>        int bp_index;
>>>
>>> -    /*
>>> -     * Prepare broadcast link message for reliable transmission,
>>> +    /* Prepare broadcast link message for reliable transmission,
>>>         * if first time trying to send it;
>>>         * preparation is skipped for broadcast link protocol messages
>>>         * since they are sent in an unreliable manner and don't need it
>>> @@ -613,11 +612,12 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
>>>            struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
>>>
>>>            if (!p)
>>> -            break;    /* no more bearers to try */
>>> +            break; /* No more bearers to try */
>>>
>>> -        tipc_nmap_diff(&bcbearer->remains, &p->nodes,
>>> &bcbearer->remains_new);
>>> +        tipc_nmap_diff(&bcbearer->remains, &p->nodes,
>>> +                   &bcbearer->remains_new);

>>      It apeears you nopt only cleaning up comments in this patch but you don't
>> mention this change in the changelog...

> That's just adding a line break to split a long line.
> Come on, seriously, is that *really* worth mentioning?

    Yes, it is. Otherwise it's an unrelated change.

> How about "tipc: cosmetic: clean up comments and break a long line"?

    Sounds good.

> Thanks,
> Gerlando

WBR, Sergei

[PATCH v3b 1/3] tipc: cosmetic: clean up comments and break a long line

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
Changes from v3:
* Added "and break a long line" to the commit message
Changes from v2:
* Split cosmetic (this patch) from functional changes (next patch)

 net/tipc/bcast.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 2655c9f..8438e01 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -584,8 +584,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 {
 	int bp_index;
 
-	/*
-	 * Prepare broadcast link message for reliable transmission,
+	/* Prepare broadcast link message for reliable transmission,
 	 * if first time trying to send it;
 	 * preparation is skipped for broadcast link protocol messages
 	 * since they are sent in an unreliable manner and don't need it
@@ -613,11 +612,12 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
 
 		if (!p)
-			break;	/* no more bearers to try */
+			break; /* No more bearers to try */
 
-		tipc_nmap_diff(&bcbearer->remains, &p->nodes, &bcbearer->remains_new);
+		tipc_nmap_diff(&bcbearer->remains, &p->nodes,
+			       &bcbearer->remains_new);
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
-			continue;	/* bearer pair doesn't add anything */
+			continue; /* Nothing added by bearer pair */
 
 		if (!tipc_bearer_blocked(p))
 			tipc_bearer_send(p, buf, &p->media->bcast_addr);
@@ -628,13 +628,14 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 			/* unable to send on either bearer */
 			continue;
 
+		/* Swap bearers for next packet */
 		if (s) {
 			bcbearer->bpairs[bp_index].primary = s;
 			bcbearer->bpairs[bp_index].secondary = p;
 		}
 
 		if (bcbearer->remains_new.count == 0)
-			break;	/* all targets reached */
+			break; /* All targets reached */
 
 		bcbearer->remains = bcbearer->remains_new;
 	}
-- 
1.7.10.1

Re: [PATCH v3b 1/3] tipc: cosmetic: clean up comments and break a long line

[David Miller]

From: Gerlando Falauto <gerlando.falauto@keymile.com>
Date: Wed,  1 May 2013 18:41:17 +0200

> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
> Changes from v3:
> * Added "and break a long line" to the commit message
> Changes from v2:
> * Split cosmetic (this patch) from functional changes (next patch)

Never resubmit patches by themselves, always resubmit the entire series.

Because when one patch has problems, I toss the entire series from patchwork,
and by resubmitting the entire series again you are also giving me important
information, you're telling me that the subsequent patches in the series
still apply cleanly even though the first one has changed.

I've tossed this patch too, resubmit this properly, thanks.

Re: [PATCH v3b 1/3] tipc: cosmetic: clean up comments and break a long line

[Gerlando Falauto]

Hi,

On 05/01/2013 07:16 PM, David Miller wrote:
> From: Gerlando Falauto <gerlando.falauto@keymile.com>
> Date: Wed,  1 May 2013 18:41:17 +0200
>
>> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
>> ---
>> Changes from v3:
>> * Added "and break a long line" to the commit message
>> Changes from v2:
>> * Split cosmetic (this patch) from functional changes (next patch)
>
> Never resubmit patches by themselves, always resubmit the entire series.

I was just wondering what happens in that case, thank you for the 
explanation, I had no idea.

>
> Because when one patch has problems, I toss the entire series from patchwork,

Is "v3b" good or shall I just use v4 instead?
Do I also need to change the version number (and add "Changes from 
previous version" lines) to the subsequent patches, even though nothing 
has actually changed)? Should have I used a cover letter to begin with?

 > and by resubmitting the entire series again you are also giving me 
important
 > information, you're telling me that the subsequent patches in the series
 > still apply cleanly even though the first one has changed.

That I don't understand. I thought it would be the other way around, 
that's why I resubmitted only the one which changed.
If I resubmit the entire series, how can you tell patches 2 and 3 are 
identical to the previous version I posted?

Last question: Is chain reply OK?

Thanks for your patience,
Gerlando


>
> I've tossed this patch too, resubmit this properly, thanks.
>

Re: [PATCH v3b 1/3] tipc: cosmetic: clean up comments and break a long line

[David Miller]

From: Gerlando Falauto <gerlando.falauto@keymile.com>
Date: Wed, 01 May 2013 19:35:09 +0200

> That I don't understand. I thought it would be the other way around,
> that's why I resubmitted only the one which changed.

Any patch you submit I assume you test applied yourself successfully,
therefore by submitting it you are saying that you validated them
as a series explicitly or know that the other patches have no conflicts.

Also, please never submit new versions of patches in replies to older
patch discussions.

Chain replies in order to construct a series are OK, especially when
you provide a proper "[PATCH 0/N] ..." initial posting explaining
the series.

Re: [PATCH v3b 1/3] tipc: cosmetic: clean up comments and break a long line

[Gerlando Falauto]

Hi,

On 05/01/2013 07:43 PM, David Miller wrote:
> From: Gerlando Falauto <gerlando.falauto@keymile.com>
> Date: Wed, 01 May 2013 19:35:09 +0200
>
>> That I don't understand. I thought it would be the other way around,
>> that's why I resubmitted only the one which changed.
>
> Any patch you submit I assume you test applied yourself successfully,
> therefore by submitting it you are saying that you validated them
> as a series explicitly or know that the other patches have no conflicts.

I understand, you're right. I hope adding "Changes from v(n-1): None" 
should also be enough to say that further review should not be necessary 
as the patch is identical to the previous version.

> Also, please never submit new versions of patches in replies to older
> patch discussions.
>
> Chain replies in order to construct a series are OK, especially when
> you provide a proper "[PATCH 0/N] ..." initial posting explaining
> the series.
>

OK, thanks.
Gerlando