* [PATCH v2] Bluetooth: Fix l2cap_tx_window_full @ 2012-02-08 13:47 Luiz Augusto von Dentz 2012-02-08 13:54 ` Ulisses Furquim 2012-04-27 13:34 ` Andrei Emeltchenko 0 siblings, 2 replies; 6+ messages in thread From: Luiz Augusto von Dentz @ 2012-02-08 13:47 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> l2cap_tx_window_full is not checking the window limit properly, first it computes based on sequence numbers which doesn't take into account the ReqSeq and always assume 64 not the real window size. To fix this now it just checks if the number of unacked frames is >= of tx window which is much simpler. Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- Add debug include/net/bluetooth/l2cap.h | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index 42fdbb8..bcfddb2 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -661,14 +661,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) static inline int l2cap_tx_window_full(struct l2cap_chan *ch) { - int sub; + BT_DBG("chan %p unacked %d tx_win %d", ch, ch->unacked_frames, + ch->remote_tx_win); - sub = (ch->next_tx_seq - ch->expected_ack_seq) % 64; - - if (sub < 0) - sub += 64; - - return sub == ch->remote_tx_win; + return ch->unacked_frames >= ch->remote_tx_win; } static inline __u16 __get_reqseq(struct l2cap_chan *chan, __u32 ctrl) -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: Fix l2cap_tx_window_full 2012-02-08 13:47 [PATCH v2] Bluetooth: Fix l2cap_tx_window_full Luiz Augusto von Dentz @ 2012-02-08 13:54 ` Ulisses Furquim 2012-02-08 13:59 ` Andrei Emeltchenko 2012-04-27 13:34 ` Andrei Emeltchenko 1 sibling, 1 reply; 6+ messages in thread From: Ulisses Furquim @ 2012-02-08 13:54 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth Hi Luiz, On Wed, Feb 8, 2012 at 11:47 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > l2cap_tx_window_full is not checking the window limit properly, first it > computes based on sequence numbers which doesn't take into account the > ReqSeq and always assume 64 not the real window size. > > To fix this now it just checks if the number of unacked frames is >= of > tx window which is much simpler. > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > Add debug > > include/net/bluetooth/l2cap.h | 10 +++------- > 1 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index 42fdbb8..bcfddb2 100644 > --- a/include/net/bluetooth/l2cap.h > +++ b/include/net/bluetooth/l2cap.h > @@ -661,14 +661,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) > > static inline int l2cap_tx_window_full(struct l2cap_chan *ch) > { > - int sub; > + BT_DBG("chan %p unacked %d tx_win %d", ch, ch->unacked_frames, > + ch->remote_tx_win); > > - sub = (ch->next_tx_seq - ch->expected_ack_seq) % 64; > - > - if (sub < 0) > - sub += 64; > - > - return sub == ch->remote_tx_win; > + return ch->unacked_frames >= ch->remote_tx_win; > } > > static inline __u16 __get_reqseq(struct l2cap_chan *chan, __u32 ctrl) > -- > 1.7.7.6 Looks good to me. Regards, -- Ulisses Furquim ProFUSION embedded systems http://profusion.mobi Mobile: +55 19 9250 0942 Skype: ulissesffs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: Fix l2cap_tx_window_full 2012-02-08 13:54 ` Ulisses Furquim @ 2012-02-08 13:59 ` Andrei Emeltchenko 2012-02-08 15:38 ` Gustavo Padovan 0 siblings, 1 reply; 6+ messages in thread From: Andrei Emeltchenko @ 2012-02-08 13:59 UTC (permalink / raw) To: Ulisses Furquim; +Cc: Luiz Augusto von Dentz, linux-bluetooth Hi all, On Wed, Feb 08, 2012 at 11:54:54AM -0200, Ulisses Furquim wrote: > Hi Luiz, > > On Wed, Feb 8, 2012 at 11:47 AM, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > l2cap_tx_window_full is not checking the window limit properly, first it > > computes based on sequence numbers which doesn't take into account the > > ReqSeq and always assume 64 not the real window size. > > > > To fix this now it just checks if the number of unacked frames is >= of > > tx window which is much simpler. > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > Add debug > > > > include/net/bluetooth/l2cap.h | 10 +++------- > > 1 files changed, 3 insertions(+), 7 deletions(-) > > > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > > index 42fdbb8..bcfddb2 100644 > > --- a/include/net/bluetooth/l2cap.h > > +++ b/include/net/bluetooth/l2cap.h > > @@ -661,14 +661,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) > > > > static inline int l2cap_tx_window_full(struct l2cap_chan *ch) > > { > > - int sub; > > + BT_DBG("chan %p unacked %d tx_win %d", ch, ch->unacked_frames, > > + ch->remote_tx_win); > > > > - sub = (ch->next_tx_seq - ch->expected_ack_seq) % 64; > > - > > - if (sub < 0) > > - sub += 64; > > - > > - return sub == ch->remote_tx_win; > > + return ch->unacked_frames >= ch->remote_tx_win; > > } > > > > static inline __u16 __get_reqseq(struct l2cap_chan *chan, __u32 ctrl) > > -- > > 1.7.7.6 > > Looks good to me. Same here, I am wondering why it was done in a such complex way? Maybe there is some logic behind? Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: Fix l2cap_tx_window_full 2012-02-08 13:59 ` Andrei Emeltchenko @ 2012-02-08 15:38 ` Gustavo Padovan 2012-02-08 16:03 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 6+ messages in thread From: Gustavo Padovan @ 2012-02-08 15:38 UTC (permalink / raw) To: Andrei Emeltchenko, Ulisses Furquim, Luiz Augusto von Dentz, linux-bluetooth Hi Andrei, * Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2012-02-08 15:59:11 +0200]: > Hi all, > > On Wed, Feb 08, 2012 at 11:54:54AM -0200, Ulisses Furquim wrote: > > Hi Luiz, > > > > On Wed, Feb 8, 2012 at 11:47 AM, Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > > > l2cap_tx_window_full is not checking the window limit properly, first it > > > computes based on sequence numbers which doesn't take into account the > > > ReqSeq and always assume 64 not the real window size. > > > > > > To fix this now it just checks if the number of unacked frames is >= of > > > tx window which is much simpler. > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > --- > > > Add debug > > > > > > include/net/bluetooth/l2cap.h | 10 +++------- > > > 1 files changed, 3 insertions(+), 7 deletions(-) > > > > > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > > > index 42fdbb8..bcfddb2 100644 > > > --- a/include/net/bluetooth/l2cap.h > > > +++ b/include/net/bluetooth/l2cap.h > > > @@ -661,14 +661,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) > > > > > > static inline int l2cap_tx_window_full(struct l2cap_chan *ch) > > > { > > > - int sub; > > > + BT_DBG("chan %p unacked %d tx_win %d", ch, ch->unacked_frames, > > > + ch->remote_tx_win); > > > > > > - sub = (ch->next_tx_seq - ch->expected_ack_seq) % 64; > > > - > > > - if (sub < 0) > > > - sub += 64; > > > - > > > - return sub == ch->remote_tx_win; > > > + return ch->unacked_frames >= ch->remote_tx_win; > > > } > > > > > > static inline __u16 __get_reqseq(struct l2cap_chan *chan, __u32 ctrl) > > > -- > > > 1.7.7.6 > > > > Looks good to me. > > Same here, I am wondering why it was done in a such complex way? Maybe > there is some logic behind? I followed the specification, next_tx_seq minus expecpted_ack_seq is the definition of the current size of the tx_window. Anyway, Acked-by: Gustavo F. Padovan <padovan@profusion.mobi> Gustavo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: Fix l2cap_tx_window_full 2012-02-08 15:38 ` Gustavo Padovan @ 2012-02-08 16:03 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 6+ messages in thread From: Luiz Augusto von Dentz @ 2012-02-08 16:03 UTC (permalink / raw) To: Andrei Emeltchenko, Ulisses Furquim, Luiz Augusto von Dentz, linux-bluetooth Hi Gustavo, On Wed, Feb 8, 2012 at 5:38 PM, Gustavo Padovan <padovan@profusion.mobi> wrote: > > I followed the specification, next_tx_seq minus expecpted_ack_seq is the > definition of the current size of the tx_window. > > Anyway, > > Acked-by: Gustavo F. Padovan <padovan@profusion.mobi> I notice it, It was actually possible to fix this by using __seq_offset but then I notice in Mat's patch he just use unacked_frames which is way simpler to read/use. Btw, there is still something else preventing me to use bigger MTU with OBEX, I can use up to 32k in kvm but only 16k if I boot it directly, at some point it just stall in the transmitter even though the receiver acks and the window is not full. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: Fix l2cap_tx_window_full 2012-02-08 13:47 [PATCH v2] Bluetooth: Fix l2cap_tx_window_full Luiz Augusto von Dentz 2012-02-08 13:54 ` Ulisses Furquim @ 2012-04-27 13:34 ` Andrei Emeltchenko 1 sibling, 0 replies; 6+ messages in thread From: Andrei Emeltchenko @ 2012-04-27 13:34 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth On Wed, Feb 08, 2012 at 03:47:35PM +0200, Luiz Augusto von Dentz wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > l2cap_tx_window_full is not checking the window limit properly, first it > computes based on sequence numbers which doesn't take into account the > ReqSeq and always assume 64 not the real window size. > > To fix this now it just checks if the number of unacked frames is >= of > tx window which is much simpler. So what is the status with this patch? Best regards Andrei Emeltchenko > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > Add debug > > include/net/bluetooth/l2cap.h | 10 +++------- > 1 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index 42fdbb8..bcfddb2 100644 > --- a/include/net/bluetooth/l2cap.h > +++ b/include/net/bluetooth/l2cap.h > @@ -661,14 +661,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) > > static inline int l2cap_tx_window_full(struct l2cap_chan *ch) > { > - int sub; > + BT_DBG("chan %p unacked %d tx_win %d", ch, ch->unacked_frames, > + ch->remote_tx_win); > > - sub = (ch->next_tx_seq - ch->expected_ack_seq) % 64; > - > - if (sub < 0) > - sub += 64; > - > - return sub == ch->remote_tx_win; > + return ch->unacked_frames >= ch->remote_tx_win; > } > > static inline __u16 __get_reqseq(struct l2cap_chan *chan, __u32 ctrl) > -- > 1.7.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-27 13:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-08 13:47 [PATCH v2] Bluetooth: Fix l2cap_tx_window_full Luiz Augusto von Dentz 2012-02-08 13:54 ` Ulisses Furquim 2012-02-08 13:59 ` Andrei Emeltchenko 2012-02-08 15:38 ` Gustavo Padovan 2012-02-08 16:03 ` Luiz Augusto von Dentz 2012-04-27 13:34 ` Andrei Emeltchenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.