Re: [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Marcel,

On Tue, Aug 31, 2021 at 1:35 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > bt_skb_sendmsg helps takes care of allocation the skb and copying the
> > the contents of msg over to the skb while checking for possible errors
> > so it should be safe to call it without holding lock_sock.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > include/net/bluetooth/bluetooth.h | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index 9125effbf448..1bfb5d3d24dd 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -420,6 +420,29 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk,
> >       return NULL;
> > }
> >
> > +/* Shall not be called with lock_sock held */
> > +static inline struct sk_buff *bt_skb_sendmsg(struct sock *sk,
> > +                                          struct msghdr *msg,
> > +                                          size_t len, int reserve)
> > +{
> > +     struct sk_buff *skb;
> > +     int err;
> > +
> > +     skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err);
> > +     if (!skb)
> > +             return ERR_PTR(err);
> > +
> > +     if (memcpy_from_msg(skb_put(skb, len), msg, len)) {
> > +             kfree_skb(skb);
> > +             return ERR_PTR(-EFAULT);
> > +     }
> > +
> > +     skb_reserve(skb, reserve);
> > +     skb->priority = sk->sk_priority;
>
> have you tested this? I remember vaguely (really vaguely actually) that skb_reserve needs to be called before you do any changes to the buffer via skb_put and others. Especially since bt_skb_send_alloc already calls skb_reserve in the first place.

Yep, that and the fact that we need both a header and footer size to
be given separately since the len should only account for the payload
itself, anyway I will send an update shortly and Ive now tested with
rfcomm-tester and they are all passing.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz