Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] packet: packet fanout rollover during socket overload
From: Eric Dumazet @ 2013-03-13 14:25 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: netdev, davem, edumazet
In-Reply-To: <1363102664-5174-1-git-send-email-willemb@google.com>

On Tue, 2013-03-12 at 11:37 -0400, Willem de Bruijn wrote:
> Minimize packet drop in a fanout group. If one socket is full,
> roll over packets to another from the group. The intended use is
> to maintain flow affinity during normal load using an rxhash or
> cpu fanout policy, while dispersing unexpected traffic storms that
> hit a single cpu, such as spoofed-source DoS flows. This mechanism
> breaks affinity for flows arriving at saturated sockets during
> those conditions.
> 
> The patch adds a fanout policy ROLLOVER that rotates between sockets,
> filling each socket before moving to the next. It also adds a fanout
> flag ROLLOVER. If passed along with any other fanout policy, the
> primary policy is applied until the chosen socket is full. Then,
> rollover selects another socket, to delay packet drop until the
> entire system is saturated.
> 
> Probing sockets is not free. Selecting the last used socket, as
> rollover does, is a greedy approach that maximizes chance of
> success, at the cost of extreme load imbalance. In practice, with
> sufficiently long queues to handle rate, sockets are drained in
> parallel and load balance looks uniform in `top`.
> 
> To avoid contention, scales counters with number of sockets and
> accesses them lockfree. Values are bounds checked to ensure
> correctness. An alternative would be to use atomic rr_cur.
> 
> Tested using an application with 9 threads pinned to CPUs, one socket
> per thread and sufficient busywork per packet operation to limits each
> thread to handling 32 Kpps. When sent 500 Kpps single UDP stream
> packets, a FANOUT_CPU setup processes 32 Kpps in total without this
> patch, 270 Kpps with the patch. Tested with read() and with a packet
> ring (V1).
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>  include/linux/if_packet.h |   2 +
>  net/packet/af_packet.c    | 112 ++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 90 insertions(+), 24 deletions(-)


> -static struct sock *fanout_demux_cpu(struct packet_fanout *f, struct sk_buff *skb, unsigned int num)
> +static unsigned int fanout_demux_rollover(struct packet_fanout *f,
> +					  struct sk_buff *skb,
> +					  unsigned int idx, unsigned int skip,
> +					  unsigned int num)
>  {
> -	unsigned int cpu = smp_processor_id();
> +	unsigned int i, j;
>  
> -	return f->arr[cpu % num];
> +	i = j = min(f->next[idx], (int) f->num_members - 1);

min_t(int, f->next[idx], f->num_members - 1);

BTW, num_members can be 0

You really should do 

int members = ACCESS_ONCE(f->num_members) - 1;

if (members < 0)
	return idx;

and only use members in your loop.


> +	do {
> +		if (i != skip && packet_rcv_has_room(pkt_sk(f->arr[i]), skb)) {
> +			if (i != j)
> +				f->next[idx] = i;
> +			return i;
> +		}
> +		if (++i >= f->num_members)
> +			i = 0;
> +	} while (i != j && idx < f->num_members);
> +
> +	return idx;
> +}
> +

^ permalink raw reply

* Re: [RFC net-next 00/11] firewire net: resource management improvements
From: David Miller @ 2013-03-13 14:24 UTC (permalink / raw)
  To: stefanr; +Cc: yoshfuji, linux1394-devel, netdev
In-Reply-To: <20130313151919.68068bae@stein>

From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Wed, 13 Mar 2013 15:19:19 +0100

> ACK for routing this through net-next.  David, do you need me to respond to
> each individual patch with the reviewer tag?  Or since I already have the
> patches in a quilt series locally, I could commit them to a branch at
> linux1394.git and send you a pull request.

Anything works for me, either the series has to be reposted to netdev
or someone sends me a pull request.

^ permalink raw reply

* Re: [PATCH] sctp: optimize searching the active path for tsns
From: Neil Horman @ 2013-03-13 14:21 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: linux-sctp@vger.kernel.org, Xufeng Zhang, davem, netdev
In-Reply-To: <514087F3.5080601@gmail.com>

On Wed, Mar 13, 2013 at 10:06:43AM -0400, Vlad Yasevich wrote:
> On 03/13/2013 09:28 AM, Neil Horman wrote:
> >On Tue, Mar 12, 2013 at 09:43:20PM -0400, Vlad Yasevich wrote:
> >>On 03/12/2013 09:20 PM, Neil Horman wrote:
> >>>On Tue, Mar 12, 2013 at 05:01:50PM -0400, Vlad Yasevich wrote:
> >>>>Hi Neil
> >>>>
> >>>>On 03/12/2013 01:29 PM, Neil Horman wrote:
> >>>>>SCTP currently attempts to optimize the search for tsns on a transport by first
> >>>>>checking the active_path, then searching alternate transports.  This operation
> >>>>>however is a bit convoluted, as we explicitly search the active path, then serch
> >>>>>all other transports, skipping the active path, when its detected.  Lets
> >>>>>optimize this by preforming a move to front on the transport_addr_list every
> >>>>>time the active_path is changed.  The active_path changes occur in relatively
> >>>>>non-critical paths, and doing so allows us to just search the
> >>>>>transport_addr_list in order, avoiding an extra conditional check in the
> >>>>>relatively hot tsn lookup path.  This also happens to fix a bug where we break
> >>>>>out of the for loop early in the tsn lookup.
> >>>>>
> >>>>>CC: Xufeng Zhang <xufengzhang.main@gmail.com>
> >>>>>CC: vyasevich@gmail.com
> >>>>>CC: davem@davemloft.net
> >>>>>CC: netdev@vger.kernel.org
> >>>>>---
> >>>>>  net/sctp/associola.c | 31 ++++++++++++-------------------
> >>>>>  1 file changed, 12 insertions(+), 19 deletions(-)
> >>>>>
> >>>>>diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> >>>>>index 43cd0dd..7af96b3 100644
> >>>>>--- a/net/sctp/associola.c
> >>>>>+++ b/net/sctp/associola.c
> >>>>>@@ -513,8 +513,11 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
> >>>>>  	 * user wants to use this new path.
> >>>>>  	 */
> >>>>>  	if ((transport->state == SCTP_ACTIVE) ||
> >>>>>-	    (transport->state == SCTP_UNKNOWN))
> >>>>>+	    (transport->state == SCTP_UNKNOWN)) {
> >>>>>+		list_del_rcu(&transport->transports);
> >>>>>+		list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
> >>>>>  		asoc->peer.active_path = transport;
> >>>>>+	}
> >>>>
> >>>>What would happen if at the same time someone is walking the list
> >>>>through the proc interfaces?
> >>>>
> >>>>Since you are effectively changing the .next pointer, I think it is
> >>>>possible to get a duplicate transport output essentially corrupting
> >>>>the output.
> >>>>
> >>>It would be the case, but you're using the rcu variants of the list_add macros
> >>>at all the points where we modify the list (some of which we do at call sites
> >>>right before we call set_primary, see sctp_assoc_add_peer, where
> >>>list_add_tail_rcu also modifies a next pointer).  So if this is a problem, its a
> >>>problem without this patch.  In fact looking at it, our list access to
> >>>transport_addr_list is broken, as we use rcu apis to modify the list but non-rcu
> >>>apis to traverse the list.  I'll need to fix that first.
> >>
> >>As long as we are under lock, we don't need rcu variants for
> >>traversal.  The traversals done by the sctp_seq_ functions already
> >>use correct rcu variants.
> >>
> >>I don't see this as a problem right now since we either delete the
> >>transport, or add it.  We don't move it to a new location in the list.
> >>With the move, what could happen is that while sctp_seq_ is printing
> >>a transport, you might move it to another spot, and the when you grab
> >>the .next pointer on the next iteration, it points to a completely
> >>different spot.
> >>
> >Ok, I see what you're saying, and looking at the seq code, with your description
> >I see how we're using half the rcu code to allow the proc interface to avoid
> >grabbing the socket lock.  But this just strikes me a poor coding.  Its
> >confusing to say the least, and begging for mistakes like the one I just made to
> >be made again.  Regardless of necessisty, it seems to me the code would be both
> >more readable and understandible if we modified it so that we used the rcu api
> >consistently to access that list.
> >Neil
> >
> 
> Can you elaborate a bit on why you believe we are using half of the
> rcu code?
> 
We're using the rcu list add/del apis on the write side when
we modify the transport_addr_list, but we're using the non-rcu primitives on the
read side, save for the proc interface.  Granted that generally safe, exepct for
any case in which you do exactly what we were speaking about above.  I know were
not doing that now, but it seems to me it would be better to use the rcu
primitives consistently, so that it was clear how to access the list.  It would
prevent mistakes like the one I just made, as well as other possible mistakes,
in which future coding errors.

> I think to support the move operation you are proposing here, you
> need something like
> 	list_for_each_entry_safe_rcu()
> 
Not to the best of my knoweldge.  Consistent use of the rcu list access
primitives is safe against rcu list mutation primitives, as long as the read
side is protected by the rcu_read_lock.  As long as thats locked, any mutations
won't be seen by the read context until after we exit the read side critical
section.

> where the .next pointer is fetched through rcu_dereference() to
> guard against its possible.
> 
You won't see the updated next pointer until the read critical side ends.  Thats
why you need to do an rcu_read_lock in addition to grabbing the socket lock.

> But even that would only work if the move happens to the the earlier
> spot (like head) of the list.  If the move happens to the later part
> of the list (like tail), you may end up visiting the same list node
> twice.
> 
Again, not if you hold the rcu_read_lock.  Doing so creates a quiescent point in
which the status of the list is held until such time as read side critical
section exits.  The move (specifically the delete and the add) won't be seen by
the reading context until that quiescent point completes, which by definition is
when that reader is done reading.

Neil

> I think rcu simply can't guard against this.
> 
> -vlad
> 
> 

^ permalink raw reply

* Re: [RFC net-next 00/11] firewire net: resource management improvements
From: Stefan Richter @ 2013-03-13 14:19 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki, David Miller; +Cc: linux1394-devel, netdev
In-Reply-To: <5139419A.2040309@linux-ipv6.org>

On Mar 08 YOSHIFUJI Hideaki wrote:
> This series of patches tries to improve resource management of IP over Firewire
> device.
> 
> Because upcoming IPv6 over Firewire patch depends on this series, it would
> be better to let this series of patches go through Dave's net-next tree, if
> firewire people do not mind.
> 
> --yoshfuji
> 
> YOSHIFUJI Hideaki (11):
>   firewire net: No need to reset dev->local_fifo after failure of
>     fw_core_add_address_handler().
>   firewire net: Introduce fwnet_fifo_{start,stop}() helpers.
>   firewire net: Setup broadcast and local fifo independently.
>   firewire net: Check dev->broadcast_state inside
>     fwnet_broadcast_start().
>   firewire net: Fix memory leakage in fwnet_remove().
>   firewire net: Clear dev->broadcast_rcv_context and
>     dev->broadcast_state after destruction of context.
>   firewire net: Omit checking dev->broadcast_rcv_context in
>     fwnet_broadcast_start().
>   firewire net: Fix leakage of kmap for broadcast receive buffer.
>   firewire net: Allocate dev->broadcast_rcv_buffer_ptrs early.
>   firewire net: Introduce fwnet_broadcast_stop() to destroy broadcast
>     resources.
>   firewire net: Release broadcast/fifo resources on ifdown.
> 
>  drivers/firewire/net.c |  177 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 105 insertions(+), 72 deletions(-)
> 

All 11 patches:
Reviewed-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

ACK for routing this through net-next.  David, do you need me to respond to
each individual patch with the reviewer tag?  Or since I already have the
patches in a quilt series locally, I could commit them to a branch at
linux1394.git and send you a pull request.
-- 
Stefan Richter
-=====-===-= --== -==-=
http://arcgraph.de/sr/

^ permalink raw reply

* [PATCH] rtnetlink: Mask the rta_type when range checking
From: Vlad Yasevich @ 2013-03-13 14:18 UTC (permalink / raw)
  To: netdev; +Cc: davem, Vlad Yasevich

Range/validity checks on rta_type in rtnetlink_rcv_msg() do
not account for flags that may be set.  This causes the function
to return -EINVAL when flags are set on the type (for example
NLA_F_NESTED).

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 net/core/rtnetlink.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1868625..dc5edf1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2538,7 +2538,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		struct rtattr *attr = (void *)nlh + NLMSG_ALIGN(min_len);
 
 		while (RTA_OK(attr, attrlen)) {
-			unsigned int flavor = attr->rta_type;
+			unsigned int flavor = attr->rta_type & NLA_TYPE_MASK;
 			if (flavor) {
 				if (flavor > rta_max[sz_idx])
 					return -EINVAL;
-- 
1.7.7.6

^ permalink raw reply related

* Re: [PATCH] sctp: don't break the loop while meeting the active_path so as to find the matched transport
From: David Miller @ 2013-03-13 14:11 UTC (permalink / raw)
  To: vyasevich; +Cc: xufeng.zhang, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <514084B0.4050906@gmail.com>

From: Vlad Yasevich <vyasevich@gmail.com>
Date: Wed, 13 Mar 2013 09:52:48 -0400

> On 03/08/2013 02:39 AM, Xufeng Zhang wrote:
>> sctp_assoc_lookup_tsn() function searchs which transport a certain TSN
>> was sent on, if not found in the active_path transport, then go search
>> all the other transports in the peer's transport_addr_list, however,
>> we
>> should continue to the next entry rather than break the loop when meet
>> the active_path transport.
>>
>> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
 ...
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
...
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH] sctp: optimize searching the active path for tsns
From: Vlad Yasevich @ 2013-03-13 14:06 UTC (permalink / raw)
  To: Neil Horman; +Cc: linux-sctp@vger.kernel.org, Xufeng Zhang, davem, netdev
In-Reply-To: <20130313132809.GA17592@hmsreliant.think-freely.org>

On 03/13/2013 09:28 AM, Neil Horman wrote:
> On Tue, Mar 12, 2013 at 09:43:20PM -0400, Vlad Yasevich wrote:
>> On 03/12/2013 09:20 PM, Neil Horman wrote:
>>> On Tue, Mar 12, 2013 at 05:01:50PM -0400, Vlad Yasevich wrote:
>>>> Hi Neil
>>>>
>>>> On 03/12/2013 01:29 PM, Neil Horman wrote:
>>>>> SCTP currently attempts to optimize the search for tsns on a transport by first
>>>>> checking the active_path, then searching alternate transports.  This operation
>>>>> however is a bit convoluted, as we explicitly search the active path, then serch
>>>>> all other transports, skipping the active path, when its detected.  Lets
>>>>> optimize this by preforming a move to front on the transport_addr_list every
>>>>> time the active_path is changed.  The active_path changes occur in relatively
>>>>> non-critical paths, and doing so allows us to just search the
>>>>> transport_addr_list in order, avoiding an extra conditional check in the
>>>>> relatively hot tsn lookup path.  This also happens to fix a bug where we break
>>>>> out of the for loop early in the tsn lookup.
>>>>>
>>>>> CC: Xufeng Zhang <xufengzhang.main@gmail.com>
>>>>> CC: vyasevich@gmail.com
>>>>> CC: davem@davemloft.net
>>>>> CC: netdev@vger.kernel.org
>>>>> ---
>>>>>   net/sctp/associola.c | 31 ++++++++++++-------------------
>>>>>   1 file changed, 12 insertions(+), 19 deletions(-)
>>>>>
>>>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>>>> index 43cd0dd..7af96b3 100644
>>>>> --- a/net/sctp/associola.c
>>>>> +++ b/net/sctp/associola.c
>>>>> @@ -513,8 +513,11 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
>>>>>   	 * user wants to use this new path.
>>>>>   	 */
>>>>>   	if ((transport->state == SCTP_ACTIVE) ||
>>>>> -	    (transport->state == SCTP_UNKNOWN))
>>>>> +	    (transport->state == SCTP_UNKNOWN)) {
>>>>> +		list_del_rcu(&transport->transports);
>>>>> +		list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
>>>>>   		asoc->peer.active_path = transport;
>>>>> +	}
>>>>
>>>> What would happen if at the same time someone is walking the list
>>>> through the proc interfaces?
>>>>
>>>> Since you are effectively changing the .next pointer, I think it is
>>>> possible to get a duplicate transport output essentially corrupting
>>>> the output.
>>>>
>>> It would be the case, but you're using the rcu variants of the list_add macros
>>> at all the points where we modify the list (some of which we do at call sites
>>> right before we call set_primary, see sctp_assoc_add_peer, where
>>> list_add_tail_rcu also modifies a next pointer).  So if this is a problem, its a
>>> problem without this patch.  In fact looking at it, our list access to
>>> transport_addr_list is broken, as we use rcu apis to modify the list but non-rcu
>>> apis to traverse the list.  I'll need to fix that first.
>>
>> As long as we are under lock, we don't need rcu variants for
>> traversal.  The traversals done by the sctp_seq_ functions already
>> use correct rcu variants.
>>
>> I don't see this as a problem right now since we either delete the
>> transport, or add it.  We don't move it to a new location in the list.
>> With the move, what could happen is that while sctp_seq_ is printing
>> a transport, you might move it to another spot, and the when you grab
>> the .next pointer on the next iteration, it points to a completely
>> different spot.
>>
> Ok, I see what you're saying, and looking at the seq code, with your description
> I see how we're using half the rcu code to allow the proc interface to avoid
> grabbing the socket lock.  But this just strikes me a poor coding.  Its
> confusing to say the least, and begging for mistakes like the one I just made to
> be made again.  Regardless of necessisty, it seems to me the code would be both
> more readable and understandible if we modified it so that we used the rcu api
> consistently to access that list.
> Neil
>

Can you elaborate a bit on why you believe we are using half of the rcu 
code?

I think to support the move operation you are proposing here, you need 
something like
	list_for_each_entry_safe_rcu()

where the .next pointer is fetched through rcu_dereference() to guard 
against its possible.

But even that would only work if the move happens to the the earlier 
spot (like head) of the list.  If the move happens to the later part
of the list (like tail), you may end up visiting the same list node
twice.

I think rcu simply can't guard against this.

-vlad

^ permalink raw reply

* Re: [PATCH] sctp: Use correct sideffect command in duplicate cookie handling
From: David Miller @ 2013-03-13 14:04 UTC (permalink / raw)
  To: vyasevich; +Cc: netdev, linux-sctp, nhorman
In-Reply-To: <1363139603-14042-1-git-send-email-vyasevich@gmail.com>

From: Vlad Yasevich <vyasevich@gmail.com>
Date: Tue, 12 Mar 2013 21:53:23 -0400

> When SCTP is done processing a duplicate cookie chunk, it tries
> to delete a newly created association.  For that, it has to set
> the right association for the side-effect processing to work.
> However, when it uses the SCTP_CMD_NEW_ASOC command, that performs
> more work then really needed (like hashing the associationa and
> assigning it an id) and there is no point to do that only to
> delete the association as a next step.  In fact, it also creates
> an impossible condition where an association may be found by
> the getsockopt() call, and that association is empty.  This
> causes a crash in some sctp getsockopts.
> 
> The solution is rather simple.  We simply use SCTP_CMD_SET_ASOC
> command that doesn't have all the overhead and does exactly
> what we need.
> 
> Reported-by: Karl Heiss <kheiss@gmail.com>
> Tested-by: Karl Heiss <kheiss@gmail.com>
> CC: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] tg3: 5715 does not link up when autoneg off
From: David Miller @ 2013-03-13 14:00 UTC (permalink / raw)
  To: nsujir; +Cc: netdev, marcinmiotk81, mchan
In-Reply-To: <1363138368-10651-1-git-send-email-nsujir@broadcom.com>

From: "Nithin Nayak Sujir" <nsujir@broadcom.com>
Date: Tue, 12 Mar 2013 18:32:48 -0700

> From: Nithin Sujir <nsujir@broadcom.com>
> 
> Commit d13ba512cbba7de5d55d7a3b2aae7d83c8921457 cleaned up the autoneg
> advertisement by removing some dead code. One effect of this change was that
> the advertisement register would not be updated if autoneg is turned off.
> 
> This exposed a bug on the 5715 device w.r.t linking. The 5715 defaults
> to advertise only 10Mb Full duplex. But with autoneg disabled, it needs
> the configured speed enabled in the advertisement register to link up.
> 
> This patch adds the work around to advertise all speeds on the 5715 when
> autoneg is disabled.
> 
> Reported-by: Marcin Miotk <marcinmiotk81@gmail.com>
> Reviewed-by: Benjamin Li <benli@broadcom.com>
> Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

^ permalink raw reply

* Re: [PATCH] sctp: don't break the loop while meeting the active_path so as to find the matched transport
From: Vlad Yasevich @ 2013-03-13 13:52 UTC (permalink / raw)
  To: Xufeng Zhang; +Cc: nhorman, davem, linux-sctp, netdev, linux-kernel
In-Reply-To: <1362728377-11025-1-git-send-email-xufeng.zhang@windriver.com>

On 03/08/2013 02:39 AM, Xufeng Zhang wrote:
> sctp_assoc_lookup_tsn() function searchs which transport a certain TSN
> was sent on, if not found in the active_path transport, then go search
> all the other transports in the peer's transport_addr_list, however, we
> should continue to the next entry rather than break the loop when meet
> the active_path transport.
>
> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
> ---
>   net/sctp/associola.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 43cd0dd..d2709e2 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1079,7 +1079,7 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
>   			transports) {
>
>   		if (transport == active)
> -			break;
> +			continue;
>   		list_for_each_entry(chunk, &transport->transmitted,
>   				transmitted_list) {
>   			if (key == chunk->subh.data_hdr->tsn) {
>

Acked-by: Vlad Yasevich <vyasevich@gmail.com>

-vlad

^ permalink raw reply

* Re: [PATCH net,stable-3.8] net: qmi_wwan: set correct altsetting for Gobi 1K devices
From: Dan Williams @ 2013-03-13 13:43 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: linux-usb, netdev
In-Reply-To: <1363177517-23524-1-git-send-email-bjorn@mork.no>

On Wed, 2013-03-13 at 13:25 +0100, Bjørn Mork wrote:
> commit bd877e4 ("net: qmi_wwan: use a single bind function for
> all device types") made Gobi 1K devices fail probing.
> 
> Using the number of endpoints in the default altsetting to decide
> whether the function use one or two interfaces is wrong.  Other
> altsettings may provide more endpoints.
> 
> With Gobi 1K devices, USB interface #3's altsetting is 0 by default, but
> altsetting 0 only provides one interrupt endpoint and is not sufficent
> for QMI.  Altsetting 1 provides all 3 endpoints required for qmi_wwan
> and works with QMI. Gobi 1K layout for intf#3 is:
> 
>     Interface Descriptor:  255/255/255
>       bInterfaceNumber        3
>       bAlternateSetting       0
>       Endpoint Descriptor:  Interrupt IN
>     Interface Descriptor:  255/255/255
>       bInterfaceNumber        3
>       bAlternateSetting       1
>       Endpoint Descriptor:  Interrupt IN
>       Endpoint Descriptor:  Bulk IN
>       Endpoint Descriptor:  Bulk OUT
> 
> Prior to commit bd877e4, we would call usbnet_get_endpoints
> before giving up finding enough endpoints. Removing the early
> endpoint number test and the strict functional descriptor
> requirement allow qmi_wwan_bind to continue until
> usbnet_get_endpoints has made the final attempt to collect
> endpoints.  This restores the behaviour from before commit
> bd877e4 without losing the added benefit of using a single bind
> function.
> 
> The driver has always required a CDC Union functional descriptor
> for two-interface functions. Using the existence of this
> descriptor to detect two-interface functions is the logically
> correct method.
> 
> Reported-by: Dan Williams <dcbw@redhat.com>
> Signed-off-by: Bjørn Mork

Works on my UML290, Gobi3K, Gobi1K, Gobi2K, and E362.

Tested-by: Dan Williams <dcbw@redhat.com>

>  <bjorn@mork.no>
> ---
> Dan,
> 
> could you test this on a Gobi 1K device?  It should fix the problem,
> but I'd like to verify my assumptions for once :)
> 
> This needs to go to stable-3.8 as well.
> 
> 
> Bjørn
> 
> 
>  drivers/net/usb/qmi_wwan.c |   49 +++++++++++++++-----------------------------
>  1 file changed, 16 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index efb5c7c..968d5d5 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -139,16 +139,9 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>  
>  	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));
>  
> -	/* control and data is shared? */
> -	if (intf->cur_altsetting->desc.bNumEndpoints == 3) {
> -		info->control = intf;
> -		info->data = intf;
> -		goto shared;
> -	}
> -
> -	/* else require a single interrupt status endpoint on control intf */
> -	if (intf->cur_altsetting->desc.bNumEndpoints != 1)
> -		goto err;
> +	/* set up initial state */
> +	info->control = intf;
> +	info->data = intf;
>  
>  	/* and a number of CDC descriptors */
>  	while (len > 3) {
> @@ -207,25 +200,14 @@ next_desc:
>  		buf += h->bLength;
>  	}
>  
> -	/* did we find all the required ones? */
> -	if (!(found & (1 << USB_CDC_HEADER_TYPE)) ||
> -	    !(found & (1 << USB_CDC_UNION_TYPE))) {
> -		dev_err(&intf->dev, "CDC functional descriptors missing\n");
> -		goto err;
> -	}
> -
> -	/* verify CDC Union */
> -	if (desc->bInterfaceNumber != cdc_union->bMasterInterface0) {
> -		dev_err(&intf->dev, "bogus CDC Union: master=%u\n", cdc_union->bMasterInterface0);
> -		goto err;
> -	}
> -
> -	/* need to save these for unbind */
> -	info->control = intf;
> -	info->data = usb_ifnum_to_if(dev->udev,	cdc_union->bSlaveInterface0);
> -	if (!info->data) {
> -		dev_err(&intf->dev, "bogus CDC Union: slave=%u\n", cdc_union->bSlaveInterface0);
> -		goto err;
> +	/* Use separate control and data interfaces if we found a CDC Union */
> +	if (cdc_union) {
> +		info->data = usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0);
> +		if (desc->bInterfaceNumber != cdc_union->bMasterInterface0 || !info->data) {
> +			dev_err(&intf->dev, "bogus CDC Union: master=%u, slave=%u\n",
> +				cdc_union->bMasterInterface0, cdc_union->bSlaveInterface0);
> +			goto err;
> +		}
>  	}
>  
>  	/* errors aren't fatal - we can live with the dynamic address */
> @@ -235,11 +217,12 @@ next_desc:
>  	}
>  
>  	/* claim data interface and set it up */
> -	status = usb_driver_claim_interface(driver, info->data, dev);
> -	if (status < 0)
> -		goto err;
> +	if (info->control != info->data) {
> +		status = usb_driver_claim_interface(driver, info->data, dev);
> +		if (status < 0)
> +			goto err;
> +	}
>  
> -shared:
>  	status = qmi_wwan_register_subdriver(dev);
>  	if (status < 0 && info->control != info->data) {
>  		usb_set_intfdata(info->data, NULL);

^ permalink raw reply

* Re: [PATCH] sctp: don't break the loop while meeting the active_path so as to find the matched transport
From: Neil Horman @ 2013-03-13 13:33 UTC (permalink / raw)
  To: Xufeng Zhang; +Cc: vyasevich, davem, linux-sctp, netdev, linux-kernel
In-Reply-To: <1362728377-11025-1-git-send-email-xufeng.zhang@windriver.com>

On Fri, Mar 08, 2013 at 03:39:37PM +0800, Xufeng Zhang wrote:
> sctp_assoc_lookup_tsn() function searchs which transport a certain TSN
> was sent on, if not found in the active_path transport, then go search
> all the other transports in the peer's transport_addr_list, however, we
> should continue to the next entry rather than break the loop when meet
> the active_path transport.
> 
> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
> ---
>  net/sctp/associola.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 43cd0dd..d2709e2 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1079,7 +1079,7 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
>  			transports) {
>  
>  		if (transport == active)
> -			break;
> +			continue;
>  		list_for_each_entry(chunk, &transport->transmitted,
>  				transmitted_list) {
>  			if (key == chunk->subh.data_hdr->tsn) {
> -- 
> 1.7.0.2
> 
Based on discussion with Vlad, it seems theres arguably some work to do on
access to the transport_addr_list before my solution is viable, so until I get
to that I'm acking this patch.

Acked-by: Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: [PATCH] tuntap: remove unused variable in __tun_detach()
From: Neil Horman @ 2013-03-13 13:30 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: davem, jasowang, mst, edumazet, yongjun_wei, netdev
In-Reply-To: <CAPgLHd-v-daRHrDqoencJOTTvxLU_RD44BWzvdHd6j5Qrr8J5Q@mail.gmail.com>

On Wed, Mar 13, 2013 at 09:03:58PM +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> The variable dev is initialized but never used
> otherwise, so remove the unused variable.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  drivers/net/tun.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index b7c457a..95837c1 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -409,14 +409,12 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
>  {
>  	struct tun_file *ntfile;
>  	struct tun_struct *tun;
> -	struct net_device *dev;
>  
>  	tun = rtnl_dereference(tfile->tun);
>  
>  	if (tun && !tfile->detached) {
>  		u16 index = tfile->queue_index;
>  		BUG_ON(index >= tun->numqueues);
> -		dev = tun->dev;
>  
>  		rcu_assign_pointer(tun->tfiles[index],
>  				   tun->tfiles[tun->numqueues - 1]);
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: [PATCH] sctp: Use correct sideffect command in duplicate cookie handling
From: Neil Horman @ 2013-03-13 13:29 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, linux-sctp
In-Reply-To: <1363139603-14042-1-git-send-email-vyasevich@gmail.com>

On Tue, Mar 12, 2013 at 09:53:23PM -0400, Vlad Yasevich wrote:
> When SCTP is done processing a duplicate cookie chunk, it tries
> to delete a newly created association.  For that, it has to set
> the right association for the side-effect processing to work.
> However, when it uses the SCTP_CMD_NEW_ASOC command, that performs
> more work then really needed (like hashing the associationa and
> assigning it an id) and there is no point to do that only to
> delete the association as a next step.  In fact, it also creates
> an impossible condition where an association may be found by
> the getsockopt() call, and that association is empty.  This
> causes a crash in some sctp getsockopts.
> 
> The solution is rather simple.  We simply use SCTP_CMD_SET_ASOC
> command that doesn't have all the overhead and does exactly
> what we need.
> 
> Reported-by: Karl Heiss <kheiss@gmail.com>
> Tested-by: Karl Heiss <kheiss@gmail.com>
> CC: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
> ---
>  net/sctp/sm_statefuns.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 5131fcf..de1a013 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -2082,7 +2082,7 @@ sctp_disposition_t sctp_sf_do_5_2_4_dupcook(struct net *net,
>  	}
>  
>  	/* Delete the tempory new association. */
> -	sctp_add_cmd_sf(commands, SCTP_CMD_NEW_ASOC, SCTP_ASOC(new_asoc));
> +	sctp_add_cmd_sf(commands, SCTP_CMD_SET_ASOC, SCTP_ASOC(new_asoc));
>  	sctp_add_cmd_sf(commands, SCTP_CMD_DELETE_TCB, SCTP_NULL());
>  
>  	/* Restore association pointer to provide SCTP command interpeter
> -- 
> 1.7.7.6
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: [PATCH] sctp: optimize searching the active path for tsns
From: Neil Horman @ 2013-03-13 13:28 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: linux-sctp, Xufeng Zhang, davem, netdev
In-Reply-To: <513FD9B8.9010300@gmail.com>

On Tue, Mar 12, 2013 at 09:43:20PM -0400, Vlad Yasevich wrote:
> On 03/12/2013 09:20 PM, Neil Horman wrote:
> >On Tue, Mar 12, 2013 at 05:01:50PM -0400, Vlad Yasevich wrote:
> >>Hi Neil
> >>
> >>On 03/12/2013 01:29 PM, Neil Horman wrote:
> >>>SCTP currently attempts to optimize the search for tsns on a transport by first
> >>>checking the active_path, then searching alternate transports.  This operation
> >>>however is a bit convoluted, as we explicitly search the active path, then serch
> >>>all other transports, skipping the active path, when its detected.  Lets
> >>>optimize this by preforming a move to front on the transport_addr_list every
> >>>time the active_path is changed.  The active_path changes occur in relatively
> >>>non-critical paths, and doing so allows us to just search the
> >>>transport_addr_list in order, avoiding an extra conditional check in the
> >>>relatively hot tsn lookup path.  This also happens to fix a bug where we break
> >>>out of the for loop early in the tsn lookup.
> >>>
> >>>CC: Xufeng Zhang <xufengzhang.main@gmail.com>
> >>>CC: vyasevich@gmail.com
> >>>CC: davem@davemloft.net
> >>>CC: netdev@vger.kernel.org
> >>>---
> >>>  net/sctp/associola.c | 31 ++++++++++++-------------------
> >>>  1 file changed, 12 insertions(+), 19 deletions(-)
> >>>
> >>>diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> >>>index 43cd0dd..7af96b3 100644
> >>>--- a/net/sctp/associola.c
> >>>+++ b/net/sctp/associola.c
> >>>@@ -513,8 +513,11 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
> >>>  	 * user wants to use this new path.
> >>>  	 */
> >>>  	if ((transport->state == SCTP_ACTIVE) ||
> >>>-	    (transport->state == SCTP_UNKNOWN))
> >>>+	    (transport->state == SCTP_UNKNOWN)) {
> >>>+		list_del_rcu(&transport->transports);
> >>>+		list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
> >>>  		asoc->peer.active_path = transport;
> >>>+	}
> >>
> >>What would happen if at the same time someone is walking the list
> >>through the proc interfaces?
> >>
> >>Since you are effectively changing the .next pointer, I think it is
> >>possible to get a duplicate transport output essentially corrupting
> >>the output.
> >>
> >It would be the case, but you're using the rcu variants of the list_add macros
> >at all the points where we modify the list (some of which we do at call sites
> >right before we call set_primary, see sctp_assoc_add_peer, where
> >list_add_tail_rcu also modifies a next pointer).  So if this is a problem, its a
> >problem without this patch.  In fact looking at it, our list access to
> >transport_addr_list is broken, as we use rcu apis to modify the list but non-rcu
> >apis to traverse the list.  I'll need to fix that first.
> 
> As long as we are under lock, we don't need rcu variants for
> traversal.  The traversals done by the sctp_seq_ functions already
> use correct rcu variants.
> 
> I don't see this as a problem right now since we either delete the
> transport, or add it.  We don't move it to a new location in the list.
> With the move, what could happen is that while sctp_seq_ is printing
> a transport, you might move it to another spot, and the when you grab
> the .next pointer on the next iteration, it points to a completely
> different spot.
> 
Ok, I see what you're saying, and looking at the seq code, with your description
I see how we're using half the rcu code to allow the proc interface to avoid
grabbing the socket lock.  But this just strikes me a poor coding.  Its
confusing to say the least, and begging for mistakes like the one I just made to
be made again.  Regardless of necessisty, it seems to me the code would be both
more readable and understandible if we modified it so that we used the rcu api
consistently to access that list.
Neil

^ permalink raw reply

* [PATCH] tuntap: remove unused variable in __tun_detach()
From: Wei Yongjun @ 2013-03-13 13:03 UTC (permalink / raw)
  To: davem, jasowang, mst, edumazet, nhorman; +Cc: yongjun_wei, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

The variable dev is initialized but never used
otherwise, so remove the unused variable.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/tun.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b7c457a..95837c1 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -409,14 +409,12 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 {
 	struct tun_file *ntfile;
 	struct tun_struct *tun;
-	struct net_device *dev;
 
 	tun = rtnl_dereference(tfile->tun);
 
 	if (tun && !tfile->detached) {
 		u16 index = tfile->queue_index;
 		BUG_ON(index >= tun->numqueues);
-		dev = tun->dev;
 
 		rcu_assign_pointer(tun->tfiles[index],
 				   tun->tfiles[tun->numqueues - 1]);

^ permalink raw reply related

* [PATCH -next] sfc: remove duplicated include from efx.c
From: Wei Yongjun @ 2013-03-13 13:02 UTC (permalink / raw)
  To: linux-net-drivers, bhutchings; +Cc: yongjun_wei, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove duplicated include.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/ethernet/sfc/efx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index f050248..78c3324 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -21,7 +21,6 @@
 #include <linux/ethtool.h>
 #include <linux/topology.h>
 #include <linux/gfp.h>
-#include <linux/pci.h>
 #include <linux/cpu_rmap.h>
 #include <linux/aer.h>
 #include "net_driver.h"

^ permalink raw reply related

* [PATCH] Revert "ip_gre: make ipgre_tunnel_xmit() not parse network header as IP unconditionally"
From: Timo Teräs @ 2013-03-13 12:37 UTC (permalink / raw)
  To: netdev, Isaku Yamahata, Eric Dumazet, David S. Miller; +Cc: Timo Teräs

This reverts commit 412ed94744d16806fbec3bd250fd94e71cde5a1f.

The commit is wrong as tiph points to the outer IPv4 header which is
installed at ipgre_header() and not the inner one which is protocol dependant.

This commit broke succesfully opennhrp which use PF_PACKET socket with
ETH_P_NHRP protocol. Additionally ssl_addr is set to the link-layer
IPv4 address. This address is written by ipgre_header() to the skb
earlier, and this is the IPv4 header tiph should point to - regardless
of the inner protocol payload.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
 net/ipv4/ip_gre.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
 
This commit appeared in 3.8.x. So should go to 3.8.x-stable.

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index d0ef0e6..91d66db 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -798,10 +798,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 
 	if (dev->header_ops && dev->type == ARPHRD_IPGRE) {
 		gre_hlen = 0;
-		if (skb->protocol == htons(ETH_P_IP))
-			tiph = (const struct iphdr *)skb->data;
-		else
-			tiph = &tunnel->parms.iph;
+		tiph = (const struct iphdr *)skb->data;
 	} else {
 		gre_hlen = tunnel->hlen;
 		tiph = &tunnel->parms.iph;
-- 
1.8.1.5

^ permalink raw reply related

* [PATCH net,stable-3.8] net: qmi_wwan: set correct altsetting for Gobi 1K devices
From: Bjørn Mork @ 2013-03-13 12:25 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	Bjørn Mork

commit bd877e4 ("net: qmi_wwan: use a single bind function for
all device types") made Gobi 1K devices fail probing.

Using the number of endpoints in the default altsetting to decide
whether the function use one or two interfaces is wrong.  Other
altsettings may provide more endpoints.

With Gobi 1K devices, USB interface #3's altsetting is 0 by default, but
altsetting 0 only provides one interrupt endpoint and is not sufficent
for QMI.  Altsetting 1 provides all 3 endpoints required for qmi_wwan
and works with QMI. Gobi 1K layout for intf#3 is:

    Interface Descriptor:  255/255/255
      bInterfaceNumber        3
      bAlternateSetting       0
      Endpoint Descriptor:  Interrupt IN
    Interface Descriptor:  255/255/255
      bInterfaceNumber        3
      bAlternateSetting       1
      Endpoint Descriptor:  Interrupt IN
      Endpoint Descriptor:  Bulk IN
      Endpoint Descriptor:  Bulk OUT

Prior to commit bd877e4, we would call usbnet_get_endpoints
before giving up finding enough endpoints. Removing the early
endpoint number test and the strict functional descriptor
requirement allow qmi_wwan_bind to continue until
usbnet_get_endpoints has made the final attempt to collect
endpoints.  This restores the behaviour from before commit
bd877e4 without losing the added benefit of using a single bind
function.

The driver has always required a CDC Union functional descriptor
for two-interface functions. Using the existence of this
descriptor to detect two-interface functions is the logically
correct method.

Reported-by: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
Dan,

could you test this on a Gobi 1K device?  It should fix the problem,
but I'd like to verify my assumptions for once :)

This needs to go to stable-3.8 as well.


Bjørn


 drivers/net/usb/qmi_wwan.c |   49 +++++++++++++++-----------------------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index efb5c7c..968d5d5 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -139,16 +139,9 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));
 
-	/* control and data is shared? */
-	if (intf->cur_altsetting->desc.bNumEndpoints == 3) {
-		info->control = intf;
-		info->data = intf;
-		goto shared;
-	}
-
-	/* else require a single interrupt status endpoint on control intf */
-	if (intf->cur_altsetting->desc.bNumEndpoints != 1)
-		goto err;
+	/* set up initial state */
+	info->control = intf;
+	info->data = intf;
 
 	/* and a number of CDC descriptors */
 	while (len > 3) {
@@ -207,25 +200,14 @@ next_desc:
 		buf += h->bLength;
 	}
 
-	/* did we find all the required ones? */
-	if (!(found & (1 << USB_CDC_HEADER_TYPE)) ||
-	    !(found & (1 << USB_CDC_UNION_TYPE))) {
-		dev_err(&intf->dev, "CDC functional descriptors missing\n");
-		goto err;
-	}
-
-	/* verify CDC Union */
-	if (desc->bInterfaceNumber != cdc_union->bMasterInterface0) {
-		dev_err(&intf->dev, "bogus CDC Union: master=%u\n", cdc_union->bMasterInterface0);
-		goto err;
-	}
-
-	/* need to save these for unbind */
-	info->control = intf;
-	info->data = usb_ifnum_to_if(dev->udev,	cdc_union->bSlaveInterface0);
-	if (!info->data) {
-		dev_err(&intf->dev, "bogus CDC Union: slave=%u\n", cdc_union->bSlaveInterface0);
-		goto err;
+	/* Use separate control and data interfaces if we found a CDC Union */
+	if (cdc_union) {
+		info->data = usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0);
+		if (desc->bInterfaceNumber != cdc_union->bMasterInterface0 || !info->data) {
+			dev_err(&intf->dev, "bogus CDC Union: master=%u, slave=%u\n",
+				cdc_union->bMasterInterface0, cdc_union->bSlaveInterface0);
+			goto err;
+		}
 	}
 
 	/* errors aren't fatal - we can live with the dynamic address */
@@ -235,11 +217,12 @@ next_desc:
 	}
 
 	/* claim data interface and set it up */
-	status = usb_driver_claim_interface(driver, info->data, dev);
-	if (status < 0)
-		goto err;
+	if (info->control != info->data) {
+		status = usb_driver_claim_interface(driver, info->data, dev);
+		if (status < 0)
+			goto err;
+	}
 
-shared:
 	status = qmi_wwan_register_subdriver(dev);
 	if (status < 0 && info->control != info->data) {
 		usb_set_intfdata(info->data, NULL);
-- 
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: [iproute2] Fix -oneline output when alias present
From: Roopa Prabhu @ 2013-03-13 12:20 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: shemminger, netdev
In-Reply-To: <20130313094859.GA3097@amd64.fatal.se>

On 3/13/13 2:48 AM, Andreas Henriksson wrote:
> Hi!
>
> Just wanted to point out some minor style issues with your patch.
> See below...
>
> On Tue, Mar 12, 2013 at 12:03:47PM -0000, roopa@cumulusnetworks.com wrote:
> [...]
>> -	if (do_link&&  tb[IFLA_IFALIAS])
>> -		fprintf(fp,"\n    alias %s",
>> +	if (do_link&&  tb[IFLA_IFALIAS]) {
>> +		fprintf(fp, "%s", _SL_);
>> +		fprintf(fp,"    alias %s",
>
>                            ^^^ missing space here.

Oh, I hadn't noticed that. I did not introduce the style problem there.
I just happened to pick the _SL_line from else where in the code.
Since this patch is in, I will submit a patch to fix the spacing.
>
> Also, why not use a single fprintf?

I was just following convention here. The SL line seems to be on a line 
of its own else where in the code. Which i did think was cleaner.


Thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/4] Allow bridge to function in non-promisc mode
From: Vlad Yasevich @ 2013-03-13 12:12 UTC (permalink / raw)
  To: Oleg A. Arkhangelsky
  Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org
In-Reply-To: <473541363155743@web2d.yandex.ru>

On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
>
>
> 13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
>
>> The series adds an ability for the bridge to function in non-promiscuous mode.
>
> What is the practical applications for such setup? In other words,
> in which cases I would want to put bridge into non-promiscuous
> mode and specify some uplink ports?
>

On of the applications would be when bridge is an edge device servicing 
a VM deployment.  Each of the VMs knows the mac address that the guest
has and may program that mac onto the uplinks.

-vlad

^ permalink raw reply

* RE: [PATCH net-next v3] netxen: write IP address to firmware when using bonding
From: Rajesh Borundia @ 2013-03-13 11:51 UTC (permalink / raw)
  To: Nikolay Aleksandrov, netdev
  Cc: David Miller, agospoda@redhat.com, Sony Chacko
In-Reply-To: <1363092541-12391-1-git-send-email-nikolay@redhat.com>

-----Original Message-----
>From: Nikolay Aleksandrov [mailto:nikolay@redhat.com]
>Sent: Tuesday, March 12, 2013 6:19 PM
>To: netdev
>Cc: Rajesh Borundia; David Miller; agospoda@redhat.com; Sony Chacko
>Subject: [PATCH net-next v3] netxen: write IP address to firmware when
>using bonding
>
>This patch allows LRO aggregation on bonded devices that contain an
>NX3031 device. It also adds a for_each_netdev_in_bond_rcu(bond, slave)
>macro which executes for each slave that has bond as master.
>
>V3: After testing and discussing this with Rajesh, I decided to keep the
>    vlan ip cache and just rename it to ip_cache since it will store
>bond
>    ip addresses too. A new master flag has been added to the ip cache
>to
>    denote that the address has been added because of a master device.
>    I've taken care of the enslave/release cases by checking for various
>    combinations of events and flags (e.g. netxen has a master, it's a
>    bond master and it's not marked as a slave means it is being
>enslaved
>    and is dev_open()ed in bond_enslave).
>    I've changed netxen_free_ip_list() to have a "master" parameter
>which
>    causes all IP addresses marked as master to be deleted (used when a
>    netxen is being released). I've made the patch use the new upper
>    device API as well. The following cases were tested:
>    - bond -> netxen
>    - vlan -> netxen
>    - vlan -> bond -> netxen

Acked-by : Rajesh Borundia <rajesh.borundia@qlogic.com>

^ permalink raw reply

* Re: rt2x00: Kconfig symbols RALINK_RT288X and RALINK_RT305X
From: John Crispin @ 2013-03-13 11:19 UTC (permalink / raw)
  To: Jonas Gorski
  Cc: Paul Bolle, Ivo van Doorn, Gertjan van Wingerde, Helmut Schaa,
	John W. Linville, linux-wireless, users, netdev, linux-kernel
In-Reply-To: <CAOiHx=keYYoCGgHfrSKXOHQQvG1mrt60ems3dcXki1K06MHg2g@mail.gmail.com>

On 13/03/13 11:35, Jonas Gorski wrote:
> On 13 March 2013 11:03, Paul Bolle<pebolle@tiscali.nl>  wrote:
>> On Wed, 2013-03-13 at 10:51 +0100, Jonas Gorski wrote:
>>> The actual accepted Kconfig symbol names are different though, so they
>>> should be changed in rt2x00 to match them (SOC_RT288X and SOC_RT305X).
>> Thanks. Note that I could not find an actual Kconfig symbol SOC_RT288X!
> Ah, yes, the inital submission only included RT305X support, not
> RT288X (and neither of the newer chips). These will come later.
>
>> Anyhow, I guess somebody has the (trivial) patch to convert
>> RALINK_RT...X and CONFIG_RALINK_RT...X to their SOC_* equivalents queued
>> for inclusion in v3.9-rcX. Is that correct?
> Yes, that should be everything. John Crispin, anything missing from that?
>
>
> Jonas
>
>
Hi,

I will look into this later this week.

John

^ permalink raw reply

* Re: Trying to implement secondary loopback
From: richard -rw- weinberger @ 2013-03-13 11:13 UTC (permalink / raw)
  To: Thomas Martitz; +Cc: netdev, davem, edumazet, herbert, ebiederm
In-Reply-To: <513F1A17.1000809@hhi.fraunhofer.de>

On Tue, Mar 12, 2013 at 1:05 PM, Thomas Martitz
<thomas.martitz@hhi.fraunhofer.de> wrote:
> Hello folks,
>
> (I'm sending this mail a second time (with a few people in CC) because I
> received no response.)
>
> I'm Thomas Martitz, working on my Master Thesis: an Ethernet NIC. As part of
> my work I want to develop a custom loopback interface (for testing
> purposes).
>
> However I'm facing a problem: It doesn't appear to be possible for a second
> loopback device to exist. Even when copying (and module'ifying)
> drivers/net/loopback.c the resulting device doesn't behave like the standard
> lo interface.
>
> After examing the code I found this line (in loopback_net_init()):
>
> loopback.c:206         net->loopback_dev = dev;
>
> This suggests that each network namespace can only have one loopback
> interface. And indeed after modifying my custom interface to include this
> particular line it begins to work, but obviously at the same time the
> standard lo interface stops working.
>
> So my questions are:
> * Is this on purpose/expected?
> * Is there anyway around it so that I can have a custom loopback interface
> without touching lo's functionality.
> * Generally, what's the proper way (if any) implement a secondary loopback
> interface?

The only really question that matters is, why do you need a second
loopback interface?
Why can't you use any other pseudo interface?

-- 
Thanks,
//richard

^ permalink raw reply

* (unknown), 
From: Mr Peter Steve @ 2013-03-13  9:57 UTC (permalink / raw)





Do You Need A Loan?If Yes Apply With Loan Amount. Duration. Full Name. Country. Address. Phone Number,

^ 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