Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Eric Dumazet @ 2012-10-22 21:52 UTC (permalink / raw)
  To: Brian Haley; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <5085BF0A.2030602@hp.com>

On Mon, 2012-10-22 at 17:47 -0400, Brian Haley wrote:
> On 10/22/2012 05:37 PM, Eric Dumazet wrote:
> > On Mon, 2012-10-22 at 17:20 -0400, Brian Haley wrote:
> > 
> >> It's not much different from what's there:
> >>
> >> 	setsockopt("foo");
> >>
> >> 	rename foo -> bar
> >>
> >> 	index = getsockopt();
> >> 	if_indextoname(index) -> "bar"
> >>
> >> I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *'
> >> to getsockopt(), I don't think any other value is non-symmetrical like this.
> >>
> >> -Brian
> > 
> > I meant another cpu can be changing dev->name[] content while the
> > strcpy() is done, and you get a mangled devname, like "for" or "bao"
> > instead of "foo" or "bar"
> 
> Even when holding the rcu_read_lock()?  I'd have to hold the rtnl lock there?

Yes, rcu_read_lock() only makes sure the device doesnt disappear.

But its name can be changed.

You could use a seqcount_t, so that readers dont have to lock rtnl.

But do we really want to return a name here, I am not yet convinced.

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Brian Haley @ 2012-10-22 21:47 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <1350941831.8609.1355.camel@edumazet-glaptop>

On 10/22/2012 05:37 PM, Eric Dumazet wrote:
> On Mon, 2012-10-22 at 17:20 -0400, Brian Haley wrote:
> 
>> It's not much different from what's there:
>>
>> 	setsockopt("foo");
>>
>> 	rename foo -> bar
>>
>> 	index = getsockopt();
>> 	if_indextoname(index) -> "bar"
>>
>> I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *'
>> to getsockopt(), I don't think any other value is non-symmetrical like this.
>>
>> -Brian
> 
> I meant another cpu can be changing dev->name[] content while the
> strcpy() is done, and you get a mangled devname, like "for" or "bao"
> instead of "foo" or "bar"

Even when holding the rcu_read_lock()?  I'd have to hold the rtnl lock there?

-Brian

^ permalink raw reply

* Re: [PATCH] tcp: Reject invalid ack_seq to Fast Open sockets
From: Yuchung Cheng @ 2012-10-22 21:46 UTC (permalink / raw)
  To: H.K. Jerry Chu; +Cc: davem, netdev, ncardwell
In-Reply-To: <1350941196-31224-1-git-send-email-hkchu@google.com>

