netdev - Re: [PATCH v2 2/3] net: usb: cdc_enc: New driver exposing USB CDC encapsulated protocols

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <201201152133.38183.oliver@neukum.org>
Date:	Sun, 15 Jan 2012 21:33:38 +0100
From:	Oliver Neukum <oliver@...kum.org>
To:	Bjørn Mork <bjorn@...k.no>
Cc:	netdev@...r.kernel.org, linux-usb@...r.kernel.org,
	Dan Williams <dcbw@...hat.com>,
	Thomas Schäfer <tschaefer@...nline.de>
Subject: Re: [PATCH v2 2/3] net: usb: cdc_enc: New driver exposing USB CDC encapsulated protocols

Am Sonntag, 15. Januar 2012, 20:52:38 schrieb Bjørn Mork:

> There is one fundamental design issue which worries me though: cdc-wdm
> doesn't support multiple simultaneous readers/writers.  This is sort of
> a blocker for my use case, as I will need to keep a reader/writer for
> the driver internal wwan management at all times.  So two simultaneous
> clients are necessary to actually have something available for
> userspace at all.

What exactly do you need? The ability to write while another task is reading?

> Are there any reasons for this choice, and do you think it's feasible

Easier to get the locking right.

> changing it without destroying the advantage of the well tested code?  I
> seem to remember that cdc-wdm allowed simultaneous reading/writing in
> the past, but this changed somewhere between 2.6.32 and 3.0. I have an

There might have been a locking bug which was easy to fix this way.

> Ericsson modem with two such AT command speaking interfaces, and I use a
> simple shell script to configure it.  At one point I had to switch from
> using /dev/cdc-wdm0 to /dev/ttyACM0 because the former started hanging
> on constructs like this:
> 
>     while read x; do
>     case "$x" in
>          foo)
>               echo -e "AT+bar\r" >/dev/cdc-wdm0
>               ;;
>          ...
>         esac
>     done </dev/cdc-wdm0
>               
> 
> This used to work with 2.6.32, but that might have been just pure luck?
> Anyway, that change really made cdc-wdm much less usable for me, and
> that sort of proves that supporting simultaneous read/write would be an
> improvement even for the existing supported devices.

Then feel free to add it, just beware of deadlocks.

> And then there are a couple of minor issues which I believe needs
> sorting out.  If I understand the WDM spec correctly (without bothering
> to actually read if, of course), then it only defines the interface but
> not the protocol.  Exactly like the embedded CDC Ethernet command
> interface.  So I have the Ericsson modem which which speaks an AT
> command protocol, of course with a number of vendor specific AT commands
> included.  And then I have the Huawei modem which speaks QMI.  Just
> mapping these to a number of /dev/cdc-wdmX interfaces isn't going to
> help user applications much.  They need to be told *what* protocol to
> use on the character device.  And the driver should know this, either by
> detection or maybe best: vid/pid based tables and sane fallbacks.

I see.

> Do you think it would be possible to add that (or similar meta data) to
> the cdc-wdmX devices?  Maybe letting it default to "AT"?  Or "unknown"?
> Any thoughts on this?  How do actual user applications deal with this?

Well, you need a pointer at the correct "control" interface anyway. Wouldn't you better
put the "protocol" attribute there?

	Regards
		Oliver
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html