Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1421489 > unrolled thread

[PATCH 0/5] ipvs: fix backup sync daemon with IPv6, and minor updates

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] ipvs: fix backup sync daemon with IPv6, and minor updates Quentin Armitage <quentin@armitage.org.uk> - 2016-06-14 04:00 +0200
    [PATCH 3/5] ipvs: Don't check result < 0 after setting result = 0 Quentin Armitage <quentin@armitage.org.uk> - 2016-06-14 04:00 +0200
    [PATCH 2/5] ipvs: Stop calling __dev_get_by_name() repeatedly when starting sync daemon Quentin Armitage <quentin@armitage.org.uk> - 2016-06-14 04:00 +0200
    [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE Quentin Armitage <quentin@armitage.org.uk> - 2016-06-14 04:00 +0200
      Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE Eric Dumazet <eric.dumazet@gmail.com> - 2016-06-14 05:20 +0200
      Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE Eric Dumazet <eric.dumazet@gmail.com> - 2016-06-14 05:20 +0200
    [PATCH 5/5] ipvs: log additional sync daemon parameters Quentin Armitage <quentin@armitage.org.uk> - 2016-06-14 04:00 +0200
    [PATCH 1/5] ipvs: Enable setting IPv6 multicast address for ipvs sync daemon backup Quentin Armitage <quentin@armitage.org.uk> - 2016-06-14 04:00 +0200

#1421489 — [PATCH 0/5] ipvs: fix backup sync daemon with IPv6, and minor updates

This series of patches arise from discovering that:
ipvsadm --start-daemon backup --mcast-group IPv6_address ...
would always fail.

The first patch resolves the problem. The second and third patches are
optimizations that were noticed while investigating the original problem.
The fourth patch adds a lock which appears to have been omitted, and the
final patch adds the recently added sync daemon multicast parameters to
the log messages that are written when the sync daemons start.

Quentin Armitage (5):
  ipvs: Enable setting IPv6 multicast address for ipvs sync daemon
    backup
  ipvs: Stop calling __dev_get_by_name() repeatedly when starting sync
    daemon
  ipvs: Don't check result < 0 after setting result = 0
  ipvs: Lock socket before setting SK_CAN_REUSE
  ipvs: log additional sync daemon parameters

 net/netfilter/ipvs/ip_vs_sync.c |  107 ++++++++++++++++++++-------------------
 1 files changed, 54 insertions(+), 53 deletions(-)

-- 
1.7.7.6

[toc] | [next] | [standalone]

#1421491 — [PATCH 3/5] ipvs: Don't check result < 0 after setting result = 0

Move the block testing result < 0 to avoid the test immediately
after setting result = 0

Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
---
 net/netfilter/ipvs/ip_vs_sync.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 3181fd0..29d73d8 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1497,13 +1497,13 @@ static struct socket *make_send_sock(struct netns_ipvs *ipvs, int id, struct net
 			  ipvs->mcast_ifn, &addr);
 
 		result = bind_mcastif_addr(sock, dev);
+		if (result < 0) {
+			pr_err("Error binding address of the mcast interface\n");
+			goto error;
+		}
 	}
 	else
 		result = 0;
-	if (result < 0) {
-		pr_err("Error binding address of the mcast interface\n");
-		goto error;
-	}
 
 	get_mcast_sockaddr(&mcast_addr, &salen, &ipvs->mcfg, id);
 	result = sock->ops->connect(sock, (struct sockaddr *) &mcast_addr,
-- 
1.7.7.6

[toc] | [prev] | [next] | [standalone]

#1421495 — [PATCH 2/5] ipvs: Stop calling __dev_get_by_name() repeatedly when starting sync daemon

Optimise starting sync daemons by using the result of the first call to
__dev_get_by_name() and pass the result or ifindex to subsequent functions
to avoid them having to call __dev_get_by_name() again.

Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
---
 net/netfilter/ipvs/ip_vs_sync.c |   66 +++++++++++++-------------------------
 1 files changed, 23 insertions(+), 43 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 85b48f1..3181fd0 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1356,28 +1356,22 @@ static void set_mcast_pmtudisc(struct sock *sk, int val)
 /*
  *      Specifiy default interface for outgoing multicasts
  */
-static int set_mcast_if(struct sock *sk, char *ifname)
+static int set_mcast_if(struct sock *sk, int ifindex)
 {
-	struct net_device *dev;
 	struct inet_sock *inet = inet_sk(sk);
-	struct net *net = sock_net(sk);
-
-	dev = __dev_get_by_name(net, ifname);
-	if (!dev)
-		return -ENODEV;
 
-	if (sk->sk_bound_dev_if && dev->ifindex != sk->sk_bound_dev_if)
+	if (sk->sk_bound_dev_if && ifindex != sk->sk_bound_dev_if)
 		return -EINVAL;
 
 	lock_sock(sk);
-	inet->mc_index = dev->ifindex;
+	inet->mc_index = ifindex;
 	/*  inet->mc_addr  = 0; */
 #ifdef CONFIG_IP_VS_IPV6
 	if (sk->sk_family == AF_INET6) {
 		struct ipv6_pinfo *np = inet6_sk(sk);
 
 		/* IPV6_MULTICAST_IF */
-		np->mcast_oif = dev->ifindex;
+		np->mcast_oif = ifindex;
 	}
 #endif
 	release_sock(sk);
@@ -1392,23 +1386,18 @@ static int set_mcast_if(struct sock *sk, char *ifname)
  *      in the in_addr structure passed in as a parameter.
  */
 static int
-join_mcast_group(struct sock *sk, struct in_addr *addr, char *ifname)
+join_mcast_group(struct sock *sk, struct in_addr *addr, int ifindex)
 {
-	struct net *net = sock_net(sk);
 	struct ip_mreqn mreq;
-	struct net_device *dev;
 	int ret;
 
 	memset(&mreq, 0, sizeof(mreq));
 	memcpy(&mreq.imr_multiaddr, addr, sizeof(struct in_addr));
 
-	dev = __dev_get_by_name(net, ifname);
-	if (!dev)
-		return -ENODEV;
-	if (sk->sk_bound_dev_if && dev->ifindex != sk->sk_bound_dev_if)
+	if (sk->sk_bound_dev_if && ifindex != sk->sk_bound_dev_if)
 		return -EINVAL;
 
-	mreq.imr_ifindex = dev->ifindex;
+	mreq.imr_ifindex = ifindex;
 
 	lock_sock(sk);
 	ret = ip_mc_join_group(sk, &mreq);
@@ -1419,45 +1408,31 @@ join_mcast_group(struct sock *sk, struct in_addr *addr, char *ifname)
 
 #ifdef CONFIG_IP_VS_IPV6
 static int join_mcast_group6(struct sock *sk, struct in6_addr *addr,
-			     char *ifname)
+			     int ifindex)
 {
-	struct net *net = sock_net(sk);
-	struct net_device *dev;
 	int ret;
 
-	dev = __dev_get_by_name(net, ifname);
-	if (!dev)
-		return -ENODEV;
-	if (sk->sk_bound_dev_if && dev->ifindex != sk->sk_bound_dev_if)
+	if (sk->sk_bound_dev_if && ifindex != sk->sk_bound_dev_if)
 		return -EINVAL;
 
 	lock_sock(sk);
-	ret = ipv6_sock_mc_join(sk, dev->ifindex, addr);
+	ret = ipv6_sock_mc_join(sk, ifindex, addr);
 	release_sock(sk);
 
 	return ret;
 }
 #endif
 
-static int bind_mcastif_addr(struct socket *sock, char *ifname)
+static int bind_mcastif_addr(struct socket *sock, struct net_device *dev)
 {
-	struct net *net = sock_net(sock->sk);
-	struct net_device *dev;
 	__be32 addr;
 	struct sockaddr_in sin;
 
-	dev = __dev_get_by_name(net, ifname);
-	if (!dev)
-		return -ENODEV;
-
 	addr = inet_select_addr(dev, 0, RT_SCOPE_UNIVERSE);
 	if (!addr)
 		pr_err("You probably need to specify IP address on "
 		       "multicast interface.\n");
 
-	IP_VS_DBG(7, "binding socket with (%s) %pI4\n",
-		  ifname, &addr);
-
 	/* Now bind the socket with the address of multicast interface */
 	sin.sin_family	     = AF_INET;
 	sin.sin_addr.s_addr  = addr;
@@ -1489,7 +1464,7 @@ static void get_mcast_sockaddr(union ipvs_sockaddr *sa, int *salen,
 /*
  *      Set up sending multicast socket over UDP
  */
-static struct socket *make_send_sock(struct netns_ipvs *ipvs, int id)
+static struct socket *make_send_sock(struct netns_ipvs *ipvs, int id, struct net_device *dev)
 {
 	/* multicast addr */
 	union ipvs_sockaddr mcast_addr;
@@ -1503,7 +1478,7 @@ static struct socket *make_send_sock(struct netns_ipvs *ipvs, int id)
 		pr_err("Error during creation of socket; terminating\n");
 		return ERR_PTR(result);
 	}
-	result = set_mcast_if(sock->sk, ipvs->mcfg.mcast_ifn);
+	result = set_mcast_if(sock->sk, dev->ifindex);
 	if (result < 0) {
 		pr_err("Error setting outbound mcast interface\n");
 		goto error;
@@ -1517,8 +1492,12 @@ static struct socket *make_send_sock(struct netns_ipvs *ipvs, int id)
 	if (result > 0)
 		set_sock_size(sock->sk, 1, result);
 
-	if (AF_INET == ipvs->mcfg.mcast_af)
-		result = bind_mcastif_addr(sock, ipvs->mcfg.mcast_ifn);
+	if (AF_INET == ipvs->mcfg.mcast_af) {
+		IP_VS_DBG(7, "binding socket with (%s) %pI4\n",
+			  ipvs->mcast_ifn, &addr);
+
+		result = bind_mcastif_addr(sock, dev);
+	}
 	else
 		result = 0;
 	if (result < 0) {
@@ -1559,6 +1538,7 @@ static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id, int ifi
 		pr_err("Error during creation of socket; terminating\n");
 		return ERR_PTR(result);
 	}
+
 	/* it is equivalent to the REUSEADDR option in user-space */
 	sock->sk->sk_reuse = SK_CAN_REUSE;
 	result = sysctl_sync_sock_size(ipvs);
@@ -1577,11 +1557,11 @@ static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id, int ifi
 #ifdef CONFIG_IP_VS_IPV6
 	if (ipvs->bcfg.mcast_af == AF_INET6)
 		result = join_mcast_group6(sock->sk, &mcast_addr.in6.sin6_addr,
-					   ipvs->bcfg.mcast_ifn);
+					   ifindex);
 	else
 #endif
 		result = join_mcast_group(sock->sk, &mcast_addr.in.sin_addr,
-					  ipvs->bcfg.mcast_ifn);
+					  ifindex);
 	if (result < 0) {
 		pr_err("Error joining to the multicast group\n");
 		goto error;
@@ -1867,7 +1847,7 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
 	tinfo = NULL;
 	for (id = 0; id < count; id++) {
 		if (state == IP_VS_STATE_MASTER)
-			sock = make_send_sock(ipvs, id);
+			sock = make_send_sock(ipvs, id, dev);
 		else
 			sock = make_receive_sock(ipvs, id, dev->ifindex);
 		if (IS_ERR(sock)) {
-- 
1.7.7.6

[toc] | [prev] | [next] | [standalone]

#1421496 — [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE

When other settings are changed in the socket it is locked, so
lock the socket before setting SK_CAN_REUSE.

Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
---
 net/netfilter/ipvs/ip_vs_sync.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 29d73d8..dfac9ef 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1540,7 +1540,9 @@ static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id, int ifi
 	}
 
 	/* it is equivalent to the REUSEADDR option in user-space */
+	lock_sock(sock->sk);
 	sock->sk->sk_reuse = SK_CAN_REUSE;
+	release_sock(sock->sk);
 	result = sysctl_sync_sock_size(ipvs);
 	if (result > 0)
 		set_sock_size(sock->sk, 0, result);
-- 
1.7.7.6

[toc] | [prev] | [next] | [standalone]

#1421555 — Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE

On Mon, 2016-06-13 at 20:12 -0700, Eric Dumazet wrote:

> 
> Have you tested this patch ?

Hmm, please ignore this question ;)

[toc] | [prev] | [next] | [standalone]

#1421557 — Re: [PATCH 4/5] ipvs: Lock socket before setting SK_CAN_REUSE

On Tue, 2016-06-14 at 02:43 +0100, Quentin Armitage wrote:
> When other settings are changed in the socket it is locked, so
> lock the socket before setting SK_CAN_REUSE.
> 
> Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
> ---
>  net/netfilter/ipvs/ip_vs_sync.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 29d73d8..dfac9ef 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -1540,7 +1540,9 @@ static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id, int ifi
>  	}
>  
>  	/* it is equivalent to the REUSEADDR option in user-space */
> +	lock_sock(sock->sk);
>  	sock->sk->sk_reuse = SK_CAN_REUSE;
> +	release_sock(sock->sk);
>  	result = sysctl_sync_sock_size(ipvs);
>  	if (result > 0)
>  		set_sock_size(sock->sk, 0, result);


Have you tested this patch ?

[toc] | [prev] | [next] | [standalone]

#1421498 — [PATCH 5/5] ipvs: log additional sync daemon parameters

Add new multicast parameters to log messages when sync daemons start.

Commits <e4ff67513096> ("ipvs: add sync_maxlen parameter for the sync
daemon") and <d33288172e72> ("ipvs: add more mcast parameters for the
sync daemon") added additional multicast parameters, but didn't add
them to the log messages when the sync daemons started.

Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
---
 net/netfilter/ipvs/ip_vs_sync.c |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index dfac9ef..e5f6739 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1670,8 +1670,17 @@ static int sync_thread_master(void *data)
 	struct ip_vs_sync_buff *sb;
 
 	pr_info("sync thread started: state = MASTER, mcast_ifn = %s, "
-		"syncid = %d, id = %d\n",
-		ipvs->mcfg.mcast_ifn, ipvs->mcfg.syncid, tinfo->id);
+			"syncid = %d, id = %d, maxlen = %d\n",
+			ipvs->mcfg.mcast_ifn, ipvs->mcfg.syncid,
+			tinfo->id, ipvs->mcfg.sync_maxlen);
+	if (ipvs->mcfg.mcast_af == AF_INET)
+		pr_info("                   : group = %pI4, port = %d, ttl = %d\n",
+			&ipvs->mcfg.mcast_group.in, ipvs->mcfg.mcast_port,
+			ipvs->mcfg.mcast_ttl);
+	else
+		pr_info("                   : group = %pI6c, port = %d, ttl = %d\n",
+			&ipvs->mcfg.mcast_group.in6, ipvs->mcfg.mcast_port,
+			ipvs->mcfg.mcast_ttl);
 
 	for (;;) {
 		sb = next_sync_buff(ipvs, ms);
@@ -1724,8 +1733,17 @@ static int sync_thread_backup(void *data)
 	int len;
 
 	pr_info("sync thread started: state = BACKUP, mcast_ifn = %s, "
-		"syncid = %d, id = %d\n",
-		ipvs->bcfg.mcast_ifn, ipvs->bcfg.syncid, tinfo->id);
+			"syncid = %d, id = %d, maxlen = %d\n",
+			ipvs->bcfg.mcast_ifn, ipvs->bcfg.syncid,
+			tinfo->id, ipvs->bcfg.sync_maxlen);
+	if (ipvs->bcfg.mcast_af == AF_INET)
+		pr_info("                   : group = %pI4, port = %d, ttl = %d\n",
+			&ipvs->bcfg.mcast_group.in, ipvs->bcfg.mcast_port,
+			ipvs->bcfg.mcast_ttl);
+	else
+		pr_info("                   : group = %pI6c, port = %d, ttl = %d\n",
+			&ipvs->bcfg.mcast_group.in6, ipvs->bcfg.mcast_port,
+			ipvs->bcfg.mcast_ttl);
 
 	while (!kthread_should_stop()) {
 		wait_event_interruptible(*sk_sleep(tinfo->sock->sk),
-- 
1.7.7.6

[toc] | [prev] | [next] | [standalone]

#1421499 — [PATCH 1/5] ipvs: Enable setting IPv6 multicast address for ipvs sync daemon backup

When using HEAD from https://git.kernel.org/cgit/utils/kernel/ipvsadm/ipvsadm.git/,
the command:
ipvsadm --start-daemon backup --mcast-interface eth0.60 --mcast-group ff01::1:81
fails with the error message:
Argument list too long

whereas both:
ipvsadm --start-daemon master --mcast-interface eth0.60 --mcast-group ff01::1:81
and:
ipvsadm --start-daemon backup --mcast-interface eth0.60 --mcast-group 224.0.0.81
are successful.

The error message "Argument list too long" isn't helpful. The error occurs
because an IPv6 address is given in backup mode.

The error is in make_receive_sock() in net/netfilter/ipvs/ip_vs_sync.c, since
it fails to set the interface on the address or the socket before calling
inet6_bind() (via sock->ops->bind), where the test 'if (!sk->sk_bound_dev_if)'
failed.

Setting sin6_scope_id on the sockaddr before calling inet6_bind() resolves
the issue.

Signed-off-by: Quentin Armitage <quentin@armitage.org.uk>
---
 net/netfilter/ipvs/ip_vs_sync.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 803001a..85b48f1 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1545,7 +1545,7 @@ error:
 /*
  *      Set up receiving multicast socket over UDP
  */
-static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id)
+static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id, int ifindex)
 {
 	/* multicast addr */
 	union ipvs_sockaddr mcast_addr;
@@ -1566,6 +1566,7 @@ static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id)
 		set_sock_size(sock->sk, 0, result);
 
 	get_mcast_sockaddr(&mcast_addr, &salen, &ipvs->bcfg, id);
+	((struct sockaddr_in6 *)&mcast_addr)->sin6_scope_id = ifindex;
 	result = sock->ops->bind(sock, (struct sockaddr *)&mcast_addr, salen);
 	if (result < 0) {
 		pr_err("Error binding to the multicast addr\n");
@@ -1868,7 +1869,7 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
 		if (state == IP_VS_STATE_MASTER)
 			sock = make_send_sock(ipvs, id);
 		else
-			sock = make_receive_sock(ipvs, id);
+			sock = make_receive_sock(ipvs, id, dev->ifindex);
 		if (IS_ERR(sock)) {
 			result = PTR_ERR(sock);
 			goto outtinfo;
-- 
1.7.7.6

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web