* [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list
2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long
@ 2016-12-20 5:49 ` Xin Long
2016-12-20 11:30 ` Marcelo Ricardo Leitner
2016-12-20 11:29 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Marcelo Ricardo Leitner
2016-12-21 14:16 ` Neil Horman
2 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2016-12-20 5:49 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman
sctp.local_addr_list is a global address list that is supposed to include
all the local addresses. sctp updates this list according to NETDEV_UP/
NETDEV_DOWN notifications.
However, if multiple NICs have the same address, the global list would
have duplicate addresses. Even if for one NIC, promote secondaries in
__inet_del_ifa can also lead to accumulating duplicate addresses.
When sctp binds address 'ANY' and creates a connection, it copies all
the addresses from global list into asoc's bind addr list, which makes
sctp pack the duplicate addresses into INIT/INIT_ACK packets.
This patch is to filter the duplicate addresses when copying the addrs
from global list in sctp_copy_local_addr_list and unpacking addr_param
from cookie in sctp_raw_to_bind_addrs to asoc's bind addr list.
Note that we can't filter the duplicate addrs when global address list
gets updated, As NETDEV_DOWN event may remove an addr that still exists
in another NIC.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/bind_addr.c | 3 +++
net/sctp/protocol.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index 401c607..1ebc184 100644
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -292,6 +292,8 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list,
}
af->from_addr_param(&addr, rawaddr, htons(port), 0);
+ if (sctp_bind_addr_state(bp, &addr) != -1)
+ goto next;
retval = sctp_add_bind_addr(bp, &addr, sizeof(addr),
SCTP_ADDR_SRC, gfp);
if (retval) {
@@ -300,6 +302,7 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list,
break;
}
+next:
len = ntohs(param->length);
addrs_len -= len;
raw_addr_list += len;
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index da5d82b..616a942 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -220,6 +220,9 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp,
!(copy_flags & SCTP_ADDR6_PEERSUPP)))
continue;
+ if (sctp_bind_addr_state(bp, &addr->a) != -1)
+ continue;
+
error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a),
SCTP_ADDR_SRC, GFP_ATOMIC);
if (error)
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list
2016-12-20 5:49 ` [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long
@ 2016-12-20 11:30 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-12-20 11:30 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
On Tue, Dec 20, 2016 at 01:49:50PM +0800, Xin Long wrote:
> sctp.local_addr_list is a global address list that is supposed to include
> all the local addresses. sctp updates this list according to NETDEV_UP/
> NETDEV_DOWN notifications.
>
> However, if multiple NICs have the same address, the global list would
> have duplicate addresses. Even if for one NIC, promote secondaries in
> __inet_del_ifa can also lead to accumulating duplicate addresses.
>
> When sctp binds address 'ANY' and creates a connection, it copies all
> the addresses from global list into asoc's bind addr list, which makes
> sctp pack the duplicate addresses into INIT/INIT_ACK packets.
>
> This patch is to filter the duplicate addresses when copying the addrs
> from global list in sctp_copy_local_addr_list and unpacking addr_param
> from cookie in sctp_raw_to_bind_addrs to asoc's bind addr list.
>
> Note that we can't filter the duplicate addrs when global address list
> gets updated, As NETDEV_DOWN event may remove an addr that still exists
> in another NIC.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/bind_addr.c | 3 +++
> net/sctp/protocol.c | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
> index 401c607..1ebc184 100644
> --- a/net/sctp/bind_addr.c
> +++ b/net/sctp/bind_addr.c
> @@ -292,6 +292,8 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list,
> }
>
> af->from_addr_param(&addr, rawaddr, htons(port), 0);
> + if (sctp_bind_addr_state(bp, &addr) != -1)
> + goto next;
> retval = sctp_add_bind_addr(bp, &addr, sizeof(addr),
> SCTP_ADDR_SRC, gfp);
> if (retval) {
> @@ -300,6 +302,7 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list,
> break;
> }
>
> +next:
> len = ntohs(param->length);
> addrs_len -= len;
> raw_addr_list += len;
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index da5d82b..616a942 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -220,6 +220,9 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp,
> !(copy_flags & SCTP_ADDR6_PEERSUPP)))
> continue;
>
> + if (sctp_bind_addr_state(bp, &addr->a) != -1)
> + continue;
> +
> error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a),
> SCTP_ADDR_SRC, GFP_ATOMIC);
> if (error)
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list
2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long
2016-12-20 5:49 ` [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long
@ 2016-12-20 11:29 ` Marcelo Ricardo Leitner
2016-12-21 14:16 ` Neil Horman
2 siblings, 0 replies; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-12-20 11:29 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
On Tue, Dec 20, 2016 at 01:49:49PM +0800, Xin Long wrote:
> This patch is to reduce indent level by using continue when the addr
> is not allowed, and also drop end_copy by using break.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/protocol.c | 37 +++++++++++++++++++------------------
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 7b523e3..da5d82b 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -205,26 +205,27 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp,
> list_for_each_entry_rcu(addr, &net->sctp.local_addr_list, list) {
> if (!addr->valid)
> continue;
> - if (sctp_in_scope(net, &addr->a, scope)) {
> - /* Now that the address is in scope, check to see if
> - * the address type is really supported by the local
> - * sock as well as the remote peer.
> - */
> - if ((((AF_INET == addr->a.sa.sa_family) &&
> - (copy_flags & SCTP_ADDR4_PEERSUPP))) ||
> - (((AF_INET6 == addr->a.sa.sa_family) &&
> - (copy_flags & SCTP_ADDR6_ALLOWED) &&
> - (copy_flags & SCTP_ADDR6_PEERSUPP)))) {
> - error = sctp_add_bind_addr(bp, &addr->a,
> - sizeof(addr->a),
> - SCTP_ADDR_SRC, GFP_ATOMIC);
> - if (error)
> - goto end_copy;
> - }
> - }
> + if (!sctp_in_scope(net, &addr->a, scope))
> + continue;
> +
> + /* Now that the address is in scope, check to see if
> + * the address type is really supported by the local
> + * sock as well as the remote peer.
> + */
> + if (addr->a.sa.sa_family == AF_INET &&
> + !(copy_flags & SCTP_ADDR4_PEERSUPP))
> + continue;
> + if (addr->a.sa.sa_family == AF_INET6 &&
> + (!(copy_flags & SCTP_ADDR6_ALLOWED) ||
> + !(copy_flags & SCTP_ADDR6_PEERSUPP)))
> + continue;
> +
> + error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a),
> + SCTP_ADDR_SRC, GFP_ATOMIC);
> + if (error)
> + break;
> }
>
> -end_copy:
> rcu_read_unlock();
> return error;
> }
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list
2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long
2016-12-20 5:49 ` [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long
2016-12-20 11:29 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Marcelo Ricardo Leitner
@ 2016-12-21 14:16 ` Neil Horman
2 siblings, 0 replies; 6+ messages in thread
From: Neil Horman @ 2016-12-21 14:16 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner
On Tue, Dec 20, 2016 at 01:49:49PM +0800, Xin Long wrote:
> This patch is to reduce indent level by using continue when the addr
> is not allowed, and also drop end_copy by using break.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/protocol.c | 37 +++++++++++++++++++------------------
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 7b523e3..da5d82b 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -205,26 +205,27 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp,
> list_for_each_entry_rcu(addr, &net->sctp.local_addr_list, list) {
> if (!addr->valid)
> continue;
> - if (sctp_in_scope(net, &addr->a, scope)) {
> - /* Now that the address is in scope, check to see if
> - * the address type is really supported by the local
> - * sock as well as the remote peer.
> - */
> - if ((((AF_INET == addr->a.sa.sa_family) &&
> - (copy_flags & SCTP_ADDR4_PEERSUPP))) ||
> - (((AF_INET6 == addr->a.sa.sa_family) &&
> - (copy_flags & SCTP_ADDR6_ALLOWED) &&
> - (copy_flags & SCTP_ADDR6_PEERSUPP)))) {
> - error = sctp_add_bind_addr(bp, &addr->a,
> - sizeof(addr->a),
> - SCTP_ADDR_SRC, GFP_ATOMIC);
> - if (error)
> - goto end_copy;
> - }
> - }
> + if (!sctp_in_scope(net, &addr->a, scope))
> + continue;
> +
> + /* Now that the address is in scope, check to see if
> + * the address type is really supported by the local
> + * sock as well as the remote peer.
> + */
> + if (addr->a.sa.sa_family == AF_INET &&
> + !(copy_flags & SCTP_ADDR4_PEERSUPP))
> + continue;
> + if (addr->a.sa.sa_family == AF_INET6 &&
> + (!(copy_flags & SCTP_ADDR6_ALLOWED) ||
> + !(copy_flags & SCTP_ADDR6_PEERSUPP)))
> + continue;
> +
> + error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a),
> + SCTP_ADDR_SRC, GFP_ATOMIC);
> + if (error)
> + break;
> }
>
> -end_copy:
> rcu_read_unlock();
> return error;
> }
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 6+ messages in thread