diff mbox

[1/3] Set socket flags on restore using sock_setsockopt() where possible (v2)

Message ID 1251134884-24491-2-git-send-email-danms@us.ibm.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Dan Smith Aug. 24, 2009, 5:28 p.m. UTC 
Fail on the TIMESTAMPING_* flags for the moment, with a TODO in place to
handle them later.

Also remove other explicit flag checks because they're no longer copied
blindly into the socket object, so existing checks will be sufficient.

Changes in v2:
 - Avoid removing the sock->sk_socket check before sync'ing the socket.flags
 - Rename sock_rst_flags() to sock_restore_flags()
 - Rebase on top of Oren's cleanup patch

Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: Dan Smith <danms@us.ibm.com>
---
 net/checkpoint.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 102 insertions(+), 11 deletions(-)

Comments

Dave Hansen Aug. 24, 2009, 5:48 p.m. UTC | #1 
On Mon, 2009-08-24 at 10:28 -0700, Dan Smith wrote:
> 
> +static int sock_restore_flags(struct socket *sock,
> +                             struct ckpt_hdr_socket *h)
> +{
> +       int ret;
> +       int v = 1;
> +       unsigned long sk_flags = h->sock.flags;
> +       unsigned long sock_flags = h->socket.flags;
> +
> +       if (test_and_clear_bit(SOCK_URGINLINE, &sk_flags)) {
> +               ret = sock_setsockopt(sock, SOL_SOCKET, SO_OOBINLINE,
> +                                     (char *)&v, sizeof(v));
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (test_and_clear_bit(SOCK_KEEPOPEN, &sk_flags)) {
> +               ret = sock_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
> +                                     (char *)&v, sizeof(v));
> +               if (ret)
> +                       return ret;
> +       }

Would it make more sense to do this programatically?

cr_sock_restore_flag(struct socket *sock, unsigned long sck_flag,
		     struct ckpt_hdr_socket *h, unsigned long sock_flag)
{
	unsigned long sk_flags = h->sock.flags;
	unsigned long sock_flags = h->socket.flags;

	
	if (!test_and_clear_bit(sk, &sk_flags))
		return 0;

	return sock_setsockopt(sock, sock_flag, SO_OOBINLINE,
 			       (char *)&v, sizeof(v));
}

Then, each call becomes:

	ret = cr_sock_restore_flag(sock, cr_sock, SOCK_URGINLINE, SO_OOBINLINE);
	if (ret)
		return ret;

	ret = cr_sock_restore_flag(sock, cr_sock, SOCK_KEEPOPEN, SO_KEEPALIVE);
	if (ret)
		return ret;

Or, you could spell the flags out in a (better named) structure:

struct sock_flagpair
{
	unsigned long sock_flag;
	unsigned long sk_flag;
}	

struct sock_flagpair sock_flagpairs[] = {
	{ SOCK_URGINLINE, SO_OOBINLINE },
	{ SOCK_KEEPOPEN, SO_KEEPALIVE},
	...
};

And just walk through the array to do the restore:

static int sock_restore_flags(struct socket *sock,
                             struct ckpt_hdr_socket *h)
{
	int i;

	for (i = 0; i < ARRAY_SIZE(sock_flagpairs); i++) {
		int ret;
		unsigned long sock_flag = sock_flagpairs[i].sock_flag;
		unsigned long sk_flag = sock_flagpairs[i].sk_flag;
		ret = cr_sock_restore_flag(sock, cr_sock, sock_flag, sk_flag);
		if (ret)
			break;
	}
	...
}

-- Dave

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Smith Aug. 24, 2009, 6:03 p.m. UTC | #2 
DH> Would it make more sense to do this programatically?

If it pleases the court, sure :)
diff mbox

Patch

diff --git a/net/checkpoint.c b/net/checkpoint.c
index c64483e..fdbf8e7 100644
--- a/net/checkpoint.c
+++ b/net/checkpoint.c
@@ -178,10 +178,6 @@  static int sock_cptrst_verify(struct ckpt_hdr_socket *h)
 	if (!ckpt_validate_errno(h->sock.err))
 		return -EINVAL;
 
-	/* None of our supported types use this flag */
-	if (h->sock.flags & SOCK_DESTROY)
-		return -EINVAL;
-
 	return 0;
 }
 
@@ -238,11 +234,97 @@  static int sock_cptrst_bufopts(int op, struct sock *sk,
 	return 0;
 }
 
+static int sock_restore_flags(struct socket *sock,
+                             struct ckpt_hdr_socket *h)
+{
+       int ret;
+       int v = 1;
+       unsigned long sk_flags = h->sock.flags;
+       unsigned long sock_flags = h->socket.flags;
+
+       if (test_and_clear_bit(SOCK_URGINLINE, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_OOBINLINE,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_KEEPOPEN, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_BROADCAST, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_BROADCAST,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_RCVTSTAMP, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_TIMESTAMP,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_RCVTSTAMPNS, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_TIMESTAMPNS,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_DBG, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_DEBUG,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_LOCALROUTE, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_DONTROUTE,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_PASSCRED, &sock_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_PASSCRED,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       /* TODO: Handle SOCK_TIMESTAMPING_* flags */
+       if (test_bit(SOCK_TIMESTAMPING_TX_HARDWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_TX_SOFTWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_RX_HARDWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_RX_SOFTWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_SOFTWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_RAW_HARDWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_SYS_HARDWARE, &sk_flags)) {
+               ckpt_debug("SOF_TIMESTAMPING_* flags are not supported\n");
+               return -ENOSYS;
+       }
+
+       /* Anything that is still set in the flags that isn't part of
+        * our protocol's default set, indicates an error
+        */
+       if (sk_flags & ~sock->sk->sk_flags) {
+               ckpt_debug("Unhandled sock flags: %lx\n", sk_flags);
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
 static int sock_cptrst(struct ckpt_ctx *ctx, struct sock *sk,
 		       struct ckpt_hdr_socket *h, int op)
 {
 	if (sk->sk_socket) {
-		CKPT_COPY(op, h->socket.flags, sk->sk_socket->flags);
 		CKPT_COPY(op, h->socket.state, sk->sk_socket->state);
 	}
 
@@ -259,12 +341,6 @@  static int sock_cptrst(struct ckpt_ctx *ctx, struct sock *sk,
 	CKPT_COPY(op, h->sock.state, sk->sk_state);
 	CKPT_COPY(op, h->sock.backlog, sk->sk_max_ack_backlog);
 
-	/* TODO:
-	 * Break out setting each of the flags to use setsockopt() or
-	 * perform proper security check
-	 */
-	CKPT_COPY(op, h->sock.flags, sk->sk_flags);
-
 	if (sock_cptrst_bufopts(op, sk, h))
 		return -EINVAL;
 
@@ -298,6 +374,21 @@  static int sock_cptrst(struct ckpt_ctx *ctx, struct sock *sk,
 		return -EINVAL;
 	}
 
+	if (op == CKPT_CPT) {
+		h->sock.flags = sk->sk_flags;
+		h->socket.flags = sk->sk_socket->flags;
+	} else {
+		int ret;
+		mm_segment_t old_fs;
+
+		old_fs = get_fs();
+		set_fs(KERNEL_DS);
+		ret = sock_restore_flags(sk->sk_socket, h);
+		set_fs(old_fs);
+		if (ret)
+			return ret;
+	}
+
 	if ((h->socket.state == SS_CONNECTED) &&
 	    (h->sock.state != TCP_ESTABLISHED)) {
 		ckpt_debug("socket/sock in inconsistent state: %i/%i",