2011-06-15 03:54:06

by Nami

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: FTP and OPP over L2CAP

From: nami <[email protected]>


Signed-off-by: Nami <[email protected]>
---
glib/obex-lowlevel.c | 2 +-
include/openobex/obex.h | 2 +-
include/openobex/obex_const.h | 5 +++++
lib/obex.c | 3 ++-
lib/obex_main.c | 5 ++++-
lib/obex_transport.h | 1 +
6 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/glib/obex-lowlevel.c b/glib/obex-lowlevel.c
index 65172fc..660fc37 100644
--- a/glib/obex-lowlevel.c
+++ b/glib/obex-lowlevel.c
@@ -496,7 +496,7 @@ obex_t *obex_open(int fd, obex_callback_t *callback, void *data)

OBEX_SetTransportMTU(handle, sizeof(context->buf), sizeof(context->buf));

- if (FdOBEX_TransportSetup(handle, fd, fd, 0) < 0) {
+ if (FdOBEX_TransportSetup(handle, fd, fd, 0, OBEX_MT_STREAM) < 0) {
OBEX_Cleanup(handle);
return NULL;
}
diff --git a/include/openobex/obex.h b/include/openobex/obex.h
index f948113..0d7afec 100644
--- a/include/openobex/obex.h
+++ b/include/openobex/obex.h
@@ -147,7 +147,7 @@ OPENOBEX_SYMBOL(int) BtOBEX_TransportConnect(obex_t *self, bt_addr_t *src, bt_ad
/*
* OBEX File API
*/
-OPENOBEX_SYMBOL(int) FdOBEX_TransportSetup(obex_t *self, int rfd, int wfd, int mtu);
+OPENOBEX_SYMBOL(int) FdOBEX_TransportSetup(obex_t *self, int rfd, int wfd, int mtu, int fmt);

/*
* OBEX interface discovery API
diff --git a/include/openobex/obex_const.h b/include/openobex/obex_const.h
index cb7afeb..4a0a7e9 100644
--- a/include/openobex/obex_const.h
+++ b/include/openobex/obex_const.h
@@ -302,6 +302,11 @@ enum obex_rsp_mode {
OBEX_RSP_MODE_SINGLE = 1, /**< single response mode (SRM) */
};

+enum obex_transport_format{
+ OBEX_MT_STREAM ,
+ OBEX_MT_SEQPACKET
+};
+
/* Min, Max and default transport MTU */
#define OBEX_DEFAULT_MTU 1024
#define OBEX_MINIMUM_MTU 255
diff --git a/lib/obex.c b/lib/obex.c
index 5ca14a2..06c890a 100644
--- a/lib/obex.c
+++ b/lib/obex.c
@@ -1118,7 +1118,7 @@ int CALLAPI BtOBEX_TransportConnect(obex_t *self, bdaddr_t *src,
\return -1 or negative error code on error
*/
LIB_SYMBOL
-int CALLAPI FdOBEX_TransportSetup(obex_t *self, int rfd, int wfd, int mtu)
+int CALLAPI FdOBEX_TransportSetup(obex_t *self, int rfd, int wfd, int mtu, int fmt)
{
DEBUG(4, "\n");

@@ -1131,6 +1131,7 @@ int CALLAPI FdOBEX_TransportSetup(obex_t *self, int rfd, int wfd, int mtu)
self->trans.fd = rfd;
self->trans.data.fd.writefd = wfd;
self->trans.mtu = mtu ? mtu : self->mtu_tx_max;
+ self->trans.fmt = fmt;
return obex_transport_connect_request(self);
}

diff --git a/lib/obex_main.c b/lib/obex_main.c
index 387f271..8730419 100644
--- a/lib/obex_main.c
+++ b/lib/obex_main.c
@@ -301,7 +301,10 @@ int obex_data_indication(obex_t *self)

/* First we need 3 bytes to be able to know how much data to read */
if (msg->data_size < sizeof(*hdr)) {
- actual = obex_transport_read(self, sizeof(*hdr)-msg->data_size);
+ if(self->trans.fmt == OBEX_MT_STREAM)
+ actual = obex_transport_read(self, sizeof(*hdr)-msg->data_size);
+ else
+ actual = obex_transport_read(self, self->mtu_rx);

DEBUG(4, "Got %d bytes\n", actual);

diff --git a/lib/obex_transport.h b/lib/obex_transport.h
index a39f563..e80a5cb 100644
--- a/lib/obex_transport.h
+++ b/lib/obex_transport.h
@@ -114,6 +114,7 @@ typedef struct obex_transport {

int connected; /* Link connection state */
unsigned int mtu; /* Tx MTU of the link */
+ enum obex_transport_format fmt; /* Data transport foramt */

} obex_transport_t;

--
1.7.1



2011-06-15 15:01:43

by Piotr Zgórecki

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: FTP and OPP over L2CAP

Marcel,

Allow me to quote GOEP this time :)
"
>From 7.1 L2CAP Interoperability Requirements
7.1.1 MTU option
According to [1], OBEX packets must fit completely within a single
L2CAP SDU, so an implementation shall either configure the L2CAP MTU
to be large enough to hold the largest OBEX packet or configure the
OBEX packet size to be small enough to fit within the L2CAP MTU. In
any case the OBEX packet size should be set as large as possible,
especially in the case that the Single Response Mode feature of IrOBEX
is not supported. By definition:
=95 The L2CAP MTU refers to the SDU size
=95 The SDU is the application packet (OBEX packet in GOEP)
"

So you can't hide the MTUs from OBEX. What is the point of using a
streaming socket then ?
I think it's a good illustration of the point I was trying to make in
my previous email. L2CAP uses SDUs, which unlike e.g. segments in TCP
_are_ exposed to the upper layers, and you'll find running away from
them rather difficult :)

Cheers,
Piotr

On 15 June 2011 15:21, Marcel Holtmann <[email protected]> wrote:
> Hi Tim,
>
>> > +enum obex_transport_format{
>> > + =A0 =A0 =A0OBEX_MT_STREAM ,
>> > + =A0 =A0 =A0OBEX_MT_SEQPACKET
>> > +};
>> > +
>> > And more important, I have no idea why are you doing this anyway. Even
>> > with L2CAP this should run over SOCK_STREAM.
>>
>> Is that really so? =A0I expect exposing L2CAP ERTM as a stream could bre=
ak protocols above that expect datagram-boundary preservation (eg RTP). =A0=
Or can break protocols that mandate that only one higher-layer packet exist=
s in each L2CAP SDU. =A0For these the socket needs to be packet based.
>>
>> So, my surprise is that ERTM is exposed as SOCK_STREAM. =A0So I do have =
an idea why the patch mentions sequenced packet...
>
> you can do both for ERTM actually. You can create SOCK_SEQPACKET and
> have it negotiate ERTM. However you can not run SOCK_STREAM on a Basic
> Mode connection.
>
> But the real benefit for higher layer protocols is if you become a
> SOCK_STREAM. Because you are MTU free now. Using SOCK_SEQPACKET with
> ERTM or Basic Mode is not really much of a difference.
>
> Especially with OBEX that has its own MTU anyway in the first place has
> always been run on SOCK_STREAM so far, what is the different that you
> expect by running it of SOCK_SEQPACKET with ERTM?
>
> Regards
>
> Marcel
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>

2011-06-15 14:21:24

by Marcel Holtmann

[permalink] [raw]
Subject: RE: [PATCH 2/2] Bluetooth: FTP and OPP over L2CAP

Hi Tim,

> > +enum obex_transport_format{
> > + OBEX_MT_STREAM ,
> > + OBEX_MT_SEQPACKET
> > +};
> > +
> > And more important, I have no idea why are you doing this anyway. Even
> > with L2CAP this should run over SOCK_STREAM.
>
> Is that really so? I expect exposing L2CAP ERTM as a stream could break protocols above that expect datagram-boundary preservation (eg RTP). Or can break protocols that mandate that only one higher-layer packet exists in each L2CAP SDU. For these the socket needs to be packet based.
>
> So, my surprise is that ERTM is exposed as SOCK_STREAM. So I do have an idea why the patch mentions sequenced packet...

you can do both for ERTM actually. You can create SOCK_SEQPACKET and
have it negotiate ERTM. However you can not run SOCK_STREAM on a Basic
Mode connection.

But the real benefit for higher layer protocols is if you become a
SOCK_STREAM. Because you are MTU free now. Using SOCK_SEQPACKET with
ERTM or Basic Mode is not really much of a difference.

Especially with OBEX that has its own MTU anyway in the first place has
always been run on SOCK_STREAM so far, what is the different that you
expect by running it of SOCK_SEQPACKET with ERTM?

Regards

Marcel



2011-06-15 12:24:08

by tim.howes

[permalink] [raw]
Subject: RE: [PATCH 2/2] Bluetooth: FTP and OPP over L2CAP

SGkgTWFyY2VsLA0KDQoNCj4gK2VudW0gb2JleF90cmFuc3BvcnRfZm9ybWF0ew0KPiArICAgICAg
T0JFWF9NVF9TVFJFQU0gLA0KPiArICAgICAgT0JFWF9NVF9TRVFQQUNLRVQNCj4gK307DQo+ICsN
Cj4gQW5kIG1vcmUgaW1wb3J0YW50LCBJIGhhdmUgbm8gaWRlYSB3aHkgYXJlIHlvdSBkb2luZyB0
aGlzIGFueXdheS4gRXZlbg0KPiB3aXRoIEwyQ0FQIHRoaXMgc2hvdWxkIHJ1biBvdmVyIFNPQ0tf
U1RSRUFNLg0KDQpJcyB0aGF0IHJlYWxseSBzbz8gIEkgZXhwZWN0IGV4cG9zaW5nIEwyQ0FQIEVS
VE0gYXMgYSBzdHJlYW0gY291bGQgYnJlYWsgcHJvdG9jb2xzIGFib3ZlIHRoYXQgZXhwZWN0IGRh
dGFncmFtLWJvdW5kYXJ5IHByZXNlcnZhdGlvbiAoZWcgUlRQKS4gIE9yIGNhbiBicmVhayBwcm90
b2NvbHMgdGhhdCBtYW5kYXRlIHRoYXQgb25seSBvbmUgaGlnaGVyLWxheWVyIHBhY2tldCBleGlz
dHMgaW4gZWFjaCBMMkNBUCBTRFUuICBGb3IgdGhlc2UgdGhlIHNvY2tldCBuZWVkcyB0byBiZSBw
YWNrZXQgYmFzZWQuDQoNClNvLCBteSBzdXJwcmlzZSBpcyB0aGF0IEVSVE0gaXMgZXhwb3NlZCBh
cyBTT0NLX1NUUkVBTS4gIFNvIEkgZG8gaGF2ZSBhbiBpZGVhIHdoeSB0aGUgcGF0Y2ggbWVudGlv
bnMgc2VxdWVuY2VkIHBhY2tldC4uLg0KDQo+IEkgYXNzdW1lZCB0aGF0IE9QUCBhbmQgRlRQIG92
ZXIgTDJDQVAgYXJlIHJlcXVpcmluZyBlTDJDQVAgYWN0dWFsbHkNCj4gd2hpY2ggaXMgcmUtcHJl
c2VudGVkIGFzIGEgc3RyZWFtLg0KDQpZZXMsIE9CRVggb3ZlciBMMkNBUCByZXF1aXJlcyBlbmhh
bmNlZCByZXRyYW5zbWlzc2lvbiBtb2RlLg0KDQpUaW0NCg0KVGhpcyBtZXNzYWdlIGlzIGZvciB0
aGUgZGVzaWduYXRlZCByZWNpcGllbnQgb25seSBhbmQgbWF5IGNvbnRhaW4gcHJpdmlsZWdlZCwg
cHJvcHJpZXRhcnksIG9yIG90aGVyd2lzZSBwcml2YXRlIGluZm9ybWF0aW9uLiAgSWYgeW91IGhh
dmUgcmVjZWl2ZWQgaXQgaW4gZXJyb3IsIHBsZWFzZSBub3RpZnkgdGhlIHNlbmRlciBpbW1lZGlh
dGVseSBhbmQgZGVsZXRlIHRoZSBvcmlnaW5hbC4gIEFueSBvdGhlciB1c2Ugb2YgdGhlIGVtYWls
IGJ5IHlvdSBpcyBwcm9oaWJpdGVkLg0K

2011-06-15 11:17:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: FTP and OPP over L2CAP

Hi Nami,

> From: nami <[email protected]>
>
>
> Signed-off-by: Nami <[email protected]>

same as with the other one.

> ---
> glib/obex-lowlevel.c | 2 +-
> include/openobex/obex.h | 2 +-
> include/openobex/obex_const.h | 5 +++++
> lib/obex.c | 3 ++-
> lib/obex_main.c | 5 ++++-
> lib/obex_transport.h | 1 +
> 6 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/glib/obex-lowlevel.c b/glib/obex-lowlevel.c
> index 65172fc..660fc37 100644
> --- a/glib/obex-lowlevel.c
> +++ b/glib/obex-lowlevel.c
> @@ -496,7 +496,7 @@ obex_t *obex_open(int fd, obex_callback_t *callback, void *data)
>
> OBEX_SetTransportMTU(handle, sizeof(context->buf), sizeof(context->buf));
>
> - if (FdOBEX_TransportSetup(handle, fd, fd, 0) < 0) {
> + if (FdOBEX_TransportSetup(handle, fd, fd, 0, OBEX_MT_STREAM) < 0) {
> OBEX_Cleanup(handle);
> return NULL;
> }
> diff --git a/include/openobex/obex.h b/include/openobex/obex.h
> index f948113..0d7afec 100644
> --- a/include/openobex/obex.h
> +++ b/include/openobex/obex.h
> @@ -147,7 +147,7 @@ OPENOBEX_SYMBOL(int) BtOBEX_TransportConnect(obex_t *self, bt_addr_t *src, bt_ad
> /*
> * OBEX File API
> */
> -OPENOBEX_SYMBOL(int) FdOBEX_TransportSetup(obex_t *self, int rfd, int wfd, int mtu);
> +OPENOBEX_SYMBOL(int) FdOBEX_TransportSetup(obex_t *self, int rfd, int wfd, int mtu, int fmt);

this is breaking public API.

> /*
> * OBEX interface discovery API
> diff --git a/include/openobex/obex_const.h b/include/openobex/obex_const.h
> index cb7afeb..4a0a7e9 100644
> --- a/include/openobex/obex_const.h
> +++ b/include/openobex/obex_const.h
> @@ -302,6 +302,11 @@ enum obex_rsp_mode {
> OBEX_RSP_MODE_SINGLE = 1, /**< single response mode (SRM) */
> };
>
> +enum obex_transport_format{
> + OBEX_MT_STREAM ,
> + OBEX_MT_SEQPACKET
> +};
> +

And more important, I have no idea why are you doing this anyway. Even
with L2CAP this should run over SOCK_STREAM.

I assumed that OPP and FTP over L2CAP are requiring eL2CAP actually
which is re-presented as a stream.

Regards

Marcel