On Mon, Oct 22, 2012 at 2:26 PM, H.K. Jerry Chu <hkchu@google.com> wrote:
> From: Jerry Chu <hkchu@google.com>
>
> A packet with an invalid ack_seq may cause a TCP Fast Open socket to switch
> to the unexpected TCP_CLOSING state, triggering a BUG_ON kernel panic.
>
> When a FIN packet with an invalid ack_seq# arrives at a socket in
> the TCP_FIN_WAIT1 state, rather than discarding the packet, the current
> code will accept the FIN, causing state transition to TCP_CLOSING.
>
> This may be a small deviation from RFC793, which seems to say that the
> packet should be dropped. Unfortunately I did not expect this case for
> Fast Open hence it will trigger a BUG_ON panic.
>
> It turns out there is really nothing bad about a TFO socket going into
> TCP_CLOSING state so I could just remove the BUG_ON statements. But after
> some thought I think it's better to treat this case like TCP_SYN_RECV
> and return a RST to the confused peer who caused the unacceptable ack_seq
> to be generated in the first place.
>
> Signed-off-by: H.K. Jerry Chu <hkchu@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
> ---
>  net/ipv4/tcp_input.c |   12 ++++++++++--
>  net/ipv4/tcp_timer.c |    2 +-
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 60cf836..ce3e3c7 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5970,7 +5970,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
>
>         req = tp->fastopen_rsk;
>         if (req != NULL) {
> -               BUG_ON(sk->sk_state != TCP_SYN_RECV &&
> +               WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
>                     sk->sk_state != TCP_FIN_WAIT1);
>
>                 if (tcp_check_req(sk, skb, req, NULL, true) == NULL)
> @@ -6059,7 +6059,15 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
>                          * ACK we have received, this would have acknowledged
>                          * our SYNACK so stop the SYNACK timer.
>                          */
> -                       if (acceptable && req != NULL) {
> +                       if (req != NULL) {
> +                               /* Return RST if ack_seq is invalid.
> +                                * Note that RFC793 only says to generate a
> +                                * DUPACK for it but for TCP Fast Open it seems
> +                                * better to treat this case like TCP_SYN_RECV
> +                                * above.
> +                                */
> +                               if (!acceptable)
> +                                       return 1;
>                                 /* We no longer need the request sock. */
>                                 reqsk_fastopen_remove(sk, req, false);
>                                 tcp_rearm_rto(sk);
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index fc04711..a24f4a4 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -347,7 +347,7 @@ void tcp_retransmit_timer(struct sock *sk)
>                 return;
>         }
>         if (tp->fastopen_rsk) {
> -               BUG_ON(sk->sk_state != TCP_SYN_RECV &&
> +               WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
>                     sk->sk_state != TCP_FIN_WAIT1);
>                 tcp_fastopen_synack_timer(sk);
>                 /* Before we receive ACK to our SYN-ACK don't retransmit
> --
> 1.7.7.3
>

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Eric Dumazet @ 2012-10-22 21:37 UTC (permalink / raw)
  To: Brian Haley; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <5085B8B0.6010500@hp.com>

On Mon, 2012-10-22 at 17:20 -0400, Brian Haley wrote:

> It's not much different from what's there:
> 
> 	setsockopt("foo");
> 
> 	rename foo -> bar
> 
> 	index = getsockopt();
> 	if_indextoname(index) -> "bar"
> 
> I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *'
> to getsockopt(), I don't think any other value is non-symmetrical like this.
> 
> -Brian

I meant another cpu can be changing dev->name[] content while the
strcpy() is done, and you get a mangled devname, like "for" or "bao"
instead of "foo" or "bar"

But yes, I obviously understood your point about "char *" and "int *"

^ permalink raw reply

* [PATCH] tcp: Reject invalid ack_seq to Fast Open sockets
From: H.K. Jerry Chu @ 2012-10-22 21:26 UTC (permalink / raw)
  To: davem; +Cc: netdev, ncardwell, ycheng, Jerry Chu

From: Jerry Chu <hkchu@google.com>

A packet with an invalid ack_seq may cause a TCP Fast Open socket to switch
to the unexpected TCP_CLOSING state, triggering a BUG_ON kernel panic.

When a FIN packet with an invalid ack_seq# arrives at a socket in
the TCP_FIN_WAIT1 state, rather than discarding the packet, the current
code will accept the FIN, causing state transition to TCP_CLOSING.

This may be a small deviation from RFC793, which seems to say that the
packet should be dropped. Unfortunately I did not expect this case for
Fast Open hence it will trigger a BUG_ON panic.

It turns out there is really nothing bad about a TFO socket going into
TCP_CLOSING state so I could just remove the BUG_ON statements. But after
some thought I think it's better to treat this case like TCP_SYN_RECV
and return a RST to the confused peer who caused the unacceptable ack_seq
to be generated in the first place.

Signed-off-by: H.K. Jerry Chu <hkchu@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_input.c |   12 ++++++++++--
 net/ipv4/tcp_timer.c |    2 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 60cf836..ce3e3c7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5970,7 +5970,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 
 	req = tp->fastopen_rsk;
 	if (req != NULL) {
-		BUG_ON(sk->sk_state != TCP_SYN_RECV &&
+		WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
 		    sk->sk_state != TCP_FIN_WAIT1);
 
 		if (tcp_check_req(sk, skb, req, NULL, true) == NULL)
@@ -6059,7 +6059,15 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
 			 * ACK we have received, this would have acknowledged
 			 * our SYNACK so stop the SYNACK timer.
 			 */
-			if (acceptable && req != NULL) {
+			if (req != NULL) {
+				/* Return RST if ack_seq is invalid.
+				 * Note that RFC793 only says to generate a
+				 * DUPACK for it but for TCP Fast Open it seems
+				 * better to treat this case like TCP_SYN_RECV
+				 * above.
+				 */
+				if (!acceptable)
+					return 1;
 				/* We no longer need the request sock. */
 				reqsk_fastopen_remove(sk, req, false);
 				tcp_rearm_rto(sk);
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index fc04711..a24f4a4 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -347,7 +347,7 @@ void tcp_retransmit_timer(struct sock *sk)
 		return;
 	}
 	if (tp->fastopen_rsk) {
-		BUG_ON(sk->sk_state != TCP_SYN_RECV &&
+		WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
 		    sk->sk_state != TCP_FIN_WAIT1);
 		tcp_fastopen_synack_timer(sk);
 		/* Before we receive ACK to our SYN-ACK don't retransmit
-- 
1.7.7.3

^ permalink raw reply related

* Re: [BUG] Kernel recieves DNS reply, but doesn't deliver it to a waiting application
From: Andrew Savchenko @ 2012-10-22 21:27 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1350888489.8609.322.camel@edumazet-glaptop>


[-- Attachment #1.1: Type: text/plain, Size: 2580 bytes --]

Hi,

On Mon, 22 Oct 2012 08:48:09 +0200 Eric Dumazet wrote:
> On Mon, 2012-10-22 at 07:36 +0400, Andrew Savchenko wrote:
[...]
> > This host has four NICs:
> > 2x Realtek Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (10ec:8139)
> > 2x D-Link System Inc DGE-528T Gigabit Ethernet Adapter (1462:223c)
> > 
> > One D-Link card is operating on 1Gbit/FD with mtu 7000, other cards
> > are used as 100Mbit/FD mtu 1500.
> > 
> > 2 D-Link and 1 Realtek card are forming a bridge and remaining Realtek
> > card is used for an uplink. This host serves as a NAT between
> > the bridge and the uplink (both MASQUERADE and DNAT are used). Also it
> > has several ipsec tunnels for multiple hosts (mostly AH), l2tp tunnel
> > (independent from ipsec) and serves as a multicast router using
> > mrouted. Rather sophisticated ebtables, iptables and ipset setup is
> > used.
> 
> Some driver or protocol stack is messing with skb->truesize, as
> your /proc/net/udp file contains anomalies :
> 
> $ cat /proc/net/udp
>   sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt   uid  timeout inode ref pointer drops
> ...
>   323: 074A070A:007B 00000000:0000 07 FFFDF700:00000000 00:00000000 00000000   123        0 254469 2 ffff88003d581880 0
> ...
>   323: 00FCA8C0:007B 00000000:0000 07 FFFFF900:00000000 00:00000000 00000000     0        0 5187 2 ffff880039993880 0
> 
> Its clearly not possible to get tx_queue = 0xFFFDF700 or 0xFFFFF900
> 
> So what drivers handle following IP addresses : 192.168.252.0 , 10.7.74.7  ?

192.168.252.0 is handled by eth0 interface running on Realtek
Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (10ec:8139) NIC.
Kernel driver 8139too. This interface handles multiple subnetworks:

# ip addr show eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 1000 
link/ether 00:80:48:30:ca:f3 brd ff:ff:ff:ff:ff:ff
inet 10.51.15.126/25 brd 10.51.15.127 scope global eth0
inet 192.168.252.0/31 scope global eth0

10.7.74.7 is an l2tp connection handled by ppp over l2tp:
CONFIG_PPPOL2TP=y
It is running on top of eth0 described above.

# ip addr show ppp0
65: ppp0: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1400 qdisc pfifo_fast state UNKNOWN qlen 3
link/ppp 
inet 10.7.74.7 peer 10.7.2.18/32 scope global ppp0

I don't know why both interfaces have unknown state: they are up and
running.

> lspci -v

Attached.

> lsmod

lsmod returns nothing: I compiled everything needed for daily use
into the kernel.

Best regards,
Andrew Savchenko

[-- Attachment #1.2: lspci-v --]
[-- Type: application/octet-stream, Size: 5774 bytes --]

00:00.0 Host bridge: Intel Corporation 82915G/P/GV/GL/PL/910GL Memory Controller Hub (rev 0e)
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, fast devsel, latency 0
	Capabilities: [e0] Vendor Specific Information: Len=09 <?>

00:02.0 VGA compatible controller: Intel Corporation 82915G/GV/910GL Integrated Graphics Controller (rev 0e) (prog-if 00 [VGA controller])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, fast devsel, latency 0, IRQ 10
	Memory at fbe80000 (32-bit, non-prefetchable) [size=512K]
	I/O ports at cc00 [size=8]
	Memory at d0000000 (32-bit, prefetchable) [size=256M]
	Memory at fbe40000 (32-bit, non-prefetchable) [size=256K]
	Expansion ROM at <unassigned> [disabled]
	Capabilities: [d0] Power Management version 2

00:1d.0 USB controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #1 (rev 04) (prog-if 00 [UHCI])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, medium devsel, latency 0, IRQ 23
	I/O ports at c880 [size=32]
	Kernel driver in use: uhci_hcd

00:1d.1 USB controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #2 (rev 04) (prog-if 00 [UHCI])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, medium devsel, latency 0, IRQ 19
	I/O ports at c800 [size=32]
	Kernel driver in use: uhci_hcd

00:1d.2 USB controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #3 (rev 04) (prog-if 00 [UHCI])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, medium devsel, latency 0, IRQ 18
	I/O ports at c480 [size=32]
	Kernel driver in use: uhci_hcd

00:1d.3 USB controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #4 (rev 04) (prog-if 00 [UHCI])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, medium devsel, latency 0, IRQ 16
	I/O ports at c400 [size=32]
	Kernel driver in use: uhci_hcd

00:1d.7 USB controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB2 EHCI Controller (rev 04) (prog-if 20 [EHCI])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, medium devsel, latency 0, IRQ 23
	Memory at fbe3bc00 (32-bit, non-prefetchable) [size=1K]
	Capabilities: [50] Power Management version 2
	Capabilities: [58] Debug port: BAR=1 offset=00a0
	Kernel driver in use: ehci_hcd

00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev d4) (prog-if 01 [Subtractive decode])
	Flags: bus master, fast devsel, latency 0
	Bus: primary=00, secondary=02, subordinate=02, sec-latency=64
	I/O behind bridge: 0000d000-0000efff
	Memory behind bridge: fbf00000-fbffffff
	Capabilities: [50] Subsystem: Micro-Star International Co., Ltd. Device 7223

00:1f.0 ISA bridge: Intel Corporation 82801FB/FR (ICH6/ICH6R) LPC Interface Bridge (rev 04)
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, medium devsel, latency 0

00:1f.1 IDE interface: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) IDE Controller (rev 04) (prog-if 8a [Master SecP PriP])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, medium devsel, latency 0, IRQ 18
	I/O ports at 01f0 [size=8]
	I/O ports at 03f4
	I/O ports at 0170 [size=8]
	I/O ports at 0374
	I/O ports at ffa0 [size=16]
	Kernel driver in use: ata_piix

00:1f.2 IDE interface: Intel Corporation 82801FB/FW (ICH6/ICH6W) SATA Controller (rev 04) (prog-if 8f [Master SecP SecO PriP PriO])
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: bus master, 66MHz, medium devsel, latency 0, IRQ 19
	I/O ports at c080 [size=8]
	I/O ports at c000 [size=4]
	I/O ports at bc00 [size=8]
	I/O ports at b880 [size=4]
	I/O ports at b800 [size=16]
	Capabilities: [70] Power Management version 2
	Kernel driver in use: ata_piix

00:1f.3 SMBus: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) SMBus Controller (rev 04)
	Subsystem: Micro-Star International Co., Ltd. Device 7223
	Flags: medium devsel, IRQ 19
	I/O ports at 0400 [size=32]
	Kernel driver in use: i801_smbus

02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (rev 10)
	Subsystem: Compex FN22-3(A) LinxPRO Ethernet Adapter
	Flags: bus master, medium devsel, latency 128, IRQ 16
	I/O ports at e800 [size=256]
	Memory at fbfffc00 (32-bit, non-prefetchable) [size=256]
	Expansion ROM at fbfe0000 [disabled] [size=64K]
	Capabilities: [50] Power Management version 2
	Kernel driver in use: 8139too

02:01.0 Ethernet controller: D-Link System Inc DGE-528T Gigabit Ethernet Adapter (rev 10)
	Subsystem: D-Link System Inc DGE-528T Gigabit Ethernet Adapter
	Flags: bus master, 66MHz, medium devsel, latency 64, IRQ 17
	I/O ports at e400 [size=256]
	Memory at fbfff800 (32-bit, non-prefetchable) [size=256]
	Expansion ROM at fbfc0000 [disabled] [size=128K]
	Capabilities: [dc] Power Management version 2
	Kernel driver in use: r8169

02:02.0 Ethernet controller: D-Link System Inc DGE-528T Gigabit Ethernet Adapter (rev 10)
	Subsystem: D-Link System Inc DGE-528T Gigabit Ethernet Adapter
	Flags: bus master, 66MHz, medium devsel, latency 64, IRQ 18
	I/O ports at e000 [size=256]
	Memory at fbfff400 (32-bit, non-prefetchable) [size=256]
	Expansion ROM at fbfa0000 [disabled] [size=128K]
	Capabilities: [dc] Power Management version 2
	Kernel driver in use: r8169

02:06.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (rev 10)
	Subsystem: Micro-Star International Co., Ltd. Device 223c
	Flags: bus master, medium devsel, latency 64, IRQ 20
	I/O ports at d800 [size=256]
	Memory at fbfff000 (32-bit, non-prefetchable) [size=256]
	Expansion ROM at fbf90000 [disabled] [size=64K]
	Capabilities: [50] Power Management version 2
	Kernel driver in use: 8139too


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Brian Haley @ 2012-10-22 21:23 UTC (permalink / raw)
  To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <5085AC71.7060904@parallels.com>

On 10/22/2012 04:28 PM, Pavel Emelyanov wrote:
> On 10/22/2012 10:04 PM, Brian Haley wrote:
>> On 10/19/2012 05:55 AM, Pavel Emelyanov wrote:
>>> The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but
>>> cannot be get via sockopt API. The only way we can find the device id a
>>> socket is bound to is via sock-diag interface. But the diag works only on
>>> hashed sockets, while the opt in question can be set for yet unhashed one.
>>>
>>> That said, in order to know what device a socket is bound to (we do want
>>> to know this in checkpoint-restore project) I propose to make this option
>>> getsockopt-able and report the respective device index.
>>>
>>> Another solution to the problem might be to teach the sock-diag reporting
>>> info on unhashed sockets. Should I go this way instead?
>>>
>>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>>>
>>> ---
>>>
>>> diff --git a/net/core/sock.c b/net/core/sock.c
>>> index 8a146cf..c49412c 100644
>>> --- a/net/core/sock.c
>>> +++ b/net/core/sock.c
>>> @@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
>>>  	case SO_NOFCS:
>>>  		v.val = sock_flag(sk, SOCK_NOFCS);
>>>  		break;
>>> +	case SO_BINDTODEVICE:
>>> +		v.val = sk->sk_bound_dev_if;
>>> +		break;
>>>  	default:
>>>  		return -ENOPROTOOPT;
>>>  	}
>>
>> Doesn't this make the set and get non-symmetrical?  For example, setsockopt()
>> would take "eth0", but getsockopt() would return 2.
> 
> It will, but since device name and index are two equal device "IDs" I assumed
> it would be OK.
> 
> However, some comments inline.
> 
>> The following patch would return a string, or -ENODEV if not set.

Sorry, my description is not quite right, this should return something like this
to be correct:

  0 on success
    optlen zero if interface not set
    optlen > zero if set and optval filled-in

  -errno on failure

>> +static int sock_getbindtodevice(struct sock *sk, char __user *optval,
>> +				int __user *optlen, int len)
>> +{
>> +	int ret = -ENOPROTOOPT;
>> +#ifdef CONFIG_NETDEVICES
>> +	struct net *net = sock_net(sk);
>> +	struct net_device *dev;
>> +	char devname[IFNAMSIZ];
>> +
>> +	ret = 0;
>> +	if (sk->sk_bound_dev_if == 0)
>> +		goto out;
> 
> It will return 0 if device is not set, thus making it impossible to detect
> this situation.

See below.

>> +	ret = -EINVAL;
>> +	if (len < IFNAMSIZ)
>> +		goto out;
>> +	if (len > IFNAMSIZ)
>> +		len = IFNAMSIZ;
>> +
>> +	rcu_read_lock();
>> +	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
>> +	if (dev)
>> +		strcpy(dev->name, devname);
>> +	rcu_read_unlock();
>> +	ret = -ENODEV;
>> +	if (!dev)
>> +		goto out;
>> +
>> +	ret = -EFAULT;
>> +	if (copy_to_user(optval, devname, len))
>> +		goto out;
>> +
>> +	if (put_user(len, optlen))
>> +		goto out;
> 
> What's the point in reporting IFNAMSIZ to the userspace always, taking
> into account that this constant is exported there anyway?

I should have put an RFC on the patch :)

In the case that there is no interface, the length returned would be zero,
indicating nothing was there.

I can post another version.

-Brian

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Brian Haley @ 2012-10-22 21:20 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <1350938702.8609.1299.camel@edumazet-glaptop>

On 10/22/2012 04:45 PM, Eric Dumazet wrote:
> On Mon, 2012-10-22 at 14:04 -0400, Brian Haley wrote:
> 
>> +	char devname[IFNAMSIZ];
>> +
>> +	ret = 0;
>> +	if (sk->sk_bound_dev_if == 0)
>> +		goto out;
>> +
>> +	ret = -EINVAL;
>> +	if (len < IFNAMSIZ)
>> +		goto out;
>> +	if (len > IFNAMSIZ)
>> +		len = IFNAMSIZ;
>> +
>> +	rcu_read_lock();
>> +	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
>> +	if (dev)
>> +		strcpy(dev->name, devname);
>> +	rcu_read_unlock();
>> +	ret = -ENODEV;
> 
> You probably meant
> 
> 	strcpy(devname, dev->name)

Yes, that was a stupid mistake, I'll fix it.

> By the way, this is not really safe in case device is renamed

It's not much different from what's there:

	setsockopt("foo");

	rename foo -> bar

	index = getsockopt();
	if_indextoname(index) -> "bar"

I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *'
to getsockopt(), I don't think any other value is non-symmetrical like this.

-Brian

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Pavel Emelyanov @ 2012-10-22 21:10 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Brian Haley, Linux Netdev List, David Miller
In-Reply-To: <1350938702.8609.1299.camel@edumazet-glaptop>

On 10/23/2012 12:45 AM, Eric Dumazet wrote:
> On Mon, 2012-10-22 at 14:04 -0400, Brian Haley wrote:
> 
>> +	char devname[IFNAMSIZ];
>> +
>> +	ret = 0;
>> +	if (sk->sk_bound_dev_if == 0)
>> +		goto out;
>> +
>> +	ret = -EINVAL;
>> +	if (len < IFNAMSIZ)
>> +		goto out;
>> +	if (len > IFNAMSIZ)
>> +		len = IFNAMSIZ;
>> +
>> +	rcu_read_lock();
>> +	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
>> +	if (dev)
>> +		strcpy(dev->name, devname);
>> +	rcu_read_unlock();
>> +	ret = -ENODEV;
> 
> You probably meant
> 
> 	strcpy(devname, dev->name)
> 
> By the way, this is not really safe in case device is renamed

Good point, actually. Getting a device name may be not very safe in terms of --
once we have the name there's no 100% guarantee, that this name corresponds to the
actual device the socket is bound to (it could be renamed after we strcpy-ed its
name). This problem doesn't exist when we get device index, as it cannot be changed.

Thanks,
Pavel

^ permalink raw reply

* [PATCH v2 net-next 11/13] net: cdc_ncm: do not bind to NCM compatible MBIM devices
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Bjørn Mork, Greg Suarez
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

The MBIM specification allows a MBIM device to disguise
itself as NCM for backwards compatibility, using additional
altsettings with different subclass (control) or protocol
(data):

C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=100mA
I:  If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0d Prot=00 Driver=cdc_mbim
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=7ms
I:* If#= 0 Alt= 1 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=7ms
I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
I:  If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
E:  Ad=81(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
I:* If#= 1 Alt= 2 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
E:  Ad=81(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
E:  Ad=02(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms

If the MBIM driver is enabled then that should have priority
for devices providing such a NCM 1.0 backward compatibility
mode.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
Signed-off-by: Greg Suarez <gsuarez@smithmicro.com>
---
 drivers/net/usb/cdc_ncm.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 17b4ba0..0ed03b1 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -563,6 +563,33 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int ret;
 
+	/* The MBIM spec defines a NCM compatible default altsetting,
+	 * which we may have matched:
+	 *
+	 *  "Functions that implement both NCM 1.0 and MBIM (an
+	 *   “NCM/MBIM function”) according to this recommendation
+	 *   shall provide two alternate settings for the
+	 *   Communication Interface.  Alternate setting 0, and the
+	 *   associated class and endpoint descriptors, shall be
+	 *   constructed according to the rules given for the
+	 *   Communication Interface in section 5 of [USBNCM10].
+	 *   Alternate setting 1, and the associated class and
+	 *   endpoint descriptors, shall be constructed according to
+	 *   the rules given in section 6 (USB Device Model) of this
+	 *   specification."
+	 *
+	 * Do not bind to such interfaces, allowing cdc_mbim to handle
+	 * them
+	 */
+#if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
+	if ((intf->num_altsetting == 2) &&
+	    !usb_set_interface(dev->udev,
+			       intf->cur_altsetting->desc.bInterfaceNumber,
+			       CDC_NCM_COMM_ALTSETTING_MBIM) &&
+	    cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
+		return -ENODEV;
+#endif
+
 	/* NCM data altsetting is always 1 */
 	ret = cdc_ncm_bind_common(dev, intf, 1);
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 03/13] net: cdc_ncm: adding MBIM support to ncm_setup
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Greg Suarez, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

From: Greg Suarez <gsuarez@smithmicro.com>

MBIM and NCM are very similar, so we can reuse most of the
setup and bind logic in cdc_ncm for CDC MBIM devices.  Handle
a few minor differences in ncm_setup.

Signed-off-by: Greg Suarez <gsuarez@smithmicro.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |   50 +++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 32bdf54..77c6728 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -64,6 +64,9 @@
 /* Minimum value for MaxDatagramSize, ch. 6.2.9 */
 #define	CDC_NCM_MIN_DATAGRAM_SIZE		1514	/* bytes */
 
+/* Minimum value for MaxDatagramSize, ch. 8.1.3 */
+#define CDC_MBIM_MIN_DATAGRAM_SIZE              2048    /* bytes */
+
 #define	CDC_NCM_MIN_TX_PKT			512	/* bytes */
 
 /* Default value for MaxDatagramSize */
@@ -98,6 +101,7 @@ struct cdc_ncm_ctx {
 	struct tasklet_struct bh;
 
 	const struct usb_cdc_ncm_desc *func_desc;
+	const struct usb_cdc_mbim_desc *mbim_desc;
 	const struct usb_cdc_header_desc *header_desc;
 	const struct usb_cdc_union_desc *union_desc;
 	const struct usb_cdc_ether_desc *ether_desc;
@@ -158,7 +162,10 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	u8 flags;
 	u8 iface_no;
 	int err;
+	int eth_hlen;
 	u16 ntb_fmt_supported;
+	u32 min_dgram_size;
+	u32 min_hdr_size;
 
 	iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
@@ -184,10 +191,19 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	ctx->tx_max_datagrams = le16_to_cpu(ctx->ncm_parm.wNtbOutMaxDatagrams);
 	ntb_fmt_supported = le16_to_cpu(ctx->ncm_parm.bmNtbFormatsSupported);
 
-	if (ctx->func_desc != NULL)
+	eth_hlen = ETH_HLEN;
+	min_dgram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
+	min_hdr_size = CDC_NCM_MIN_HDR_SIZE;
+	if (ctx->mbim_desc != NULL) {
+		flags = ctx->mbim_desc->bmNetworkCapabilities;
+		eth_hlen = 0;
+		min_dgram_size = CDC_MBIM_MIN_DATAGRAM_SIZE;
+		min_hdr_size = 0;
+	} else if (ctx->func_desc != NULL) {
 		flags = ctx->func_desc->bmNetworkCapabilities;
-	else
+	} else {
 		flags = 0;
+	}
 
 	pr_debug("dwNtbInMaxSize=%u dwNtbOutMaxSize=%u "
 		 "wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u "
@@ -237,7 +253,7 @@ size_err:
 
 	/* verify maximum size of transmitted NTB in bytes */
 	if ((ctx->tx_max <
-	    (CDC_NCM_MIN_HDR_SIZE + CDC_NCM_MIN_DATAGRAM_SIZE)) ||
+	    (min_hdr_size + min_dgram_size)) ||
 	    (ctx->tx_max > CDC_NCM_NTB_MAX_SIZE_TX)) {
 		pr_debug("Using default maximum transmit length=%d\n",
 						CDC_NCM_NTB_MAX_SIZE_TX);
@@ -279,8 +295,8 @@ size_err:
 	}
 
 	/* adjust TX-remainder according to NCM specification. */
-	ctx->tx_remainder = ((ctx->tx_remainder - ETH_HLEN) &
-						(ctx->tx_modulus - 1));
+	ctx->tx_remainder = ((ctx->tx_remainder - eth_hlen) &
+			     (ctx->tx_modulus - 1));
 
 	/* additional configuration */
 
@@ -307,12 +323,18 @@ size_err:
 			pr_debug("Setting NTB format to 16-bit failed\n");
 	}
 
-	ctx->max_datagram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
+	ctx->max_datagram_size = min_dgram_size;
 
 	/* set Max Datagram Size (MTU) */
 	if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
 		__le16 *max_datagram_size;
-		u16 eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
+		u16 eth_max_sz;
+		if (ctx->ether_desc != NULL)
+			eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
+		else if (ctx->mbim_desc != NULL)
+			eth_max_sz = le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
+		else
+			goto max_dgram_err;
 
 		max_datagram_size = kzalloc(sizeof(*max_datagram_size),
 				GFP_KERNEL);
@@ -329,7 +351,7 @@ size_err:
 				2, 1000);
 		if (err < 0) {
 			pr_debug("GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
-						CDC_NCM_MIN_DATAGRAM_SIZE);
+				 min_dgram_size);
 		} else {
 			ctx->max_datagram_size =
 				le16_to_cpu(*max_datagram_size);
@@ -338,12 +360,10 @@ size_err:
 					ctx->max_datagram_size = eth_max_sz;
 
 			if (ctx->max_datagram_size > CDC_NCM_MAX_DATAGRAM_SIZE)
-				ctx->max_datagram_size =
-						CDC_NCM_MAX_DATAGRAM_SIZE;
+				ctx->max_datagram_size = CDC_NCM_MAX_DATAGRAM_SIZE;
 
-			if (ctx->max_datagram_size < CDC_NCM_MIN_DATAGRAM_SIZE)
-				ctx->max_datagram_size =
-					CDC_NCM_MIN_DATAGRAM_SIZE;
+			if (ctx->max_datagram_size < min_dgram_size)
+				ctx->max_datagram_size = min_dgram_size;
 
 			/* if value changed, update device */
 			if (ctx->max_datagram_size !=
@@ -364,8 +384,8 @@ size_err:
 	}
 
 max_dgram_err:
-	if (ctx->netdev->mtu != (ctx->max_datagram_size - ETH_HLEN))
-		ctx->netdev->mtu = ctx->max_datagram_size - ETH_HLEN;
+	if (ctx->netdev->mtu != (ctx->max_datagram_size - eth_hlen))
+		ctx->netdev->mtu = ctx->max_datagram_size - eth_hlen;
 
 	return 0;
 }
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 07/13] net: cdc_ncm: refactoring for tx multiplexing
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

Adding multiplexed NDP support to cdc_ncm_fill_tx_frame, allowing
transmissions of multiple independent sessions within the same NTB.

Refactoring the code quite a bit to avoid having to store copies
of multiple NDPs being prepared for tx. The old code would still
reserve enough room for a maximum sized NDP in the skb so we might
as well keep them in the skb while they are being prepared.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |  247 +++++++++++++++++++--------------------------
 1 file changed, 102 insertions(+), 145 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 6688a15..2dd2734 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -88,14 +88,11 @@
 	(sizeof(struct usb_cdc_ncm_nth16) + sizeof(struct usb_cdc_ncm_ndp16) + \
 	(CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16))
 
-struct cdc_ncm_data {
-	struct usb_cdc_ncm_nth16 nth16;
-	struct usb_cdc_ncm_ndp16 ndp16;
-	struct usb_cdc_ncm_dpe16 dpe16[CDC_NCM_DPT_DATAGRAMS_MAX + 1];
-};
+#define CDC_NCM_NDP_SIZE \
+	(sizeof(struct usb_cdc_ncm_ndp16) +				\
+	      (CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16))
 
 struct cdc_ncm_ctx {
-	struct cdc_ncm_data tx_ncm;
 	struct usb_cdc_ncm_ntb_parameters ncm_parm;
 	struct hrtimer tx_timer;
 	struct tasklet_struct bh;
@@ -117,13 +114,12 @@ struct cdc_ncm_ctx {
 
 	struct sk_buff *tx_curr_skb;
 	struct sk_buff *tx_rem_skb;
+	__le32 tx_rem_sign;
 
 	spinlock_t mtx;
 	atomic_t stop;
 
 	u32 tx_timer_pending;
-	u32 tx_curr_offset;
-	u32 tx_curr_last_offset;
 	u32 tx_curr_frame_num;
 	u32 rx_speed;
 	u32 tx_speed;
@@ -658,51 +654,75 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 	return ret;
 }
 
-static void cdc_ncm_zero_fill(u8 *ptr, u32 first, u32 end, u32 max)
+static void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t remainder, size_t max)
 {
-	if (first >= max)
-		return;
-	if (first >= end)
-		return;
-	if (end > max)
-		end = max;
-	memset(ptr + first, 0, end - first);
+	size_t align = ALIGN(skb->len, modulus) - skb->len + remainder;
+
+	if (skb->len + align > max)
+		align = max - skb->len;
+	if (align && skb_tailroom(skb) >= align)
+		memset(skb_put(skb, align), 0, align);
+}
+
+/* return a pointer to a valid struct usb_cdc_ncm_ndp16 of type sign, possibly
+ * allocating a new one within skb
+ */
+static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign, size_t reserve)
+{
+	struct usb_cdc_ncm_ndp16 *ndp16 = NULL;
+	struct usb_cdc_ncm_nth16 *nth16 = (void *)skb->data;
+	size_t ndpoffset = le16_to_cpu(nth16->wNdpIndex);
+
+	/* follow the chain of NDPs, looking for a match */
+	while (ndpoffset) {
+		ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb->data + ndpoffset);
+		if  (ndp16->dwSignature == sign)
+			return ndp16;
+		ndpoffset = le16_to_cpu(ndp16->wNextNdpIndex);
+	}
+
+	/* align new NDP */
+	cdc_ncm_align_tail(skb, ctx->tx_ndp_modulus, 0, ctx->tx_max);
+
+	/* verify that there is room for the NDP and the datagram (reserve) */
+	if ((ctx->tx_max - skb->len - reserve) < CDC_NCM_NDP_SIZE)
+		return NULL;
+
+	/* link to it */
+	if (ndp16)
+		ndp16->wNextNdpIndex = cpu_to_le16(skb->len);
+	else
+		nth16->wNdpIndex = cpu_to_le16(skb->len);
+
+	/* push a new empty NDP */
+	ndp16 = (struct usb_cdc_ncm_ndp16 *)memset(skb_put(skb, CDC_NCM_NDP_SIZE), 0, CDC_NCM_NDP_SIZE);
+	ndp16->dwSignature = sign;
+	ndp16->wLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_ndp16) + sizeof(struct usb_cdc_ncm_dpe16));
+	return ndp16;
 }
 
 static struct sk_buff *
-cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
+cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 {
+	struct usb_cdc_ncm_nth16 *nth16;
+	struct usb_cdc_ncm_ndp16 *ndp16;
 	struct sk_buff *skb_out;
-	u32 rem;
-	u32 offset;
-	u32 last_offset;
-	u16 n = 0, index;
+	u16 n = 0, index, ndplen;
 	u8 ready2send = 0;
 
 	/* if there is a remaining skb, it gets priority */
-	if (skb != NULL)
+	if (skb != NULL) {
 		swap(skb, ctx->tx_rem_skb);
-	else
+		swap(sign, ctx->tx_rem_sign);
+	} else {
 		ready2send = 1;
-
-	/*
-	 * +----------------+
-	 * | skb_out        |
-	 * +----------------+
-	 *           ^ offset
-	 *        ^ last_offset
-	 */
+	}
 
 	/* check if we are resuming an OUT skb */
-	if (ctx->tx_curr_skb != NULL) {
-		/* pop variables */
-		skb_out = ctx->tx_curr_skb;
-		offset = ctx->tx_curr_offset;
-		last_offset = ctx->tx_curr_last_offset;
-		n = ctx->tx_curr_frame_num;
+	skb_out = ctx->tx_curr_skb;
 
-	} else {
-		/* reset variables */
+	/* allocate a new OUT skb */
+	if (!skb_out) {
 		skb_out = alloc_skb((ctx->tx_max + 1), GFP_ATOMIC);
 		if (skb_out == NULL) {
 			if (skb != NULL) {
@@ -711,35 +731,21 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 			}
 			goto exit_no_skb;
 		}
+		/* fill out the initial 16-bit NTB header */
+		nth16 = (struct usb_cdc_ncm_nth16 *)memset(skb_put(skb_out, sizeof(struct usb_cdc_ncm_nth16)), 0, sizeof(struct usb_cdc_ncm_nth16));
+		nth16->dwSignature = cpu_to_le32(USB_CDC_NCM_NTH16_SIGN);
+		nth16->wHeaderLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_nth16));
+		nth16->wSequence = cpu_to_le16(ctx->tx_seq++);
 
-		/* make room for NTH and NDP */
-		offset = ALIGN(sizeof(struct usb_cdc_ncm_nth16),
-					ctx->tx_ndp_modulus) +
-					sizeof(struct usb_cdc_ncm_ndp16) +
-					(ctx->tx_max_datagrams + 1) *
-					sizeof(struct usb_cdc_ncm_dpe16);
-
-		/* store last valid offset before alignment */
-		last_offset = offset;
-		/* align first Datagram offset correctly */
-		offset = ALIGN(offset, ctx->tx_modulus) + ctx->tx_remainder;
-		/* zero buffer till the first IP datagram */
-		cdc_ncm_zero_fill(skb_out->data, 0, offset, offset);
-		n = 0;
+		/* count total number of frames in this NTB */
 		ctx->tx_curr_frame_num = 0;
 	}
 
-	for (; n < ctx->tx_max_datagrams; n++) {
-		/* check if end of transmit buffer is reached */
-		if (offset >= ctx->tx_max) {
-			ready2send = 1;
-			break;
-		}
-		/* compute maximum buffer size */
-		rem = ctx->tx_max - offset;
-
+	for (n = ctx->tx_curr_frame_num; n < ctx->tx_max_datagrams; n++) {
+		/* send any remaining skb first */
 		if (skb == NULL) {
 			skb = ctx->tx_rem_skb;
+			sign = ctx->tx_rem_sign;
 			ctx->tx_rem_skb = NULL;
 
 			/* check for end of skb */
@@ -747,7 +753,14 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 				break;
 		}
 
-		if (skb->len > rem) {
+		/* get the appropriate NDP for this skb */
+		ndp16 = cdc_ncm_ndp(ctx, skb_out, sign, skb->len + ctx->tx_modulus + ctx->tx_remainder);
+
+		/* align beginning of next frame */
+		cdc_ncm_align_tail(skb_out,  ctx->tx_modulus, ctx->tx_remainder, ctx->tx_max);
+
+		/* check if we had enough room left for both NDP and frame */
+		if (!ndp16 || skb_out->len + skb->len > ctx->tx_max) {
 			if (n == 0) {
 				/* won't fit, MTU problem? */
 				dev_kfree_skb_any(skb);
@@ -760,31 +773,30 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 					ctx->netdev->stats.tx_dropped++;
 				}
 				ctx->tx_rem_skb = skb;
+				ctx->tx_rem_sign = sign;
 				skb = NULL;
 				ready2send = 1;
 			}
 			break;
 		}
 
-		memcpy(((u8 *)skb_out->data) + offset, skb->data, skb->len);
-
-		ctx->tx_ncm.dpe16[n].wDatagramLength = cpu_to_le16(skb->len);
-		ctx->tx_ncm.dpe16[n].wDatagramIndex = cpu_to_le16(offset);
-
-		/* update offset */
-		offset += skb->len;
+		/* calculate frame number withing this NDP */
+		ndplen = le16_to_cpu(ndp16->wLength);
+		index = (ndplen - sizeof(struct usb_cdc_ncm_ndp16)) / sizeof(struct usb_cdc_ncm_dpe16) - 1;
 
-		/* store last valid offset before alignment */
-		last_offset = offset;
-
-		/* align offset correctly */
-		offset = ALIGN(offset, ctx->tx_modulus) + ctx->tx_remainder;
-
-		/* zero padding */
-		cdc_ncm_zero_fill(skb_out->data, last_offset, offset,
-								ctx->tx_max);
+		/* OK, add this skb */
+		ndp16->dpe16[index].wDatagramLength = cpu_to_le16(skb->len);
+		ndp16->dpe16[index].wDatagramIndex = cpu_to_le16(skb_out->len);
+		ndp16->wLength = cpu_to_le16(ndplen + sizeof(struct usb_cdc_ncm_dpe16));
+		memcpy(skb_put(skb_out, skb->len), skb->data, skb->len);
 		dev_kfree_skb_any(skb);
 		skb = NULL;
+
+		/* send now if this NDP is full */
+		if (index >= CDC_NCM_DPT_DATAGRAMS_MAX) {
+			ready2send = 1;
+			break;
+		}
 	}
 
 	/* free up any dangling skb */
@@ -800,16 +812,12 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 		/* wait for more frames */
 		/* push variables */
 		ctx->tx_curr_skb = skb_out;
-		ctx->tx_curr_offset = offset;
-		ctx->tx_curr_last_offset = last_offset;
 		goto exit_no_skb;
 
 	} else if ((n < ctx->tx_max_datagrams) && (ready2send == 0)) {
 		/* wait for more frames */
 		/* push variables */
 		ctx->tx_curr_skb = skb_out;
-		ctx->tx_curr_offset = offset;
-		ctx->tx_curr_last_offset = last_offset;
 		/* set the pending count */
 		if (n < CDC_NCM_RESTART_TIMER_DATAGRAM_CNT)
 			ctx->tx_timer_pending = CDC_NCM_TIMER_PENDING_CNT;
@@ -820,75 +828,24 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 		/* variables will be reset at next call */
 	}
 
-	/* check for overflow */
-	if (last_offset > ctx->tx_max)
-		last_offset = ctx->tx_max;
-
-	/* revert offset */
-	offset = last_offset;
-
 	/*
 	 * If collected data size is less or equal CDC_NCM_MIN_TX_PKT bytes,
 	 * we send buffers as it is. If we get more data, it would be more
 	 * efficient for USB HS mobile device with DMA engine to receive a full
 	 * size NTB, than canceling DMA transfer and receiving a short packet.
 	 */
-	if (offset > CDC_NCM_MIN_TX_PKT)
-		offset = ctx->tx_max;
-
-	/* final zero padding */
-	cdc_ncm_zero_fill(skb_out->data, last_offset, offset, ctx->tx_max);
-
-	/* store last offset */
-	last_offset = offset;
-
-	if (((last_offset < ctx->tx_max) && ((last_offset %
-			le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0)) ||
-	    (((last_offset == ctx->tx_max) && ((ctx->tx_max %
-		le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0)) &&
-		(ctx->tx_max < le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)))) {
-		/* force short packet */
-		*(((u8 *)skb_out->data) + last_offset) = 0;
-		last_offset++;
-	}
+	if (skb_out->len > CDC_NCM_MIN_TX_PKT)
+		/* final zero padding */
+		memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0, ctx->tx_max - skb_out->len);
 
-	/* zero the rest of the DPEs plus the last NULL entry */
-	for (; n <= CDC_NCM_DPT_DATAGRAMS_MAX; n++) {
-		ctx->tx_ncm.dpe16[n].wDatagramLength = 0;
-		ctx->tx_ncm.dpe16[n].wDatagramIndex = 0;
-	}
-
-	/* fill out 16-bit NTB header */
-	ctx->tx_ncm.nth16.dwSignature = cpu_to_le32(USB_CDC_NCM_NTH16_SIGN);
-	ctx->tx_ncm.nth16.wHeaderLength =
-					cpu_to_le16(sizeof(ctx->tx_ncm.nth16));
-	ctx->tx_ncm.nth16.wSequence = cpu_to_le16(ctx->tx_seq);
-	ctx->tx_ncm.nth16.wBlockLength = cpu_to_le16(last_offset);
-	index = ALIGN(sizeof(struct usb_cdc_ncm_nth16), ctx->tx_ndp_modulus);
-	ctx->tx_ncm.nth16.wNdpIndex = cpu_to_le16(index);
-
-	memcpy(skb_out->data, &(ctx->tx_ncm.nth16), sizeof(ctx->tx_ncm.nth16));
-	ctx->tx_seq++;
-
-	/* fill out 16-bit NDP table */
-	ctx->tx_ncm.ndp16.dwSignature =
-				cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN);
-	rem = sizeof(ctx->tx_ncm.ndp16) + ((ctx->tx_curr_frame_num + 1) *
-					sizeof(struct usb_cdc_ncm_dpe16));
-	ctx->tx_ncm.ndp16.wLength = cpu_to_le16(rem);
-	ctx->tx_ncm.ndp16.wNextNdpIndex = 0; /* reserved */
-
-	memcpy(((u8 *)skb_out->data) + index,
-						&(ctx->tx_ncm.ndp16),
-						sizeof(ctx->tx_ncm.ndp16));
-
-	memcpy(((u8 *)skb_out->data) + index + sizeof(ctx->tx_ncm.ndp16),
-					&(ctx->tx_ncm.dpe16),
-					(ctx->tx_curr_frame_num + 1) *
-					sizeof(struct usb_cdc_ncm_dpe16));
+	/* do we need to prevent a ZLP? */
+	if (((skb_out->len % le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0) &&
+	    (skb_out->len < le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)) && skb_tailroom(skb_out))
+		*skb_put(skb_out, 1) = 0;	/* force short packet */
 
-	/* set frame length */
-	skb_put(skb_out, last_offset);
+	/* set final frame length */
+	nth16 = (struct usb_cdc_ncm_nth16 *)skb_out->data;
+	nth16->wBlockLength = cpu_to_le16(skb_out->len);
 
 	/* return skb */
 	ctx->tx_curr_skb = NULL;
@@ -955,7 +912,7 @@ cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 		goto error;
 
 	spin_lock_bh(&ctx->mtx);
-	skb_out = cdc_ncm_fill_tx_frame(ctx, skb);
+	skb_out = cdc_ncm_fill_tx_frame(ctx, skb, cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN));
 	spin_unlock_bh(&ctx->mtx);
 	return skb_out;
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 02/13] USB: cdc: add MBIM constants and structures
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Greg Suarez, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

From: Greg Suarez <gsuarez@smithmicro.com>

Based on revision 1.0 of "Universal Serial Bus Communications
Class Subclass Specification for Mobile Broadband Interface
Model" available from www.usb.org

Signed-off-by: Greg Suarez <gsuarez@smithmicro.com>
[bmork: added DSS defines]
Signed-off-by: Bjørn Mork <bjorn@mork.no>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/usb/cdc.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/include/linux/usb/cdc.h b/include/linux/usb/cdc.h
index 81a9279..f35aa0a 100644
--- a/include/linux/usb/cdc.h
+++ b/include/linux/usb/cdc.h
@@ -19,6 +19,7 @@
 #define USB_CDC_SUBCLASS_OBEX			0x0b
 #define USB_CDC_SUBCLASS_EEM			0x0c
 #define USB_CDC_SUBCLASS_NCM			0x0d
+#define USB_CDC_SUBCLASS_MBIM			0x0e
 
 #define USB_CDC_PROTO_NONE			0
 
@@ -33,6 +34,7 @@
 #define USB_CDC_PROTO_EEM			7
 
 #define USB_CDC_NCM_PROTO_NTB			1
+#define USB_CDC_MBIM_PROTO_NTB			2
 
 /*-------------------------------------------------------------------------*/
 
@@ -53,6 +55,7 @@
 #define USB_CDC_DMM_TYPE		0x14
 #define USB_CDC_OBEX_TYPE		0x15
 #define USB_CDC_NCM_TYPE		0x1a
+#define USB_CDC_MBIM_TYPE		0x1b
 
 /* "Header Functional Descriptor" from CDC spec  5.2.3.1 */
 struct usb_cdc_header_desc {
@@ -187,6 +190,21 @@ struct usb_cdc_ncm_desc {
 	__le16	bcdNcmVersion;
 	__u8	bmNetworkCapabilities;
 } __attribute__ ((packed));
+
+/* "MBIM Control Model Functional Descriptor" */
+struct usb_cdc_mbim_desc {
+	__u8	bLength;
+	__u8	bDescriptorType;
+	__u8	bDescriptorSubType;
+
+	__le16	bcdMBIMVersion;
+	__le16  wMaxControlMessage;
+	__u8    bNumberFilters;
+	__u8    bMaxFilterSize;
+	__le16  wMaxSegmentSize;
+	__u8    bmNetworkCapabilities;
+} __attribute__ ((packed));
+
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -332,6 +350,11 @@ struct usb_cdc_ncm_nth32 {
 #define USB_CDC_NCM_NDP32_CRC_SIGN	0x316D636E /* ncm1 */
 #define USB_CDC_NCM_NDP32_NOCRC_SIGN	0x306D636E /* ncm0 */
 
+#define USB_CDC_MBIM_NDP16_IPS_SIGN     0x00535049 /* IPS<sessionID> : IPS0 for now */
+#define USB_CDC_MBIM_NDP32_IPS_SIGN     0x00737069 /* ips<sessionID> : ips0 for now */
+#define USB_CDC_MBIM_NDP16_DSS_SIGN     0x00535344 /* DSS<sessionID> */
+#define USB_CDC_MBIM_NDP32_DSS_SIGN     0x00737364 /* dss<sessionID> */
+
 /* 16-bit NCM Datagram Pointer Entry */
 struct usb_cdc_ncm_dpe16 {
 	__le16	wDatagramIndex;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 10/13] net: cdc_mbim: build the MBIM driver
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/Kconfig  |   18 ++++++++++++++++++
 drivers/net/usb/Makefile |    1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index c1ae769..2ab8043 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -219,6 +219,24 @@ config USB_NET_CDC_NCM
 	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
 	    * Ericsson F5521gw Mobile Broadband Module
 
+config USB_NET_CDC_MBIM
+	tristate "CDC MBIM support"
+	depends on USB_USBNET
+	select USB_WDM
+	select USB_NET_CDC_NCM
+	help
+	  This driver provides support for CDC MBIM (Mobile Broadband
+	  Interface Model) devices. The CDC MBIM specification is
+	  available from <http://www.usb.org/>.
+
+	  MBIM devices require configuration using the management
+	  protocol defined by the MBIM specification.  This driver
+	  provides unfiltered access to the MBIM control channel
+	  through the associated /dev/cdc-wdmx character device.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cdc_mbim.
+
 config USB_NET_DM9601
 	tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices"
 	depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index bf06300..4786913 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,4 +31,5 @@ obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
 obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
 obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
 obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
+obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 08/13] net: cdc_ncm: export shared symbols and definitions
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

Move symbols and definitons which can be shared with a
MBIM driver in a new header.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c   |   97 ++++-----------------------------
 include/linux/usb/cdc_ncm.h |  124 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+), 86 deletions(-)
 create mode 100644 include/linux/usb/cdc_ncm.h

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 2dd2734..17b4ba0 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -51,90 +51,10 @@
 #include <linux/atomic.h>
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc.h>
+#include <linux/usb/cdc_ncm.h>
 
 #define	DRIVER_VERSION				"14-Mar-2012"
 
-/* CDC NCM subclass 3.2.1 */
-#define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10
-
-/* Maximum NTB length */
-#define	CDC_NCM_NTB_MAX_SIZE_TX			32768	/* bytes */
-#define	CDC_NCM_NTB_MAX_SIZE_RX			32768	/* bytes */
-
-/* Minimum value for MaxDatagramSize, ch. 6.2.9 */
-#define	CDC_NCM_MIN_DATAGRAM_SIZE		1514	/* bytes */
-
-/* Minimum value for MaxDatagramSize, ch. 8.1.3 */
-#define CDC_MBIM_MIN_DATAGRAM_SIZE              2048    /* bytes */
-
-#define	CDC_NCM_MIN_TX_PKT			512	/* bytes */
-
-/* Default value for MaxDatagramSize */
-#define	CDC_NCM_MAX_DATAGRAM_SIZE		8192	/* bytes */
-
-/*
- * Maximum amount of datagrams in NCM Datagram Pointer Table, not counting
- * the last NULL entry.
- */
-#define	CDC_NCM_DPT_DATAGRAMS_MAX		40
-
-/* Restart the timer, if amount of datagrams is less than given value */
-#define	CDC_NCM_RESTART_TIMER_DATAGRAM_CNT	3
-#define	CDC_NCM_TIMER_PENDING_CNT		2
-#define CDC_NCM_TIMER_INTERVAL			(400UL * NSEC_PER_USEC)
-
-/* The following macro defines the minimum header space */
-#define	CDC_NCM_MIN_HDR_SIZE \
-	(sizeof(struct usb_cdc_ncm_nth16) + sizeof(struct usb_cdc_ncm_ndp16) + \
-	(CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16))
-
-#define CDC_NCM_NDP_SIZE \
-	(sizeof(struct usb_cdc_ncm_ndp16) +				\
-	      (CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16))
-
-struct cdc_ncm_ctx {
-	struct usb_cdc_ncm_ntb_parameters ncm_parm;
-	struct hrtimer tx_timer;
-	struct tasklet_struct bh;
-
-	const struct usb_cdc_ncm_desc *func_desc;
-	const struct usb_cdc_mbim_desc *mbim_desc;
-	const struct usb_cdc_header_desc *header_desc;
-	const struct usb_cdc_union_desc *union_desc;
-	const struct usb_cdc_ether_desc *ether_desc;
-
-	struct net_device *netdev;
-	struct usb_device *udev;
-	struct usb_host_endpoint *in_ep;
-	struct usb_host_endpoint *out_ep;
-	struct usb_host_endpoint *status_ep;
-	struct usb_interface *intf;
-	struct usb_interface *control;
-	struct usb_interface *data;
-
-	struct sk_buff *tx_curr_skb;
-	struct sk_buff *tx_rem_skb;
-	__le32 tx_rem_sign;
-
-	spinlock_t mtx;
-	atomic_t stop;
-
-	u32 tx_timer_pending;
-	u32 tx_curr_frame_num;
-	u32 rx_speed;
-	u32 tx_speed;
-	u32 rx_max;
-	u32 tx_max;
-	u32 max_datagram_size;
-	u16 tx_max_datagrams;
-	u16 tx_remainder;
-	u16 tx_modulus;
-	u16 tx_ndp_modulus;
-	u16 tx_seq;
-	u16 rx_seq;
-	u16 connected;
-};
-
 static void cdc_ncm_txpath_bh(unsigned long param);
 static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
@@ -447,7 +367,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
 	.nway_reset = usbnet_nway_reset,
 };
 
-static int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
+int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
 {
 	struct cdc_ncm_ctx *ctx;
 	struct usb_driver *driver;
@@ -605,8 +525,9 @@ error:
 	dev_info(&dev->udev->dev, "bind() failure\n");
 	return -ENODEV;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_bind_common);
 
-static void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
+void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 	struct usb_driver *driver = driver_of(intf);
@@ -636,6 +557,7 @@ static void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 	usb_set_intfdata(ctx->intf, NULL);
 	cdc_ncm_free(ctx);
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_unbind);
 
 static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 {
@@ -701,7 +623,7 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_
 	return ndp16;
 }
 
-static struct sk_buff *
+struct sk_buff *
 cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 {
 	struct usb_cdc_ncm_nth16 *nth16;
@@ -858,6 +780,7 @@ exit_no_skb:
 		cdc_ncm_tx_timeout_start(ctx);
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_fill_tx_frame);
 
 static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx)
 {
@@ -924,7 +847,7 @@ error:
 }
 
 /* verify NTB header and return offset of first NDP, or negative error */
-static int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
+int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
 {
 	struct usb_cdc_ncm_nth16 *nth16;
 	int len;
@@ -966,9 +889,10 @@ static int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_
 error:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_nth16);
 
 /* verify NDP header and return number of datagrams, or negative error */
-static int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
+int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
 {
 	struct usb_cdc_ncm_ndp16 *ndp16;
 	int ret = -EINVAL;
@@ -999,6 +923,7 @@ static int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
 error:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
 
 static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
new file mode 100644
index 0000000..d1719a7
--- /dev/null
+++ b/include/linux/usb/cdc_ncm.h
@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) ST-Ericsson 2010-2012
+ * Contact: Alexey Orishko <alexey.orishko@stericsson.com>
+ * Original author: Hans Petter Selasky <hans.petter.selasky@stericsson.com>
+ *
+ * USB Host Driver for Network Control Model (NCM)
+ * http://www.usb.org/developers/devclass_docs/NCM10.zip
+ *
+ * The NCM encoding, decoding and initialization logic
+ * derives from FreeBSD 8.x. if_cdce.c and if_cdcereg.h
+ *
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose this file to be licensed under the terms
+ * of the GNU General Public License (GPL) Version 2 or the 2-clause
+ * BSD license listed below:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/* CDC NCM subclass 3.2.1 */
+#define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10
+
+/* Maximum NTB length */
+#define	CDC_NCM_NTB_MAX_SIZE_TX			32768	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_RX			32768	/* bytes */
+
+/* Minimum value for MaxDatagramSize, ch. 6.2.9 */
+#define	CDC_NCM_MIN_DATAGRAM_SIZE		1514	/* bytes */
+
+/* Minimum value for MaxDatagramSize, ch. 8.1.3 */
+#define CDC_MBIM_MIN_DATAGRAM_SIZE		2048	/* bytes */
+
+#define	CDC_NCM_MIN_TX_PKT			512	/* bytes */
+
+/* Default value for MaxDatagramSize */
+#define	CDC_NCM_MAX_DATAGRAM_SIZE		8192	/* bytes */
+
+/*
+ * Maximum amount of datagrams in NCM Datagram Pointer Table, not counting
+ * the last NULL entry.
+ */
+#define	CDC_NCM_DPT_DATAGRAMS_MAX		40
+
+/* Restart the timer, if amount of datagrams is less than given value */
+#define	CDC_NCM_RESTART_TIMER_DATAGRAM_CNT	3
+#define	CDC_NCM_TIMER_PENDING_CNT		2
+#define CDC_NCM_TIMER_INTERVAL			(400UL * NSEC_PER_USEC)
+
+/* The following macro defines the minimum header space */
+#define	CDC_NCM_MIN_HDR_SIZE \
+	(sizeof(struct usb_cdc_ncm_nth16) + sizeof(struct usb_cdc_ncm_ndp16) + \
+	(CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16))
+
+#define CDC_NCM_NDP_SIZE \
+	(sizeof(struct usb_cdc_ncm_ndp16) +				\
+	      (CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16))
+
+struct cdc_ncm_ctx {
+	struct usb_cdc_ncm_ntb_parameters ncm_parm;
+	struct hrtimer tx_timer;
+	struct tasklet_struct bh;
+
+	const struct usb_cdc_ncm_desc *func_desc;
+	const struct usb_cdc_mbim_desc   *mbim_desc;
+	const struct usb_cdc_header_desc *header_desc;
+	const struct usb_cdc_union_desc *union_desc;
+	const struct usb_cdc_ether_desc *ether_desc;
+
+	struct net_device *netdev;
+	struct usb_device *udev;
+	struct usb_host_endpoint *in_ep;
+	struct usb_host_endpoint *out_ep;
+	struct usb_host_endpoint *status_ep;
+	struct usb_interface *intf;
+	struct usb_interface *control;
+	struct usb_interface *data;
+
+	struct sk_buff *tx_curr_skb;
+	struct sk_buff *tx_rem_skb;
+	__le32 tx_rem_sign;
+
+	spinlock_t mtx;
+	atomic_t stop;
+
+	u32 tx_timer_pending;
+	u32 tx_curr_frame_num;
+	u32 rx_speed;
+	u32 tx_speed;
+	u32 rx_max;
+	u32 tx_max;
+	u32 max_datagram_size;
+	u16 tx_max_datagrams;
+	u16 tx_remainder;
+	u16 tx_modulus;
+	u16 tx_ndp_modulus;
+	u16 tx_seq;
+	u16 rx_seq;
+	u16 connected;
+};
+
+extern int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
+extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
+extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
+extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
+extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 00/13] Adding a USB CDC MBIM driver
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Bjørn Mork

[for those who reviewed v1 of this series: patch #1 is the only one
changed in v2]

The USB Communications Device Class "Mobile Broadband Interface Model"
(MBIM) is the USB-IFs alternative to the current chipset/vendor
specific solutions to "Mobile Broadband" device management. The
specification, including the management protocol description, can be
downloaded from http://www.usb.org/developers/devclass_docs#approved

This driver implementing most MBIM features with the exception of
32bit NTB and NDP headers.

An important design principle has been reusing as much as possible of
existing kernel code, in particular the cdc_ncm and cdc_wdm drivers.
The CDC MBIM protocol is based on CDC NCM, and much of the setup and
framing logic is very similar.

One important addition in MBIM compared to NCM is the new control
protocol embedded in CDC commands.  This protocol is comprehensive and
support a number of policy decisions necessary for modern mobile
broadband devices often having multiple radio interfaces.  Based on
early comments and the experiences with the qmi_wwan driver, knowledge
of the control protocol has been kept completely out of the driver.
This is userspace material.  Like with qmi_wwan, a control protocol
interface is exported to userspace using the cdc_wdm subdriver API,
associating a /dev/cdc-wdmX character device with the network device
for the management application.

Patch 1 works around a firmware bug in one of the MBIM test devices

Patch 2 adds new MBIM definitions to the cdc.h header file

Patches 3 to 8 refactor the cdc_ncm driver to enable reusing common
parts for MBIM.

Patches 9 and 10 add the new cdc_mbim driver

Patch 11 prevents cdc_ncm from binding to backward compatible MBIM
devices

Patches 12 and 13 extend the MBIM driver to support multiplexed
sessions

The changes to the cdc_ncm driver has been tested and verified to work
with an Ericsson F5521gw device.  The new cdc_mbim driver has been
tested with a Huawei E367u-2 device with MBIM firmware, and other
currently undisclosed devices.

Enjoy!

patch series changelog:
v2:
 - dropped v1 patch 1
 - reworked the firmware bug workaround in v1 patch 2 (which is now 1)
 - acks noted



Bjørn Mork (9):
  net: cdc_ncm: workaround NTB input size firmware bug
  net: cdc_ncm: process chained NDPs
  net: cdc_ncm: splitting rx_fixup for code reuse
  net: cdc_ncm: refactoring for tx multiplexing
  net: cdc_ncm: export shared symbols and definitions
  net: cdc_mbim: build the MBIM driver
  net: cdc_ncm: do not bind to NCM compatible MBIM devices
  net: cdc_ncm: map MBIM IPS SessionID to VLAN ID
  net: cdc_mbim: Device Service Stream support

Greg Suarez (4):
  USB: cdc: add MBIM constants and structures
  net: cdc_ncm: adding MBIM support to ncm_setup
  net: cdc_ncm: refactor bind preparing for MBIM support
  net: cdc_mbim: adding MBIM driver

 drivers/net/usb/Kconfig     |   18 ++
 drivers/net/usb/Makefile    |    1 +
 drivers/net/usb/cdc_mbim.c  |  412 ++++++++++++++++++++++++++++++
 drivers/net/usb/cdc_ncm.c   |  584 ++++++++++++++++++++-----------------------
 include/linux/usb/cdc.h     |   23 ++
 include/linux/usb/cdc_ncm.h |  134 ++++++++++
 6 files changed, 861 insertions(+), 311 deletions(-)
 create mode 100644 drivers/net/usb/cdc_mbim.c
 create mode 100644 include/linux/usb/cdc_ncm.h

-- 
1.7.10.4

^ permalink raw reply

* [PATCH v2 net-next 05/13] net: cdc_ncm: process chained NDPs
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

The NCM 1.0 spefication makes provisions for linking more than
one NDP into a single NTB. This is important for MBIM support,
where these NDPs might be of different types.

Following the chain of NDPs is also correct for NCM, and will
not change anything in the common case where there is only
one NDP

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |   35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 643e643..3406934 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -977,6 +977,8 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 	struct usb_cdc_ncm_nth16 *nth16;
 	struct usb_cdc_ncm_ndp16 *ndp16;
 	struct usb_cdc_ncm_dpe16 *dpe16;
+	int ndpoffset;
+	int loopcount = 50; /* arbitrary max preventing infinite loop */
 
 	if (ctx == NULL)
 		goto error;
@@ -1010,25 +1012,24 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 	}
 	ctx->rx_seq = le16_to_cpu(nth16->wSequence);
 
-	len = le16_to_cpu(nth16->wNdpIndex);
-	if ((len + sizeof(struct usb_cdc_ncm_ndp16)) > skb_in->len) {
-		pr_debug("invalid DPT16 index <%u>\n",
-					le16_to_cpu(nth16->wNdpIndex));
+	ndpoffset = le16_to_cpu(nth16->wNdpIndex);
+next_ndp:
+	if ((ndpoffset + sizeof(struct usb_cdc_ncm_ndp16)) > skb_in->len) {
+		pr_debug("invalid NDP offset  <%u>\n", ndpoffset);
 		goto error;
 	}
-
-	ndp16 = (struct usb_cdc_ncm_ndp16 *)(((u8 *)skb_in->data) + len);
+	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
 
 	if (le32_to_cpu(ndp16->dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
 		pr_debug("invalid DPT16 signature <%u>\n",
 					le32_to_cpu(ndp16->dwSignature));
-		goto error;
+		goto err_ndp;
 	}
 
 	if (le16_to_cpu(ndp16->wLength) < USB_CDC_NCM_NDP16_LENGTH_MIN) {
 		pr_debug("invalid DPT16 length <%u>\n",
 					le32_to_cpu(ndp16->dwSignature));
-		goto error;
+		goto err_ndp;
 	}
 
 	nframes = ((le16_to_cpu(ndp16->wLength) -
@@ -1036,15 +1037,15 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 					sizeof(struct usb_cdc_ncm_dpe16));
 	nframes--; /* we process NDP entries except for the last one */
 
-	len += sizeof(struct usb_cdc_ncm_ndp16);
+	ndpoffset += sizeof(struct usb_cdc_ncm_ndp16);
 
-	if ((len + nframes * (sizeof(struct usb_cdc_ncm_dpe16))) >
+	if ((ndpoffset + nframes * (sizeof(struct usb_cdc_ncm_dpe16))) >
 								skb_in->len) {
 		pr_debug("Invalid nframes = %d\n", nframes);
-		goto error;
+		goto err_ndp;
 	}
 
-	dpe16 = (struct usb_cdc_ncm_dpe16 *)(((u8 *)skb_in->data) + len);
+	dpe16 = (struct usb_cdc_ncm_dpe16 *)(skb_in->data + ndpoffset);
 
 	for (x = 0; x < nframes; x++, dpe16++) {
 		offset = le16_to_cpu(dpe16->wDatagramIndex);
@@ -1056,7 +1057,7 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 		 */
 		if ((offset == 0) || (len == 0)) {
 			if (!x)
-				goto error; /* empty NTB */
+				goto err_ndp; /* empty NTB */
 			break;
 		}
 
@@ -1067,7 +1068,7 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 					"offset[%u]=%u, length=%u, skb=%p\n",
 					x, offset, len, skb_in);
 			if (!x)
-				goto error;
+				goto err_ndp;
 			break;
 
 		} else {
@@ -1080,6 +1081,12 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 			usbnet_skb_return(dev, skb);
 		}
 	}
+err_ndp:
+	/* are there more NDPs to process? */
+	ndpoffset = le16_to_cpu(ndp16->wNextNdpIndex);
+	if (ndpoffset && loopcount--)
+		goto next_ndp;
+
 	return 1;
 error:
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 13/13] net: cdc_mbim: Device Service Stream support
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Greg Kroah-Hartman, Alexey Orishko,
	Greg Suarez, Fangxiaozhi (Franko), Dan Williams,
	Aleksander Morgado, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn@mork.no>

MBIM devices can support up to 256 generic streams called
Device Service Streams (DSS). The MBIM spec says

   The format of the Device Service Stream payload depends
   on the device service (as identified by the corresponding
   UUID) that is used when opening the data stream.

Example use cases are serial AT command interfaces and NMEA
data streams. We cannot make any assumptions about these
device services.

Adding support for Device Service Stream by extending
the MBIM session to VLAN mapping scheme, allocating
VLAN IDs 256 to 511 for DSS, using the DSS SessionID
as the lower 8bit of the VLAN ID.

Using a netdev for DSS keeps the device framing intact and
allows userspace to do whatever it want with the streams.
For example, exporting an AT command interface using DSS
session #0 to a PTY for use with a terminal application like
minicom:

  vconfig add wwan0 256
  ip link set dev wwan0 up
  ip link set dev wwan0.256 up
  socat INTERFACE:wwan0.256,type=2 PTY:,echo=0,link=/tmp/modem

Device configuration must be done using MBIM control commands
over the /dev/cdc-wdmx device. The userspace management
application should coordinate host VLAN configuration and the
device MBIM configuration using the device capabilities to
find out if it needs to set up PTY mappings etc.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_mbim.c |   58 ++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 45f5f50..42f51c7 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -149,12 +149,27 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 		/* mapping VLANs to MBIM sessions:
 		 *   no tag     => IPS session <0>
 		 *   1 - 255    => IPS session <vlanid>
-		 *   256 - 4095 => unsupported, drop
+		 *   256 - 511  => DSS session <vlanid - 256>
+		 *   512 - 4095 => unsupported, drop
 		 */
 		vlan_get_tag(skb, &tci);
 
 		switch (tci & 0x0f00) {
 		case 0x0000: /* VLAN ID 0 - 255 */
+			/* verify that datagram is IPv4 or IPv6 */
+			skb_reset_mac_header(skb);
+			switch (eth_hdr(skb)->h_proto) {
+			case htons(ETH_P_IP):
+			case htons(ETH_P_IPV6):
+				break;
+			default:
+				goto error;
+			}
+			c = (u8 *)&sign;
+			c[3] = tci;
+			break;
+		case 0x0100: /* VLAN ID 256 - 511 */
+			sign = cpu_to_le32(USB_CDC_MBIM_NDP16_DSS_SIGN);
 			c = (u8 *)&sign;
 			c[3] = tci;
 			break;
@@ -163,16 +178,7 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 				  "unsupported tci=0x%04x\n", tci);
 			goto error;
 		}
-
-		skb_reset_mac_header(skb);
-		switch (eth_hdr(skb)->h_proto) {
-		case htons(ETH_P_IP):
-		case htons(ETH_P_IPV6):
-			skb_pull(skb, ETH_HLEN);
-			break;
-		default:
-			goto error;
-		}
+		skb_pull(skb, ETH_HLEN);
 	}
 
 	spin_lock_bh(&ctx->mtx);
@@ -189,21 +195,23 @@ error:
 
 static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_t len, u16 tci)
 {
-	__be16 proto;
+	__be16 proto = htons(ETH_P_802_3);
 	struct sk_buff *skb = NULL;
 
-	if (len < sizeof(struct iphdr))
-		goto err;
+	if (tci < 256) { /* IPS session? */
+		if (len < sizeof(struct iphdr))
+			goto err;
 
-	switch (*buf & 0xf0) {
-	case 0x40:
-		proto = htons(ETH_P_IP);
-		break;
-	case 0x60:
-		proto = htons(ETH_P_IPV6);
-		break;
-	default:
-		goto err;
+		switch (*buf & 0xf0) {
+		case 0x40:
+			proto = htons(ETH_P_IP);
+			break;
+		case 0x60:
+			proto = htons(ETH_P_IPV6);
+			break;
+		default:
+			goto err;
+		}
 	}
 
 	skb = netdev_alloc_skb_ip_align(dev->net,  len + ETH_HLEN);
@@ -259,6 +267,10 @@ next_ndp:
 		c = (u8 *)&ndp16->dwSignature;
 		tci = c[3];
 		break;
+	case cpu_to_le32(USB_CDC_MBIM_NDP16_DSS_SIGN):
+		c = (u8 *)&ndp16->dwSignature;
+		tci = c[3] + 256;
+		break;
 	default:
 		netif_dbg(dev, rx_err, dev->net,
 			  "unsupported NDP signature <0x%08x>\n",
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 net-next 12/13] net: cdc_ncm: map MBIM IPS SessionID to VLAN ID
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum,
	Greg Kroah-Hartman, Alexey Orishko, Greg Suarez,
	Fangxiaozhi (Franko), Dan Williams, Aleksander Morgado,
	Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>

MBIM devices can support up to 256 independent IP Streams.
The main network device will only handle SessionID 0. Mapping
SessionIDs 1 to 255 to VLANs using the SessionID as VLAN ID
allow userspace to use these streams with traditional tools
like vconfig.

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_mbim.c |   51 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 2f21139..45f5f50 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 #include <linux/ip.h>
 #include <linux/mii.h>
 #include <linux/usb.h>
@@ -107,6 +108,9 @@ static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	/* MBIM cannot do ARP */
 	dev->net->flags |= IFF_NOARP;
+
+	/* no need to put the VLAN tci in the packet headers */
+	dev->net->features |= NETIF_F_HW_VLAN_TX;
 err:
 	return ret;
 }
@@ -131,6 +135,9 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 	struct sk_buff *skb_out;
 	struct cdc_mbim_state *info = (void *)&dev->data;
 	struct cdc_ncm_ctx *ctx = info->ctx;
+	__le32 sign = cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN);
+	u16 tci = 0;
+	u8 *c;
 
 	if (!ctx)
 		goto error;
@@ -139,6 +146,24 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 		if (skb->len <= sizeof(ETH_HLEN))
 			goto error;
 
+		/* mapping VLANs to MBIM sessions:
+		 *   no tag     => IPS session <0>
+		 *   1 - 255    => IPS session <vlanid>
+		 *   256 - 4095 => unsupported, drop
+		 */
+		vlan_get_tag(skb, &tci);
+
+		switch (tci & 0x0f00) {
+		case 0x0000: /* VLAN ID 0 - 255 */
+			c = (u8 *)&sign;
+			c[3] = tci;
+			break;
+		default:
+			netif_err(dev, tx_err, dev->net,
+				  "unsupported tci=0x%04x\n", tci);
+			goto error;
+		}
+
 		skb_reset_mac_header(skb);
 		switch (eth_hdr(skb)->h_proto) {
 		case htons(ETH_P_IP):
@@ -151,7 +176,7 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 	}
 
 	spin_lock_bh(&ctx->mtx);
-	skb_out = cdc_ncm_fill_tx_frame(ctx, skb, cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN));
+	skb_out = cdc_ncm_fill_tx_frame(ctx, skb, sign);
 	spin_unlock_bh(&ctx->mtx);
 	return skb_out;
 
@@ -162,11 +187,14 @@ error:
 	return NULL;
 }
 
-static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_t len)
+static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_t len, u16 tci)
 {
 	__be16 proto;
 	struct sk_buff *skb = NULL;
 
+	if (len < sizeof(struct iphdr))
+		goto err;
+
 	switch (*buf & 0xf0) {
 	case 0x40:
 		proto = htons(ETH_P_IP);
@@ -191,6 +219,10 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_
 
 	/* add datagram */
 	memcpy(skb_put(skb, len), buf, len);
+
+	/* map MBIM session to VLAN */
+	if (tci)
+		vlan_put_tag(skb, tci);
 err:
 	return skb;
 }
@@ -198,7 +230,8 @@ err:
 static int cdc_mbim_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_ncm_ctx *ctx = info->ctx;
 	int len;
 	int nframes;
 	int x;
@@ -207,6 +240,8 @@ static int cdc_mbim_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 	struct usb_cdc_ncm_dpe16 *dpe16;
 	int ndpoffset;
 	int loopcount = 50; /* arbitrary max preventing infinite loop */
+	u8 *c;
+	u16 tci;
 
 	ndpoffset = cdc_ncm_rx_verify_nth16(ctx, skb_in);
 	if (ndpoffset < 0)
@@ -219,9 +254,10 @@ next_ndp:
 
 	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
 
-	/* only supporting IPS Session #0 for now */
-	switch (ndp16->dwSignature) {
+	switch (ndp16->dwSignature & cpu_to_le32(0x00ffffff)) {
 	case cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN):
+		c = (u8 *)&ndp16->dwSignature;
+		tci = c[3];
 		break;
 	default:
 		netif_dbg(dev, rx_err, dev->net,
@@ -247,8 +283,7 @@ next_ndp:
 		}
 
 		/* sanity checking */
-		if (((offset + len) > skb_in->len) || (len > ctx->rx_max) ||
-		    (len < sizeof(struct iphdr))) {
+		if (((offset + len) > skb_in->len) || (len > ctx->rx_max)) {
 			netif_dbg(dev, rx_err, dev->net,
 				  "invalid frame detected (ignored) offset[%u]=%u, length=%u, skb=%p\n",
 				  x, offset, len, skb_in);
@@ -256,7 +291,7 @@ next_ndp:
 				goto err_ndp;
 			break;
 		} else {
-			skb = cdc_mbim_process_dgram(dev, skb_in->data + offset, len);
+			skb = cdc_mbim_process_dgram(dev, skb_in->data + offset, len, tci);
 			if (!skb)
 				goto error;
 			usbnet_skb_return(dev, skb);
-- 
1.7.10.4

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

^ permalink raw reply related

* [PATCH v2 net-next 09/13] net: cdc_mbim: adding MBIM driver
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum,
	Greg Kroah-Hartman, Alexey Orishko, Greg Suarez,
	Fangxiaozhi (Franko), Dan Williams, Aleksander Morgado,
	Greg Suarez, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>

From: Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>

The CDC Mobile Broadband Interface Model (MBIM) specification
extends CDC NCM by
 - removing the redundant ethernet header from the point-to-point
   USB channel
 - adding support for multiple IP (v4 and/or v6) sessions multiplexed
   on the same USB channel
 - adding a MBIM control channel encapsulated in CDC
 - adding Device Service Streams (DSS), which are non IP generic data
   streams multiplexed on the same USB channel as the IP sessions

MBIM devices are managed using the dedicated control channel, and no
data will flow on the data channel until a control session has been
established.  This driver has no knowledge of MBIM control messages.
It just exports the control channel to a /dev/cdc-wdmX character
device for userspace management applications. Such an application is
therefore required to use this driver.

This patch implements basic MBIM support, reusing the NCM and WDM driver
APIs, currently limited to IP sessions with SessionID 0. DSS and
multiplexed IP sessions are not yet supported.

Signed-off-by: Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_mbim.c  |  365 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/cdc_ncm.h |   10 ++
 2 files changed, 375 insertions(+)
 create mode 100644 drivers/net/usb/cdc_mbim.c

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
new file mode 100644
index 0000000..2f21139
--- /dev/null
+++ b/drivers/net/usb/cdc_mbim.c
@@ -0,0 +1,365 @@
+/*
+ * Copyright (c) 2012  Smith Micro Software, Inc.
+ * Copyright (c) 2012  Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
+ *
+ * This driver is based on and reuse most of cdc_ncm, which is
+ * Copyright (C) ST-Ericsson 2010-2012
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* driver specific data - must match cdc_ncm usage */
+struct cdc_mbim_state {
+	struct cdc_ncm_ctx *ctx;
+	atomic_t pmcount;
+	struct usb_driver *subdriver;
+	struct usb_interface *control;
+	struct usb_interface *data;
+};
+
+/* using a counter to merge subdriver requests with our own into a combined state */
+static int cdc_mbim_manage_power(struct usbnet *dev, int on)
+{
+	struct cdc_mbim_state *info = (void *)&dev->data;
+	int rv = 0;
+
+	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
+
+	if ((on && atomic_add_return(1, &info->pmcount) == 1) || (!on && atomic_dec_and_test(&info->pmcount))) {
+		/* need autopm_get/put here to ensure the usbcore sees the new value */
+		rv = usb_autopm_get_interface(dev->intf);
+		if (rv < 0)
+			goto err;
+		dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
+{
+	struct usbnet *dev = usb_get_intfdata(intf);
+
+	/* can be called while disconnecting */
+	if (!dev)
+		return 0;
+
+	return cdc_mbim_manage_power(dev, status);
+}
+
+
+static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+	struct cdc_ncm_ctx *ctx;
+	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+	int ret = -ENODEV;
+	u8 data_altsetting = CDC_NCM_DATA_ALTSETTING_NCM;
+	struct cdc_mbim_state *info = (void *)&dev->data;
+
+	/* see if interface supports MBIM alternate setting */
+	if (intf->num_altsetting == 2) {
+		if (!cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
+			usb_set_interface(dev->udev,
+					  intf->cur_altsetting->desc.bInterfaceNumber,
+					  CDC_NCM_COMM_ALTSETTING_MBIM);
+		data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
+	}
+
+	/* Probably NCM, defer for cdc_ncm_bind */
+	if (!cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
+		goto err;
+
+	ret = cdc_ncm_bind_common(dev, intf, data_altsetting);
+	if (ret)
+		goto err;
+
+	ctx = info->ctx;
+
+	/* The MBIM descriptor and the status endpoint are required */
+	if (ctx->mbim_desc && dev->status)
+		subdriver = usb_cdc_wdm_register(ctx->control,
+						 &dev->status->desc,
+						 le16_to_cpu(ctx->mbim_desc->wMaxControlMessage),
+						 cdc_mbim_wdm_manage_power);
+	if (IS_ERR(subdriver)) {
+		ret = PTR_ERR(subdriver);
+		cdc_ncm_unbind(dev, intf);
+		goto err;
+	}
+
+	/* can't let usbnet use the interrupt endpoint */
+	dev->status = NULL;
+	info->subdriver = subdriver;
+
+	/* MBIM cannot do ARP */
+	dev->net->flags |= IFF_NOARP;
+err:
+	return ret;
+}
+
+static void cdc_mbim_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_ncm_ctx *ctx = info->ctx;
+
+	/* disconnect subdriver from control interface */
+	if (info->subdriver && info->subdriver->disconnect)
+		info->subdriver->disconnect(ctx->control);
+	info->subdriver = NULL;
+
+	/* let NCM unbind clean up both control and data interface */
+	cdc_ncm_unbind(dev, intf);
+}
+
+
+static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
+{
+	struct sk_buff *skb_out;
+	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_ncm_ctx *ctx = info->ctx;
+
+	if (!ctx)
+		goto error;
+
+	if (skb) {
+		if (skb->len <= sizeof(ETH_HLEN))
+			goto error;
+
+		skb_reset_mac_header(skb);
+		switch (eth_hdr(skb)->h_proto) {
+		case htons(ETH_P_IP):
+		case htons(ETH_P_IPV6):
+			skb_pull(skb, ETH_HLEN);
+			break;
+		default:
+			goto error;
+		}
+	}
+
+	spin_lock_bh(&ctx->mtx);
+	skb_out = cdc_ncm_fill_tx_frame(ctx, skb, cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN));
+	spin_unlock_bh(&ctx->mtx);
+	return skb_out;
+
+error:
+	if (skb)
+		dev_kfree_skb_any(skb);
+
+	return NULL;
+}
+
+static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_t len)
+{
+	__be16 proto;
+	struct sk_buff *skb = NULL;
+
+	switch (*buf & 0xf0) {
+	case 0x40:
+		proto = htons(ETH_P_IP);
+		break;
+	case 0x60:
+		proto = htons(ETH_P_IPV6);
+		break;
+	default:
+		goto err;
+	}
+
+	skb = netdev_alloc_skb_ip_align(dev->net,  len + ETH_HLEN);
+	if (!skb)
+		goto err;
+
+	/* add an ethernet header */
+	skb_put(skb, ETH_HLEN);
+	skb_reset_mac_header(skb);
+	eth_hdr(skb)->h_proto = proto;
+	memset(eth_hdr(skb)->h_source, 0, ETH_ALEN);
+	memcpy(eth_hdr(skb)->h_dest, dev->net->dev_addr, ETH_ALEN);
+
+	/* add datagram */
+	memcpy(skb_put(skb, len), buf, len);
+err:
+	return skb;
+}
+
+static int cdc_mbim_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+{
+	struct sk_buff *skb;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	int len;
+	int nframes;
+	int x;
+	int offset;
+	struct usb_cdc_ncm_ndp16 *ndp16;
+	struct usb_cdc_ncm_dpe16 *dpe16;
+	int ndpoffset;
+	int loopcount = 50; /* arbitrary max preventing infinite loop */
+
+	ndpoffset = cdc_ncm_rx_verify_nth16(ctx, skb_in);
+	if (ndpoffset < 0)
+		goto error;
+
+next_ndp:
+	nframes = cdc_ncm_rx_verify_ndp16(skb_in, ndpoffset);
+	if (nframes < 0)
+		goto error;
+
+	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
+
+	/* only supporting IPS Session #0 for now */
+	switch (ndp16->dwSignature) {
+	case cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN):
+		break;
+	default:
+		netif_dbg(dev, rx_err, dev->net,
+			  "unsupported NDP signature <0x%08x>\n",
+			  le32_to_cpu(ndp16->dwSignature));
+		goto err_ndp;
+
+	}
+
+	dpe16 = ndp16->dpe16;
+	for (x = 0; x < nframes; x++, dpe16++) {
+		offset = le16_to_cpu(dpe16->wDatagramIndex);
+		len = le16_to_cpu(dpe16->wDatagramLength);
+
+		/*
+		 * CDC NCM ch. 3.7
+		 * All entries after first NULL entry are to be ignored
+		 */
+		if ((offset == 0) || (len == 0)) {
+			if (!x)
+				goto err_ndp; /* empty NTB */
+			break;
+		}
+
+		/* sanity checking */
+		if (((offset + len) > skb_in->len) || (len > ctx->rx_max) ||
+		    (len < sizeof(struct iphdr))) {
+			netif_dbg(dev, rx_err, dev->net,
+				  "invalid frame detected (ignored) offset[%u]=%u, length=%u, skb=%p\n",
+				  x, offset, len, skb_in);
+			if (!x)
+				goto err_ndp;
+			break;
+		} else {
+			skb = cdc_mbim_process_dgram(dev, skb_in->data + offset, len);
+			if (!skb)
+				goto error;
+			usbnet_skb_return(dev, skb);
+		}
+	}
+err_ndp:
+	/* are there more NDPs to process? */
+	ndpoffset = le16_to_cpu(ndp16->wNextNdpIndex);
+	if (ndpoffset && loopcount--)
+		goto next_ndp;
+
+	return 1;
+error:
+	return 0;
+}
+
+static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	int ret = 0;
+	struct usbnet *dev = usb_get_intfdata(intf);
+	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_ncm_ctx *ctx = info->ctx;
+
+	if (ctx == NULL) {
+		ret = -1;
+		goto error;
+	}
+
+	ret = usbnet_suspend(intf, message);
+	if (ret < 0)
+		goto error;
+
+	if (intf == ctx->control && info->subdriver && info->subdriver->suspend)
+		ret = info->subdriver->suspend(intf, message);
+	if (ret < 0)
+		usbnet_resume(intf);
+
+error:
+	return ret;
+}
+
+static int cdc_mbim_resume(struct usb_interface *intf)
+{
+	int  ret = 0;
+	struct usbnet *dev = usb_get_intfdata(intf);
+	struct cdc_mbim_state *info = (void *)&dev->data;
+	struct cdc_ncm_ctx *ctx = info->ctx;
+	bool callsub = (intf == ctx->control && info->subdriver && info->subdriver->resume);
+
+	if (callsub)
+		ret = info->subdriver->resume(intf);
+	if (ret < 0)
+		goto err;
+	ret = usbnet_resume(intf);
+	if (ret < 0 && callsub && info->subdriver->suspend)
+		info->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+	return ret;
+}
+
+static const struct driver_info cdc_mbim_info = {
+	.description = "CDC MBIM",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.bind = cdc_mbim_bind,
+	.unbind = cdc_mbim_unbind,
+	.manage_power = cdc_mbim_manage_power,
+	.rx_fixup = cdc_mbim_rx_fixup,
+	.tx_fixup = cdc_mbim_tx_fixup,
+};
+
+static const struct usb_device_id mbim_devs[] = {
+	/* This duplicate NCM entry is intentional. MBIM devices can
+	 * be disguised as NCM by default, and this is necessary to
+	 * allow us to bind the correct driver_info to such devices.
+	 *
+	 * bind() will sort out this for us, selecting the correct
+	 * entry and reject the other
+	 */
+	{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
+	  .driver_info = (unsigned long)&cdc_mbim_info,
+	},
+	{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
+	  .driver_info = (unsigned long)&cdc_mbim_info,
+	},
+	{
+	},
+};
+MODULE_DEVICE_TABLE(usb, mbim_devs);
+
+static struct usb_driver cdc_mbim_driver = {
+	.name = "cdc_mbim",
+	.id_table = mbim_devs,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = cdc_mbim_suspend,
+	.resume = cdc_mbim_resume,
+	.reset_resume =	cdc_mbim_resume,
+	.supports_autosuspend = 1,
+	.disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(cdc_mbim_driver);
+
+MODULE_AUTHOR("Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>");
+MODULE_AUTHOR("Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>");
+MODULE_DESCRIPTION("USB CDC MBIM host driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index d1719a7..3b8f9d4 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -36,6 +36,12 @@
  * SUCH DAMAGE.
  */
 
+#define CDC_NCM_COMM_ALTSETTING_NCM		0
+#define CDC_NCM_COMM_ALTSETTING_MBIM		1
+
+#define CDC_NCM_DATA_ALTSETTING_NCM		1
+#define CDC_NCM_DATA_ALTSETTING_MBIM		2
+
 /* CDC NCM subclass 3.2.1 */
 #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10
 
@@ -74,6 +80,10 @@
 	(sizeof(struct usb_cdc_ncm_ndp16) +				\
 	      (CDC_NCM_DPT_DATAGRAMS_MAX + 1) * sizeof(struct usb_cdc_ncm_dpe16))
 
+#define cdc_ncm_comm_intf_is_mbim(x)  ((x)->desc.bInterfaceSubClass == USB_CDC_SUBCLASS_MBIM && \
+				       (x)->desc.bInterfaceProtocol == USB_CDC_PROTO_NONE)
+#define cdc_ncm_data_intf_is_mbim(x)  ((x)->desc.bInterfaceProtocol == USB_CDC_MBIM_PROTO_NTB)
+
 struct cdc_ncm_ctx {
 	struct usb_cdc_ncm_ntb_parameters ncm_parm;
 	struct hrtimer tx_timer;
-- 
1.7.10.4

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

^ permalink raw reply related

* [PATCH v2 net-next 06/13] net: cdc_ncm: splitting rx_fixup for code reuse
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum,
	Greg Kroah-Hartman, Alexey Orishko, Greg Suarez,
	Fangxiaozhi (Franko), Dan Williams, Aleksander Morgado,
	Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>

Verifying and handling received MBIM and NCM frames will need
to be different in three areas:
 - verifying the NDP signature
 - checking valid datagram length
 - datagram header manipulation

This makes it inconvenient to share rx_fixup in whole.  But
some verification parts are common.  Split these out in separate
functions.

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |   83 ++++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 28 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 3406934..6688a15 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -966,19 +966,12 @@ error:
 	return NULL;
 }
 
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+/* verify NTB header and return offset of first NDP, or negative error */
+static int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
 {
-	struct sk_buff *skb;
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
-	int len;
-	int nframes;
-	int x;
-	int offset;
 	struct usb_cdc_ncm_nth16 *nth16;
-	struct usb_cdc_ncm_ndp16 *ndp16;
-	struct usb_cdc_ncm_dpe16 *dpe16;
-	int ndpoffset;
-	int loopcount = 50; /* arbitrary max preventing infinite loop */
+	int len;
+	int ret = -EINVAL;
 
 	if (ctx == NULL)
 		goto error;
@@ -1012,40 +1005,74 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 	}
 	ctx->rx_seq = le16_to_cpu(nth16->wSequence);
 
-	ndpoffset = le16_to_cpu(nth16->wNdpIndex);
-next_ndp:
+	ret = le16_to_cpu(nth16->wNdpIndex);
+error:
+	return ret;
+}
+
+/* verify NDP header and return number of datagrams, or negative error */
+static int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
+{
+	struct usb_cdc_ncm_ndp16 *ndp16;
+	int ret = -EINVAL;
+
 	if ((ndpoffset + sizeof(struct usb_cdc_ncm_ndp16)) > skb_in->len) {
 		pr_debug("invalid NDP offset  <%u>\n", ndpoffset);
 		goto error;
 	}
 	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
 
-	if (le32_to_cpu(ndp16->dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
-		pr_debug("invalid DPT16 signature <%u>\n",
-					le32_to_cpu(ndp16->dwSignature));
-		goto err_ndp;
-	}
-
 	if (le16_to_cpu(ndp16->wLength) < USB_CDC_NCM_NDP16_LENGTH_MIN) {
 		pr_debug("invalid DPT16 length <%u>\n",
 					le32_to_cpu(ndp16->dwSignature));
-		goto err_ndp;
+		goto error;
 	}
 
-	nframes = ((le16_to_cpu(ndp16->wLength) -
+	ret = ((le16_to_cpu(ndp16->wLength) -
 					sizeof(struct usb_cdc_ncm_ndp16)) /
 					sizeof(struct usb_cdc_ncm_dpe16));
-	nframes--; /* we process NDP entries except for the last one */
-
-	ndpoffset += sizeof(struct usb_cdc_ncm_ndp16);
+	ret--; /* we process NDP entries except for the last one */
 
-	if ((ndpoffset + nframes * (sizeof(struct usb_cdc_ncm_dpe16))) >
+	if ((sizeof(struct usb_cdc_ncm_ndp16) + ret * (sizeof(struct usb_cdc_ncm_dpe16))) >
 								skb_in->len) {
-		pr_debug("Invalid nframes = %d\n", nframes);
-		goto err_ndp;
+		pr_debug("Invalid nframes = %d\n", ret);
+		ret = -EINVAL;
 	}
 
-	dpe16 = (struct usb_cdc_ncm_dpe16 *)(skb_in->data + ndpoffset);
+error:
+	return ret;
+}
+
+static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+{
+	struct sk_buff *skb;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	int len;
+	int nframes;
+	int x;
+	int offset;
+	struct usb_cdc_ncm_ndp16 *ndp16;
+	struct usb_cdc_ncm_dpe16 *dpe16;
+	int ndpoffset;
+	int loopcount = 50; /* arbitrary max preventing infinite loop */
+
+	ndpoffset = cdc_ncm_rx_verify_nth16(ctx, skb_in);
+	if (ndpoffset < 0)
+		goto error;
+
+next_ndp:
+	nframes = cdc_ncm_rx_verify_ndp16(skb_in, ndpoffset);
+	if (nframes < 0)
+		goto error;
+
+	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
+
+	if (le32_to_cpu(ndp16->dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
+		pr_debug("invalid DPT16 signature <%u>\n",
+			 le32_to_cpu(ndp16->dwSignature));
+		goto err_ndp;
+	}
+	dpe16 = ndp16->dpe16;
 
 	for (x = 0; x < nframes; x++, dpe16++) {
 		offset = le16_to_cpu(dpe16->wDatagramIndex);
-- 
1.7.10.4

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

^ permalink raw reply related

* [PATCH v2 net-next 04/13] net: cdc_ncm: refactor bind preparing for MBIM support
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum,
	Greg Kroah-Hartman, Alexey Orishko, Greg Suarez,
	Fangxiaozhi (Franko), Dan Williams, Aleksander Morgado,
	Greg Suarez, Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>

From: Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>

NCM and MBIM can share most of the bind function.  Split
out the shareable part and add MBIM functional descriptor
parsing.

Signed-off-by: Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |   47 +++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 77c6728..643e643 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -451,7 +451,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
 	.nway_reset = usbnet_nway_reset,
 };
 
-static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
+static int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
 {
 	struct cdc_ncm_ctx *ctx;
 	struct usb_driver *driver;
@@ -525,6 +525,13 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 			ctx->func_desc = (const struct usb_cdc_ncm_desc *)buf;
 			break;
 
+		case USB_CDC_MBIM_TYPE:
+			if (buf[0] < sizeof(*(ctx->mbim_desc)))
+				break;
+
+			ctx->mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
+			break;
+
 		default:
 			break;
 		}
@@ -537,7 +544,7 @@ advance:
 
 	/* check if we got everything */
 	if ((ctx->control == NULL) || (ctx->data == NULL) ||
-	    (ctx->ether_desc == NULL) || (ctx->control != intf))
+	    ((!ctx->mbim_desc) && ((ctx->ether_desc == NULL) || (ctx->control != intf))))
 		goto error;
 
 	/* claim interfaces, if any */
@@ -557,7 +564,7 @@ advance:
 		goto error2;
 
 	/* configure data interface */
-	temp = usb_set_interface(dev->udev, iface_no, 1);
+	temp = usb_set_interface(dev->udev, iface_no, data_altsetting);
 	if (temp)
 		goto error2;
 
@@ -574,11 +581,13 @@ advance:
 	usb_set_intfdata(ctx->control, dev);
 	usb_set_intfdata(ctx->intf, dev);
 
-	temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
-	if (temp)
-		goto error2;
+	if (ctx->ether_desc) {
+		temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
+		if (temp)
+			goto error2;
+		dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
+	}
 
-	dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
 
 	dev->in = usb_rcvbulkpipe(dev->udev,
 		ctx->in_ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
@@ -587,13 +596,6 @@ advance:
 	dev->status = ctx->status_ep;
 	dev->rx_urb_size = ctx->rx_max;
 
-	/*
-	 * We should get an event when network connection is "connected" or
-	 * "disconnected". Set network connection in "disconnected" state
-	 * (carrier is OFF) during attach, so the IP network stack does not
-	 * start IPv6 negotiation and more.
-	 */
-	netif_carrier_off(dev->net);
 	ctx->tx_speed = ctx->rx_speed = 0;
 	return 0;
 
@@ -639,6 +641,23 @@ static void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 	cdc_ncm_free(ctx);
 }
 
+static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+	int ret;
+
+	/* NCM data altsetting is always 1 */
+	ret = cdc_ncm_bind_common(dev, intf, 1);
+
+	/*
+	 * We should get an event when network connection is "connected" or
+	 * "disconnected". Set network connection in "disconnected" state
+	 * (carrier is OFF) during attach, so the IP network stack does not
+	 * start IPv6 negotiation and more.
+	 */
+	netif_carrier_off(dev->net);
+	return ret;
+}
+
 static void cdc_ncm_zero_fill(u8 *ptr, u32 first, u32 end, u32 max)
 {
 	if (first >= max)
-- 
1.7.10.4

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

^ permalink raw reply related

* [PATCH v2 net-next 01/13] net: cdc_ncm: workaround NTB input size firmware bug
From: Bjørn Mork @ 2012-10-22 20:56 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum,
	Greg Kroah-Hartman, Alexey Orishko, Greg Suarez,
	Fangxiaozhi (Franko), Dan Williams, Aleksander Morgado,
	Bjørn Mork
In-Reply-To: <1350939400-27209-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>

Some devices do not support the 8 byte variants of the NTB input
size control messages despite announcing such support in their
NCM or MBIM functional descriptor.

According to the NCM specification, all devices must support the
4 byte variant regardless of whether or not the flag is set:

  If bit D5 is set in the bmNetworkCapabilities field of
  function’s NCM Functional Descriptor, the host may
  set wLength either to 4 or to 8. If wLength is 4, the
  function shall assume that wNtbInMaxDatagrams is to be
  set to zero. If wLength is 8, then the function shall
  use the provided value as the limit. The function shall
  return an error response (a STALL PID) if wLength is set
  to any other value.

We do not set wNtbInMaxDatagrams in any case, so we can just as
well unconditionally use the 4 byte variant without losing any
functionality.  This works around the known firmware bug, and
simplifies the code considerably.

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |   46 +++++++++++++--------------------------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..32bdf54 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -215,41 +215,21 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	/* inform device about NTB input size changes */
 	if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
+		__le32 *dwNtbInMaxSize;
 
-		if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
-			struct usb_cdc_ncm_ndp_input_size *ndp_in_sz;
-
-			ndp_in_sz = kzalloc(sizeof(*ndp_in_sz), GFP_KERNEL);
-			if (!ndp_in_sz) {
-				err = -ENOMEM;
-				goto size_err;
-			}
-
-			err = usb_control_msg(ctx->udev,
-					usb_sndctrlpipe(ctx->udev, 0),
-					USB_CDC_SET_NTB_INPUT_SIZE,
-					USB_TYPE_CLASS | USB_DIR_OUT
-					 | USB_RECIP_INTERFACE,
-					0, iface_no, ndp_in_sz, 8, 1000);
-			kfree(ndp_in_sz);
-		} else {
-			__le32 *dwNtbInMaxSize;
-			dwNtbInMaxSize = kzalloc(sizeof(*dwNtbInMaxSize),
-					GFP_KERNEL);
-			if (!dwNtbInMaxSize) {
-				err = -ENOMEM;
-				goto size_err;
-			}
-			*dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
-
-			err = usb_control_msg(ctx->udev,
-					usb_sndctrlpipe(ctx->udev, 0),
-					USB_CDC_SET_NTB_INPUT_SIZE,
-					USB_TYPE_CLASS | USB_DIR_OUT
-					 | USB_RECIP_INTERFACE,
-					0, iface_no, dwNtbInMaxSize, 4, 1000);
-			kfree(dwNtbInMaxSize);
+		dwNtbInMaxSize = kzalloc(sizeof(*dwNtbInMaxSize), GFP_KERNEL);
+		if (!dwNtbInMaxSize) {
+			err = -ENOMEM;
+			goto size_err;
 		}
+		*dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
+		err = usb_control_msg(ctx->udev,
+				      usb_sndctrlpipe(ctx->udev, 0),
+				      USB_CDC_SET_NTB_INPUT_SIZE,
+				      USB_TYPE_CLASS | USB_DIR_OUT
+				      | USB_RECIP_INTERFACE,
+				      0, iface_no, dwNtbInMaxSize, 4, 1000);
+		kfree(dwNtbInMaxSize);
 size_err:
 		if (err < 0)
 			pr_debug("Setting NTB Input Size failed\n");
-- 
1.7.10.4

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

^ permalink raw reply related

* RE: [net-next 12/13] ixgbe: Add support for pipeline reset
From: Skidmore, Donald C @ 2012-10-22 20:45 UTC (permalink / raw)
  To: Martin Josefsson, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <CAP1qwHgSra4i=+O5JczYxHJLn40sqC4DdYX+_jijXSwTqYeuzg@mail.gmail.com>

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Martin Josefsson
Sent: Sunday, October 21, 2012 5:06 AM
To: Kirsher, Jeffrey T
Cc: davem@davemloft.net; Skidmore, Donald C; netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
Subject: Re: [net-next 12/13] ixgbe: Add support for pipeline reset

On Sat, Oct 20, 2012 at 8:26 AM, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> From: Don Skidmore <donald.c.skidmore@intel.com>

Hi Jeff, Don,

> Calling the ixgbe_reset_pipeline_82599 function will ensure a full 
> pipeline reset on all 82599 devices.  This is necessary to avoid possible link issues.
> Since this patch accomplishes this by modifying AUTOC.LMS we need to 
> wrap all AUTOC writes when LESM is enabled.

The section below doesn't look right.
Old behaviour gave LMS == 011b (SFI)
New behaviour gives LMS == 000b (1 GbE link (no backplane auto-negotiation))

LMS is reset to 0 before ixgbe_reset_pipeline_82599() is called.
Looks like that part of the old code wasn't removed.

> @@ -137,28 +135,37 @@ static s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
>                 usleep_range(hw->eeprom.semaphore_delay * 1000,
>                              hw->eeprom.semaphore_delay * 2000);
>
> +               /* Need SW/FW semaphore around AUTOC writes if LESM on,
> +                * likewise reset_pipeline requires lock as it also writes
> +                * AUTOC.
> +                */
> +               if (ixgbe_verify_lesm_fw_enabled_82599(hw)) {
> +                       ret_val = hw->mac.ops.acquire_swfw_sync(hw,
> +                                                       IXGBE_GSSR_MAC_CSR_SM);
> +                       if (ret_val)
> +                               goto setup_sfp_out;
> +
> +                       got_lock = true;
> +               }
> +
>                 /* Now restart DSP by setting Restart_AN and clearing LMS */
>                 IXGBE_WRITE_REG(hw, IXGBE_AUTOC, ((IXGBE_READ_REG(hw,
>                                 IXGBE_AUTOC) & ~IXGBE_AUTOC_LMS_MASK) |
>                                 IXGBE_AUTOC_AN_RESTART));
>
> -               /* Wait for AN to leave state 0 */
> -               for (i = 0; i < 10; i++) {
> -                       usleep_range(4000, 8000);
> -                       reg_anlp1 = IXGBE_READ_REG(hw, IXGBE_ANLP1);
> -                       if (reg_anlp1 & IXGBE_ANLP1_AN_STATE_MASK)
> -                               break;
> +               ret_val = ixgbe_reset_pipeline_82599(hw);
> +
> +               if (got_lock) {
> +                       hw->mac.ops.release_swfw_sync(hw,
> +                                                     IXGBE_GSSR_MAC_CSR_SM);
> +                       got_lock = false;
>                 }
> -               if (!(reg_anlp1 & IXGBE_ANLP1_AN_STATE_MASK)) {
> -                       hw_dbg(hw, "sfp module setup not complete\n");
> +
> +               if (ret_val) {
> +                       hw_dbg(hw, " sfp module setup not 
> + complete\n");
>                         ret_val = IXGBE_ERR_SFP_SETUP_NOT_COMPLETE;
>                         goto setup_sfp_out;
>                 }
> -
> -               /* Restart DSP by setting Restart_AN and return to SFI mode */
> -               IXGBE_WRITE_REG(hw, IXGBE_AUTOC, (IXGBE_READ_REG(hw,
> -                               IXGBE_AUTOC) | IXGBE_AUTOC_LMS_10G_SERIAL |
> -                               IXGBE_AUTOC_AN_RESTART));
>         }
>
>  setup_sfp_out:

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

Hi Martin,

Thanks for pointing this problem out and I believe I see what is wrong.  I'll work with Jeff to get a new patch created and tested.

Thanks again,
-Don Skidmore <donald.c.skidmore@intel.com>

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Eric Dumazet @ 2012-10-22 20:45 UTC (permalink / raw)
  To: Brian Haley; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <50858ABD.2000206@hp.com>

On Mon, 2012-10-22 at 14:04 -0400, Brian Haley wrote:

> +	char devname[IFNAMSIZ];
> +
> +	ret = 0;
> +	if (sk->sk_bound_dev_if == 0)
> +		goto out;
> +
> +	ret = -EINVAL;
> +	if (len < IFNAMSIZ)
> +		goto out;
> +	if (len > IFNAMSIZ)
> +		len = IFNAMSIZ;
> +
> +	rcu_read_lock();
> +	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
> +	if (dev)
> +		strcpy(dev->name, devname);
> +	rcu_read_unlock();
> +	ret = -ENODEV;

You probably meant

	strcpy(devname, dev->name)

By the way, this is not really safe in case device is renamed

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox