* [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
@ 2017-03-18 11:12 ` Xin Long
0 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-03-18 11:12 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Neil Horman, Vlad Yasevich, ja
Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
a temporary variable "confirm" in sctp_packet_transmit.
But it broke the rule that longer lines should be above shorter ones.
Besides, this variable is not necessary, so this patch is to just
remove it and use tp->dst_pending_confirm directly.
Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/output.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 71ce6b9..1224421 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -546,7 +546,6 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
struct sctp_association *asoc = tp->asoc;
struct sctp_chunk *chunk, *tmp;
int pkt_count, gso = 0;
- int confirm;
struct dst_entry *dst;
struct sk_buff *head;
struct sctphdr *sh;
@@ -625,13 +624,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
asoc->peer.last_sent_to = tp;
}
head->ignore_df = packet->ipfragok;
- confirm = tp->dst_pending_confirm;
- if (confirm)
+ if (tp->dst_pending_confirm)
skb_set_dst_pending_confirm(head, 1);
/* neighbour should be confirmed on successful transmission or
* positive error
*/
- if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
+ if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
+ tp->dst_pending_confirm)
tp->dst_pending_confirm = 0;
out:
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
2017-03-18 11:12 ` Xin Long
@ 2017-03-18 11:48 ` Julian Anastasov
-1 siblings, 0 replies; 10+ messages in thread
From: Julian Anastasov @ 2017-03-18 11:48 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Neil Horman, Vlad Yasevich
Hello,
On Sat, 18 Mar 2017, Xin Long wrote:
> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
> a temporary variable "confirm" in sctp_packet_transmit.
>
> But it broke the rule that longer lines should be above shorter ones.
> Besides, this variable is not necessary, so this patch is to just
> remove it and use tp->dst_pending_confirm directly.
>
> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/output.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 71ce6b9..1224421 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -546,7 +546,6 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> struct sctp_association *asoc = tp->asoc;
> struct sctp_chunk *chunk, *tmp;
> int pkt_count, gso = 0;
> - int confirm;
> struct dst_entry *dst;
> struct sk_buff *head;
> struct sctphdr *sh;
> @@ -625,13 +624,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> asoc->peer.last_sent_to = tp;
> }
> head->ignore_df = packet->ipfragok;
> - confirm = tp->dst_pending_confirm;
> - if (confirm)
> + if (tp->dst_pending_confirm)
> skb_set_dst_pending_confirm(head, 1);
> /* neighbour should be confirmed on successful transmission or
> * positive error
> */
> - if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
> + if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> + tp->dst_pending_confirm)
> tp->dst_pending_confirm = 0;
>
> out:
> --
I played safe here, I was not sure if currently
or some day in the future the SCTP stack can allow another
thread to set tp->dst_pending_confirm concurrently with the
sending. My idea was only when skb was used to confirm
neighbour only then to clear the indication. I guess, your
patch is ok because we should be locking the socket
everywhere.
Regards
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
@ 2017-03-18 11:48 ` Julian Anastasov
0 siblings, 0 replies; 10+ messages in thread
From: Julian Anastasov @ 2017-03-18 11:48 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Neil Horman, Vlad Yasevich
Hello,
On Sat, 18 Mar 2017, Xin Long wrote:
> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
> a temporary variable "confirm" in sctp_packet_transmit.
>
> But it broke the rule that longer lines should be above shorter ones.
> Besides, this variable is not necessary, so this patch is to just
> remove it and use tp->dst_pending_confirm directly.
>
> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/output.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 71ce6b9..1224421 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -546,7 +546,6 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> struct sctp_association *asoc = tp->asoc;
> struct sctp_chunk *chunk, *tmp;
> int pkt_count, gso = 0;
> - int confirm;
> struct dst_entry *dst;
> struct sk_buff *head;
> struct sctphdr *sh;
> @@ -625,13 +624,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> asoc->peer.last_sent_to = tp;
> }
> head->ignore_df = packet->ipfragok;
> - confirm = tp->dst_pending_confirm;
> - if (confirm)
> + if (tp->dst_pending_confirm)
> skb_set_dst_pending_confirm(head, 1);
> /* neighbour should be confirmed on successful transmission or
> * positive error
> */
> - if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
> + if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> + tp->dst_pending_confirm)
> tp->dst_pending_confirm = 0;
>
> out:
> --
I played safe here, I was not sure if currently
or some day in the future the SCTP stack can allow another
thread to set tp->dst_pending_confirm concurrently with the
sending. My idea was only when skb was used to confirm
neighbour only then to clear the indication. I guess, your
patch is ok because we should be locking the socket
everywhere.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
2017-03-18 11:48 ` Julian Anastasov
@ 2017-03-18 16:26 ` Xin Long
-1 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-03-18 16:26 UTC (permalink / raw)
To: Julian Anastasov
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Neil Horman, Vlad Yasevich
On Sat, Mar 18, 2017 at 7:48 PM, Julian Anastasov <ja@ssi.bg> wrote:
>
> Hello,
>
> On Sat, 18 Mar 2017, Xin Long wrote:
>
>> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
>> a temporary variable "confirm" in sctp_packet_transmit.
>>
>> But it broke the rule that longer lines should be above shorter ones.
>> Besides, this variable is not necessary, so this patch is to just
>> remove it and use tp->dst_pending_confirm directly.
>>
>> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>> net/sctp/output.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/sctp/output.c b/net/sctp/output.c
>> index 71ce6b9..1224421 100644
>> --- a/net/sctp/output.c
>> +++ b/net/sctp/output.c
>> @@ -546,7 +546,6 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
>> struct sctp_association *asoc = tp->asoc;
>> struct sctp_chunk *chunk, *tmp;
>> int pkt_count, gso = 0;
>> - int confirm;
>> struct dst_entry *dst;
>> struct sk_buff *head;
>> struct sctphdr *sh;
>> @@ -625,13 +624,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
>> asoc->peer.last_sent_to = tp;
>> }
>> head->ignore_df = packet->ipfragok;
>> - confirm = tp->dst_pending_confirm;
>> - if (confirm)
>> + if (tp->dst_pending_confirm)
>> skb_set_dst_pending_confirm(head, 1);
>> /* neighbour should be confirmed on successful transmission or
>> * positive error
>> */
>> - if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
>> + if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
>> + tp->dst_pending_confirm)
>> tp->dst_pending_confirm = 0;
>>
>> out:
>> --
>
> I played safe here, I was not sure if currently
> or some day in the future the SCTP stack can allow another
> thread to set tp->dst_pending_confirm concurrently with the
> sending. My idea was only when skb was used to confirm
> neighbour only then to clear the indication. I guess, your
> patch is ok because we should be locking the socket
> everywhere.
Yeps, It's safe, as all the codes for dst_pending_confirm are
under the sock lock protection.
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
@ 2017-03-18 16:26 ` Xin Long
0 siblings, 0 replies; 10+ messages in thread
From: Xin Long @ 2017-03-18 16:26 UTC (permalink / raw)
To: Julian Anastasov
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Neil Horman, Vlad Yasevich
On Sat, Mar 18, 2017 at 7:48 PM, Julian Anastasov <ja@ssi.bg> wrote:
>
> Hello,
>
> On Sat, 18 Mar 2017, Xin Long wrote:
>
>> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
>> a temporary variable "confirm" in sctp_packet_transmit.
>>
>> But it broke the rule that longer lines should be above shorter ones.
>> Besides, this variable is not necessary, so this patch is to just
>> remove it and use tp->dst_pending_confirm directly.
>>
>> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>> net/sctp/output.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/sctp/output.c b/net/sctp/output.c
>> index 71ce6b9..1224421 100644
>> --- a/net/sctp/output.c
>> +++ b/net/sctp/output.c
>> @@ -546,7 +546,6 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
>> struct sctp_association *asoc = tp->asoc;
>> struct sctp_chunk *chunk, *tmp;
>> int pkt_count, gso = 0;
>> - int confirm;
>> struct dst_entry *dst;
>> struct sk_buff *head;
>> struct sctphdr *sh;
>> @@ -625,13 +624,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
>> asoc->peer.last_sent_to = tp;
>> }
>> head->ignore_df = packet->ipfragok;
>> - confirm = tp->dst_pending_confirm;
>> - if (confirm)
>> + if (tp->dst_pending_confirm)
>> skb_set_dst_pending_confirm(head, 1);
>> /* neighbour should be confirmed on successful transmission or
>> * positive error
>> */
>> - if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
>> + if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
>> + tp->dst_pending_confirm)
>> tp->dst_pending_confirm = 0;
>>
>> out:
>> --
>
> I played safe here, I was not sure if currently
> or some day in the future the SCTP stack can allow another
> thread to set tp->dst_pending_confirm concurrently with the
> sending. My idea was only when skb was used to confirm
> neighbour only then to clear the indication. I guess, your
> patch is ok because we should be locking the socket
> everywhere.
Yeps, It's safe, as all the codes for dst_pending_confirm are
under the sock lock protection.
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
2017-03-18 11:12 ` Xin Long
@ 2017-03-18 19:05 ` Neil Horman
-1 siblings, 0 replies; 10+ messages in thread
From: Neil Horman @ 2017-03-18 19:05 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Vlad Yasevich, ja
On Sat, Mar 18, 2017 at 07:12:22PM +0800, Xin Long wrote:
> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
> a temporary variable "confirm" in sctp_packet_transmit.
>
> But it broke the rule that longer lines should be above shorter ones.
> Besides, this variable is not necessary, so this patch is to just
> remove it and use tp->dst_pending_confirm directly.
>
> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/output.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 71ce6b9..1224421 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -546,7 +546,6 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> struct sctp_association *asoc = tp->asoc;
> struct sctp_chunk *chunk, *tmp;
> int pkt_count, gso = 0;
> - int confirm;
> struct dst_entry *dst;
> struct sk_buff *head;
> struct sctphdr *sh;
> @@ -625,13 +624,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> asoc->peer.last_sent_to = tp;
> }
> head->ignore_df = packet->ipfragok;
> - confirm = tp->dst_pending_confirm;
> - if (confirm)
> + if (tp->dst_pending_confirm)
> skb_set_dst_pending_confirm(head, 1);
> /* neighbour should be confirmed on successful transmission or
> * positive error
> */
> - if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
> + if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> + tp->dst_pending_confirm)
> tp->dst_pending_confirm = 0;
>
> out:
> --
> 2.1.0
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
@ 2017-03-18 19:05 ` Neil Horman
0 siblings, 0 replies; 10+ messages in thread
From: Neil Horman @ 2017-03-18 19:05 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
Vlad Yasevich, ja
On Sat, Mar 18, 2017 at 07:12:22PM +0800, Xin Long wrote:
> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
> a temporary variable "confirm" in sctp_packet_transmit.
>
> But it broke the rule that longer lines should be above shorter ones.
> Besides, this variable is not necessary, so this patch is to just
> remove it and use tp->dst_pending_confirm directly.
>
> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/output.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 71ce6b9..1224421 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -546,7 +546,6 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> struct sctp_association *asoc = tp->asoc;
> struct sctp_chunk *chunk, *tmp;
> int pkt_count, gso = 0;
> - int confirm;
> struct dst_entry *dst;
> struct sk_buff *head;
> struct sctphdr *sh;
> @@ -625,13 +624,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
> asoc->peer.last_sent_to = tp;
> }
> head->ignore_df = packet->ipfragok;
> - confirm = tp->dst_pending_confirm;
> - if (confirm)
> + if (tp->dst_pending_confirm)
> skb_set_dst_pending_confirm(head, 1);
> /* neighbour should be confirmed on successful transmission or
> * positive error
> */
> - if (tp->af_specific->sctp_xmit(head, tp) >= 0 && confirm)
> + if (tp->af_specific->sctp_xmit(head, tp) >= 0 &&
> + tp->dst_pending_confirm)
> tp->dst_pending_confirm = 0;
>
> out:
> --
> 2.1.0
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
2017-03-18 11:12 ` Xin Long
@ 2017-03-22 1:31 ` David Miller
-1 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-03-22 1:31 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, vyasevich, ja
From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 18 Mar 2017 19:12:22 +0800
> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
> a temporary variable "confirm" in sctp_packet_transmit.
>
> But it broke the rule that longer lines should be above shorter ones.
> Besides, this variable is not necessary, so this patch is to just
> remove it and use tp->dst_pending_confirm directly.
>
> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net] sctp: remove temporary variable confirm from sctp_packet_transmit
@ 2017-03-22 1:31 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-03-22 1:31 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, vyasevich, ja
From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 18 Mar 2017 19:12:22 +0800
> Commit c86a773c7802 ("sctp: add dst_pending_confirm flag") introduced
> a temporary variable "confirm" in sctp_packet_transmit.
>
> But it broke the rule that longer lines should be above shorter ones.
> Besides, this variable is not necessary, so this patch is to just
> remove it and use tp->dst_pending_confirm directly.
>
> Fixes: c86a773c7802 ("sctp: add dst_pending_confirm flag")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread