Skip to content

Commit

Permalink
rxrpc: Keep the skb private record of the Rx header in host byte order
Browse files Browse the repository at this point in the history
Currently, a copy of the Rx packet header is copied into the the sk_buff
private data so that we can advance the pointer into the buffer,
potentially discarding the original.  At the moment, this copy is held in
network byte order, but this means we're doing a lot of unnecessary
translations.

The reasons it was done this way are that we need the values in network
byte order occasionally and we can use the copy, slightly modified, as part
of an iov array when sending an ack or an abort packet.

However, it seems more reasonable on review that it would be better kept in
host byte order and that we make up a new header when we want to send
another packet.

To this end, rename the original header struct to rxrpc_wire_header (with
BE fields) and institute a variant called rxrpc_host_header that has host
order fields.  Change the struct in the sk_buff private data into an
rxrpc_host_header and translate the values when filling it in.

This further allows us to keep values kept in various structures in host
byte order rather than network byte order and allows removal of some fields
that are byteswapped duplicates.

Signed-off-by: David Howells <[email protected]>
  • Loading branch information
dhowells committed Mar 4, 2016
1 parent 4c198ad commit 0d12f8a
Show file tree
Hide file tree
Showing 18 changed files with 432 additions and 384 deletions.
4 changes: 1 addition & 3 deletions include/rxrpc/packet.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ typedef __be32 rxrpc_serial_net_t; /* on-the-wire Rx message serial number */
* on-the-wire Rx packet header
* - all multibyte fields should be in network byte order
*/
struct rxrpc_header {
struct rxrpc_wire_header {
__be32 epoch; /* client boot timestamp */

__be32 cid; /* connection and channel ID */
Expand Down Expand Up @@ -68,8 +68,6 @@ struct rxrpc_header {

} __packed;

#define __rxrpc_header_off(X) offsetof(struct rxrpc_header,X)

extern const char *rxrpc_pkts[];

/*****************************************************************************/
Expand Down
20 changes: 7 additions & 13 deletions net/rxrpc/af_rxrpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static struct proto rxrpc_proto;
static const struct proto_ops rxrpc_rpc_ops;

/* local epoch for detecting local-end reset */
__be32 rxrpc_epoch;
u32 rxrpc_epoch;

/* current debugging ID */
atomic_t rxrpc_debug_id;
Expand Down Expand Up @@ -125,7 +125,6 @@ static int rxrpc_bind(struct socket *sock, struct sockaddr *saddr, int len)
struct sock *sk = sock->sk;
struct rxrpc_local *local;
struct rxrpc_sock *rx = rxrpc_sk(sk), *prx;
__be16 service_id;
int ret;

_enter("%p,%p,%d", rx, saddr, len);
Expand All @@ -152,14 +151,12 @@ static int rxrpc_bind(struct socket *sock, struct sockaddr *saddr, int len)

rx->local = local;
if (srx->srx_service) {
service_id = htons(srx->srx_service);
write_lock_bh(&local->services_lock);
list_for_each_entry(prx, &local->services, listen_link) {
if (prx->service_id == service_id)
if (prx->srx.srx_service == srx->srx_service)
goto service_in_use;
}

rx->service_id = service_id;
list_add_tail(&rx->listen_link, &local->services);
write_unlock_bh(&local->services_lock);

Expand Down Expand Up @@ -276,7 +273,6 @@ struct rxrpc_call *rxrpc_kernel_begin_call(struct socket *sock,
struct rxrpc_transport *trans;
struct rxrpc_call *call;
struct rxrpc_sock *rx = rxrpc_sk(sock->sk);
__be16 service_id;

_enter(",,%x,%lx", key_serial(key), user_call_ID);

Expand All @@ -299,16 +295,15 @@ struct rxrpc_call *rxrpc_kernel_begin_call(struct socket *sock,
atomic_inc(&trans->usage);
}

service_id = rx->service_id;
if (srx)
service_id = htons(srx->srx_service);
if (!srx)
srx = &rx->srx;

if (!key)
key = rx->key;
if (key && !key->payload.data[0])
key = NULL; /* a no-security key */

bundle = rxrpc_get_bundle(rx, trans, key, service_id, gfp);
bundle = rxrpc_get_bundle(rx, trans, key, srx->srx_service, gfp);
if (IS_ERR(bundle)) {
call = ERR_CAST(bundle);
goto out;
Expand Down Expand Up @@ -425,7 +420,6 @@ static int rxrpc_connect(struct socket *sock, struct sockaddr *addr,
}

rx->trans = trans;
rx->service_id = htons(srx->srx_service);
rx->sk.sk_state = RXRPC_CLIENT_CONNECTED;

release_sock(&rx->sk);
Expand Down Expand Up @@ -778,7 +772,7 @@ static struct proto rxrpc_proto = {
.name = "RXRPC",
.owner = THIS_MODULE,
.obj_size = sizeof(struct rxrpc_sock),
.max_header = sizeof(struct rxrpc_header),
.max_header = sizeof(struct rxrpc_wire_header),
};

static const struct net_proto_family rxrpc_family_ops = {
Expand All @@ -796,7 +790,7 @@ static int __init af_rxrpc_init(void)

BUILD_BUG_ON(sizeof(struct rxrpc_skb_priv) > FIELD_SIZEOF(struct sk_buff, cb));

rxrpc_epoch = htonl(get_seconds());
rxrpc_epoch = get_seconds();

ret = -ENOMEM;
rxrpc_call_jar = kmem_cache_create(
Expand Down
40 changes: 24 additions & 16 deletions net/rxrpc/ar-accept.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* generate a connection-level abort
*/
static int rxrpc_busy(struct rxrpc_local *local, struct sockaddr_rxrpc *srx,
struct rxrpc_header *hdr)
struct rxrpc_wire_header *whdr)
{
struct msghdr msg;
struct kvec iov[1];
Expand All @@ -36,25 +36,21 @@ static int rxrpc_busy(struct rxrpc_local *local, struct sockaddr_rxrpc *srx,

_enter("%d,,", local->debug_id);

whdr->type = RXRPC_PACKET_TYPE_BUSY;
whdr->serial = htonl(1);

msg.msg_name = &srx->transport.sin;
msg.msg_namelen = sizeof(srx->transport.sin);
msg.msg_control = NULL;
msg.msg_controllen = 0;
msg.msg_flags = 0;

hdr->seq = 0;
hdr->type = RXRPC_PACKET_TYPE_BUSY;
hdr->flags = 0;
hdr->userStatus = 0;
hdr->_rsvd = 0;

iov[0].iov_base = hdr;
iov[0].iov_len = sizeof(*hdr);
iov[0].iov_base = whdr;
iov[0].iov_len = sizeof(*whdr);

len = iov[0].iov_len;

hdr->serial = htonl(1);
_proto("Tx BUSY %%%u", ntohl(hdr->serial));
_proto("Tx BUSY %%1");

ret = kernel_sendmsg(local->socket, &msg, iov, 1, len);
if (ret < 0) {
Expand Down Expand Up @@ -211,8 +207,8 @@ void rxrpc_accept_incoming_calls(struct work_struct *work)
struct rxrpc_skb_priv *sp;
struct sockaddr_rxrpc srx;
struct rxrpc_sock *rx;
struct rxrpc_wire_header whdr;
struct sk_buff *skb;
__be16 service_id;
int ret;

_enter("%d", local->debug_id);
Expand Down Expand Up @@ -240,6 +236,19 @@ void rxrpc_accept_incoming_calls(struct work_struct *work)

sp = rxrpc_skb(skb);

/* Set up a response packet header in case we need it */
whdr.epoch = htonl(sp->hdr.epoch);
whdr.cid = htonl(sp->hdr.cid);
whdr.callNumber = htonl(sp->hdr.callNumber);
whdr.seq = htonl(sp->hdr.seq);
whdr.serial = 0;
whdr.flags = 0;
whdr.type = 0;
whdr.userStatus = 0;
whdr.securityIndex = sp->hdr.securityIndex;
whdr._rsvd = 0;
whdr.serviceId = htons(sp->hdr.serviceId);

/* determine the remote address */
memset(&srx, 0, sizeof(srx));
srx.srx_family = AF_RXRPC;
Expand All @@ -256,18 +265,17 @@ void rxrpc_accept_incoming_calls(struct work_struct *work)
}

/* get the socket providing the service */
service_id = sp->hdr.serviceId;
read_lock_bh(&local->services_lock);
list_for_each_entry(rx, &local->services, listen_link) {
if (rx->service_id == service_id &&
if (rx->srx.srx_service == sp->hdr.serviceId &&
rx->sk.sk_state != RXRPC_CLOSE)
goto found_service;
}
read_unlock_bh(&local->services_lock);
goto invalid_service;

found_service:
_debug("found service %hd", ntohs(rx->service_id));
_debug("found service %hd", rx->srx.srx_service);
if (sk_acceptq_is_full(&rx->sk))
goto backlog_full;
sk_acceptq_added(&rx->sk);
Expand Down Expand Up @@ -296,7 +304,7 @@ void rxrpc_accept_incoming_calls(struct work_struct *work)
backlog_full:
read_unlock_bh(&local->services_lock);
busy:
rxrpc_busy(local, &srx, &sp->hdr);
rxrpc_busy(local, &srx, &whdr);
rxrpc_free_skb(skb);
goto process_next_packet;

Expand Down
Loading

0 comments on commit 0d12f8a

Please sign in to comment.