[PATCH] [RFC] HDP: reject MDL reconnection with different MTU

[PATCH] [RFC] HDP: reject MDL reconnection with different MTU

[Elvis Pfützenreuter]

From: Elvis Pfutzenreuter <epx@signove.com>

This patch implements refusal of a MDL reconnection if the new L2CAP
connection presents a different MTU. Accordingly to HDP spec. item 3.5.

It aims to pass the TC_SNK_HCT_BV_07_C PTS test. (It does not pass yet
because PTS itself seems to have issues. See ticket 7121 for details.)
---
 health/hdp.c       |   17 +++++++++++++++--
 health/hdp_types.h |    1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/health/hdp.c b/health/hdp.c
index dc1f803..4f89958 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -874,6 +874,7 @@ static gboolean check_channel_conf(struct hdp_channel *chan)
 	GError *err = NULL;
 	GIOChannel *io;
 	uint8_t mode;
+	uint16_t mtu;
 	int fd;
 
 	fd = mcap_mdl_get_fd(chan->mdl);
@@ -883,6 +884,7 @@ static gboolean check_channel_conf(struct hdp_channel *chan)
 
 	if (!bt_io_get(io, BT_IO_L2CAP, &err,
 			BT_IO_OPT_MODE, &mode,
+			BT_IO_OPT_OMTU, &mtu,
 			BT_IO_OPT_INVALID)) {
 		error("Error: %s", err->message);
 		g_io_channel_unref(io);
@@ -894,13 +896,24 @@ static gboolean check_channel_conf(struct hdp_channel *chan)
 
 	switch (chan->config) {
 	case HDP_RELIABLE_DC:
-		return mode == L2CAP_MODE_ERTM;
+		if (mode != L2CAP_MODE_ERTM)
+			return FALSE;
+		break;
 	case HDP_STREAMING_DC:
-		return mode == L2CAP_MODE_STREAMING;
+		if (mode != L2CAP_MODE_STREAMING)
+			return FALSE;
+		break;
 	default:
 		error("Error: Connected with unknown configuration");
 		return FALSE;
 	}
+
+	DBG("MDL MTU: %d; Channel MTU: %d", mtu, chan->mtu);
+
+	if (!chan->mtu)
+		chan->mtu = mtu;
+
+	return chan->mtu == mtu;
 }
 
 static void hdp_mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
diff --git a/health/hdp_types.h b/health/hdp_types.h
index fffdb32..714cf9a 100644
--- a/health/hdp_types.h
+++ b/health/hdp_types.h
@@ -120,6 +120,7 @@ struct hdp_channel {
 	uint8_t			config;		/* Channel configuration */
 	uint8_t			mdep;		/* Remote MDEP */
 	uint16_t		mdlid;		/* Data channel Id */
+	uint16_t		mtu;		/* Channel MTU */
 	struct hdp_echo_data	*edata;		/* private data used by echo channels */
 	gint			ref;		/* Reference counter */
 };
-- 
1.7.0.4

Re: [PATCH] [RFC] HDP: reject MDL reconnection with different MTU

[José Antonio Santos Cadenas]

 Hi Elvis,

 On Thu, 13 Jan 2011 21:11:04 -0200, Elvis Pfützenreuter wrote:
> From: Elvis Pfutzenreuter <epx@signove.com>
>
> This patch implements refusal of a MDL reconnection if the new L2CAP
> connection presents a different MTU. Accordingly to HDP spec. item 
> 3.5.
>
> It aims to pass the TC_SNK_HCT_BV_07_C PTS test. (It does not pass 
> yet
> because PTS itself seems to have issues. See ticket 7121 for 
> details.)

 Nice catch!

 The patch seems OK for me, but please, let me some time for review this 
 issue more deeply before pushing the patch.

 Regards.

 Jose.

