[PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

These patches together supply secure client-side RxRPC connectivity as a Linux
kernel socket family.  Only the transport/session side is supplied - the
presentation side (marshalling the data) is left to the client.  Copies of the
patches can be found here:

	http://people.redhat.com/~dhowells/rxrpc/01-crypto-kernel-buff.diff
	http://people.redhat.com/~dhowells/rxrpc/02-move-skb-generic.diff
	http://people.redhat.com/~dhowells/rxrpc/03-timers.diff
	http://people.redhat.com/~dhowells/rxrpc/04-keys.diff
	http://people.redhat.com/~dhowells/rxrpc/05-af_rxrpc.diff

The userspace access methods make use of the control data passed to/by
sendmsg() and recvmsg().  See the three simple test programs:

	http://people.redhat.com/~dhowells/rxrpc/klog.c
	http://people.redhat.com/~dhowells/rxrpc/rxrpc.c
	http://people.redhat.com/~dhowells/rxrpc/listen.c

I've attached the current in-kernel documentation to this message.

TODO:

 (*) Make it possible for the client socket to be used to go to more than one
     destination.

 (*) Make fs/afs/ use it and delete the current contents of net/rxrpc/

 (*) Make certain parameters (such as connection timeouts) userspace
     configurable.

 (*) Make userspace utilities use it; librxrpc.

 (*) Userspace documentation.

 (*) KerberosV security.

David


			    ======================
			    RxRPC NETWORK PROTOCOL
			    ======================

The RxRPC protocol driver provides a reliable two-phase transport on top of UDP
that can be used to perform RxRPC remote operations.  This is done over sockets
of AF_RXRPC family, using sendmsg() and recvmsg() with control data to send and
receive data, aborts and errors.

Contents of this document:

 (*) Overview.

 (*) RxRPC protocol summary.

 (*) AF_RXRPC driver model.

 (*) Security.

 (*) Example client usage.

 (*) Example server usage.


========
OVERVIEW
========

RxRPC is a two-layer protocol.  There is a session layer which provides
reliable virtual connections using UDP over IPv4 (or IPv6) as the transport
layer, but implements a real network protocol; and there's the presentation
layer which renders structured data to binary blobs and back again using XDR
(as does SunRPC):

		+-------------+
		| Application |
		+-------------+
		|     XDR     |		Presentation
		+-------------+
		|    RxRPC    |		Session
		+-------------+
		|     UDP     |		Transport
		+-------------+


AF_RXRPC provides:

 (1) Part of an RxRPC facility for both kernel and userspace applications by
     making the session part of it a Linux network protocol (AF_RXRPC).

 (2) A two-phase protocol.  The client transmits a blob (the request) and then
     receives a blob (the reply), and the server receives the request and then
     transmits the reply.

 (3) Retention of the reusable bits of the transport system set up for one call
     to speed up subsequent calls.

 (4) A secure protocol, using the Linux kernel's key retention facility to
     manage security on the client end.  The server end must of necessity be
     more active in security negotiations.

AF_RXRPC does not provide XDR marshalling/presentation facilities.  That is
left to the application.  AF_RXRPC only deals in blobs.  Even the operation ID
is just the first four bytes of the request blob, and as such is beyond the
kernel's interest.


Sockets of AF_RXRPC family are:

 (1) created as type SOCK_RPC;

 (2) provided with a protocol of the type of underlying transport they're going
     to use - currently only PF_INET is supported.


The Andrew File System (AFS) is an example of an application that uses this and
that has both kernel (filesystem) and userspace (utility) components.


======================
RXRPC PROTOCOL SUMMARY
======================

An overview of the RxRPC protocol:

 (*) RxRPC sits on top of another networking protocol (UDP is the only option
     currently), and uses this to provide network transport.  UDP ports, for
     example, provide transport endpoints.

 (*) RxRPC supports multiple virtual "connections" from any given transport
     endpoint, thus allowing the endpoints to be shared, even to the same
     remote endpoint.

 (*) Each connection goes to a particular "service".  A connection may not go
     to multiple services.  A service may be considered the RxRPC equivalent of
     a port number.  AF_RXRPC permits multiple services to share an endpoint.

 (*) Client-originating packets are marked, thus a transport endpoint can be
     shared between client and server connections (connections have a
     direction).

 (*) Up to a billion connections may be supported concurrently between one
     local transport endpoint and one service on one remote endpoint.  An RxRPC
     connection is described by seven numbers:

	Local address	}
	Local port	} Transport (UDP) address
	Remote address	}
	Remote port	}
	Direction
	Connection ID
	Service ID

 (*) Each RxRPC operation is a "call".  A connection may make up to four
     billion calls, but only up to four calls may be in progress on a
     connection at any one time.

 (*) Calls are two-phase and asymmetric: the client sends its request data,
     which the service receives; then the service transmits the reply data
     which the client receives.

 (*) The data blobs are of indefinite size, the end of a phase is marked with a
     flag in the packet.  The number of packets of data making up one blob may
     not exceed 4 billion, however, as this would cause the sequence number to
     wrap.

 (*) The first four bytes of the request data are the service operation ID.

 (*) Security is negotiated on a per-connection basis.  The connection is
     initiated by the first data packet on it arriving.  If security is
     requested, the server then issues a "challenge" and then the client
     replies with a "response".  If the response is successful, the security is
     set for the lifetime of that connection, and all subsequent calls made
     upon it use that same security.  In the event that the server lets a
     connection lapse before the client, the security will be renegotiated if
     the client uses the connection again.

 (*) Calls use ACK packets to handle reliability.  Data packets are also
     explicitly sequenced per call.

 (*) There are two types of positive acknowledgement: hard-ACKs and soft-ACKs.
     A hard-ACK indicates to the far side that all the data received to a point
     has been received and processed; a soft-ACK indicates that the data has
     been received but may yet be discarded and re-requested.  The sender may
     not discard any transmittable packets until they've been hard-ACK'd.

 (*) Reception of a reply data packet implicitly hard-ACK's all the data
     packets that make up the request.

 (*) An call is complete when the request has been sent, the reply has been
     received and the final hard-ACK on the last packet of the reply has
     reached the server.

 (*) An call may be aborted by either end at any time up to its completion.


=====================
AF_RXRPC DRIVER MODEL
=====================

About the AF_RXRPC driver:

 (*) The AF_RXRPC protocol transparently uses internal sockets of the transport
     protocol to represent transport endpoints.

 (*) AF_RXRPC sockets map onto RxRPC connection bundles.  Actual RxRPC
     connections are handled transparently.  One client socket may be used to
     make multiple simultaneous calls to the same service.  One server socket
     may handle calls from many clients.

 (*) Additional parallel client connections will be initiated to support extra
     concurrent calls, up to a tunable limit.

 (*) Each connection is retained for a certain amount of time [tunable] after
     the last call currently using it has completed in case a new call is made
     that could reuse it.

 (*) Each internal UDP socket is retained [tunable] for a certain amount of
     time [tunable] after the last connection using it discarded, in case a new
     connection is made that could use it.

 (*) A client-side connection is only shared between calls if they have have
     the same key struct describing their security (and assuming the calls
     would otherwise share the connection).  Non-secured calls would also be
     able to share connections with each other.

 (*) A server-side connection is shared if the client says it is.

 (*) ACK'ing is handled by the protocol driver automatically, including ping
     replying.

 (*) SO_KEEPALIVE automatically pings the other side to keep the connection
     alive [TODO].

 (*) If an ICMP error is received, all calls affected by that error will be
     aborted with an appropriate network error passed through recvmsg().


Interaction with the user of the RxRPC socket:

 (*) A socket is made into a server socket by binding an address with a
     non-zero service ID.

 (*) In the client, sending a request is achieved with one or more sendmsgs,
     followed by the reply being received with one or more recvmsgs.

 (*) The first sendmsg for a request to be sent from a client contains a tag to
     be used in all other sendmsgs or recvmsgs associated with that call.  The
     tag is carried in the control data.

 (*) Once the client has received the last message associated with a call, the
     tag is guaranteed not to be seen again, and so it can be used to pin
     client resources.  A new call can then be initiated with the same tag
     without fear of interference.

 (*) In the server, a request is received with one or more recvmsgs, then the
     the reply is transmitted with one or more sendmsgs, and then the final ACK
     is received with a last recvmsg].

 (*) When sending data, sendmsg is given MSG_MORE if there's more data to come.

 (*) An abort may be issued by adding an control message to the control data.
     Issuing an abort terminates the kernel's use of that call's tag.

 (*) Aborts, busy notifications and challenge packets are collected by recvmsg
     with control data message to indicate the context.  Receiving an abort or
     a busy message terminates the kernel's use of that call's tag.

 (*) The control data part of the msghdr struct is used for a number of things:

     (*) The tag of the intended or affected call.

     (*) Sending or receiving errors, aborts and busy notifications.

     (*) Sending debug requests and receiving debug replies [TODO].

 (*) When the kernel has received and set up an incoming call, it sends a
     message to server application to let it know there's a new call awaiting
     its acceptance [recvmsg reports a special control message].  The server
     application then uses sendmsg to assign a tag to the new call.  Once that
     is done, the first part of the request data will be delivered by recvmsg.

 (*) The server application has to provide the server socket with a keyring of
     secret keys corresponding to the security types it permits.  When a secure
     connection is being set up, the kernel looks up the appropriate secret key
     in the keyring and then sends a challenge packet to the client and
     receives a response packet.  The kernel then checks the authorisation of
     the packet and either aborts the connection or sets up the security.

 (*) The name of the key a client will use to secure its communications is
     nominated by a socket option.


========
SECURITY
========

Currently, only the kerberos 4 equivalent protocol has been implemented
(security index 2 - rxkad).  This requires the rxkad module to be loaded and,
on the client, tickets of the appropriate type to be obtained from the AFS
kaserver or the kerberos server and installed as "rxrpc" type keys.  This is
normally done using the klog program.  An example simple klog program can be
found at:

	http://people.redhat.com/~dhowells/rxrpc/klog.c

The payload provided to add_key() on the client should be of the following
form:

	struct rxrpc_key_sec2_v1 {
		uint16_t	security_index;	/* 2 */
		uint16_t	ticket_length;	/* length of ticket[] */
		uint32_t	expiry;		/* time at which expires */
		uint8_t		kvno;		/* key version number */
		uint8_t		__pad[3];
		uint8_t		session_key[8];	/* DES session key */
		uint8_t		ticket[0];	/* the encrypted ticket */
	};

Where the ticket blob is just appended to the above structure.


For the server, keys of type "rxrpc_s" must be made available to the server.
They have a description of "<serviceID>:<securityIndex>" (eg: "52:2" for an
rxkad key for the AFS VL service).  When such a key is created, it should be
given the server's secret key as the instantiation data (see the example
below).

	add_key("rxrpc_s", "52:2", secret_key, 8, keyring);

A keyring is passed to the server socket by naming it in a sockopt.  The server
socket then looks the server secret keys up in this keyring when secure
incoming connections are made.  This can be seen in an example program that can
be found at:

	http://people.redhat.com/~dhowells/rxrpc/listen.c


====================
EXAMPLE CLIENT USAGE
====================

A client would issue an operation by:

 (1) An RxRPC socket is set up by:

	client = socket(AF_RXRPC, SOCK_RPC, PF_INET);

     Where the third parameter indicates the protocol family of the transport
     socket used - usually IPv4 but it can also be IPv6 [TODO].

 (2) A local address can optionally be bound:

	struct sockaddr_rxrpc srx = {
		.srx_family	= AF_RXRPC,
		.srx_service	= 0,  /* we're a client */
		.transport_type	= SOCK_DGRAM,	/* type of transport socket */
		.transport.sin_family	= AF_INET,
		.transport.sin_port	= htons(7000), /* AFS callback */
		.transport.sin_address	= 0,  /* all local interfaces */
	};
	bind(client, &srx, sizeof(srx));

     This specifies the local UDP port to be used.  If not given, a random
     non-privileged port will be used.  A UDP port may be shared between
     several unrelated RxRPC sockets.  Security is handled on a basis of
     per-RxRPC virtual connection.

 (3) The security is set:

	const char *key = "AFS:cambridge.redhat.com";
	setsockopt(client, SOL_RXRPC, RXRPC_SECURITY_KEY, key, strlen(key));

     This issues a request_key() to get the key representing the security
     context.  The minimum security level can be set:

	unsigned int sec = RXRPC_SECURITY_ENCRYPTED;
	setsockopt(client, SOL_RXRPC, RXRPC_MIN_SECURITY_LEVEL,
		   &sec, sizeof(sec));

 (4) The server to be contacted is then specified:

	struct sockaddr_rxrpc srx = {
		.srx_family	= AF_RXRPC,
		.srx_service	= VL_SERVICE_ID,
		.transport_type	= SOCK_DGRAM,	/* type of transport socket */
		.transport.sin_family	= AF_INET,
		.transport.sin_port	= htons(7005), /* AFS volume manager */
		.transport.sin_address	= ...,
	};
	connect(client, &srx, sizeof(srx));

 (5) The request is then sent:

	sendmsg(client, msg, 0);

 (6) And the reply received:

	recvmsg(client, msg, 0);

     If an abort or error occurred, this will be returned in the control data
     buffer.


====================
EXAMPLE SERVER USAGE
====================

A server would be set up to accept operations in the following manner:

 (1) An RxRPC socket is created by:

	server = socket(AF_RXRPC, SOCK_RPC, PF_INET);

     Where the third parameter indicates the address type of the transport
     socket used - usually IPv4.

 (2) Security is set up if desired by giving the socket a keyring with server
     secret keys in it:

	keyring = add_key("keyring", "AFSkeys", NULL, 0,
			  KEY_SPEC_PROCESS_KEYRING);

	const char secret_key[8] = {
		0xa7, 0x83, 0x8a, 0xcb, 0xc7, 0x83, 0xec, 0x94 };
	add_key("rxrpc_s", "52:2", secret_key, 8, keyring);

	setsockopt(server, SOL_RXRPC, RXRPC_SECURITY_KEYRING, "AFSkeys", 7);

     The keyring can be manipulated after it has been given to the socket. This
     permits the server to add more keys, replace keys, etc. whilst it is live.

 (2) A local address would be bound:

	struct sockaddr_rxrpc srx = {
		.srx_family	= AF_RXRPC,
		.srx_service	= VL_SERVICE_ID, /* RxRPC service ID */
		.transport_type	= SOCK_DGRAM,	/* type of transport socket */
		.transport.sin_family	= AF_INET,
		.transport.sin_port	= htons(7000), /* AFS callback */
		.transport.sin_address	= 0,  /* all local interfaces */
	};
	bind(server, &srx, sizeof(srx));

 (3) The server would then be set to listen out for incoming calls:

	listen(server, 100);

 (4) The kernel would notify the server of pending incoming connections by
     sending it a message for each.  This would be received with recvmsg() on
     the server socket.  It would have no data, and would have a single
     dataless control message attached:

	RXRPC_NEW_CALL

     The address that can be passed back by recvmsg() at this point should be
     ignored since the call for which the message was posted may have gone by
     the time it is accepted - in which case the first call still on the queue
     will be accepted.

 (5) The server then accepts the new call by issuing a sendmsg() with two
     pieces of control data and no actual data:

	RXRPC_ACCEPT		- indicate connection acceptance
	RXRPC_USER_CALL_ID	- specify user ID for this call

 (6) The first request data packet will then be posted to the server socket for
     recvmsg() to pick up.  At that point, the RxRPC address for the call can
     be read from the address fields in the msghdr struct.

     Subsequent request data packets will be posted to the server socket for
     recvmsg() to collect as they arrive.  The last packet in the request will
     be posted with MSG_EOR set in msghdr::msg_flags.

     All data packets will be delivered with the following control message
     attached:

	RXRPC_USER_CALL_ID	- specifies the user ID for this call

 (8) The reply data should then be posted to the server socket using a series
     of sendmsg() calls, each with the following control messages attached:

	RXRPC_USER_CALL_ID	- specifies the user ID for this call

     MSG_MORE should be set in msghdr::msg_flags on all but the last call.

 (9) The final ACK from the client will be posted for retrieval by recvmsg()
     when it is received.  It will take the form of a dataless message with two
     control messages attached:

	RXRPC_USER_CALL_ID	- specifies the user ID for this call
	RXRPC_ACK		- indicates final ACK (no data)

(10) Up to the point the final packet of reply data is sent, the call can be
     aborted by calling sendmsg() with a dataless message with the following
     control messages attached:

	RXRPC_USER_CALL_ID	- specifies the user ID for this call
	RXRPC_ABORT		- indicates abort code (4 byte data)

Note that all the communications for a particular service take place through
the one server socket, using control messages on sendmsg() and recvmsg() to
determine the call affected.

[PATCH 1/5] AF_RXRPC: Add blkcipher accessors for using kernel data directly [try #2]

[David Howells]

Add blkcipher accessors for using kernel data directly without the use of
scatter lists.

Also add a CRYPTO_ALG_DMA algorithm capability flag to permit or deny the use
of DMA and hardware accelerators.  A hardware accelerator may not be used to
access any arbitrary piece of kernel memory lest it not be in a DMA'able
region.  Only software algorithms may do that.

If kernel data is going to be accessed directly, then CRYPTO_ALG_DMA must, for
instance, be passed in the mask of crypto_alloc_blkcipher(), but not the type.

Signed-Off-By: David Howells <dhowells@redhat.com>
---

 crypto/blkcipher.c     |    2 ++
 crypto/pcbc.c          |   62 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/crypto.h |   53 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 116 insertions(+), 1 deletions(-)

diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
index b5befe8..4498b2d 100644
--- a/crypto/blkcipher.c
+++ b/crypto/blkcipher.c
@@ -376,6 +376,8 @@ static int crypto_init_blkcipher_ops(struct crypto_tfm *tfm, u32 type, u32 mask)
 	crt->setkey = setkey;
 	crt->encrypt = alg->encrypt;
 	crt->decrypt = alg->decrypt;
+	crt->encrypt_kernel = alg->encrypt_kernel;
+	crt->decrypt_kernel = alg->decrypt_kernel;
 
 	addr = (unsigned long)crypto_tfm_ctx(tfm);
 	addr = ALIGN(addr, align);
diff --git a/crypto/pcbc.c b/crypto/pcbc.c
index 5174d7f..fa76111 100644
--- a/crypto/pcbc.c
+++ b/crypto/pcbc.c
@@ -126,6 +126,36 @@ static int crypto_pcbc_encrypt(struct blkcipher_desc *desc,
 	return err;
 }
 
+static int crypto_pcbc_encrypt_kernel(struct blkcipher_desc *desc,
+				      u8 *dst, const u8 *src,
+				      unsigned int nbytes)
+{
+	struct blkcipher_walk walk;
+	struct crypto_blkcipher *tfm = desc->tfm;
+	struct crypto_pcbc_ctx *ctx = crypto_blkcipher_ctx(tfm);
+	struct crypto_cipher *child = ctx->child;
+	void (*xor)(u8 *, const u8 *, unsigned int bs) = ctx->xor;
+
+	BUG_ON(crypto_tfm_alg_capabilities(crypto_cipher_tfm(child)) &
+	       CRYPTO_ALG_DMA);
+
+	if (nbytes == 0)
+		return 0;
+
+	memset(&walk, 0, sizeof(walk));
+	walk.src.virt.addr = (u8 *) src;
+	walk.dst.virt.addr = (u8 *) dst;
+	walk.nbytes = nbytes;
+	walk.total = nbytes;
+	walk.iv = desc->info;
+
+	if (walk.src.virt.addr == walk.dst.virt.addr)
+		nbytes = crypto_pcbc_encrypt_inplace(desc, &walk, child, xor);
+	else
+		nbytes = crypto_pcbc_encrypt_segment(desc, &walk, child, xor);
+	return 0;
+}
+
 static int crypto_pcbc_decrypt_segment(struct blkcipher_desc *desc,
 				       struct blkcipher_walk *walk,
 				       struct crypto_cipher *tfm,
@@ -211,6 +241,36 @@ static int crypto_pcbc_decrypt(struct blkcipher_desc *desc,
 	return err;
 }
 
+static int crypto_pcbc_decrypt_kernel(struct blkcipher_desc *desc,
+				      u8 *dst, const u8 *src,
+				      unsigned int nbytes)
+{
+	struct blkcipher_walk walk;
+	struct crypto_blkcipher *tfm = desc->tfm;
+	struct crypto_pcbc_ctx *ctx = crypto_blkcipher_ctx(tfm);
+	struct crypto_cipher *child = ctx->child;
+	void (*xor)(u8 *, const u8 *, unsigned int bs) = ctx->xor;
+
+	BUG_ON(crypto_tfm_alg_capabilities(crypto_cipher_tfm(child)) &
+		CRYPTO_ALG_DMA);
+
+	if (nbytes == 0)
+		return 0;
+
+	memset(&walk, 0, sizeof(walk));
+	walk.src.virt.addr = (u8 *) src;
+	walk.dst.virt.addr = (u8 *) dst;
+	walk.nbytes = nbytes;
+	walk.total = nbytes;
+	walk.iv = desc->info;
+
+	if (walk.src.virt.addr == walk.dst.virt.addr)
+		nbytes = crypto_pcbc_decrypt_inplace(desc, &walk, child, xor);
+	else
+		nbytes = crypto_pcbc_decrypt_segment(desc, &walk, child, xor);
+	return 0;
+}
+
 static void xor_byte(u8 *a, const u8 *b, unsigned int bs)
 {
 	do {
@@ -313,6 +373,8 @@ static struct crypto_instance *crypto_pcbc_alloc(void *param, unsigned int len)
 	inst->alg.cra_blkcipher.setkey = crypto_pcbc_setkey;
 	inst->alg.cra_blkcipher.encrypt = crypto_pcbc_encrypt;
 	inst->alg.cra_blkcipher.decrypt = crypto_pcbc_decrypt;
+	inst->alg.cra_blkcipher.encrypt_kernel = crypto_pcbc_encrypt_kernel;
+	inst->alg.cra_blkcipher.decrypt_kernel = crypto_pcbc_decrypt_kernel;
 
 out_put_alg:
 	crypto_mod_put(alg);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 779aa78..ce092fe 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -40,7 +40,10 @@
 #define CRYPTO_ALG_LARVAL		0x00000010
 #define CRYPTO_ALG_DEAD			0x00000020
 #define CRYPTO_ALG_DYING		0x00000040
-#define CRYPTO_ALG_ASYNC		0x00000080
+
+#define CRYPTO_ALG_CAP_MASK		0x00000180	/* capabilities mask */
+#define CRYPTO_ALG_ASYNC		0x00000080	/* capable of async operation */
+#define CRYPTO_ALG_DMA			0x00000100	/* capable of using of DMA */
 
 /*
  * Set this bit if and only if the algorithm requires another algorithm of
@@ -125,6 +128,10 @@ struct blkcipher_alg {
 	int (*decrypt)(struct blkcipher_desc *desc,
 		       struct scatterlist *dst, struct scatterlist *src,
 		       unsigned int nbytes);
+	int (*encrypt_kernel)(struct blkcipher_desc *desc, u8 *dst,
+			      const u8 *src, unsigned int nbytes);
+	int (*decrypt_kernel)(struct blkcipher_desc *desc, u8 *dst,
+			      const u8 *src, unsigned int nbytes);
 
 	unsigned int min_keysize;
 	unsigned int max_keysize;
@@ -240,6 +247,10 @@ struct blkcipher_tfm {
 		       struct scatterlist *src, unsigned int nbytes);
 	int (*decrypt)(struct blkcipher_desc *desc, struct scatterlist *dst,
 		       struct scatterlist *src, unsigned int nbytes);
+	int (*encrypt_kernel)(struct blkcipher_desc *desc, u8 *dst,
+			      const u8 *src, unsigned int nbytes);
+	int (*decrypt_kernel)(struct blkcipher_desc *desc, u8 *dst,
+			      const u8 *src, unsigned int nbytes);
 };
 
 struct cipher_tfm {
@@ -372,6 +383,11 @@ static inline u32 crypto_tfm_alg_type(struct crypto_tfm *tfm)
 	return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK;
 }
 
+static inline u32 crypto_tfm_alg_capabilities(struct crypto_tfm *tfm)
+{
+	return tfm->__crt_alg->cra_flags & CRYPTO_ALG_CAP_MASK;
+}
+
 static inline unsigned int crypto_tfm_alg_blocksize(struct crypto_tfm *tfm)
 {
 	return tfm->__crt_alg->cra_blocksize;
@@ -529,6 +545,23 @@ static inline int crypto_blkcipher_encrypt_iv(struct blkcipher_desc *desc,
 	return crypto_blkcipher_crt(desc->tfm)->encrypt(desc, dst, src, nbytes);
 }
 
+static inline void crypto_blkcipher_encrypt_kernel(struct blkcipher_desc *desc,
+						   u8 *dst, const u8 *src,
+						   unsigned int nbytes)
+{
+	desc->info = crypto_blkcipher_crt(desc->tfm)->iv;
+	crypto_blkcipher_crt(desc->tfm)->encrypt_kernel(desc, dst, src,
+							nbytes);
+}
+
+static inline void crypto_blkcipher_encrypt_kernel_iv(struct blkcipher_desc *desc,
+						      u8 *dst, const u8 *src,
+						      unsigned int nbytes)
+{
+	crypto_blkcipher_crt(desc->tfm)->encrypt_kernel(desc, dst, src,
+							nbytes);
+}
+
 static inline int crypto_blkcipher_decrypt(struct blkcipher_desc *desc,
 					   struct scatterlist *dst,
 					   struct scatterlist *src,
@@ -546,6 +579,24 @@ static inline int crypto_blkcipher_decrypt_iv(struct blkcipher_desc *desc,
 	return crypto_blkcipher_crt(desc->tfm)->decrypt(desc, dst, src, nbytes);
 }
 
+static inline void crypto_blkcipher_decrypt_kernel(struct blkcipher_desc *desc,
+						   u8 *dst, const u8 *src,
+						   unsigned int nbytes)
+{
+	desc->info = crypto_blkcipher_crt(desc->tfm)->iv;
+	crypto_blkcipher_crt(desc->tfm)->decrypt_kernel(desc, dst, src,
+							nbytes);
+}
+
+static inline
+void crypto_blkcipher_decrypt_kernel_iv(struct blkcipher_desc *desc,
+					u8 *dst, const u8 *src,
+					unsigned int nbytes)
+{
+	crypto_blkcipher_crt(desc->tfm)->decrypt_kernel(desc, dst, src,
+							nbytes);
+}
+
 static inline void crypto_blkcipher_set_iv(struct crypto_blkcipher *tfm,
 					   const u8 *src, unsigned int len)
 {

Re: [PATCH 1/5] AF_RXRPC: Add blkcipher accessors for using kernel data directly [try #2]

[Christoph Hellwig]

On Fri, Mar 16, 2007 at 12:50:16PM +0000, David Howells wrote:
> Add blkcipher accessors for using kernel data directly without the use of
> scatter lists.

I don't quite understand all these indirections.  What's the problem
with just having a helper that builds the scatterlist for you?

> Also add a CRYPTO_ALG_DMA algorithm capability flag to permit or deny the use
> of DMA and hardware accelerators.  A hardware accelerator may not be used to
> access any arbitrary piece of kernel memory lest it not be in a DMA'able
> region.  Only software algorithms may do that.

We allow dma access to arbitary pieces of _dynamically_ allocated kernel
memory, and I think using the crypto subsystem on the stack is not allowed
at all.


But the even bigger question is, how does this relate to rxrpc?

> +static inline
> +void crypto_blkcipher_decrypt_kernel_iv(struct blkcipher_desc *desc,

very odd line split, please put the function name on the same line
as the qualifiers, or everything including the void on a line of it's
own.  The first option seems to be the preffered style in this file.

Re: [PATCH 1/5] AF_RXRPC: Add blkcipher accessors for using kernel data directly [try #2]

[David Howells]

Christoph Hellwig <hch@infradead.org> wrote:

> I don't quite understand all these indirections.  What's the problem
> with just having a helper that builds the scatterlist for you?

I was trying to avoid building a scatterlist completely.  There's not much
point because the scatterlist approach involves finding out the page struct and
then kmapping it just so that the FCrypt algorithm can read 8 or 16 bytes of
data from kernel space.  Why do that if we can avoid it?  It's a waste of
processing time, and has to be done on every secure packet.

> We allow dma access to arbitary pieces of _dynamically_ allocated kernel
> memory, and I think using the crypto subsystem on the stack is not allowed
> at all.

FCrypt is only available in software as far as I know.  For producing and
checking packet signatures, using hardware support would be a waste of time as
the size of the crunched data is so small (a single 8-byte fragment per
packet).

> But the even bigger question is, how does this relate to rxrpc?

RxRPC has security features, up to and including full packet content encryption
if you select it.

> very odd line split

It's not really odd.  The "static" and "inline" don't actually add anything to
the function template.  They're merely scope limiters / optimisation
controllers, and so make a lot of sense placed on their own line.

David

Re: [PATCH 1/5] AF_RXRPC: Add blkcipher accessors for using kernel data directly [try #2]

[Alan Cox]

> > very odd line split
> 
> It's not really odd.  The "static" and "inline" don't actually add anything to
> the function template.  They're merely scope limiters / optimisation
> controllers, and so make a lot of sense placed on their own line.

As do many things but the goal of the coding style is consistency and
almost all other code doesn't have the static inline wasting an extra
display line.

Re: [PATCH 1/5] AF_RXRPC: Add blkcipher accessors for using kernel data directly [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> As do many things but the goal of the coding style is consistency and
> almost all other code doesn't have the static inline wasting an extra
> display line.

Actually it doesn't waste an extra display line.  Either the static inline is
on a line of its own, or the first argument is on a line of its own.  Either
way it uses up two lines - unless you want to split it up further and have
four lines as per Christoph's second suggestion (static inline / void /
funcname / 1st arg).

David

[PATCH 2/5] AF_RXRPC: Move generic skbuff stuff from XFRM code to generic code [try #2]

[David Howells]

Move generic skbuff stuff from XFRM code to generic code so that AF_RXRPC can
use it too.

Signed-Off-By: David Howells <dhowells@redhat.com>
---

 include/linux/skbuff.h |    4 +
 include/net/esp.h      |    2 -
 net/core/skbuff.c      |  173 ++++++++++++++++++++++++++++++++++++++++++++++++
 net/xfrm/xfrm_algo.c   |  169 -----------------------------------------------
 4 files changed, 177 insertions(+), 171 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 4ff3940..8701b12 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1481,5 +1481,9 @@ static inline int skb_is_gso(const struct sk_buff *skb)
 	return skb_shinfo(skb)->gso_size;
 }
 
+struct scatterlist;
+extern int skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len);
+extern int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer);
+
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff --git a/include/net/esp.h b/include/net/esp.h
index 713d039..d05d8d2 100644
--- a/include/net/esp.h
+++ b/include/net/esp.h
@@ -40,8 +40,6 @@ struct esp_data
 	} auth;
 };
 
-extern int skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len);
-extern int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer);
 extern void *pskb_put(struct sk_buff *skb, struct sk_buff *tail, int len);
 
 static inline int esp_mac_digest(struct esp_data *esp, struct sk_buff *skb,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 820761f..a70c152 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -55,6 +55,7 @@
 #include <linux/cache.h>
 #include <linux/rtnetlink.h>
 #include <linux/init.h>
+#include <linux/scatterlist.h>
 
 #include <net/protocol.h>
 #include <net/dst.h>
@@ -2059,6 +2060,175 @@ void __init skb_init(void)
 						NULL, NULL);
 }
 
+/*
+ * fill a scatter-gather list with pointers into a part of a socket buffer
+ * chain
+ */
+int
+skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
+{
+	int start = skb_headlen(skb);
+	int i, copy = start - offset;
+	int elt = 0;
+
+	if (copy > 0) {
+		if (copy > len)
+			copy = len;
+		sg[elt].page = virt_to_page(skb->data + offset);
+		sg[elt].offset = (unsigned long)(skb->data + offset) % PAGE_SIZE;
+		sg[elt].length = copy;
+		elt++;
+		if ((len -= copy) == 0)
+			return elt;
+		offset += copy;
+	}
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		int end;
+
+		BUG_TRAP(start <= offset + len);
+
+		end = start + skb_shinfo(skb)->frags[i].size;
+		if ((copy = end - offset) > 0) {
+			skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+			if (copy > len)
+				copy = len;
+			sg[elt].page = frag->page;
+			sg[elt].offset = frag->page_offset+offset-start;
+			sg[elt].length = copy;
+			elt++;
+			if (!(len -= copy))
+				return elt;
+			offset += copy;
+		}
+		start = end;
+	}
+
+	if (skb_shinfo(skb)->frag_list) {
+		struct sk_buff *list = skb_shinfo(skb)->frag_list;
+
+		for (; list; list = list->next) {
+			int end;
+
+			BUG_TRAP(start <= offset + len);
+
+			end = start + list->len;
+			if ((copy = end - offset) > 0) {
+				if (copy > len)
+					copy = len;
+				elt += skb_to_sgvec(list, sg+elt, offset - start, copy);
+				if ((len -= copy) == 0)
+					return elt;
+				offset += copy;
+			}
+			start = end;
+		}
+	}
+	BUG_ON(len);
+	return elt;
+}
+
+/*
+ * Check that skb data bits are writable. If they are not, copy data
+ * to newly created private area. If "tailbits" is given, make sure that
+ * tailbits bytes beyond current end of skb are writable.
+ *
+ * Returns amount of elements of scatterlist to load for subsequent
+ * transformations and pointer to writable trailer skb.
+ */
+int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
+{
+	int copyflag;
+	int elt;
+	struct sk_buff *skb1, **skb_p;
+
+	/* If skb is cloned or its head is paged, reallocate
+	 * head pulling out all the pages (pages are considered not writable
+	 * at the moment even if they are anonymous).
+	 */
+	if ((skb_cloned(skb) || skb_shinfo(skb)->nr_frags) &&
+	    __pskb_pull_tail(skb, skb_pagelen(skb)-skb_headlen(skb)) == NULL)
+		return -ENOMEM;
+
+	/* Easy case. Most of packets will go this way. */
+	if (!skb_shinfo(skb)->frag_list) {
+		/* A little of trouble, not enough of space for trailer.
+		 * This should not happen, when stack is tuned to generate
+		 * good frames. OK, on miss we reallocate and reserve even more
+		 * space, 128 bytes is fair. */
+
+		if (skb_tailroom(skb) < tailbits &&
+		    pskb_expand_head(skb, 0, tailbits-skb_tailroom(skb)+128, GFP_ATOMIC))
+			return -ENOMEM;
+
+		/* Voila! */
+		*trailer = skb;
+		return 1;
+	}
+
+	/* Misery. We are in troubles, going to mincer fragments... */
+
+	elt = 1;
+	skb_p = &skb_shinfo(skb)->frag_list;
+	copyflag = 0;
+
+	while ((skb1 = *skb_p) != NULL) {
+		int ntail = 0;
+
+		/* The fragment is partially pulled by someone,
+		 * this can happen on input. Copy it and everything
+		 * after it. */
+
+		if (skb_shared(skb1))
+			copyflag = 1;
+
+		/* If the skb is the last, worry about trailer. */
+
+		if (skb1->next == NULL && tailbits) {
+			if (skb_shinfo(skb1)->nr_frags ||
+			    skb_shinfo(skb1)->frag_list ||
+			    skb_tailroom(skb1) < tailbits)
+				ntail = tailbits + 128;
+		}
+
+		if (copyflag ||
+		    skb_cloned(skb1) ||
+		    ntail ||
+		    skb_shinfo(skb1)->nr_frags ||
+		    skb_shinfo(skb1)->frag_list) {
+			struct sk_buff *skb2;
+
+			/* Fuck, we are miserable poor guys... */
+			if (ntail == 0)
+				skb2 = skb_copy(skb1, GFP_ATOMIC);
+			else
+				skb2 = skb_copy_expand(skb1,
+						       skb_headroom(skb1),
+						       ntail,
+						       GFP_ATOMIC);
+			if (unlikely(skb2 == NULL))
+				return -ENOMEM;
+
+			if (skb1->sk)
+				skb_set_owner_w(skb2, skb1->sk);
+
+			/* Looking around. Are we still alive?
+			 * OK, link new skb, drop old one */
+
+			skb2->next = skb1->next;
+			*skb_p = skb2;
+			kfree_skb(skb1);
+			skb1 = skb2;
+		}
+		elt++;
+		*trailer = skb1;
+		skb_p = &skb1->next;
+	}
+
+	return elt;
+}
+
 EXPORT_SYMBOL(___pskb_trim);
 EXPORT_SYMBOL(__kfree_skb);
 EXPORT_SYMBOL(kfree_skb);
@@ -2093,3 +2263,6 @@ EXPORT_SYMBOL(skb_seq_read);
 EXPORT_SYMBOL(skb_abort_seq_read);
 EXPORT_SYMBOL(skb_find_text);
 EXPORT_SYMBOL(skb_append_datato_frags);
+
+EXPORT_SYMBOL_GPL(skb_to_sgvec);
+EXPORT_SYMBOL_GPL(skb_cow_data);
diff --git a/net/xfrm/xfrm_algo.c b/net/xfrm/xfrm_algo.c
index f373a8a..6249a94 100644
--- a/net/xfrm/xfrm_algo.c
+++ b/net/xfrm/xfrm_algo.c
@@ -612,175 +612,6 @@ EXPORT_SYMBOL_GPL(skb_icv_walk);
 
 #if defined(CONFIG_INET_ESP) || defined(CONFIG_INET_ESP_MODULE) || defined(CONFIG_INET6_ESP) || defined(CONFIG_INET6_ESP_MODULE)
 
-/* Looking generic it is not used in another places. */
-
-int
-skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
-{
-	int start = skb_headlen(skb);
-	int i, copy = start - offset;
-	int elt = 0;
-
-	if (copy > 0) {
-		if (copy > len)
-			copy = len;
-		sg[elt].page = virt_to_page(skb->data + offset);
-		sg[elt].offset = (unsigned long)(skb->data + offset) % PAGE_SIZE;
-		sg[elt].length = copy;
-		elt++;
-		if ((len -= copy) == 0)
-			return elt;
-		offset += copy;
-	}
-
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
-		int end;
-
-		BUG_TRAP(start <= offset + len);
-
-		end = start + skb_shinfo(skb)->frags[i].size;
-		if ((copy = end - offset) > 0) {
-			skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
-
-			if (copy > len)
-				copy = len;
-			sg[elt].page = frag->page;
-			sg[elt].offset = frag->page_offset+offset-start;
-			sg[elt].length = copy;
-			elt++;
-			if (!(len -= copy))
-				return elt;
-			offset += copy;
-		}
-		start = end;
-	}
-
-	if (skb_shinfo(skb)->frag_list) {
-		struct sk_buff *list = skb_shinfo(skb)->frag_list;
-
-		for (; list; list = list->next) {
-			int end;
-
-			BUG_TRAP(start <= offset + len);
-
-			end = start + list->len;
-			if ((copy = end - offset) > 0) {
-				if (copy > len)
-					copy = len;
-				elt += skb_to_sgvec(list, sg+elt, offset - start, copy);
-				if ((len -= copy) == 0)
-					return elt;
-				offset += copy;
-			}
-			start = end;
-		}
-	}
-	BUG_ON(len);
-	return elt;
-}
-EXPORT_SYMBOL_GPL(skb_to_sgvec);
-
-/* Check that skb data bits are writable. If they are not, copy data
- * to newly created private area. If "tailbits" is given, make sure that
- * tailbits bytes beyond current end of skb are writable.
- *
- * Returns amount of elements of scatterlist to load for subsequent
- * transformations and pointer to writable trailer skb.
- */
-
-int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
-{
-	int copyflag;
-	int elt;
-	struct sk_buff *skb1, **skb_p;
-
-	/* If skb is cloned or its head is paged, reallocate
-	 * head pulling out all the pages (pages are considered not writable
-	 * at the moment even if they are anonymous).
-	 */
-	if ((skb_cloned(skb) || skb_shinfo(skb)->nr_frags) &&
-	    __pskb_pull_tail(skb, skb_pagelen(skb)-skb_headlen(skb)) == NULL)
-		return -ENOMEM;
-
-	/* Easy case. Most of packets will go this way. */
-	if (!skb_shinfo(skb)->frag_list) {
-		/* A little of trouble, not enough of space for trailer.
-		 * This should not happen, when stack is tuned to generate
-		 * good frames. OK, on miss we reallocate and reserve even more
-		 * space, 128 bytes is fair. */
-
-		if (skb_tailroom(skb) < tailbits &&
-		    pskb_expand_head(skb, 0, tailbits-skb_tailroom(skb)+128, GFP_ATOMIC))
-			return -ENOMEM;
-
-		/* Voila! */
-		*trailer = skb;
-		return 1;
-	}
-
-	/* Misery. We are in troubles, going to mincer fragments... */
-
-	elt = 1;
-	skb_p = &skb_shinfo(skb)->frag_list;
-	copyflag = 0;
-
-	while ((skb1 = *skb_p) != NULL) {
-		int ntail = 0;
-
-		/* The fragment is partially pulled by someone,
-		 * this can happen on input. Copy it and everything
-		 * after it. */
-
-		if (skb_shared(skb1))
-			copyflag = 1;
-
-		/* If the skb is the last, worry about trailer. */
-
-		if (skb1->next == NULL && tailbits) {
-			if (skb_shinfo(skb1)->nr_frags ||
-			    skb_shinfo(skb1)->frag_list ||
-			    skb_tailroom(skb1) < tailbits)
-				ntail = tailbits + 128;
-		}
-
-		if (copyflag ||
-		    skb_cloned(skb1) ||
-		    ntail ||
-		    skb_shinfo(skb1)->nr_frags ||
-		    skb_shinfo(skb1)->frag_list) {
-			struct sk_buff *skb2;
-
-			/* Fuck, we are miserable poor guys... */
-			if (ntail == 0)
-				skb2 = skb_copy(skb1, GFP_ATOMIC);
-			else
-				skb2 = skb_copy_expand(skb1,
-						       skb_headroom(skb1),
-						       ntail,
-						       GFP_ATOMIC);
-			if (unlikely(skb2 == NULL))
-				return -ENOMEM;
-
-			if (skb1->sk)
-				skb_set_owner_w(skb2, skb1->sk);
-
-			/* Looking around. Are we still alive?
-			 * OK, link new skb, drop old one */
-
-			skb2->next = skb1->next;
-			*skb_p = skb2;
-			kfree_skb(skb1);
-			skb1 = skb2;
-		}
-		elt++;
-		*trailer = skb1;
-		skb_p = &skb1->next;
-	}
-
-	return elt;
-}
-EXPORT_SYMBOL_GPL(skb_cow_data);
-
 void *pskb_put(struct sk_buff *skb, struct sk_buff *tail, int len)
 {
 	if (tail != skb) {

Re: [PATCH 2/5] AF_RXRPC: Move generic skbuff stuff from XFRM code to generic code [try #2]

[Christoph Hellwig]

On Fri, Mar 16, 2007 at 12:50:21PM +0000, David Howells wrote:
> Move generic skbuff stuff from XFRM code to generic code so that AF_RXRPC can
> use it too.
> 
> Signed-Off-By: David Howells <dhowells@redhat.com>
> ---
> 
>  include/linux/skbuff.h |    4 +
>  include/net/esp.h      |    2 -
>  net/core/skbuff.c      |  173 ++++++++++++++++++++++++++++++++++++++++++++++++
>  net/xfrm/xfrm_algo.c   |  169 -----------------------------------------------
>  4 files changed, 177 insertions(+), 171 deletions(-)

> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1481,5 +1481,9 @@ static inline int skb_is_gso(const struct sk_buff *skb)
>  	return skb_shinfo(skb)->gso_size;
>  }
>  
> +struct scatterlist;

normally we try to put structure forward declarations at the top of the
header instead of spreading it around all over.

> +extern int skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len);
> +extern int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer);

please make sure no line is longer than 80 characters.  Also shouldn't
prototypes normally be above inlines?  Or at least grouped into logical
areas?

> +/*
> + * fill a scatter-gather list with pointers into a part of a socket buffer
> + * chain
> + */

This could probably use a kdoc comment now that it's a public symbol.

> +/*
> + * Check that skb data bits are writable. If they are not, copy data
> + * to newly created private area. If "tailbits" is given, make sure that
> + * tailbits bytes beyond current end of skb are writable.
> + *
> + * Returns amount of elements of scatterlist to load for subsequent
> + * transformations and pointer to writable trailer skb.
> + */

Same here.

[PATCH 3/5] AF_RXRPC: Make it possible to merely try to cancel timers and delayed work [try #2]

[David Howells]

Export try_to_del_timer_sync() for use by the RxRPC module.

Add a try_to_cancel_delayed_work() so that it is possible to merely attempt to
cancel a delayed work timer.

Signed-Off-By: David Howells <dhowells@redhat.com>
---

 include/linux/workqueue.h |   21 +++++++++++++++++++++
 kernel/timer.c            |    2 ++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 2a7b38d..40a61ae 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -204,4 +204,25 @@ static inline int cancel_delayed_work(struct delayed_work *work)
 	return ret;
 }
 
+/**
+ * try_to_cancel_delayed_work - Try to kill pending scheduled, delayed work
+ * @work: the work to cancel
+ *
+ * Try to kill off a pending schedule_delayed_work().
+ * - The timer may still be running afterwards, and if so, the work may still
+ *   be pending
+ * - Returns -1 if timer still active, 1 if timer removed, 0 if not scheduled
+ * - Can be called from the work routine; if it's still pending, just return
+ *   and it'll be called again.
+ */
+static inline int try_to_cancel_delayed_work(struct delayed_work *work)
+{
+	int ret;
+
+	ret = try_to_del_timer_sync(&work->timer);
+	if (ret > 0)
+		work_release(&work->work);
+	return ret;
+}
+
 #endif
diff --git a/kernel/timer.c b/kernel/timer.c
index 797cccb..447506a 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -505,6 +505,8 @@ out:
 	return ret;
 }
 
+EXPORT_SYMBOL(try_to_del_timer_sync);
+
 /**
  * del_timer_sync - deactivate a timer and wait for the handler to finish.
  * @timer: the timer to be deactivated

Re: [PATCH 3/5] AF_RXRPC: Make it possible to merely try to cancel timers and delayed work [try #2]

[Alan Cox]

> +/**
> + * try_to_cancel_delayed_work - Try to kill pending scheduled, delayed work
> + * @work: the work to cancel
> + *
> + * Try to kill off a pending schedule_delayed_work().
> + * - The timer may still be running afterwards, and if so, the work may still
> + *   be pending
> + * - Returns -1 if timer still active, 1 if timer removed, 0 if not scheduled
> + * - Can be called from the work routine; if it's still pending, just return
> + *   and it'll be called again.
> + */
> +static inline int try_to_cancel_delayed_work(struct delayed_work *work)
> +{
> +	int ret;

This expands to a fair bit of code and IMHO isn't an inline candidate.

Re: [PATCH 3/5] AF_RXRPC: Make it possible to merely try to cancel timers and delayed work [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > +static inline int try_to_cancel_delayed_work(struct delayed_work *work)
> > +{
> > +	int ret;
> 
> This expands to a fair bit of code and IMHO isn't an inline candidate.

Then why is cancel_delayed_work() inline?  It's very similar in terms of
expansion.

And it's not a fair bit of code.  try_to_cancel_delayed_work() is out of line,
and work_release() is just clear_bit() in disguise.

David

[PATCH 4/5] AF_RXRPC: Key facility changes for AF_RXRPC [try #2]

[David Howells]

Export the keyring key type definition.

Add extra alternative types into the key's type_data union to make it more
useful.

Signed-Off-By: David Howells <dhowells@redhat.com>
---

 include/linux/key.h     |    2 ++
 security/keys/keyring.c |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 169f05e..a9220e7 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -160,6 +160,8 @@ struct key {
 	 */
 	union {
 		struct list_head	link;
+		unsigned long		x[2];
+		void			*p[2];
 	} type_data;
 
 	/* key data
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index ad45ce7..88292e3 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -66,6 +66,8 @@ struct key_type key_type_keyring = {
 	.read		= keyring_read,
 };
 
+EXPORT_SYMBOL(key_type_keyring);
+
 /*
  * semaphore to serialise link/link calls to prevent two link calls in parallel
  * introducing a cycle

Re: [PATCH 4/5] AF_RXRPC: Key facility changes for AF_RXRPC [try #2]

[Christoph Hellwig]

On Fri, Mar 16, 2007 at 12:50:31PM +0000, David Howells wrote:
> Export the keyring key type definition.
> 
> Add extra alternative types into the key's type_data union to make it more
> useful.

You wrote the code so there must be some reason for this, but these
changes look rather odd to me :)

> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index ad45ce7..88292e3 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -66,6 +66,8 @@ struct key_type key_type_keyring = {
>  	.read		= keyring_read,
>  };
>  
> +EXPORT_SYMBOL(key_type_keyring);
> +

Having a type exported is really odd, how is this supposed to be a public API?

Re: [PATCH 4/5] AF_RXRPC: Key facility changes for AF_RXRPC [try #2]

[David Howells]

Christoph Hellwig <hch@infradead.org> wrote:

> You wrote the code so there must be some reason for this, but these
> changes look rather odd to me :)

The union is for use by the type in any way it sees fit, but it may not want to
use it as a list_head.  So all I've done is to make it available as a pair of
arbitrary pointers or a pair of arbitrary numbers.

Maybe a better way would be to have an overlay struct that's private to the
type, sort of how sk_buff::cb works.

> Having a type exported is really odd, how is this supposed to be a public API?

Keyrings are a special type.

It occurred to me whilst doing this that the best way to achieve what I wanted
was by dealing with rings of keys.  What I needed was for the server app to
give the kernel a key for each security type it wanted to support, which the
kernel would then have to retain.  It seems natural to use a keyring to do the
retension as that's its purpose.

Another way to look at it is that in the client I need just one key at once,
and I can get that from the process as it's setting up the connection.
However, in the server I need to have several keys, and I need them available
up front because the server app doesn't set up a connection, the kernel does,
and it needs the keys immediately.

I'll add a mention to Documentation/keys.txt to record this exportation.

David

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Christoph Hellwig]

On Fri, Mar 16, 2007 at 12:50:08PM +0000, David Howells wrote:
>  (*) Make fs/afs/ use it and delete the current contents of net/rxrpc/

Having this done would be a really useful use case for getting the code
merged..

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Alan Cox]

> Sockets of AF_RXRPC family are:
> 
>  (1) created as type SOCK_RPC;

There is no such socket type and please don't invent one

If your messages are datagrams and unreliable -> SOCK_DGRAM
If your messages are datagrams and reliable -> SOCK_RDM
If your messages are datagrams, reliable and ordered -> SOCK_SEQPACKET
If your messages are streams of bits, reliable and ordered -> SOCK_STREAM

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> If your messages are datagrams and unreliable -> SOCK_DGRAM

Nope.

> If your messages are datagrams and reliable -> SOCK_RDM

Nope.

> If your messages are datagrams, reliable and ordered -> SOCK_SEQPACKET

Nope.

> If your messages are streams of bits, reliable and ordered -> SOCK_STREAM

Nope.

David

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Alan Cox]

On Fri, 16 Mar 2007 14:23:13 +0000
David Howells <dhowells@redhat.com> wrote:

> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > If your messages are datagrams and unreliable -> SOCK_DGRAM
> 
> Nope.
> 
> > If your messages are datagrams and reliable -> SOCK_RDM
> 
> Nope.
> 
> > If your messages are datagrams, reliable and ordered -> SOCK_SEQPACKET
> 
> Nope.
> 
> > If your messages are streams of bits, reliable and ordered -> SOCK_STREAM
> 
> Nope.

So your messages are neither reliable not unreliable, nor ordered, nor
unordered.

NAK

Until you work out what your messages actually are and use a proper
standard socket type. And "but there are higher layers" isn't relevant
here, this is true for Appletalk as well and it doesn't have to go
inventing new types for everything as you seem to.

Alan

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> So your messages are neither reliable not unreliable, nor ordered, nor
> unordered.

If you look at the LHS of each of your list of mappings again, you'll see
you've made an assumption there in considering it to be an exhaustive list.
You've assumed a service must be either a datagram service or a stream service;
I content that RxRPC is neither.

Let me explain.

RxRPC is not a datagram service because:

	A datagram is, to quote the Internet's Request for Comments 1594, "a
	self-contained, independent entity of data carrying sufficient
	information to be routed from the source to the destination computer
	without reliance on earlier exchanges between this source and
	destination computer and the transporting network.

An RxRPC operation fails the "without reliance on earlier exchanges" bit, and
I'd argue that it possibly fails the "self-contained" and "independent" bits
too.

RxRPC does use a datagram service as its transport, but it takes a minimum of
three packets to complete an operation: a request from the client, a reply from
the server and a final ACK from the client.  The second and third packets are
dependent on the first to give them meaning.

Looking at RxRPC from a client app point of view, you have to issue a request
packet and then await for the reply _associated_ with it (as marked by the
control data).  In the server app, you receive a request message, and then have
to make a reply _associated_ with that request.

Furthermore, you can have several operations in progress simultaneously on a
socket.  They are ordered with respect to themselves only; they are not ordered
with respect to each other.


RxRPC is also not a streamed data service because it doesn't have a stream of
data (either continuous [STREAM] or broken [SEQPACKET]) of indefinite length.
It has operations, each of which follows a sequence of discrete phases
(request, secure, secured, reply, final ACK).  Each operation is independent of
the other operations, and may overlap temporally with those others.


So, to summarise, I think this is a "new" type of flow model because:

 (1) It has discrete components (operations) rather than a flow.

 (2) The discrete components may overlap temporally (are unordered with respect
     to each other).

 (3) Each discrete component consists of a sequence of phases, first in one
     direction then the other and then back again rather than discrete
     independent messages (are ordered with respect to themselves).

 (4) It is reliable.

It also has security features, but I don't think that needs to be part of the
definition, even though negotiating it does require the use of extra packets of
special types.

> Until you work out what your messages actually are

I know what they are; and I don't think that what's available covers it.

> and use a proper standard socket type.

Assuming that that list is exhaustive...

> And "but there are higher layers" isn't relevant here, this is true for
> Appletalk as well and it doesn't have to go inventing new types for
> everything as you seem to.

The fact that there are higher layers is irrelevant.

David

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Alan Cox]

> I know what they are; and I don't think that what's available covers it.
> 
> > and use a proper standard socket type.
> 
> Assuming that that list is exhaustive...

SOCK_RDM seems to match perfectly well here. The point isn't to enumerate
everything in the universe the point is to find "works like" parallels
good enough to avoid more special casing.

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Kyle Moffett]

On Mar 16, 2007, at 10:11:41, Alan Cox wrote:
>> I know what they are; and I don't think that what's available  
>> covers it.
>>
>>> and use a proper standard socket type.
>>
>> Assuming that that list is exhaustive...
>
> SOCK_RDM seems to match perfectly well here. The point isn't to  
> enumerate everything in the universe the point is to find "works  
> like" parallels good enough to avoid more special casing.

IMHO the problem with classifying RxRPC as a "reliable datagram"  
socket is that even an atomic unidirectional communication isn't a  
single datagram, it's at least 3; there is shared connection state  
and security context on both sides which pertains to a collection of  
independent and possibly simultaneous RxRPC calls.  From the digging  
around that I did in the kernel socket code a while ago I don't see a  
cleaner way of implementing it than a new SOCK_RXRPC.

Cheers,
Kyle Moffett

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Alan Cox]

> IMHO the problem with classifying RxRPC as a "reliable datagram"  
> socket is that even an atomic unidirectional communication isn't a  
> single datagram, it's at least 3; there is shared connection state  

Thats fine. Any *reliable* protocol sends more than one packet per
message you send. RDM is for reliable messages

> independent and possibly simultaneous RxRPC calls.  From the digging  
> around that I did in the kernel socket code a while ago I don't see a  
> cleaner way of implementing it than a new SOCK_RXRPC.

I disagree entirely. I've still seen no evidence you need a new type for
this. What does AIX do  out of interest ?

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > IMHO the problem with classifying RxRPC as a "reliable datagram"  
> > socket is that even an atomic unidirectional communication isn't a  
> > single datagram, it's at least 3; there is shared connection state  
> 
> Thats fine. Any *reliable* protocol sends more than one packet per
> message you send. RDM is for reliable messages

I'm not arguing with that.  Why I think RDM is the wrong thing to use is that
there seems to be a perception that RDM involves a unidirectional, single
message transmission.  You yourself defined RDM to be a datagram service.
RxRPC is not, in my opinion, a datagram service, and neither is it a stream
service.

Interestingly, searching for SOCK_RDM definitions with google shows there's
some disagreement as to what it represents.  Some seem to thing it's connection
oriented and some that it's connectionless, and some seem to think it's
ordered, and some not.

> > independent and possibly simultaneous RxRPC calls.  From the digging  
> > around that I did in the kernel socket code a while ago I don't see a  
> > cleaner way of implementing it than a new SOCK_RXRPC.

Well, I suggest SOCK_RPC, not SOCK_RXRPC.  There's no particular reason such a
flow type has to be limited to RxRPC.

> I disagree entirely.

That's your prerogative.

> I've still seen no evidence you need a new type for this.

Conversely, you haven't shown that any of the existing types are suitable
either.

Strictly, the "type" parameter for RxRPC family sockets is completely
irrelevant as only one flow type is supported anyway; but still, I'd like to
get it right.

Looking at the socket(2) manpage, I see the definition of the SOCK_RDM to be:

	Provides a reliable datagram layer that does not guarantee ordering.

I'd say that that precludes applicability on two points: firstly, RxRPC is not
a datagram layer by the definition I provided in a previous email; and
secondly, RxRPC *does* provide some ordering guarantees.

> What does AIX do out of interest ?

I've no way to find out what AIX does, but I suspect it simply doesn't.

The way OpenAFS does things is that it has an RxRPC library that can be
compiled either in userspace or in kernel space.  Userspace AFS utilities use
the userspace-compiled library, which simply uses a UDP socket as its kernel
interface; the kernel code uses the same library, but compiled for existence
within the kernel.

David

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Alan Cox]

> message transmission.  You yourself defined RDM to be a datagram service.
> RxRPC is not, in my opinion, a datagram service, and neither is it a stream
> service.

Message is what I should have said.

> Interestingly, searching for SOCK_RDM definitions with google shows there's
> some disagreement as to what it represents.  Some seem to thing it's connection
> oriented and some that it's connectionless, and some seem to think it's
> ordered, and some not.

Which is just fine, does it need to be one or the other. SOCK_DGRAM an be
both connection oriented or connectionless.

> 
> > > independent and possibly simultaneous RxRPC calls.  From the digging  
> > > around that I did in the kernel socket code a while ago I don't see a  
> > > cleaner way of implementing it than a new SOCK_RXRPC.
> 
> Well, I suggest SOCK_RPC, not SOCK_RXRPC.  There's no particular reason such a
> flow type has to be limited to RxRPC.

Other RPC types use normal socket types.

> I'd say that that precludes applicability on two points: firstly, RxRPC is not
> a datagram layer by the definition I provided in a previous email; and
> secondly, RxRPC *does* provide some ordering guarantees.

So you want SOCK_SEQPACKET perhaps then ?

> > What does AIX do out of interest ?
> 
> I've no way to find out what AIX does, but I suspect it simply doesn't.

Perhaps someone with AIX boxes around (say @ibm.com) could answer ?

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > message transmission.  You yourself defined RDM to be a datagram service.
> > RxRPC is not, in my opinion, a datagram service, and neither is it a stream
> > service.
> 
> Message is what I should have said.

socket(2) also says datagram...

> Which is just fine, does it need to be one or the other. SOCK_DGRAM an be
> both connection oriented or connectionless.

Is SOCK_DGRAM ever connection-oriented?  Over an AF_INET socket it isn't really
so.  Yes, you can use connect() to point a UDP socket at a particular target,
but you haven't really made a connection.

> > Well, I suggest SOCK_RPC, not SOCK_RXRPC.  There's no particular reason
> > such a flow type has to be limited to RxRPC.
> 
> Other RPC types use normal socket types.

They do?  Examples please.  I didn't think Linux, at least, has any other RPC
socket families, though I could be wrong as I haven't made a thorough study of
them.

> > I'd say that that precludes applicability on two points: firstly, RxRPC is
> > not a datagram layer by the definition I provided in a previous email; and
> > secondly, RxRPC *does* provide some ordering guarantees.
> 
> So you want SOCK_SEQPACKET perhaps then ?

Perhaps; but I think that's wrong also.  You yourself defined SEQPACKET to be a
datagram service, and FC6 socket(2) agrees.  The POSIX definition, however
disagrees[*]: it claims that SEQPACKET is a stream-based service that permits
record markers to be embedded in the stream.  I can see how the two can be two
views of the same thing, but it looks like in either case the two flows are
independent at the socket level - which is not so for RxRPC as requests,
replies and ACKs are obviously not independent.

Furthermore:

 (1) The message flow in RxRPC is not completely ordered as seems to be
     required in SEQPACKET: packets going into the Tx socket need not come out
     of the Rx socket in the same order.  The only ordering is over the packets
     of a particular operation (as labelled by the control data).

 (2) The RxRPC sockets don't actually require a formal connection.  Even at the
     protocol level, there is no connection negotiation and no disconnection.
     A virtual connection more or less springs up spontaneously when the first
     data packet on it arrives.  Virtual connections are really only ways of
     avoiding having to set up the security for every call.

     I have made my client sockets use connect(), but that's just a convenience
     and I need to make it possible to avoid doing that to make it useful to
     the kernel.  It's similar to SOCK_DGRAM sockets in this respect.

David

[*] There has been discussion on this.

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Alan Cox]

> > Other RPC types use normal socket types.
> 
> They do?  Examples please.  I didn't think Linux, at least, has any other RPC
> socket families, though I could be wrong as I haven't made a thorough study of
> them.

SunRPC is implemented in user space and uses the existing TCP/IP layer
and socket types, even though it is using them in an RPC manner and
viewed at the RPC layer they are RPCs

>      I have made my client sockets use connect(), but that's just a convenience
>      and I need to make it possible to avoid doing that to make it useful to
>      the kernel.  It's similar to SOCK_DGRAM sockets in this respect.

So use SOCK_DGRAM, its clearly near enough.

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > > Other RPC types use normal socket types.
> > 
> > They do?  Examples please.  I didn't think Linux, at least, has any other
> > RPC socket families, though I could be wrong as I haven't made a thorough
> > study of them.
> 
> SunRPC is implemented in user space and uses the existing TCP/IP layer
> and socket types, even though it is using them in an RPC manner and
> viewed at the RPC layer they are RPCs

SunRPC is not then a suitable analogy.  There is no socket interface that
provides SunRPC as far as I know, so your example is invalid.  Yes, SunRPC is
built on top of something else, SOCK_DGRAM, SOCK_STREAM or whatever, but that's
like saying TCP is a datagram service rather than a stream service because it's
built on a datagram service (IP).  What a protocol uses out the back is pretty
much irrelevant - what is relevant is what the protocol in question actually
appears to provide to anyone using it.

> >      I have made my client sockets use connect(), but that's just a
> >      convenience and I need to make it possible to avoid doing that to
> >      make it useful to the kernel.  It's similar to SOCK_DGRAM sockets in
> >      this respect.
> 
> So use SOCK_DGRAM, its clearly near enough.

No, it's not.  SOCK_DGRAM is an unreliable, unidirectional datagram passing
service.

David

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[Alan Cox]

O> No, it's not.  SOCK_DGRAM is an unreliable, unidirectional datagram passing
> service.

Thats funny UDP receives and sends packets.

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> O> No, it's not.  SOCK_DGRAM is an unreliable, unidirectional datagram passing
> > service.
> 
> Thats funny UDP receives and sends packets.

I meant that the protocol ships a datagram from one peer to another, but
there's no tie at the protocol level to any datagrams going the other way.

David

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Miller]

From: David Howells <dhowells@redhat.com>
Date: Mon, 19 Mar 2007 15:41:38 +0000

> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > So use SOCK_DGRAM, its clearly near enough.
> 
> No, it's not.  SOCK_DGRAM is an unreliable, unidirectional datagram passing
> service.

David we're not looking for a precise match, so please stop
discussing this in those terms.  We're looking for something
close enough.

The more and more I read Alan's arguments and your resistence to
his logic, the more I side with Alan.  He's definitely right on
all the basic counts here.

Re: [PATCH 0/5] [RFC] AF_RXRPC socket family implementation [try #2]

[David Howells]

David Miller <davem@davemloft.net> wrote:

> David we're not looking for a precise match, so please stop
> discussing this in those terms.  We're looking for something
> close enough.

But we don't have one that's close.  Let me recap: according to Alan's
definitions, all (presumably all non-RAW) network services must either be
datagram services or stream services, and must be selectable from DGRAM, RDM,
SEQPACKET or STREAM.  RPC is neither.  Also, why does DCCP have its own type?
According to Alan's logic that's superfluous.

> The more and more I read Alan's arguments and your resistence to
> his logic,

Which is flawed...

> the more I side with Alan.  He's definitely right on all the basic counts
> here.

Well, perhaps *you* can explain why he's right then...

But, since you insist, I'll just remove any restrictions on the type and drop
SOCK_RPC.  Type 0 will serve just as well since there's only one choice to be
had anyway, and that's all there's likely to be.

David