Re: [PATCH 6/7] usb: cdc-wdm: support command echo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Marcel Holtmann <marcel@xxxxxxxxxxxx> writes:

> Hi Oliver,
>
>> > This is preparing for supporting devices using other
>> > protocols than AT commands.  Such devices will typically not
>> > echo back the received command.
>> > 
>> > Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
>> > ---
>> > This won't affect existing users.  I was wondering how to enable
>> > echo in a sane way.  I did not want to that the tty route using
>> > an ioctl...  Although that is probably the only possible way
>> > if configuring this is required per client?
>> > 
>> > For now it is a static device specific attribute.  The assumption
>> > is that QMI devices will have this on to enable monitoring complete
>> > transactions from a read-only process, while AT based devices have
>> > it turned off as they echo back commands themselves
>> 
>> This makes me wonder why we want to work around user space's
>> stupidity of not knowing what it sent. 
>
> actually we do not even want this. Especially not for a binary protocol
> like QMI. It is pretty much a really bad idea.

Why?  

Echo is explicitly enabled only for QMI speaking interfaces for now.  I
agree that enabling it unconditionally for any unknown protocol is a bad
idea.

But as long as we know the transported protocol is QMI, then we also
knows that it includes both response/request/notification flags and
source of the packet.  Any application will need to filter out the
packets it wants anyway, and if you only want to see them in one
direction then you filter on e.g (sender != 0x80)

The reason for wanting echo is to enable applications monitoring both
outgoing and incoming packets, which are extremely useful for debugging
and development of the real user space applications.  I believe having
this feature will help user application development a lot, and having it
from the beginning will ensure that no-one believes they can get away
with no filtering.

BTW, this is not about not remembering what *you* sent.  It's about
whether or not you are allowed to see what your friendly nextdoor
application is sending.

An example of a single req/reply:

read 13 bytes

QMUX Header:
  len:    0x000c
  sender: 0x00 (client) <= so you know it's coming from host going to device
  svc:    0x01 (wds)
  cid:    0x01 

QMI Header:
  Flags:  0x00 (request) <= and you know it's a request too
  TXN:    0x0001
  Cmd:    0x0022 (GET_PKT_STATUS)
  Size:   0x0000


read 24 bytes

QMUX Header:
  len:    0x0017
  sender: 0x80 (service) <= from device to host
  svc:    0x01 (wds)
  cid:    0x01 

QMI Header:
  Flags:  0x02 (response)  <= and it was a reply
  TXN:    0x0001
  Cmd:    0x0022 (GET_PKT_STATUS)
  Size:   0x000b

  TLV:    0x02 (WDS/Get Packet Service Status Response/Result Code)
  Size:   0x0004
  Status: 0 (SUCCESS)
  Error:  0

  TLV:    0x01 (WDS/Get Packet Service Status Response/Status)
  Size:   0x0001
  Data:   02



This particular message is a very good example of why any application
using this is going to have to filter on these headers anyway.  You will
get the same message as a notification on connect/disconnect:



read 22 bytes

QMUX Header:
  len:    0x0015
  sender: 0x80 (service)
  svc:    0x01 (wds)
  cid:    0xff (broadcast)

QMI Header:
  Flags:  0x04 (indication) <= aka notification
  TXN:    0x0000
  Cmd:    0x0022 (GET_PKT_STATUS)
  Size:   0x0009

  TLV:    0x01 (WDS/Packet Service Status Report/Status)
  Size:   0x0002
  Data:   02 00

  TLV:    0x12 (!!! UNKNOWN !!!)
  Size:   0x0001
  Data:   04


Notice the small differences in the TLVs: The indication has no "result
code", but it has an "IP version" TLV ("4" => "IPv4").  But even worse:
The "status" TLV has a different format depending on that header flag.

You cannot parse this without first parsing the flags to detect whether
it's a reply or an indication.  And both of these messages would be
received whether or not we were echoing requests.  Only the request
would not.  But given that you must parse and filter, and that receiving
requests is useful, I will very much want to keep the echo.


>> However, this seems to be a traditional capability of ttys. And the other
>> major option to configure wireless devices is a pseudo serial interface.
>> That on the gripping hand argues for doing this exactly the way a tty does
>> it, namely by supporting the very ioctl ttys use.
>
> For AT commands, this is a feature of the hardware itself. And not some
> TTY or kernel magic. And we actually switch echoing off since nobody
> ever gets this right. Not even for AT commands.
>
> So again, a really really bad idea.

I suspect that you are thinking along the lines that there should be
only one single application using this device at the time.  The fact is
that there is no need to enforce such restrictions.  The device supports
dealing with multiple clients (hence the client ID registration scheme),
and it can be very useful not having to integrate all your
network/SMS/call management/monitoring stuff in a single complex
application.

Would you want wireshark to only show you the incoming packets?


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux