Hi Mat,
* Mat Martineau <mathewm@xxxxxxxxxxxxxx> [2010-08-10 12:15:03 -0700]:
> As enhanced L2CAP PDUs arrive, it is not necessary to copy them
> in to a separate skbuff. Instead, the skbuffs can be linked
> together as fragments, only being copied in to a linear buffer
> when the data is copied to userspace. This avoids the need
> to allocate additional buffers for incoming data, and
> eliminates copying of data payloads during SDU reassembly.
> This is of greater concern with high-speed AMP links than
> with BR/EDR.
>
> Signed-off-by: Mat Martineau <mathewm@xxxxxxxxxxxxxx>
> ---
> include/net/bluetooth/l2cap.h | 1 +
> net/bluetooth/l2cap.c | 66 +++++++++++++++++++++-------------------
> 2 files changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 2f3222f..9384e87 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -357,6 +357,7 @@ struct l2cap_pinfo {
> __u16 sdu_len;
> __u16 partial_sdu_len;
> struct sk_buff *sdu;
> + struct sk_buff *sdu_last_frag;
>
> __u8 ident;
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index b485c4a..0212035 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -290,6 +290,9 @@ static void l2cap_chan_del(struct sock *sk, int err)
> skb_queue_purge(SREJ_QUEUE(sk));
> skb_queue_purge(BUSY_QUEUE(sk));
>
> + if (l2cap_pi(sk)->sdu)
> + kfree_skb(l2cap_pi(sk)->sdu);
> +
> list_for_each_entry_safe(l, tmp, SREJ_LIST(sk), list) {
> list_del(&l->list);
> kfree(l);
> @@ -3635,6 +3638,27 @@ static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_s
> return 0;
> }
>
> +static inline void append_skb_frag(struct sk_buff *skb,
> + struct sk_buff *new_frag, struct sk_buff **last_frag)
Call this l2cap_append_skb_frag()
> +{
> + /* skb->len reflects data in skb as well as all fragments
> + skb->data_len reflects only data in fragments
> + */
> + BT_DBG("skb %p, new_frag %p, *last_frag %p", skb, new_frag, *last_frag);
> +
> + if (!skb_has_frags(skb))
> + skb_shinfo(skb)->frag_list = new_frag;
> +
> + new_frag->next = NULL;
> +
> + (*last_frag)->next = new_frag;
> + *last_frag = new_frag;
> +
> + skb->len += new_frag->len;
> + skb->data_len += new_frag->len;
> + skb->truesize += new_frag->truesize;
> +}
> +
Wondering if it is possible to do that append more simple, but I looked
through the kernel code it's not possible, we have to keep last frag
pointer.
This change should go to l2cap_streaming_reassembly_sdu() as well, then
you can get ride of partial_sdu_len in the struct l2cap_pinfo.
> static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 control)
> {
> struct l2cap_pinfo *pi = l2cap_pi(sk);
> @@ -3643,7 +3667,7 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
>
> switch (control & L2CAP_CTRL_SAR) {
> case L2CAP_SDU_UNSEGMENTED:
> - if (pi->conn_state & L2CAP_CONN_SAR_SDU)
> + if (pi->sdu)
pi->sdu can do the work work of pi->conn_state & L2CAP_CONN_SAR_SDU, so
a separated patch for that change sounds better (you can change that for
the Streaming mode at the same time, it's a similar code, but more simple).
--
Gustavo F. Padovan
http://padovan.org
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
