Problems with L2CAP: deferred setup and MTU

Problems with L2CAP: deferred setup and MTU

[Elvis Pfützenreuter]

I am currently investigating two problems with L2CAP sockets. Since I am not a kernel guy, I will take an indefinite amount of time to find and fix them; perhaps someone else can find and fix it in 5 minutes, so I think it is worth reporting.

First problem is ERTM x MTU, sockets configured with a MTU != 672. Normal sockets exchange MTU in initialization:

./l2test -P 4099 -I 1000 -O 1000
l2test[3211]: Connect from 00:1B:DC:0F:C8:A9 [imtu 1000, omtu 1000, flush_to 65535, mode 0, handle 12, class 0x480100]

but ERTM sockets seem to ignore the setsockopt() value and exchange the default size (672):

./l2test -P 4099 -I 1000 -O 1000 -X ERTM
l2test[3228]: Connect from 00:1B:DC:0F:C8:A9 [imtu 1000, omtu 672, flush_to 65535, mode 3, handle 11, class 0x480100]

(obviously the initiator side uses the same parameters)


Second problem is about deferred setup in accept()ed sockets. Sometimes POLLOUT times out (or waits forever if timeout is 0). The following read(buf, 1) works, and the connection works normally after this. 

The pattern seems to be: the first accept()ed socket of a process works ok regarding POLLOUT; the following connections do not.

This is normally not a problem, but if you try to read MTU using getsockopt() right after connection is set up, and if POLLOUT failed, MTU values are not updated. If you sleep a small amount of time like 0.1s then getsockopt() returns the expected values.

Finally, I have noted that playing with deferred setup sockets yields frequent kernel panics, like 1 in 3 rounds. This has prevented me to find more elaborate patterns (like ERTM x normal sockets etc.). The panics happen at listen() side; the connect() side is not affected. (Tested with 2.6.36 stock and bluetooth-next).

--
Elvis

Re: Problems with L2CAP: deferred setup and MTU

[Gustavo F. Padovan]

Hi Elvis,

* Elvis Pfützenreuter <epx@signove.com> [2011-01-19 17:10:51 -0200]:

> I am currently investigating two problems with L2CAP sockets. Since I am not a kernel guy, I will take an indefinite amount of time to find and fix them; perhaps someone else can find and fix it in 5 minutes, so I think it is worth reporting.
> 
> First problem is ERTM x MTU, sockets configured with a MTU != 672. Normal sockets exchange MTU in initialization:
> 
> ./l2test -P 4099 -I 1000 -O 1000
> l2test[3211]: Connect from 00:1B:DC:0F:C8:A9 [imtu 1000, omtu 1000, flush_to 65535, mode 0, handle 12, class 0x480100]
> 
> but ERTM sockets seem to ignore the setsockopt() value and exchange the default size (672):
> 
> ./l2test -P 4099 -I 1000 -O 1000 -X ERTM
> l2test[3228]: Connect from 00:1B:DC:0F:C8:A9 [imtu 1000, omtu 672, flush_to 65535, mode 3, handle 11, class 0x480100]
> 
> (obviously the initiator side uses the same parameters)

I pushed I fix for this to bluetooth-next-2.6

Author: Gustavo F. Padovan <padovan@profusion.mobi>
Date:   Mon Jan 24 16:01:43 2011 -0200

    Bluetooth: Fix setting of MTU for ERTM and Streaming Mode
    
    The desired MTU should be sent in an Config_Req for all modes.
    
    Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ff191b9..1ee18c6 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2588,11 +2588,11 @@ static int l2cap_build_conf_req(struct sock *sk, void *data)
        }
 
 done:
+       if (pi->imtu != L4CAP_DEFAULT_MTU)
+               l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
+
        switch (pi->mode) {
        case L2CAP_MODE_BASIC:
-               if (pi->imtu != L2CAP_DEFAULT_MTU)
-                       l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
-
                if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
                                !(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
                        break;


-- 
Gustavo F. Padovan
http://profusion.mobi

Re: Problems with L2CAP: deferred setup and MTU

[Ville Tervo]

Hi Gustavo,

On Mon, Jan 24, 2011 at 05:09:13PM -0200, ext Gustavo F. Padovan wrote:
> Hi Elvis,
> 
> * Elvis Pfützenreuter <epx@signove.com> [2011-01-19 17:10:51 -0200]:
> 
> > I am currently investigating two problems with L2CAP sockets. Since I am not a kernel guy, I will take an indefinite amount of time to find and fix them; perhaps someone else can find and fix it in 5 minutes, so I think it is worth reporting.
> > 
> > First problem is ERTM x MTU, sockets configured with a MTU != 672. Normal sockets exchange MTU in initialization:
> > 
> > ./l2test -P 4099 -I 1000 -O 1000
> > l2test[3211]: Connect from 00:1B:DC:0F:C8:A9 [imtu 1000, omtu 1000, flush_to 65535, mode 0, handle 12, class 0x480100]
> > 
> > but ERTM sockets seem to ignore the setsockopt() value and exchange the default size (672):
> > 
> > ./l2test -P 4099 -I 1000 -O 1000 -X ERTM
> > l2test[3228]: Connect from 00:1B:DC:0F:C8:A9 [imtu 1000, omtu 672, flush_to 65535, mode 3, handle 11, class 0x480100]
> > 
> > (obviously the initiator side uses the same parameters)
> 
> I pushed I fix for this to bluetooth-next-2.6
> 
> Author: Gustavo F. Padovan <padovan@profusion.mobi>
> Date:   Mon Jan 24 16:01:43 2011 -0200
> 
>     Bluetooth: Fix setting of MTU for ERTM and Streaming Mode
>     
>     The desired MTU should be sent in an Config_Req for all modes.
>     
>     Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> 
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index ff191b9..1ee18c6 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -2588,11 +2588,11 @@ static int l2cap_build_conf_req(struct sock *sk, void *data)
>         }
>  
>  done:
> +       if (pi->imtu != L4CAP_DEFAULT_MTU)
                          ^^^
Probably you noticed this typo already?

> +               l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
> +
>         switch (pi->mode) {
>         case L2CAP_MODE_BASIC:
> -               if (pi->imtu != L2CAP_DEFAULT_MTU)
> -                       l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
> -
>                 if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
>                                 !(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
>                         break;


-- 
Ville