> ---
>  health/hdp.c       |   17 +++++++++++++++--
>  health/hdp_types.h |    1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/health/hdp.c b/health/hdp.c
> index dc1f803..4f89958 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -874,6 +874,7 @@ static gboolean check_channel_conf(struct
> hdp_channel *chan)
>  	GError *err = NULL;
>  	GIOChannel *io;
>  	uint8_t mode;
> +	uint16_t mtu;
>  	int fd;
>
>  	fd = mcap_mdl_get_fd(chan->mdl);
> @@ -883,6 +884,7 @@ static gboolean check_channel_conf(struct
> hdp_channel *chan)
>
>  	if (!bt_io_get(io, BT_IO_L2CAP, &err,
>  			BT_IO_OPT_MODE, &mode,
> +			BT_IO_OPT_OMTU, &mtu,
>  			BT_IO_OPT_INVALID)) {
>  		error("Error: %s", err->message);
>  		g_io_channel_unref(io);
> @@ -894,13 +896,24 @@ static gboolean check_channel_conf(struct
> hdp_channel *chan)
>
>  	switch (chan->config) {
>  	case HDP_RELIABLE_DC:
> -		return mode == L2CAP_MODE_ERTM;
> +		if (mode != L2CAP_MODE_ERTM)
> +			return FALSE;
> +		break;
>  	case HDP_STREAMING_DC:
> -		return mode == L2CAP_MODE_STREAMING;
> +		if (mode != L2CAP_MODE_STREAMING)
> +			return FALSE;
> +		break;
>  	default:
>  		error("Error: Connected with unknown configuration");
>  		return FALSE;
>  	}
> +
> +	DBG("MDL MTU: %d; Channel MTU: %d", mtu, chan->mtu);
> +
> +	if (!chan->mtu)
> +		chan->mtu = mtu;
> +
> +	return chan->mtu == mtu;
>  }
>
>  static void hdp_mcap_mdl_connected_cb(struct mcap_mdl *mdl, void 
> *data)
> diff --git a/health/hdp_types.h b/health/hdp_types.h
> index fffdb32..714cf9a 100644
> --- a/health/hdp_types.h
> +++ b/health/hdp_types.h
> @@ -120,6 +120,7 @@ struct hdp_channel {
>  	uint8_t			config;		/* Channel configuration */
>  	uint8_t			mdep;		/* Remote MDEP */
>  	uint16_t		mdlid;		/* Data channel Id */
> +	uint16_t		mtu;		/* Channel MTU */
>  	struct hdp_echo_data	*edata;		/* private data used by echo channels 
> */
>  	gint			ref;		/* Reference counter */
>  };

Re: [PATCH] [RFC] HDP: reject MDL reconnection with different MTU

[Jose Antonio Santos Cadenas]

2011/1/14 José Antonio Santos Cadenas <jcaden@libresoft.es>
>
> Hi Elvis,
>
> On Thu, 13 Jan 2011 21:11:04 -0200, Elvis Pfützenreuter wrote:
>>
>> From: Elvis Pfutzenreuter <epx@signove.com>
>>
>> This patch implements refusal of a MDL reconnection if the new L2CAP
>> connection presents a different MTU. Accordingly to HDP spec. item 3.5.
>>
>> It aims to pass the TC_SNK_HCT_BV_07_C PTS test. (It does not pass yet
>> because PTS itself seems to have issues. See ticket 7121 for details.)

Just a little fix, I guess the issue that you mentioned is 7212 and 7214.

Still reviewing...

>
> Nice catch!
>
> The patch seems OK for me, but please, let me some time for review this issue more deeply before pushing the patch.
>
> Regards.
>
> Jose.
>
>> ---
>>  health/hdp.c       |   17 +++++++++++++++--
>>  health/hdp_types.h |    1 +
>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/health/hdp.c b/health/hdp.c
>> index dc1f803..4f89958 100644
>> --- a/health/hdp.c
>> +++ b/health/hdp.c
>> @@ -874,6 +874,7 @@ static gboolean check_channel_conf(struct
>> hdp_channel *chan)
>>        GError *err = NULL;
>>        GIOChannel *io;
>>        uint8_t mode;
>> +       uint16_t mtu;
>>        int fd;
>>
>>        fd = mcap_mdl_get_fd(chan->mdl);
>> @@ -883,6 +884,7 @@ static gboolean check_channel_conf(struct
>> hdp_channel *chan)
>>
>>        if (!bt_io_get(io, BT_IO_L2CAP, &err,
>>                        BT_IO_OPT_MODE, &mode,
>> +                       BT_IO_OPT_OMTU, &mtu,
>>                        BT_IO_OPT_INVALID)) {
>>                error("Error: %s", err->message);
>>                g_io_channel_unref(io);
>> @@ -894,13 +896,24 @@ static gboolean check_channel_conf(struct
>> hdp_channel *chan)
>>
>>        switch (chan->config) {
>>        case HDP_RELIABLE_DC:
>> -               return mode == L2CAP_MODE_ERTM;
>> +               if (mode != L2CAP_MODE_ERTM)
>> +                       return FALSE;
>> +               break;
>>        case HDP_STREAMING_DC:
>> -               return mode == L2CAP_MODE_STREAMING;
>> +               if (mode != L2CAP_MODE_STREAMING)
>> +                       return FALSE;
>> +               break;
>>        default:
>>                error("Error: Connected with unknown configuration");
>>                return FALSE;
>>        }
>> +
>> +       DBG("MDL MTU: %d; Channel MTU: %d", mtu, chan->mtu);
>> +
>> +       if (!chan->mtu)
>> +               chan->mtu = mtu;
>> +
>> +       return chan->mtu == mtu;
>>  }
>>
>>  static void hdp_mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
>> diff --git a/health/hdp_types.h b/health/hdp_types.h
>> index fffdb32..714cf9a 100644
>> --- a/health/hdp_types.h
>> +++ b/health/hdp_types.h
>> @@ -120,6 +120,7 @@ struct hdp_channel {
>>        uint8_t                 config;         /* Channel configuration */
>>        uint8_t                 mdep;           /* Remote MDEP */
>>        uint16_t                mdlid;          /* Data channel Id */
>> +       uint16_t                mtu;            /* Channel MTU */
>>        struct hdp_echo_data    *edata;         /* private data used by echo channels */
>>        gint                    ref;            /* Reference counter */
>>  };
>
> --
> 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

Re: [PATCH] [RFC] HDP: reject MDL reconnection with different MTU

[Jose Antonio Santos Cadenas]

2011/1/14 Jose Antonio Santos Cadenas <jcaden@libresoft.es>:
> 2011/1/14 José Antonio Santos Cadenas <jcaden@libresoft.es>
>>
>> Hi Elvis,
>>
>> On Thu, 13 Jan 2011 21:11:04 -0200, Elvis Pfützenreuter wrote:
>>>
>>> From: Elvis Pfutzenreuter <epx@signove.com>
>>>
>>> This patch implements refusal of a MDL reconnection if the new L2CAP
>>> connection presents a different MTU. Accordingly to HDP spec. item 3.5.
>>>
>>> It aims to pass the TC_SNK_HCT_BV_07_C PTS test. (It does not pass yet
>>> because PTS itself seems to have issues. See ticket 7121 for details.)
>

I finish the tests I'd like to do. Just I'm concerned about why you
are using just OMTU value instead of both values IMTU and OMTU. Will
not be better to store and check both of them?

Let's see if the PTS issue gets fixed soon :).

Regards.

Jose.

Re: [PATCH] [RFC] HDP: reject MDL reconnection with different MTU

[Elvis Pfützenreuter]

On 14 Jan 2011, at 09:15 , Jose Antonio Santos Cadenas wrote:

> 2011/1/14 Jose Antonio Santos Cadenas <jcaden@libresoft.es>:
>> 2011/1/14 José Antonio Santos Cadenas <jcaden@libresoft.es>
>>> 
>>> Hi Elvis,
>>> 
>>> On Thu, 13 Jan 2011 21:11:04 -0200, Elvis Pfützenreuter wrote:
>>>> 
>>>> From: Elvis Pfutzenreuter <epx@signove.com>
>>>> 
>>>> This patch implements refusal of a MDL reconnection if the new L2CAP
>>>> connection presents a different MTU. Accordingly to HDP spec. item 3.5.
>>>> 
>>>> It aims to pass the TC_SNK_HCT_BV_07_C PTS test. (It does not pass yet
>>>> because PTS itself seems to have issues. See ticket 7121 for details.)
>> 
> 
> I finish the tests I'd like to do. Just I'm concerned about why you
> are using just OMTU value instead of both values IMTU and OMTU. Will
> not be better to store and check both of them?

Indeed. I will fix this and submit another round.