Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/8 V4] rtlwifi: Patches to fix problems shown by smatch
From: Larry Finger @ 2013-09-25 17:57 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
	netdev-u79uwXL29TY76Z2rM5mHXA

Fix smatch warnings and errors in the rtlwifi family of drivers.

V2 addresses comments by David Laight and Sergei Shtylyov.
V3 addresses further comments by David, Sergei, and Kalle Valo. The
   dead code is removed, and the variable is left in the struct.
V4 Fixes extra line removed by accident from rtl8192de/hw.c as
   noted by Sergei

Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
---

Larry Finger (8):
  rtlwifi: rtl8192du: Fix smatch errors in /rtl8192de/dm.c
  rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
  rtlwifi: rtl8192cu: Fix smatch warning in rtl8192cu/trx.c
  rtlwifi: rtl8192_common: Fix smatch errors and warnings in
    rtl8192c/dm_common.c
  rtlwifi: Fix smatch warning in pci.c
  rtlwifi: Fix smatch warnings in usb.c
  rtlwifi: rtl8188ee: Fix smatch warning in rtl8188ee/hw.c
  rtlwifi: Remove all remaining references to variable 'noise' in
    rtl_stats struct

 drivers/net/wireless/rtlwifi/pci.c                |  1 -
 drivers/net/wireless/rtlwifi/rtl8188ee/hw.c       |  1 +
 drivers/net/wireless/rtlwifi/rtl8188ee/trx.c      |  1 -
 drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c | 25 +----------------------
 drivers/net/wireless/rtlwifi/rtl8192ce/trx.c      |  1 -
 drivers/net/wireless/rtlwifi/rtl8192cu/trx.c      |  2 --
 drivers/net/wireless/rtlwifi/rtl8192de/dm.c       |  8 ++++++--
 drivers/net/wireless/rtlwifi/rtl8192de/hw.c       | 18 ----------------
 drivers/net/wireless/rtlwifi/rtl8192de/trx.c      |  1 -
 drivers/net/wireless/rtlwifi/rtl8192se/trx.c      |  1 -
 drivers/net/wireless/rtlwifi/rtl8723ae/trx.c      |  1 -
 drivers/net/wireless/rtlwifi/usb.c                |  6 ++++--
 12 files changed, 12 insertions(+), 54 deletions(-)

-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* Re: [PATCH tip/core/rcu 04/13] wireless: Apply rcu_access_pointer() to avoid sparse false positive
From: Ben Hutchings @ 2013-09-25 17:48 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Stephen Hemminger, tglx, laijs, edumazet, David S. Miller, peterz,
	fweisbec, bridge, linux-kernel, rostedt, josh, dhowells, sbw, niv,
	netdev, mathieu.desnoyers, dipankar, darren, akpm, mingo
In-Reply-To: <1380072916-31557-4-git-send-email-paulmck@linux.vnet.ibm.com>

On Tue, 2013-09-24 at 18:35 -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> The sparse checking for rcu_assign_pointer() was recently upgraded
> to reject non-__kernel address spaces.  This also rejects __rcu,
> which is almost always the right thing to do.  However, the uses in
> cfg80211_combine_bsses() and cfg80211_bss_update() are legitimate:
> They is assigning a pointer to an element from an RCU-protected list,
[...]

'They is'?  This error is also in the commit messsages for the bridge
and mac80211 patches, and maybe others that weren't sent to netdev.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH] Allow userspace code to use flag IFA_F_SECONDARY to specify an ip address to be primary or secondary ip on an interface
From: Vincent Li @ 2013-09-25 17:30 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: netdev@vger.kernel.org, linux-kernel, davem
In-Reply-To: <alpine.LFD.2.00.1309250925240.1971@ja.ssi.bg>

I think it is good idea to add these preferences flags and sorted
them, but my code knowledge is limited to implement it  as I am still
learning, I can help testing :)

On Wed, Sep 25, 2013 at 12:08 AM, Julian Anastasov <ja@ssi.bg> wrote:
>
>         Hello,
>
> On Tue, 24 Sep 2013, Vincent Li wrote:
>
>> Thanks Julian for the comments, I imagined it would not be so simple
>> as it changed old behavior with ip binary and some actions in
>> __inet_del_ifa() that I am not fully aware of. my intention is to
>> preserve the old behavior and extend the flexibility, I am unable to
>> come up with a good patch to achieve the intended behavior.
>
> ...
>
>> if someone can point me to the right patch directions or coming up
>> with better patches, it is very much appreciated.
>
>         My first idea was to use NLM_F_APPEND to implement
> 'ip addr prepend' and 'ip addr append' but the default
> operation is 'append' without providing NLM_F_APPEND, so it
> does not work.
>
>         Another idea is to add new attribute IFA_PREFERENCE in
> include/uapi/linux/if_addr.h just before __IFA_MAX, integer,
> 3 of the values are known. A preference for the used scope.
>
> /* Add as last, default */
> IFA_PREFERENCE_APPEND = 0,
>
> /* Add as last primary, before any present primary in subnet */
> IFA_PREFERENCE_PRIMARY = 128,
>
> /* First for scope */
> IFA_PREFERENCE_FIRST = 255,
>
>         We should keep it in ifa as priority, for
> sorting purposes. It can be 4-byte value, if user wants
> to copy user-defined order into preference.
>
>         Sorting order should be:
>
> - all primaries sorted by decreasing scope, decreasing
> priority and adding order
>
> - then all secondaries (IFA_F_SECONDARY) sorted by decreasing
> priority and adding order
>
>         Usage:
>
> ip addr add ... pref[erence] type_or_priority
>
> # Add floating IP (append at priority 128)
> # The primary mode is not guaranteed if another address from
> # the same subnet is already using the same or higher priority.
> ip addr add ... pref primary
> # More preferred primary
> ip addr add ... pref 129
>
> # Add first IP for scope
> ip addr add ... pref first
>
>         The scope has similar 'sorting' property but not
> for IPs in same subnet and it would be difficult to use
> it for global routes.
>
>         Thoughts?
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* ALX driver fails to resume from suspend on 3.11
From: Josh Boyer @ 2013-09-25 17:13 UTC (permalink / raw)
  To: James Cliburn, Chris Snook, Johannes Berg; +Cc: netdev, David Miller

Hi All,

We've had a couple reports [1] of the ALX module causing issues on
resume from suspend.  The reporters say their logs are flooded with
messages like:

alx 0000:04:00.0: invalid PHY speed/duplex: 0xffff

This was working fine in the 3.10 kernel.  Does anyone have any ideas
why this would be occurring now?

josh

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1011362

^ permalink raw reply

* Re: [PATCH v2] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Richard Cochran @ 2013-09-25 17:13 UTC (permalink / raw)
  To: Aida Mynzhasova
  Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1380093863-5388-1-git-send-email-aida.mynzhasova-Fmhy8gsqeTEvJsYlp49lxw@public.gmane.org>

On Wed, Sep 25, 2013 at 11:24:23AM +0400, Aida Mynzhasova wrote:
> Currently IEEE 1588 timer reference clock source is determined through
> hard-coded value in gianfar_ptp driver. This patch allows to select ptp
> clock source by means of device tree file node.

Looks okay to me now.

Acked-by: Richard Cochran <richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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

* Re: [PATCH net] ip_tunnel: Do not use stale inner_iph pointer.
From: Eric Dumazet @ 2013-09-25 17:10 UTC (permalink / raw)
  To: Pravin B Shelar; +Cc: netdev
In-Reply-To: <1380128267-16123-1-git-send-email-pshelar@nicira.com>

On Wed, 2013-09-25 at 09:57 -0700, Pravin B Shelar wrote:
> While sending packet skb_cow_head() can change skb header which
> invalidates inner_iph pointer to skb header. Following patch
> avoid using it. Found by code inspection.
> 
> This bug was introduced by commit 0e6fbc5b6c6218 (ip_tunnels: extend
> iptunnel_xmit()).
> 
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
>  net/ipv4/ip_tunnel.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Eric Dumazet <edumazet@google.com>

Thanks !

^ permalink raw reply

* ipv6: strange routing behaviors on a multi-interfaces setup
From: Emmanuel Thierry @ 2013-09-25 16:53 UTC (permalink / raw)
  To: netdev

Hello,

I'm working on multi-interfaces setups on IPv6. I found several disturbing route behaviors which sounds like bugs to me.

Both eth1 and eth2 interfaces receive RAs from distinct routers and autoconfigure:
* their slaac address
* their default route, both with the same priority
Under these conditions, the following happen depending on the expiration time of each route.


1/ A ping with a specified source address and interface may go through the wrong interface.

# ping6  -c 1 -I "<slaac_eth1>%eth1" <dest>
… uses the right interface (eth1) with the right source address (<slaac_eth1>).

# ping6  -c 1 -I "<slaac_eth2>%eth2" <dest>
… uses the wrong interface (eth1) with the right source address (<slaac_eth2>).

The ping6 utility performed a bind() on <slaac_ethx> with scope id set to <ifindex_ethx>.
If we flush the routing cache between each ping, the routing is done as expected.

As i could observe in the kernel. When ip6_pol_route() is called, oif is equal to <ifindex_eth2> but the flag RT6_LOOKUP_F_IFACE is not set. This makes routes through other interfaces to still be valid.
Shouldn't we set the RT6_LOOKUP_F_IFACE flag when a scope id is specified ?


2/ A ping from a specified source address may go through the wrong interface.

# ping6  -I "<slaac_eth2>" <dest>
… may use eth1 with <slaac_eth2>.

The ping6 utility performed a bind() on <slaac_eth2>
This is a derivative from the first one, with the significative difference that it also happens if the routing cache is empty. The most recent default route is chosen regardless of the source address.

Shouldn't we look in a first try for routes on the device corresponding to the source address, and in a second try for others ?


3/ A ping from a specified interface may go through the wrong interface with the wrong source address.

# ping6  -I "eth2" <dest>
… may use eth1 with <slaac_eth1>.

The ping6 performs a setsockopt(IPV6_PKTINFO) with <ifindex_ethx>, then a connect() to the destination. In this case, source address selection is concerned, but also routing since source address selection depends on routing.


I experienced these problems on a 3.11.1 kernel but they look to be quite recurrent in the past versions as well. 

Best regards
Emmanuel Thierry

^ permalink raw reply

* [PATCH net] ip_tunnel: Do not use stale inner_iph pointer.
From: Pravin B Shelar @ 2013-09-25 16:57 UTC (permalink / raw)
  To: netdev; +Cc: Pravin B Shelar

While sending packet skb_cow_head() can change skb header which
invalidates inner_iph pointer to skb header. Following patch
avoid using it. Found by code inspection.

This bug was introduced by commit 0e6fbc5b6c6218 (ip_tunnels: extend
iptunnel_xmit()).

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/ipv4/ip_tunnel.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index ac9fabe..d3fbad4 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -623,6 +623,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 			tunnel->err_count = 0;
 	}
 
+	tos = ip_tunnel_ecn_encap(tos, inner_iph, skb);
 	ttl = tnl_params->ttl;
 	if (ttl == 0) {
 		if (skb->protocol == htons(ETH_P_IP))
@@ -651,8 +652,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	err = iptunnel_xmit(rt, skb, fl4.saddr, fl4.daddr, protocol,
-			    ip_tunnel_ecn_encap(tos, inner_iph, skb), ttl, df,
-			    !net_eq(tunnel->net, dev_net(dev)));
+			    tos, ttl, df, !net_eq(tunnel->net, dev_net(dev)));
 	iptunnel_xmit_stats(err, &dev->stats, dev->tstats);
 
 	return;
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH net 1/2] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit
From: Pravin Shelar @ 2013-09-25 16:55 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David Miller, netdev
In-Reply-To: <20130925055418.GV7660@secunet.com>

On Tue, Sep 24, 2013 at 10:54 PM, Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> We might extend the used aera of a skb beyond the total
> headroom when we install the ipip header. Fix this by
> calling skb_cow_head() unconditionally.
>
It is better to call skb_cow_head() from ipip_tunnel_xmit() as it is
consistent with gre.

> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
>  net/ipv4/ip_tunnel.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index ac9fabe..b8ce640 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -641,13 +641,13 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>
>         max_headroom = LL_RESERVED_SPACE(rt->dst.dev) + sizeof(struct iphdr)
>                         + rt->dst.header_len;
> -       if (max_headroom > dev->needed_headroom) {
> +       if (max_headroom > dev->needed_headroom)
>                 dev->needed_headroom = max_headroom;
> -               if (skb_cow_head(skb, dev->needed_headroom)) {
> -                       dev->stats.tx_dropped++;
> -                       dev_kfree_skb(skb);
> -                       return;
> -               }
> +
> +       if (skb_cow_head(skb, dev->needed_headroom)) {
> +               dev->stats.tx_dropped++;
> +               dev_kfree_skb(skb);
> +               return;
>         }
>
>         err = iptunnel_xmit(rt, skb, fl4.saddr, fl4.daddr, protocol,
> --
> 1.7.9.5
>
> --
> 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

^ permalink raw reply

* Re: [PATCH net-next 1/2] bonding: modify the old and add new xmit hash functions
From: Nikolay Aleksandrov @ 2013-09-25 16:36 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: netdev, davem, andy, fubar, eric.dumazet
In-Reply-To: <20130925162606.GA27752@redhat.com>

On 09/25/2013 06:26 PM, Veaceslav Falico wrote:
> On Wed, Sep 25, 2013 at 05:19:57PM +0200, Nikolay Aleksandrov wrote:
>> This patch adds two new hash policy modes which use skb_flow_dissect:
>> 3 - Encapsulated layer 2+3
>> 4 - Encapsulated layer 3+4
>> There should be a good improvement for tunnel users in those modes.
>> It also changes the old hash functions to:
>> hash ^= (__force u32)flow.dst ^ (__force u32)flow.src;
>> hash ^= (hash >> 16);
>> hash ^= (hash >> 8);
>>
>> Where hash will be initialized either to L2 hash, that is
>> SRCMAC[5] XOR DSTMAC[5], or to flow->ports which should be extracted
>> from the upper layer. Flow's dst and src are also extracted based on the
>> xmit policy either directly from the buffer or by using skb_flow_dissect,
>> but in both cases if the protocol is IPv6 then dst and src are obtained by
>> ipv6_addr_hash() on the real addresses. In case of a non-dissectable
>> packet, the algorithms fall back to L2 hashing.
>> The bond_set_mode_ops() function is now obsolete and thus deleted
>> because it was used only to set the proper hash policy. Also we trim a
>> pointer from struct bonding because we no longer need to keep the hash
>> function, now there's only a single hash function - bond_xmit_hash that
>> works based on bond->params.xmit_policy.
>>
>> The hash function and skb_flow_dissect were suggested by Eric Dumazet.
>> The layer names were suggested by Andy Gospodarek, because I suck at
>> semantics.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>> ---
>> One line is intentionally left at 82 chars since it's the whole function
>> and IMO looks better that way.
>>
>> drivers/net/bonding/bond_3ad.c   |   2 +-
>> drivers/net/bonding/bond_main.c  | 211
>> +++++++++++++++------------------------
>> drivers/net/bonding/bond_sysfs.c |   2 -
>> drivers/net/bonding/bonding.h    |   3 +-
>> include/uapi/linux/if_bonding.h  |   2 +
>> 5 files changed, 86 insertions(+), 134 deletions(-)
> 
> Nice!!! Though some comments below.
> 
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index 0d8f427..b3ab703 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -2442,7 +2442,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct
>> net_device *dev)
>>         goto out;
>>     }
>>
>> -    slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
>> +    slave_agg_no = bond_xmit_hash(bond, skb, slaves_in_agg);
>>
>>     bond_for_each_slave(bond, slave) {
>>         struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
>> diff --git a/drivers/net/bonding/bond_main.c
>> b/drivers/net/bonding/bond_main.c
>> index 55bbb8b..73e416b 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -78,6 +78,7 @@
>> #include <net/netns/generic.h>
>> #include <net/pkt_sched.h>
>> #include <linux/rculist.h>
>> +#include <net/flow_keys.h>
>> #include "bonding.h"
>> #include "bond_3ad.h"
>> #include "bond_alb.h"
>> @@ -159,7 +160,8 @@ MODULE_PARM_DESC(min_links, "Minimum number of
>> available links before turning on
>> module_param(xmit_hash_policy, charp, 0);
>> MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing
>> method; "
>>                    "0 for layer 2 (default), 1 for layer 3+4, "
>> -                   "2 for layer 2+3");
>> +                   "2 for layer 2+3, 3 for encap layer 2+3, "
>> +                   "4 for encap layer 3+4");
>> module_param(arp_interval, int, 0);
>> MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
>> module_param_array(arp_ip_target, charp, NULL, 0);
>> @@ -217,6 +219,8 @@ const struct bond_parm_tbl xmit_hashtype_tbl[] = {
>> {    "layer2",        BOND_XMIT_POLICY_LAYER2},
>> {    "layer3+4",        BOND_XMIT_POLICY_LAYER34},
>> {    "layer2+3",        BOND_XMIT_POLICY_LAYER23},
>> +{    "encap2+3",        BOND_XMIT_POLICY_ENCAP23},
>> +{    "encap3+4",        BOND_XMIT_POLICY_ENCAP34},
>> {    NULL,            -1},
>> };
>>
>> @@ -3026,99 +3030,99 @@ static struct notifier_block bond_netdev_notifier
>> = {
>>
>> /*---------------------------- Hashing Policies
>> -----------------------------*/
>>
>> -/*
>> - * Hash for the output device based upon layer 2 data
>> - */
>> -static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
>> +/* L2 hash helper */
>> +static inline u32 bond_eth_hash(struct sk_buff *skb)
>> {
>>     struct ethhdr *data = (struct ethhdr *)skb->data;
>>
>>     if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto))
>> -        return (data->h_dest[5] ^ data->h_source[5]) % count;
>> +        return data->h_dest[5] ^ data->h_source[5];
>>
>>     return 0;
>> }
>>
>> -/*
>> - * Hash for the output device based upon layer 2 and layer 3 data. If
>> - * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
>> - */
>> -static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
>> +static void bond_flow_get_ports(struct sk_buff *skb, struct flow_keys *fk,
>> +                int offset)
>> +{
>> +    __be32 *ports;
>> +
>> +    ports = skb_header_pointer(skb, offset, sizeof(fk->ports), &fk->ports);
>> +    if (ports)
>> +        fk->ports = *ports;
>> +}
>> +
>> +/* Extract the appropriate headers based on bond's xmit policy */
>> +static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
>> +                  struct flow_keys *fk)
>> {
>> -    const struct ethhdr *data;
>> +    const struct ipv6hdr *iph6;
>>     const struct iphdr *iph;
>> -    const struct ipv6hdr *ipv6h;
>> -    u32 v6hash;
>> -    const __be32 *s, *d;
>> +    int noff, proto = -1, poff = -1;
> 
> Nitpick: Useless initialization of poff.
> 
Yeah, I saw it after posting and have this in the prepared v2 :-)

>> +    bool ret = false;
>> +
>> +    if (bond->params.xmit_policy > BOND_XMIT_POLICY_LAYER23)
>> +        return skb_flow_dissect(skb, fk);
>>
>> -    if (skb->protocol == htons(ETH_P_IP) &&
>> -        pskb_network_may_pull(skb, sizeof(*iph))) {
>> +    fk->ports = 0;
>> +    noff = skb_network_offset(skb);
>> +    if (skb->protocol == htons(ETH_P_IP)) {
>> +        if (!pskb_may_pull(skb, noff + sizeof(*iph)))
>> +            goto out;
>>         iph = ip_hdr(skb);
>> -        data = (struct ethhdr *)skb->data;
>> -        return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
>> -            (data->h_dest[5] ^ data->h_source[5])) % count;
>> -    } else if (skb->protocol == htons(ETH_P_IPV6) &&
>> -           pskb_network_may_pull(skb, sizeof(*ipv6h))) {
>> -        ipv6h = ipv6_hdr(skb);
>> -        data = (struct ethhdr *)skb->data;
>> -        s = &ipv6h->saddr.s6_addr32[0];
>> -        d = &ipv6h->daddr.s6_addr32[0];
>> -        v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
>> -        v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8);
>> -        return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
>> -    }
>> -
>> -    return bond_xmit_hash_policy_l2(skb, count);
>> +        fk->src = iph->saddr;
>> +        fk->dst = iph->daddr;
>> +        noff += iph->ihl << 2;
>> +        if (!ip_is_fragment(iph))
>> +            proto = iph->protocol;
>> +    } else if (skb->protocol == htons(ETH_P_IPV6)) {
>> +        if (!pskb_may_pull(skb, noff + sizeof(*iph6)))
>> +            goto out;
>> +        iph6 = ipv6_hdr(skb);
>> +        fk->src = (__force __be32)ipv6_addr_hash(&iph6->saddr);
>> +        fk->dst = (__force __be32)ipv6_addr_hash(&iph6->daddr);
>> +        noff += sizeof(*iph6);
>> +        proto = iph6->nexthdr;
>> +    }
> 
> else
>     return false;
> 
> ?
> 
Hehe, I actually had that in v1 few months ago, I was wondering why I did
it that way...

> Otherwise we might report false-positive for vlans, per example, without
> populating the flow keys.
> 
Right.

>> +    if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER34) {
>> +        poff = proto_ports_offset(proto);
>> +        if (poff >= 0)
>> +            bond_flow_get_ports(skb, fk, poff + noff);
>> +    }
> 
> One idea would be to move the same initialization code from
> skb_flow_dissect() to an external function and re-use it here?
> 
Yep, I could do an inline skb_flow_get_ports() and re-use it.

>> +    ret = true;
>> +
>> +out:
>> +    return ret;
>> }
>>
>> -/*
>> - * Hash for the output device based upon layer 3 and layer 4 data. If
>> - * the packet is a frag or not TCP or UDP, just use layer 3 data.  If it is
>> - * altogether not IP, fall back on bond_xmit_hash_policy_l2()
>> +/**
>> + * bond_xmit_hash - generate a hash value based on the xmit policy
>> + * @bond: bonding device
>> + * @skb: buffer to use for headers
>> + * @count: modulo value
>> + *
>> + * This function will extract the necessary headers from the skb buffer
>> and use
>> + * them to generate a hash based on the xmit_policy set in the bonding
>> device
>> + * which will be reduced modulo count before returning.
>>  */
>> -static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
>> +int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
>> {
>> -    u32 layer4_xor = 0;
>> -    const struct iphdr *iph;
>> -    const struct ipv6hdr *ipv6h;
>> -    const __be32 *s, *d;
>> -    const __be16 *l4 = NULL;
>> -    __be16 _l4[2];
>> -    int noff = skb_network_offset(skb);
>> -    int poff;
>> -
>> -    if (skb->protocol == htons(ETH_P_IP) &&
>> -        pskb_may_pull(skb, noff + sizeof(*iph))) {
>> -        iph = ip_hdr(skb);
>> -        poff = proto_ports_offset(iph->protocol);
>> +    struct flow_keys flow;
>> +    u32 hash;
>>
>> -        if (!ip_is_fragment(iph) && poff >= 0) {
>> -            l4 = skb_header_pointer(skb, noff + (iph->ihl << 2) + poff,
>> -                        sizeof(_l4), &_l4);
>> -            if (l4)
>> -                layer4_xor = ntohs(l4[0] ^ l4[1]);
>> -        }
>> -        return (layer4_xor ^
>> -            ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>> -    } else if (skb->protocol == htons(ETH_P_IPV6) &&
>> -           pskb_may_pull(skb, noff + sizeof(*ipv6h))) {
>> -        ipv6h = ipv6_hdr(skb);
>> -        poff = proto_ports_offset(ipv6h->nexthdr);
>> -        if (poff >= 0) {
>> -            l4 = skb_header_pointer(skb, noff + sizeof(*ipv6h) + poff,
>> -                        sizeof(_l4), &_l4);
>> -            if (l4)
>> -                layer4_xor = ntohs(l4[0] ^ l4[1]);
>> -        }
>> -        s = &ipv6h->saddr.s6_addr32[0];
>> -        d = &ipv6h->daddr.s6_addr32[0];
>> -        layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
>> -        layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^
>> -                   (layer4_xor >> 8);
>> -        return layer4_xor % count;
>> -    }
>> +    if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
>> +        !bond_flow_dissect(bond, skb, &flow))
>> +        return bond_eth_hash(skb) % count;
>> +
>> +    if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 ||
>> +        bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23)
>> +        hash = bond_eth_hash(skb);
>> +    else
>> +        hash = (__force u32)flow.ports;
>> +    hash ^= (__force u32)flow.dst ^ (__force u32)flow.src;
>> +    hash ^= (hash >> 16);
>> +    hash ^= (hash >> 8);
>>
>> -    return bond_xmit_hash_policy_l2(skb, count);
>> +    return hash % count;
>> }
>>
>> /*-------------------------- Device entry points
>> ----------------------------*/
>> @@ -3700,8 +3704,7 @@ static int bond_xmit_activebackup(struct sk_buff
>> *skb, struct net_device *bond_d
>>     return NETDEV_TX_OK;
>> }
>>
>> -/*
>> - * In bond_xmit_xor() , we determine the output device by using a pre-
>> +/* In bond_xmit_xor() , we determine the output device by using a pre-
>>  * determined xmit_hash_policy(), If the selected device is not enabled,
>>  * find the next active slave.
>>  */
>> @@ -3709,8 +3712,7 @@ static int bond_xmit_xor(struct sk_buff *skb,
>> struct net_device *bond_dev)
>> {
>>     struct bonding *bond = netdev_priv(bond_dev);
>>
>> -    bond_xmit_slave_id(bond, skb,
>> -               bond->xmit_hash_policy(skb, bond->slave_cnt));
>> +    bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb,
>> bond->slave_cnt));
>>
>>     return NETDEV_TX_OK;
>> }
>> @@ -3746,22 +3748,6 @@ static int bond_xmit_broadcast(struct sk_buff
>> *skb, struct net_device *bond_dev)
>>
>> /*------------------------- Device initialization
>> ---------------------------*/
>>
>> -static void bond_set_xmit_hash_policy(struct bonding *bond)
>> -{
>> -    switch (bond->params.xmit_policy) {
>> -    case BOND_XMIT_POLICY_LAYER23:
>> -        bond->xmit_hash_policy = bond_xmit_hash_policy_l23;
>> -        break;
>> -    case BOND_XMIT_POLICY_LAYER34:
>> -        bond->xmit_hash_policy = bond_xmit_hash_policy_l34;
>> -        break;
>> -    case BOND_XMIT_POLICY_LAYER2:
>> -    default:
>> -        bond->xmit_hash_policy = bond_xmit_hash_policy_l2;
>> -        break;
>> -    }
>> -}
>> -
>> /*
>>  * Lookup the slave that corresponds to a qid
>>  */
>> @@ -3871,38 +3857,6 @@ static netdev_tx_t bond_start_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>>     return ret;
>> }
>>
>> -/*
>> - * set bond mode specific net device operations
>> - */
>> -void bond_set_mode_ops(struct bonding *bond, int mode)
>> -{
>> -    struct net_device *bond_dev = bond->dev;
>> -
>> -    switch (mode) {
>> -    case BOND_MODE_ROUNDROBIN:
>> -        break;
>> -    case BOND_MODE_ACTIVEBACKUP:
>> -        break;
>> -    case BOND_MODE_XOR:
>> -        bond_set_xmit_hash_policy(bond);
>> -        break;
>> -    case BOND_MODE_BROADCAST:
>> -        break;
>> -    case BOND_MODE_8023AD:
>> -        bond_set_xmit_hash_policy(bond);
>> -        break;
>> -    case BOND_MODE_ALB:
>> -        /* FALLTHRU */
>> -    case BOND_MODE_TLB:
>> -        break;
>> -    default:
>> -        /* Should never happen, mode already checked */
>> -        pr_err("%s: Error: Unknown bonding mode %d\n",
>> -               bond_dev->name, mode);
>> -        break;
>> -    }
>> -}
>> -
>> static int bond_ethtool_get_settings(struct net_device *bond_dev,
>>                      struct ethtool_cmd *ecmd)
>> {
>> @@ -4004,7 +3958,6 @@ static void bond_setup(struct net_device *bond_dev)
>>     ether_setup(bond_dev);
>>     bond_dev->netdev_ops = &bond_netdev_ops;
>>     bond_dev->ethtool_ops = &bond_ethtool_ops;
>> -    bond_set_mode_ops(bond, bond->params.mode);
>>
>>     bond_dev->destructor = bond_destructor;
>>
>> diff --git a/drivers/net/bonding/bond_sysfs.c
>> b/drivers/net/bonding/bond_sysfs.c
>> index c29b836..dba3b9b 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -352,7 +352,6 @@ static ssize_t bonding_store_mode(struct device *d,
>>     /* don't cache arp_validate between modes */
>>     bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
>>     bond->params.mode = new_value;
>> -    bond_set_mode_ops(bond, bond->params.mode);
>>     pr_info("%s: setting mode to %s (%d).\n",
>>         bond->dev->name, bond_mode_tbl[new_value].modename,
>>         new_value);
>> @@ -392,7 +391,6 @@ static ssize_t bonding_store_xmit_hash(struct device *d,
>>         ret = -EINVAL;
>>     } else {
>>         bond->params.xmit_policy = new_value;
>> -        bond_set_mode_ops(bond, bond->params.mode);
>>         pr_info("%s: setting xmit hash policy to %s (%d).\n",
>>             bond->dev->name,
>>             xmit_hashtype_tbl[new_value].modename, new_value);
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index 03cf3fd..4db9ec4 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -245,7 +245,6 @@ struct bonding {
>>     char     proc_file_name[IFNAMSIZ];
>> #endif /* CONFIG_PROC_FS */
>>     struct   list_head bond_list;
>> -    int      (*xmit_hash_policy)(struct sk_buff *, int);
>>     u16      rr_tx_counter;
>>     struct   ad_bond_info ad_info;
>>     struct   alb_bond_info alb_info;
>> @@ -446,7 +445,7 @@ int bond_release(struct net_device *bond_dev, struct
>> net_device *slave_dev);
>> void bond_mii_monitor(struct work_struct *);
>> void bond_loadbalance_arp_mon(struct work_struct *);
>> void bond_activebackup_arp_mon(struct work_struct *);
>> -void bond_set_mode_ops(struct bonding *bond, int mode);
>> +int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count);
>> int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl);
>> void bond_select_active_slave(struct bonding *bond);
>> void bond_change_active_slave(struct bonding *bond, struct slave
>> *new_active);
>> diff --git a/include/uapi/linux/if_bonding.h
>> b/include/uapi/linux/if_bonding.h
>> index a17edda..9635a62 100644
>> --- a/include/uapi/linux/if_bonding.h
>> +++ b/include/uapi/linux/if_bonding.h
>> @@ -91,6 +91,8 @@
>> #define BOND_XMIT_POLICY_LAYER2        0 /* layer 2 (MAC only), default */
>> #define BOND_XMIT_POLICY_LAYER34    1 /* layer 3+4 (IP ^ (TCP || UDP)) */
>> #define BOND_XMIT_POLICY_LAYER23    2 /* layer 2+3 (IP ^ MAC) */
>> +#define BOND_XMIT_POLICY_ENCAP23    3 /* encapsulated layer 2+3 */
>> +#define BOND_XMIT_POLICY_ENCAP34    4 /* encapsulated layer 3+4 */
>>
>> typedef struct ifbond {
>>     __s32 bond_mode;
>> -- 
>> 1.8.1.4
>>
>> -- 
>> 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

^ permalink raw reply

* rx_dropped count for USB ethernet interfaces
From: David Laight @ 2013-09-25 16:22 UTC (permalink / raw)
  To: netdev

We are seeing the 'rx_dropped' count increasing on an USB ethernet interfaces
smsc95xx, but not on the e1000 interface connected to the same LAN.

Now I thought that rx_dropped was a count of the number of packets that the
MAC driver/hardware discarded - typically because the rx ring had no buffers.
This could include packets dropped in the receive stack due to other memory
limits.

However it looks as though Linux is also counting rx_dropped if the packet
can't be delivered to a protocol (at the end of __netif_receive_skb).

The LAN definitely has broadcast packets with an unknown 'ethertype',
I'd expect these to be silently discarded not counted as rx_dropped.

If I run tcpdump on the interface (even with a filter that passes nothing)
then rx_dropped doesn't change.

I think I've just worked out why I don't see these error counts
on the e1000 interface. I suspect that DHCP is getting a copy of every
packet (that can't help network performance?)

	David

^ permalink raw reply

* Re: [PATCH net-next 1/2] bonding: modify the old and add new xmit hash functions
From: Veaceslav Falico @ 2013-09-25 16:26 UTC (permalink / raw)
  To: Nikolay Aleksandrov; +Cc: netdev, davem, andy, fubar, eric.dumazet
In-Reply-To: <1380122398-7370-2-git-send-email-nikolay@redhat.com>

On Wed, Sep 25, 2013 at 05:19:57PM +0200, Nikolay Aleksandrov wrote:
>This patch adds two new hash policy modes which use skb_flow_dissect:
>3 - Encapsulated layer 2+3
>4 - Encapsulated layer 3+4
>There should be a good improvement for tunnel users in those modes.
>It also changes the old hash functions to:
>hash ^= (__force u32)flow.dst ^ (__force u32)flow.src;
>hash ^= (hash >> 16);
>hash ^= (hash >> 8);
>
>Where hash will be initialized either to L2 hash, that is
>SRCMAC[5] XOR DSTMAC[5], or to flow->ports which should be extracted
>from the upper layer. Flow's dst and src are also extracted based on the
>xmit policy either directly from the buffer or by using skb_flow_dissect,
>but in both cases if the protocol is IPv6 then dst and src are obtained by
>ipv6_addr_hash() on the real addresses. In case of a non-dissectable
>packet, the algorithms fall back to L2 hashing.
>The bond_set_mode_ops() function is now obsolete and thus deleted
>because it was used only to set the proper hash policy. Also we trim a
>pointer from struct bonding because we no longer need to keep the hash
>function, now there's only a single hash function - bond_xmit_hash that
>works based on bond->params.xmit_policy.
>
>The hash function and skb_flow_dissect were suggested by Eric Dumazet.
>The layer names were suggested by Andy Gospodarek, because I suck at
>semantics.
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>---
>One line is intentionally left at 82 chars since it's the whole function
>and IMO looks better that way.
>
> drivers/net/bonding/bond_3ad.c   |   2 +-
> drivers/net/bonding/bond_main.c  | 211 +++++++++++++++------------------------
> drivers/net/bonding/bond_sysfs.c |   2 -
> drivers/net/bonding/bonding.h    |   3 +-
> include/uapi/linux/if_bonding.h  |   2 +
> 5 files changed, 86 insertions(+), 134 deletions(-)

Nice!!! Though some comments below.

>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 0d8f427..b3ab703 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -2442,7 +2442,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
> 		goto out;
> 	}
>
>-	slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
>+	slave_agg_no = bond_xmit_hash(bond, skb, slaves_in_agg);
>
> 	bond_for_each_slave(bond, slave) {
> 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 55bbb8b..73e416b 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -78,6 +78,7 @@
> #include <net/netns/generic.h>
> #include <net/pkt_sched.h>
> #include <linux/rculist.h>
>+#include <net/flow_keys.h>
> #include "bonding.h"
> #include "bond_3ad.h"
> #include "bond_alb.h"
>@@ -159,7 +160,8 @@ MODULE_PARM_DESC(min_links, "Minimum number of available links before turning on
> module_param(xmit_hash_policy, charp, 0);
> MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; "
> 				   "0 for layer 2 (default), 1 for layer 3+4, "
>-				   "2 for layer 2+3");
>+				   "2 for layer 2+3, 3 for encap layer 2+3, "
>+				   "4 for encap layer 3+4");
> module_param(arp_interval, int, 0);
> MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
> module_param_array(arp_ip_target, charp, NULL, 0);
>@@ -217,6 +219,8 @@ const struct bond_parm_tbl xmit_hashtype_tbl[] = {
> {	"layer2",		BOND_XMIT_POLICY_LAYER2},
> {	"layer3+4",		BOND_XMIT_POLICY_LAYER34},
> {	"layer2+3",		BOND_XMIT_POLICY_LAYER23},
>+{	"encap2+3",		BOND_XMIT_POLICY_ENCAP23},
>+{	"encap3+4",		BOND_XMIT_POLICY_ENCAP34},
> {	NULL,			-1},
> };
>
>@@ -3026,99 +3030,99 @@ static struct notifier_block bond_netdev_notifier = {
>
> /*---------------------------- Hashing Policies -----------------------------*/
>
>-/*
>- * Hash for the output device based upon layer 2 data
>- */
>-static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
>+/* L2 hash helper */
>+static inline u32 bond_eth_hash(struct sk_buff *skb)
> {
> 	struct ethhdr *data = (struct ethhdr *)skb->data;
>
> 	if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto))
>-		return (data->h_dest[5] ^ data->h_source[5]) % count;
>+		return data->h_dest[5] ^ data->h_source[5];
>
> 	return 0;
> }
>
>-/*
>- * Hash for the output device based upon layer 2 and layer 3 data. If
>- * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
>- */
>-static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
>+static void bond_flow_get_ports(struct sk_buff *skb, struct flow_keys *fk,
>+				int offset)
>+{
>+	__be32 *ports;
>+
>+	ports = skb_header_pointer(skb, offset, sizeof(fk->ports), &fk->ports);
>+	if (ports)
>+		fk->ports = *ports;
>+}
>+
>+/* Extract the appropriate headers based on bond's xmit policy */
>+static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
>+			      struct flow_keys *fk)
> {
>-	const struct ethhdr *data;
>+	const struct ipv6hdr *iph6;
> 	const struct iphdr *iph;
>-	const struct ipv6hdr *ipv6h;
>-	u32 v6hash;
>-	const __be32 *s, *d;
>+	int noff, proto = -1, poff = -1;

Nitpick: Useless initialization of poff.

>+	bool ret = false;
>+
>+	if (bond->params.xmit_policy > BOND_XMIT_POLICY_LAYER23)
>+		return skb_flow_dissect(skb, fk);
>
>-	if (skb->protocol == htons(ETH_P_IP) &&
>-	    pskb_network_may_pull(skb, sizeof(*iph))) {
>+	fk->ports = 0;
>+	noff = skb_network_offset(skb);
>+	if (skb->protocol == htons(ETH_P_IP)) {
>+		if (!pskb_may_pull(skb, noff + sizeof(*iph)))
>+			goto out;
> 		iph = ip_hdr(skb);
>-		data = (struct ethhdr *)skb->data;
>-		return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
>-			(data->h_dest[5] ^ data->h_source[5])) % count;
>-	} else if (skb->protocol == htons(ETH_P_IPV6) &&
>-		   pskb_network_may_pull(skb, sizeof(*ipv6h))) {
>-		ipv6h = ipv6_hdr(skb);
>-		data = (struct ethhdr *)skb->data;
>-		s = &ipv6h->saddr.s6_addr32[0];
>-		d = &ipv6h->daddr.s6_addr32[0];
>-		v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
>-		v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8);
>-		return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
>-	}
>-
>-	return bond_xmit_hash_policy_l2(skb, count);
>+		fk->src = iph->saddr;
>+		fk->dst = iph->daddr;
>+		noff += iph->ihl << 2;
>+		if (!ip_is_fragment(iph))
>+			proto = iph->protocol;
>+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
>+		if (!pskb_may_pull(skb, noff + sizeof(*iph6)))
>+			goto out;
>+		iph6 = ipv6_hdr(skb);
>+		fk->src = (__force __be32)ipv6_addr_hash(&iph6->saddr);
>+		fk->dst = (__force __be32)ipv6_addr_hash(&iph6->daddr);
>+		noff += sizeof(*iph6);
>+		proto = iph6->nexthdr;
>+	}

else
	return false;

?

Otherwise we might report false-positive for vlans, per example, without
populating the flow keys.

>+	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER34) {
>+		poff = proto_ports_offset(proto);
>+		if (poff >= 0)
>+			bond_flow_get_ports(skb, fk, poff + noff);
>+	}

One idea would be to move the same initialization code from
skb_flow_dissect() to an external function and re-use it here?

>+	ret = true;
>+
>+out:
>+	return ret;
> }
>
>-/*
>- * Hash for the output device based upon layer 3 and layer 4 data. If
>- * the packet is a frag or not TCP or UDP, just use layer 3 data.  If it is
>- * altogether not IP, fall back on bond_xmit_hash_policy_l2()
>+/**
>+ * bond_xmit_hash - generate a hash value based on the xmit policy
>+ * @bond: bonding device
>+ * @skb: buffer to use for headers
>+ * @count: modulo value
>+ *
>+ * This function will extract the necessary headers from the skb buffer and use
>+ * them to generate a hash based on the xmit_policy set in the bonding device
>+ * which will be reduced modulo count before returning.
>  */
>-static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
>+int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
> {
>-	u32 layer4_xor = 0;
>-	const struct iphdr *iph;
>-	const struct ipv6hdr *ipv6h;
>-	const __be32 *s, *d;
>-	const __be16 *l4 = NULL;
>-	__be16 _l4[2];
>-	int noff = skb_network_offset(skb);
>-	int poff;
>-
>-	if (skb->protocol == htons(ETH_P_IP) &&
>-	    pskb_may_pull(skb, noff + sizeof(*iph))) {
>-		iph = ip_hdr(skb);
>-		poff = proto_ports_offset(iph->protocol);
>+	struct flow_keys flow;
>+	u32 hash;
>
>-		if (!ip_is_fragment(iph) && poff >= 0) {
>-			l4 = skb_header_pointer(skb, noff + (iph->ihl << 2) + poff,
>-						sizeof(_l4), &_l4);
>-			if (l4)
>-				layer4_xor = ntohs(l4[0] ^ l4[1]);
>-		}
>-		return (layer4_xor ^
>-			((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>-	} else if (skb->protocol == htons(ETH_P_IPV6) &&
>-		   pskb_may_pull(skb, noff + sizeof(*ipv6h))) {
>-		ipv6h = ipv6_hdr(skb);
>-		poff = proto_ports_offset(ipv6h->nexthdr);
>-		if (poff >= 0) {
>-			l4 = skb_header_pointer(skb, noff + sizeof(*ipv6h) + poff,
>-						sizeof(_l4), &_l4);
>-			if (l4)
>-				layer4_xor = ntohs(l4[0] ^ l4[1]);
>-		}
>-		s = &ipv6h->saddr.s6_addr32[0];
>-		d = &ipv6h->daddr.s6_addr32[0];
>-		layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
>-		layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^
>-			       (layer4_xor >> 8);
>-		return layer4_xor % count;
>-	}
>+	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
>+	    !bond_flow_dissect(bond, skb, &flow))
>+		return bond_eth_hash(skb) % count;
>+
>+	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 ||
>+	    bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23)
>+		hash = bond_eth_hash(skb);
>+	else
>+		hash = (__force u32)flow.ports;
>+	hash ^= (__force u32)flow.dst ^ (__force u32)flow.src;
>+	hash ^= (hash >> 16);
>+	hash ^= (hash >> 8);
>
>-	return bond_xmit_hash_policy_l2(skb, count);
>+	return hash % count;
> }
>
> /*-------------------------- Device entry points ----------------------------*/
>@@ -3700,8 +3704,7 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
> 	return NETDEV_TX_OK;
> }
>
>-/*
>- * In bond_xmit_xor() , we determine the output device by using a pre-
>+/* In bond_xmit_xor() , we determine the output device by using a pre-
>  * determined xmit_hash_policy(), If the selected device is not enabled,
>  * find the next active slave.
>  */
>@@ -3709,8 +3712,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
> {
> 	struct bonding *bond = netdev_priv(bond_dev);
>
>-	bond_xmit_slave_id(bond, skb,
>-			   bond->xmit_hash_policy(skb, bond->slave_cnt));
>+	bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb, bond->slave_cnt));
>
> 	return NETDEV_TX_OK;
> }
>@@ -3746,22 +3748,6 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
>
> /*------------------------- Device initialization ---------------------------*/
>
>-static void bond_set_xmit_hash_policy(struct bonding *bond)
>-{
>-	switch (bond->params.xmit_policy) {
>-	case BOND_XMIT_POLICY_LAYER23:
>-		bond->xmit_hash_policy = bond_xmit_hash_policy_l23;
>-		break;
>-	case BOND_XMIT_POLICY_LAYER34:
>-		bond->xmit_hash_policy = bond_xmit_hash_policy_l34;
>-		break;
>-	case BOND_XMIT_POLICY_LAYER2:
>-	default:
>-		bond->xmit_hash_policy = bond_xmit_hash_policy_l2;
>-		break;
>-	}
>-}
>-
> /*
>  * Lookup the slave that corresponds to a qid
>  */
>@@ -3871,38 +3857,6 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
> 	return ret;
> }
>
>-/*
>- * set bond mode specific net device operations
>- */
>-void bond_set_mode_ops(struct bonding *bond, int mode)
>-{
>-	struct net_device *bond_dev = bond->dev;
>-
>-	switch (mode) {
>-	case BOND_MODE_ROUNDROBIN:
>-		break;
>-	case BOND_MODE_ACTIVEBACKUP:
>-		break;
>-	case BOND_MODE_XOR:
>-		bond_set_xmit_hash_policy(bond);
>-		break;
>-	case BOND_MODE_BROADCAST:
>-		break;
>-	case BOND_MODE_8023AD:
>-		bond_set_xmit_hash_policy(bond);
>-		break;
>-	case BOND_MODE_ALB:
>-		/* FALLTHRU */
>-	case BOND_MODE_TLB:
>-		break;
>-	default:
>-		/* Should never happen, mode already checked */
>-		pr_err("%s: Error: Unknown bonding mode %d\n",
>-		       bond_dev->name, mode);
>-		break;
>-	}
>-}
>-
> static int bond_ethtool_get_settings(struct net_device *bond_dev,
> 				     struct ethtool_cmd *ecmd)
> {
>@@ -4004,7 +3958,6 @@ static void bond_setup(struct net_device *bond_dev)
> 	ether_setup(bond_dev);
> 	bond_dev->netdev_ops = &bond_netdev_ops;
> 	bond_dev->ethtool_ops = &bond_ethtool_ops;
>-	bond_set_mode_ops(bond, bond->params.mode);
>
> 	bond_dev->destructor = bond_destructor;
>
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index c29b836..dba3b9b 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -352,7 +352,6 @@ static ssize_t bonding_store_mode(struct device *d,
> 	/* don't cache arp_validate between modes */
> 	bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
> 	bond->params.mode = new_value;
>-	bond_set_mode_ops(bond, bond->params.mode);
> 	pr_info("%s: setting mode to %s (%d).\n",
> 		bond->dev->name, bond_mode_tbl[new_value].modename,
> 		new_value);
>@@ -392,7 +391,6 @@ static ssize_t bonding_store_xmit_hash(struct device *d,
> 		ret = -EINVAL;
> 	} else {
> 		bond->params.xmit_policy = new_value;
>-		bond_set_mode_ops(bond, bond->params.mode);
> 		pr_info("%s: setting xmit hash policy to %s (%d).\n",
> 			bond->dev->name,
> 			xmit_hashtype_tbl[new_value].modename, new_value);
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 03cf3fd..4db9ec4 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -245,7 +245,6 @@ struct bonding {
> 	char     proc_file_name[IFNAMSIZ];
> #endif /* CONFIG_PROC_FS */
> 	struct   list_head bond_list;
>-	int      (*xmit_hash_policy)(struct sk_buff *, int);
> 	u16      rr_tx_counter;
> 	struct   ad_bond_info ad_info;
> 	struct   alb_bond_info alb_info;
>@@ -446,7 +445,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
> void bond_mii_monitor(struct work_struct *);
> void bond_loadbalance_arp_mon(struct work_struct *);
> void bond_activebackup_arp_mon(struct work_struct *);
>-void bond_set_mode_ops(struct bonding *bond, int mode);
>+int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count);
> int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl);
> void bond_select_active_slave(struct bonding *bond);
> void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
>diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h
>index a17edda..9635a62 100644
>--- a/include/uapi/linux/if_bonding.h
>+++ b/include/uapi/linux/if_bonding.h
>@@ -91,6 +91,8 @@
> #define BOND_XMIT_POLICY_LAYER2		0 /* layer 2 (MAC only), default */
> #define BOND_XMIT_POLICY_LAYER34	1 /* layer 3+4 (IP ^ (TCP || UDP)) */
> #define BOND_XMIT_POLICY_LAYER23	2 /* layer 2+3 (IP ^ MAC) */
>+#define BOND_XMIT_POLICY_ENCAP23	3 /* encapsulated layer 2+3 */
>+#define BOND_XMIT_POLICY_ENCAP34	4 /* encapsulated layer 3+4 */
>
> typedef struct ifbond {
> 	__s32 bond_mode;
>-- 
>1.8.1.4
>
>--
>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

^ permalink raw reply

* Re: [PATCH 1/2] net: qmi_wwan: add Telit LE920 newer firmware support
From: Bjørn Mork @ 2013-09-25 16:15 UTC (permalink / raw)
  To: Fabio Porcedda; +Cc: netdev, linux-usb, David S. Miller, Dan Williams
In-Reply-To: <1380100886-16531-1-git-send-email-fabio.porcedda@gmail.com>

Fabio Porcedda <fabio.porcedda@gmail.com> writes:

> Newer firmware use a new pid and a different interface.
>
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  drivers/net/usb/qmi_wwan.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 6312332..5f6b6fa 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -714,6 +714,7 @@ static const struct usb_device_id products[] = {
>  	{QMI_FIXED_INTF(0x2357, 0x0201, 4)},	/* TP-LINK HSUPA Modem MA180 */
>  	{QMI_FIXED_INTF(0x2357, 0x9000, 4)},	/* TP-LINK MA260 */
>  	{QMI_FIXED_INTF(0x1bc7, 0x1200, 5)},	/* Telit LE920 */
> +	{QMI_FIXED_INTF(0x1bc7, 0x1201, 2)},	/* Telit LE920 */
>  	{QMI_FIXED_INTF(0x1e2d, 0x12d1, 4)},	/* Cinterion PLxx */
>  
>  	/* 4. Gobi 1000 devices */

You can add my

Acked-by: Bjørn Mork <bjorn@mork.no>

to this patch if you like.  But I guess you may have to resend it if the
netdev maintainer agree with me regarding the line length patch.


Bjørn

^ permalink raw reply

* Re: [PATCH] net: qmi_wwan: fix Cinterion PLXX product ID
From: Bjørn Mork @ 2013-09-25 16:10 UTC (permalink / raw)
  To: Aleksander Morgado
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, dcbw-H+wXaHxf7aLQT0dZR+AlfA,
	Hans-Christoph Schemmel, Christian Schmiedl, Nicolaus Colberg
In-Reply-To: <1380121356-28601-1-git-send-email-aleksander-bhGbAngMcJvQT0dZR+AlfA@public.gmane.org>

Aleksander Morgado <aleksander-bhGbAngMcJvQT0dZR+AlfA@public.gmane.org> writes:

> Cinterion PLXX LTE devices have a 0x0060 product ID, not 0x12d1.
>
> The blacklisting in the serial/option driver does actually use the correct PID,
> as per commit 8ff10bdb14a52e3f25d4ce09e0582a8684c1a6db ('USB: Blacklisted
> Cinterion's PLxx WWAN Interface').

Right.  This seems like a typo.  Thanks

Acked-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>


Bjørn
--
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

* Re: [PATCH net 2/2] ip_tunnel: Add fallback tunnels to the hash lists
From: Pravin Shelar @ 2013-09-25 16:03 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David Miller, netdev
In-Reply-To: <20130925055519.GW7660@secunet.com>

On Tue, Sep 24, 2013 at 10:55 PM, Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> Currently we can not update the tunnel parameters of
> the fallback tunnels because we don't find them in the
> hash lists. Fix this by adding them on initialization.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
>  net/ipv4/ip_tunnel.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index b8ce640..f2348f2 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -853,8 +853,10 @@ int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
>         /* FB netdevice is special: we have one, and only one per netns.
>          * Allowing to move it to another netns is clearly unsafe.
>          */
> -       if (!IS_ERR(itn->fb_tunnel_dev))
> +       if (!IS_ERR(itn->fb_tunnel_dev)) {
>                 itn->fb_tunnel_dev->features |= NETIF_F_NETNS_LOCAL;
> +               ip_tunnel_add(itn, netdev_priv(itn->fb_tunnel_dev));
> +       }
>         rtnl_unlock();
>
fallback tunnel s not required to be in hash table, Its is returned if
none of hashed tunnels are matched, ref ip_tunnel_lookup().
Can you post command to reproduce this issue?

>         return PTR_RET(itn->fb_tunnel_dev);
> --
> 1.7.9.5
>
> --
> 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

^ permalink raw reply

* traceroute reporting wrong nexthop addr when xfrm is involved
From: Florian Westphal @ 2013-09-25 15:42 UTC (permalink / raw)
  To: netdev

Hi.

When running traceroute to a host behind an ipsec tunnel, the first hop
appears to be the destination host instead of the "real" address, when
"flag icmp" is set on outbound xfrm policy on the 1st hop gateway.

Given

A ---IPSEC ---- B ---<Internal-Network>--- D

D is an arbitrary machine on internal network 10/8, with address
10.128.8.1

A is a client (address: 192.168.20.8), connected to ipsec gateway B with
address 192.168.20.80.  B has "flag icmp" set on its in/fwd/out xfrm policies.

A is configured to encapsulate traffic to either B or the internal network
10/8 in ESP.

B decapsulates packets, (or vv when packets go to A from 10/8 network or B).

This works fine.  However, when running traceroute from
A to D, the very first hop (B) reports the address of D instead:

A $: traceroute -n 10.128.8.1
traceroute to 10.128.8.1 (10.128.8.1), 30 hops max, 40 byte packets using UDP
 1  10.128.8.1 (10.128.8.1)  0.450 ms   0.391 ms   0.357 ms
 2  192.168.10.1 (192.168.10.1)  0.654 ms   0.585 ms   0.642 ms
 3  10.128.128.1 (10.128.128.1)  0.957 ms   0.986 ms   1.410 ms
 4  10.128.254.6 (10.128.254.6)  1.745 ms   1.656 ms   1.240 ms
 5  10.128.8.1 (10.128.8.1)  1.514 ms   1.728 ms   1.495 ms

I tracked this down to icmp.c:icmp_route_lookup()

 447         rt2 = (struct rtable *) xfrm_lookup(net, &rt2->dst,
 448                                             flowi4_to_flowi(&fl4_dec), NULL,
 449                                             XFRM_LOOKUP_ICMP);
 450         if (!IS_ERR(rt2)) {
 451                 dst_release(&rt->dst);
 452                 memcpy(fl4, &fl4_dec, sizeof(*fl4));
 453                 rt = rt2;


fl4 has the correct information, fl4.saddr is the one chosen in
icmp_send() earlier - 192.168.20.80 in my case.

xfrm_lookup() succeds and icmp_route_lookup() commits to using rt2.

In my case, fl4_dec.saddr is 10.128.8.1, the memcpy then copies the new
flowi that will be used for the icmp packet.

Removing the memcpy 'fixes' the problem, but I'm not sure if thats
even correct.

Does anyone know what the purpose of fl4_dec is, or if the behaviour
is the expected one?

Thanks,
Florian

^ permalink raw reply

* [PATCH net-next 1/2] bonding: modify the old and add new xmit hash functions
From: Nikolay Aleksandrov @ 2013-09-25 15:19 UTC (permalink / raw)
  To: netdev; +Cc: davem, andy, fubar, eric.dumazet
In-Reply-To: <1380122398-7370-1-git-send-email-nikolay@redhat.com>

This patch adds two new hash policy modes which use skb_flow_dissect:
3 - Encapsulated layer 2+3
4 - Encapsulated layer 3+4
There should be a good improvement for tunnel users in those modes.
It also changes the old hash functions to:
hash ^= (__force u32)flow.dst ^ (__force u32)flow.src;
hash ^= (hash >> 16);
hash ^= (hash >> 8);

Where hash will be initialized either to L2 hash, that is
SRCMAC[5] XOR DSTMAC[5], or to flow->ports which should be extracted
from the upper layer. Flow's dst and src are also extracted based on the
xmit policy either directly from the buffer or by using skb_flow_dissect,
but in both cases if the protocol is IPv6 then dst and src are obtained by
ipv6_addr_hash() on the real addresses. In case of a non-dissectable
packet, the algorithms fall back to L2 hashing.
The bond_set_mode_ops() function is now obsolete and thus deleted
because it was used only to set the proper hash policy. Also we trim a
pointer from struct bonding because we no longer need to keep the hash
function, now there's only a single hash function - bond_xmit_hash that
works based on bond->params.xmit_policy.

The hash function and skb_flow_dissect were suggested by Eric Dumazet.
The layer names were suggested by Andy Gospodarek, because I suck at
semantics.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
One line is intentionally left at 82 chars since it's the whole function
and IMO looks better that way.

 drivers/net/bonding/bond_3ad.c   |   2 +-
 drivers/net/bonding/bond_main.c  | 211 +++++++++++++++------------------------
 drivers/net/bonding/bond_sysfs.c |   2 -
 drivers/net/bonding/bonding.h    |   3 +-
 include/uapi/linux/if_bonding.h  |   2 +
 5 files changed, 86 insertions(+), 134 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 0d8f427..b3ab703 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2442,7 +2442,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
-	slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
+	slave_agg_no = bond_xmit_hash(bond, skb, slaves_in_agg);
 
 	bond_for_each_slave(bond, slave) {
 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 55bbb8b..73e416b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -78,6 +78,7 @@
 #include <net/netns/generic.h>
 #include <net/pkt_sched.h>
 #include <linux/rculist.h>
+#include <net/flow_keys.h>
 #include "bonding.h"
 #include "bond_3ad.h"
 #include "bond_alb.h"
@@ -159,7 +160,8 @@ MODULE_PARM_DESC(min_links, "Minimum number of available links before turning on
 module_param(xmit_hash_policy, charp, 0);
 MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; "
 				   "0 for layer 2 (default), 1 for layer 3+4, "
-				   "2 for layer 2+3");
+				   "2 for layer 2+3, 3 for encap layer 2+3, "
+				   "4 for encap layer 3+4");
 module_param(arp_interval, int, 0);
 MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
 module_param_array(arp_ip_target, charp, NULL, 0);
@@ -217,6 +219,8 @@ const struct bond_parm_tbl xmit_hashtype_tbl[] = {
 {	"layer2",		BOND_XMIT_POLICY_LAYER2},
 {	"layer3+4",		BOND_XMIT_POLICY_LAYER34},
 {	"layer2+3",		BOND_XMIT_POLICY_LAYER23},
+{	"encap2+3",		BOND_XMIT_POLICY_ENCAP23},
+{	"encap3+4",		BOND_XMIT_POLICY_ENCAP34},
 {	NULL,			-1},
 };
 
@@ -3026,99 +3030,99 @@ static struct notifier_block bond_netdev_notifier = {
 
 /*---------------------------- Hashing Policies -----------------------------*/
 
-/*
- * Hash for the output device based upon layer 2 data
- */
-static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count)
+/* L2 hash helper */
+static inline u32 bond_eth_hash(struct sk_buff *skb)
 {
 	struct ethhdr *data = (struct ethhdr *)skb->data;
 
 	if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto))
-		return (data->h_dest[5] ^ data->h_source[5]) % count;
+		return data->h_dest[5] ^ data->h_source[5];
 
 	return 0;
 }
 
-/*
- * Hash for the output device based upon layer 2 and layer 3 data. If
- * the packet is not IP, fall back on bond_xmit_hash_policy_l2()
- */
-static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
+static void bond_flow_get_ports(struct sk_buff *skb, struct flow_keys *fk,
+				int offset)
+{
+	__be32 *ports;
+
+	ports = skb_header_pointer(skb, offset, sizeof(fk->ports), &fk->ports);
+	if (ports)
+		fk->ports = *ports;
+}
+
+/* Extract the appropriate headers based on bond's xmit policy */
+static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
+			      struct flow_keys *fk)
 {
-	const struct ethhdr *data;
+	const struct ipv6hdr *iph6;
 	const struct iphdr *iph;
-	const struct ipv6hdr *ipv6h;
-	u32 v6hash;
-	const __be32 *s, *d;
+	int noff, proto = -1, poff = -1;
+	bool ret = false;
+
+	if (bond->params.xmit_policy > BOND_XMIT_POLICY_LAYER23)
+		return skb_flow_dissect(skb, fk);
 
-	if (skb->protocol == htons(ETH_P_IP) &&
-	    pskb_network_may_pull(skb, sizeof(*iph))) {
+	fk->ports = 0;
+	noff = skb_network_offset(skb);
+	if (skb->protocol == htons(ETH_P_IP)) {
+		if (!pskb_may_pull(skb, noff + sizeof(*iph)))
+			goto out;
 		iph = ip_hdr(skb);
-		data = (struct ethhdr *)skb->data;
-		return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
-			(data->h_dest[5] ^ data->h_source[5])) % count;
-	} else if (skb->protocol == htons(ETH_P_IPV6) &&
-		   pskb_network_may_pull(skb, sizeof(*ipv6h))) {
-		ipv6h = ipv6_hdr(skb);
-		data = (struct ethhdr *)skb->data;
-		s = &ipv6h->saddr.s6_addr32[0];
-		d = &ipv6h->daddr.s6_addr32[0];
-		v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
-		v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8);
-		return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
-	}
-
-	return bond_xmit_hash_policy_l2(skb, count);
+		fk->src = iph->saddr;
+		fk->dst = iph->daddr;
+		noff += iph->ihl << 2;
+		if (!ip_is_fragment(iph))
+			proto = iph->protocol;
+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
+		if (!pskb_may_pull(skb, noff + sizeof(*iph6)))
+			goto out;
+		iph6 = ipv6_hdr(skb);
+		fk->src = (__force __be32)ipv6_addr_hash(&iph6->saddr);
+		fk->dst = (__force __be32)ipv6_addr_hash(&iph6->daddr);
+		noff += sizeof(*iph6);
+		proto = iph6->nexthdr;
+	}
+	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER34) {
+		poff = proto_ports_offset(proto);
+		if (poff >= 0)
+			bond_flow_get_ports(skb, fk, poff + noff);
+	}
+	ret = true;
+
+out:
+	return ret;
 }
 
-/*
- * Hash for the output device based upon layer 3 and layer 4 data. If
- * the packet is a frag or not TCP or UDP, just use layer 3 data.  If it is
- * altogether not IP, fall back on bond_xmit_hash_policy_l2()
+/**
+ * bond_xmit_hash - generate a hash value based on the xmit policy
+ * @bond: bonding device
+ * @skb: buffer to use for headers
+ * @count: modulo value
+ *
+ * This function will extract the necessary headers from the skb buffer and use
+ * them to generate a hash based on the xmit_policy set in the bonding device
+ * which will be reduced modulo count before returning.
  */
-static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
+int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
 {
-	u32 layer4_xor = 0;
-	const struct iphdr *iph;
-	const struct ipv6hdr *ipv6h;
-	const __be32 *s, *d;
-	const __be16 *l4 = NULL;
-	__be16 _l4[2];
-	int noff = skb_network_offset(skb);
-	int poff;
-
-	if (skb->protocol == htons(ETH_P_IP) &&
-	    pskb_may_pull(skb, noff + sizeof(*iph))) {
-		iph = ip_hdr(skb);
-		poff = proto_ports_offset(iph->protocol);
+	struct flow_keys flow;
+	u32 hash;
 
-		if (!ip_is_fragment(iph) && poff >= 0) {
-			l4 = skb_header_pointer(skb, noff + (iph->ihl << 2) + poff,
-						sizeof(_l4), &_l4);
-			if (l4)
-				layer4_xor = ntohs(l4[0] ^ l4[1]);
-		}
-		return (layer4_xor ^
-			((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
-	} else if (skb->protocol == htons(ETH_P_IPV6) &&
-		   pskb_may_pull(skb, noff + sizeof(*ipv6h))) {
-		ipv6h = ipv6_hdr(skb);
-		poff = proto_ports_offset(ipv6h->nexthdr);
-		if (poff >= 0) {
-			l4 = skb_header_pointer(skb, noff + sizeof(*ipv6h) + poff,
-						sizeof(_l4), &_l4);
-			if (l4)
-				layer4_xor = ntohs(l4[0] ^ l4[1]);
-		}
-		s = &ipv6h->saddr.s6_addr32[0];
-		d = &ipv6h->daddr.s6_addr32[0];
-		layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]);
-		layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^
-			       (layer4_xor >> 8);
-		return layer4_xor % count;
-	}
+	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
+	    !bond_flow_dissect(bond, skb, &flow))
+		return bond_eth_hash(skb) % count;
+
+	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 ||
+	    bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23)
+		hash = bond_eth_hash(skb);
+	else
+		hash = (__force u32)flow.ports;
+	hash ^= (__force u32)flow.dst ^ (__force u32)flow.src;
+	hash ^= (hash >> 16);
+	hash ^= (hash >> 8);
 
-	return bond_xmit_hash_policy_l2(skb, count);
+	return hash % count;
 }
 
 /*-------------------------- Device entry points ----------------------------*/
@@ -3700,8 +3704,7 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
 	return NETDEV_TX_OK;
 }
 
-/*
- * In bond_xmit_xor() , we determine the output device by using a pre-
+/* In bond_xmit_xor() , we determine the output device by using a pre-
  * determined xmit_hash_policy(), If the selected device is not enabled,
  * find the next active slave.
  */
@@ -3709,8 +3712,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 
-	bond_xmit_slave_id(bond, skb,
-			   bond->xmit_hash_policy(skb, bond->slave_cnt));
+	bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb, bond->slave_cnt));
 
 	return NETDEV_TX_OK;
 }
@@ -3746,22 +3748,6 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
 
 /*------------------------- Device initialization ---------------------------*/
 
-static void bond_set_xmit_hash_policy(struct bonding *bond)
-{
-	switch (bond->params.xmit_policy) {
-	case BOND_XMIT_POLICY_LAYER23:
-		bond->xmit_hash_policy = bond_xmit_hash_policy_l23;
-		break;
-	case BOND_XMIT_POLICY_LAYER34:
-		bond->xmit_hash_policy = bond_xmit_hash_policy_l34;
-		break;
-	case BOND_XMIT_POLICY_LAYER2:
-	default:
-		bond->xmit_hash_policy = bond_xmit_hash_policy_l2;
-		break;
-	}
-}
-
 /*
  * Lookup the slave that corresponds to a qid
  */
@@ -3871,38 +3857,6 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return ret;
 }
 
-/*
- * set bond mode specific net device operations
- */
-void bond_set_mode_ops(struct bonding *bond, int mode)
-{
-	struct net_device *bond_dev = bond->dev;
-
-	switch (mode) {
-	case BOND_MODE_ROUNDROBIN:
-		break;
-	case BOND_MODE_ACTIVEBACKUP:
-		break;
-	case BOND_MODE_XOR:
-		bond_set_xmit_hash_policy(bond);
-		break;
-	case BOND_MODE_BROADCAST:
-		break;
-	case BOND_MODE_8023AD:
-		bond_set_xmit_hash_policy(bond);
-		break;
-	case BOND_MODE_ALB:
-		/* FALLTHRU */
-	case BOND_MODE_TLB:
-		break;
-	default:
-		/* Should never happen, mode already checked */
-		pr_err("%s: Error: Unknown bonding mode %d\n",
-		       bond_dev->name, mode);
-		break;
-	}
-}
-
 static int bond_ethtool_get_settings(struct net_device *bond_dev,
 				     struct ethtool_cmd *ecmd)
 {
@@ -4004,7 +3958,6 @@ static void bond_setup(struct net_device *bond_dev)
 	ether_setup(bond_dev);
 	bond_dev->netdev_ops = &bond_netdev_ops;
 	bond_dev->ethtool_ops = &bond_ethtool_ops;
-	bond_set_mode_ops(bond, bond->params.mode);
 
 	bond_dev->destructor = bond_destructor;
 
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index c29b836..dba3b9b 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -352,7 +352,6 @@ static ssize_t bonding_store_mode(struct device *d,
 	/* don't cache arp_validate between modes */
 	bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
 	bond->params.mode = new_value;
-	bond_set_mode_ops(bond, bond->params.mode);
 	pr_info("%s: setting mode to %s (%d).\n",
 		bond->dev->name, bond_mode_tbl[new_value].modename,
 		new_value);
@@ -392,7 +391,6 @@ static ssize_t bonding_store_xmit_hash(struct device *d,
 		ret = -EINVAL;
 	} else {
 		bond->params.xmit_policy = new_value;
-		bond_set_mode_ops(bond, bond->params.mode);
 		pr_info("%s: setting xmit hash policy to %s (%d).\n",
 			bond->dev->name,
 			xmit_hashtype_tbl[new_value].modename, new_value);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 03cf3fd..4db9ec4 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -245,7 +245,6 @@ struct bonding {
 	char     proc_file_name[IFNAMSIZ];
 #endif /* CONFIG_PROC_FS */
 	struct   list_head bond_list;
-	int      (*xmit_hash_policy)(struct sk_buff *, int);
 	u16      rr_tx_counter;
 	struct   ad_bond_info ad_info;
 	struct   alb_bond_info alb_info;
@@ -446,7 +445,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
 void bond_mii_monitor(struct work_struct *);
 void bond_loadbalance_arp_mon(struct work_struct *);
 void bond_activebackup_arp_mon(struct work_struct *);
-void bond_set_mode_ops(struct bonding *bond, int mode);
+int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count);
 int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl);
 void bond_select_active_slave(struct bonding *bond);
 void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h
index a17edda..9635a62 100644
--- a/include/uapi/linux/if_bonding.h
+++ b/include/uapi/linux/if_bonding.h
@@ -91,6 +91,8 @@
 #define BOND_XMIT_POLICY_LAYER2		0 /* layer 2 (MAC only), default */
 #define BOND_XMIT_POLICY_LAYER34	1 /* layer 3+4 (IP ^ (TCP || UDP)) */
 #define BOND_XMIT_POLICY_LAYER23	2 /* layer 2+3 (IP ^ MAC) */
+#define BOND_XMIT_POLICY_ENCAP23	3 /* encapsulated layer 2+3 */
+#define BOND_XMIT_POLICY_ENCAP34	4 /* encapsulated layer 3+4 */
 
 typedef struct ifbond {
 	__s32 bond_mode;
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 2/2] bonding: document the new xmit policy modes and update the changed ones
From: Nikolay Aleksandrov @ 2013-09-25 15:19 UTC (permalink / raw)
  To: netdev; +Cc: davem, andy, fubar, eric.dumazet
In-Reply-To: <1380122398-7370-1-git-send-email-nikolay@redhat.com>

Add new documentation for encap2+3 and encap3+4, also update the formula
for the old modes due to the changes.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
I'm not good at writing documentation so any suggestions on how to improve
this text are very welcome.

 Documentation/networking/bonding.txt | 66 ++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 9b28e71..3856ed2 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -743,21 +743,16 @@ xmit_hash_policy
 		protocol information to generate the hash.
 
 		Uses XOR of hardware MAC addresses and IP addresses to
-		generate the hash.  The IPv4 formula is
+		generate the hash.  The formula is
 
-		(((source IP XOR dest IP) AND 0xffff) XOR
-			( source MAC XOR destination MAC ))
-				modulo slave count
+		hash = source MAC XOR destination MAC
+		hash = hash XOR source IP XOR destination IP
+		hash = hash XOR (hash RSHIFT 16)
+		hash = hash XOR (hash RSHIFT 8)
+		And then hash is reduced modulo slave count.
 
-		The IPv6 formula is
-
-		hash = (source ip quad 2 XOR dest IP quad 2) XOR
-		       (source ip quad 3 XOR dest IP quad 3) XOR
-		       (source ip quad 4 XOR dest IP quad 4)
-
-		(((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
-			XOR (source MAC XOR destination MAC))
-				modulo slave count
+		If the protocol is IPv6 then the source and destination
+		addresses are first hashed using ipv6_addr_hash.
 
 		This algorithm will place all traffic to a particular
 		network peer on the same slave.  For non-IP traffic,
@@ -779,21 +774,16 @@ xmit_hash_policy
 		slaves, although a single connection will not span
 		multiple slaves.
 
-		The formula for unfragmented IPv4 TCP and UDP packets is
-
-		((source port XOR dest port) XOR
-			 ((source IP XOR dest IP) AND 0xffff)
-				modulo slave count
+		The formula for unfragmented TCP and UDP packets is
 
-		The formula for unfragmented IPv6 TCP and UDP packets is
+		hash = source port, destination port (as in the header)
+		hash = hash XOR source IP XOR destination IP
+		hash = hash XOR (hash RSHIFT 16)
+		hash = hash XOR (hash RSHIFT 8)
+		And then hash is reduced modulo slave count.
 
-		hash = (source port XOR dest port) XOR
-		       ((source ip quad 2 XOR dest IP quad 2) XOR
-			(source ip quad 3 XOR dest IP quad 3) XOR
-			(source ip quad 4 XOR dest IP quad 4))
-
-		((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
-			modulo slave count
+		If the protocol is IPv6 then the source and destination
+		addresses are first hashed using ipv6_addr_hash.
 
 		For fragmented TCP or UDP packets and all other IPv4 and
 		IPv6 protocol traffic, the source and destination port
@@ -801,10 +791,6 @@ xmit_hash_policy
 		formula is the same as for the layer2 transmit hash
 		policy.
 
-		The IPv4 policy is intended to mimic the behavior of
-		certain switches, notably Cisco switches with PFC2 as
-		well as some Foundry and IBM products.
-
 		This algorithm is not fully 802.3ad compliant.  A
 		single TCP or UDP conversation containing both
 		fragmented and unfragmented packets will see packets
@@ -815,6 +801,26 @@ xmit_hash_policy
 		conversations.  Other implementations of 802.3ad may
 		or may not tolerate this noncompliance.
 
+	encap2+3
+
+		This policy uses the same formula as layer2+3 but it
+		relies on skb_flow_dissect to obtain the header fields
+		which might result in the use of inner headers if an
+		encapsulation protocol is used. For example this will
+		improve the performance for tunnel users because the
+		packets will be distributed according to the encapsulated
+		flows.
+
+	encap3+4
+
+		This policy uses the same formula as layer3+4 but it
+		relies on skb_flow_dissect to obtain the header fields
+		which might result in the use of inner headers if an
+		encapsulation protocol is used. For example this will
+		improve the performance for tunnel users because the
+		packets will be distributed according to the encapsulated
+		flows.
+
 	The default value is layer2.  This option was added in bonding
 	version 2.6.3.  In earlier versions of bonding, this parameter
 	does not exist, and the layer2 policy is the only policy.  The
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 0/2] bonding: modify the current and add new hash functions
From: Nikolay Aleksandrov @ 2013-09-25 15:19 UTC (permalink / raw)
  To: netdev; +Cc: davem, andy, fubar, eric.dumazet

Hi all,
This is a complete remake of my old patch that modified the bonding hash
functions to use skb_flow_dissect which was suggested by Eric Dumazet.
This time around I've left the old modes although using a new hash function
again suggested by Eric, which is the same for all modes. The only
difference is the way the headers are obtained. The old modes obtain them
as before in order to address concerns about speed, but the 2 new ones use
skb_flow_dissect. The unification of the hash function allows to remove a
pointer from struct bonding and also a few extra functions that dealt with
it. Two new functions are added which take care of the hashing based on
bond->params.xmit_policy only:
bond_xmit_hash() - global function, used by XOR and 3ad modes
bond_flow_dissect() - used by bond_xmit_hash() to obtain the necessary
headers and combine them according to bond->params.xmit_policy.


Best regards,
 Nikolay Aleksandrov

Nikolay Aleksandrov (2):
  bonding: modify the old and add new xmit hash functions
  bonding: document the new xmit policy modes and update the changed
    ones

 Documentation/networking/bonding.txt |  66 ++++++-----
 drivers/net/bonding/bond_3ad.c       |   2 +-
 drivers/net/bonding/bond_main.c      | 211 ++++++++++++++---------------------
 drivers/net/bonding/bond_sysfs.c     |   2 -
 drivers/net/bonding/bonding.h        |   3 +-
 include/uapi/linux/if_bonding.h      |   2 +
 6 files changed, 122 insertions(+), 164 deletions(-)

-- 
1.8.1.4

^ permalink raw reply

* [PATCH] net: qmi_wwan: fix Cinterion PLXX product ID
From: Aleksander Morgado @ 2013-09-25 15:02 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: davem, bjorn, dcbw, Aleksander Morgado, Hans-Christoph Schemmel,
	Christian Schmiedl, Nicolaus Colberg

Cinterion PLXX LTE devices have a 0x0060 product ID, not 0x12d1.

The blacklisting in the serial/option driver does actually use the correct PID,
as per commit 8ff10bdb14a52e3f25d4ce09e0582a8684c1a6db ('USB: Blacklisted
Cinterion's PLxx WWAN Interface').

CC: Hans-Christoph Schemmel <hans-christoph.schemmel@gemalto.com>
CC: Christian Schmiedl <christian.schmiedl@gemalto.com>
CC: Nicolaus Colberg <nicolaus.colberg@gemalto.com>
Signed-off-by: Aleksander Morgado <aleksander@lanedo.com>
---
 drivers/net/usb/qmi_wwan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6312332..3d6aaf7 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -714,7 +714,7 @@ static const struct usb_device_id products[] = {
 	{QMI_FIXED_INTF(0x2357, 0x0201, 4)},	/* TP-LINK HSUPA Modem MA180 */
 	{QMI_FIXED_INTF(0x2357, 0x9000, 4)},	/* TP-LINK MA260 */
 	{QMI_FIXED_INTF(0x1bc7, 0x1200, 5)},	/* Telit LE920 */
-	{QMI_FIXED_INTF(0x1e2d, 0x12d1, 4)},	/* Cinterion PLxx */
+	{QMI_FIXED_INTF(0x1e2d, 0x0060, 4)},	/* Cinterion PLxx */
 
 	/* 4. Gobi 1000 devices */
 	{QMI_GOBI1K_DEVICE(0x05c6, 0x9212)},	/* Acer Gobi Modem Device */
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH RFC] random: introduce get_random_bytes_busy_wait_initialized
From: Hannes Frederic Sowa @ 2013-09-25 13:35 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Tom Herbert, davem, netdev, jesse.brandeburg, tytso, linux-kernel
In-Reply-To: <1380110810.3165.140.camel@edumazet-glaptop>

On Wed, Sep 25, 2013 at 05:06:50AM -0700, Eric Dumazet wrote:
> On Wed, 2013-09-25 at 11:00 +0200, Hannes Frederic Sowa wrote:
> 
> >  /*
> > + * Busy loop until the nonblocking_pool is intialized and return
> > + * random data in buf of size nbytes.
> > + *
> > + * This is used by the network stack to defer the extraction of
> > + * entropy from the nonblocking_pool until the pool is initialized.
> > + *
> > + * We need to busy loop here, because we could be called from an
> > + * atomic section.
> > + */
> > +void get_random_bytes_busy_wait_initialized(void *buf, int nbytes)
> > +{
> > +	while (!nonblocking_pool.initialized)
> > +		cpu_relax();
> > +	get_random_bytes(buf, nbytes);
> > +}
> 
> No idea if this can work if called from IRQ context.
> 
> How is nonblocking_poll initialized if host has a single cpu ?

We increase the entropy_count and can initialize the random pool from
handle_irq_event.

We can document that it should not get called from irq context and
maybe add a WARN_ON(irqs_disabled())?

^ permalink raw reply

* Re: [PATCH 2/2] net: qmi_wwan: fix checkpatch warnings
From: Bjørn Mork @ 2013-09-25 13:31 UTC (permalink / raw)
  To: Fabio Porcedda
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	David S. Miller, Dan Williams
In-Reply-To: <1380100886-16531-2-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Sorry, I really don't see the point of this.  Yes, the lines are longer
than 80 columns, but breaking them don't improve the readability at
all.  On the contrary, IMHO.

So NAK from me for this part.


Bjørn

Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/net/usb/qmi_wwan.c | 56 +++++++++++++++++++++++++++++-----------------
>  1 file changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 5f6b6fa..0e59f9e 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -143,16 +143,22 @@ static const struct net_device_ops qmi_wwan_netdev_ops = {
>  	.ndo_validate_addr	= eth_validate_addr,
>  };
>  
> -/* using a counter to merge subdriver requests with our own into a combined state */
> +/* using a counter to merge subdriver requests with our own into a
> + * combined state
> + */
>  static int qmi_wwan_manage_power(struct usbnet *dev, int on)
>  {
>  	struct qmi_wwan_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);
> +	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 */
> +	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;
> @@ -199,7 +205,8 @@ static int qmi_wwan_register_subdriver(struct usbnet *dev)
>  	atomic_set(&info->pmcount, 0);
>  
>  	/* register subdriver */
> -	subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc, 4096, &qmi_wwan_cdc_wdm_manage_power);
> +	subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc,
> +					 4096, &qmi_wwan_cdc_wdm_manage_power);
>  	if (IS_ERR(subdriver)) {
>  		dev_err(&info->control->dev, "subdriver registration failed\n");
>  		rv = PTR_ERR(subdriver);
> @@ -228,7 +235,8 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>  	struct usb_driver *driver = driver_of(intf);
>  	struct qmi_wwan_state *info = (void *)&dev->data;
>  
> -	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));
> +	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) <
> +		      sizeof(struct qmi_wwan_state)));
>  
>  	/* set up initial state */
>  	info->control = intf;
> @@ -250,7 +258,8 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>  				goto err;
>  			}
>  			if (h->bLength != sizeof(struct usb_cdc_header_desc)) {
> -				dev_dbg(&intf->dev, "CDC header len %u\n", h->bLength);
> +				dev_dbg(&intf->dev, "CDC header len %u\n",
> +					h->bLength);
>  				goto err;
>  			}
>  			break;
> @@ -260,7 +269,8 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>  				goto err;
>  			}
>  			if (h->bLength != sizeof(struct usb_cdc_union_desc)) {
> -				dev_dbg(&intf->dev, "CDC union len %u\n", h->bLength);
> +				dev_dbg(&intf->dev, "CDC union len %u\n",
> +					h->bLength);
>  				goto err;
>  			}
>  			cdc_union = (struct usb_cdc_union_desc *)buf;
> @@ -271,15 +281,15 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>  				goto err;
>  			}
>  			if (h->bLength != sizeof(struct usb_cdc_ether_desc)) {
> -				dev_dbg(&intf->dev, "CDC ether len %u\n",  h->bLength);
> +				dev_dbg(&intf->dev, "CDC ether len %u\n",
> +					h->bLength);
>  				goto err;
>  			}
>  			cdc_ether = (struct usb_cdc_ether_desc *)buf;
>  			break;
>  		}
>  
> -		/*
> -		 * Remember which CDC functional descriptors we've seen.  Works
> +		/* Remember which CDC functional descriptors we've seen.  Works
>  		 * for all types we care about, of which USB_CDC_ETHERNET_TYPE
>  		 * (0x0f) is the highest numbered
>  		 */
> @@ -293,10 +303,14 @@ next_desc:
>  
>  	/* 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);
> +		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;
>  		}
>  	}
> @@ -374,8 +388,7 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
>  	struct qmi_wwan_state *info = (void *)&dev->data;
>  	int ret;
>  
> -	/*
> -	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
> +	/* Both usbnet_suspend() and subdriver->suspend() MUST return 0
>  	 * in system sleep context, otherwise, the resume callback has
>  	 * to recover device from previous suspend failure.
>  	 */
> @@ -383,7 +396,8 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
>  	if (ret < 0)
>  		goto err;
>  
> -	if (intf == info->control && info->subdriver && info->subdriver->suspend)
> +	if (intf == info->control && info->subdriver &&
> +	    info->subdriver->suspend)
>  		ret = info->subdriver->suspend(intf, message);
>  	if (ret < 0)
>  		usbnet_resume(intf);
> @@ -396,7 +410,8 @@ static int qmi_wwan_resume(struct usb_interface *intf)
>  	struct usbnet *dev = usb_get_intfdata(intf);
>  	struct qmi_wwan_state *info = (void *)&dev->data;
>  	int ret = 0;
> -	bool callsub = (intf == info->control && info->subdriver && info->subdriver->resume);
> +	bool callsub = (intf == info->control && info->subdriver &&
> +			info->subdriver->resume);
>  
>  	if (callsub)
>  		ret = info->subdriver->resume(intf);
> @@ -777,7 +792,8 @@ static const struct usb_device_id products[] = {
>  };
>  MODULE_DEVICE_TABLE(usb, products);
>  
> -static int qmi_wwan_probe(struct usb_interface *intf, const struct usb_device_id *prod)
> +static int qmi_wwan_probe(struct usb_interface *intf,
> +			  const struct usb_device_id *prod)
>  {
>  	struct usb_device_id *id = (struct usb_device_id *)prod;
--
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

* Re: [PATCH net 1/2] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit
From: Eric Dumazet @ 2013-09-25 12:44 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David Miller, netdev
In-Reply-To: <20130925121337.GZ7660@secunet.com>

On Wed, 2013-09-25 at 14:13 +0200, Steffen Klassert wrote:
> It was commit c544193214 (GRE: Refactor GRE tunneling code.)
> when net/ipv4/ip_tunnel.c was created.

Yeah. This one begins to be very upsetting.

> 
> > 
> > (commit id and title in your changelog would be really nice)
> > 
> 
> I can send a v2 with these informations included if you want
> that.

I think that patches coming from experimented kernel developers
should always include a study of bug origin.

Otherwise, both maintainer and stable teams have to figure it, and
they sometime lack the time or context.

Plus it's good to CC patch author and reviewers so that he can learn of
its mistakes, and Ack your work as well.

Thanks

^ permalink raw reply

* Re: [PATCH net 1/2] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit
From: Steffen Klassert @ 2013-09-25 12:13 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1380110163.3165.138.camel@edumazet-glaptop>

On Wed, Sep 25, 2013 at 04:56:03AM -0700, Eric Dumazet wrote:
> On Wed, 2013-09-25 at 07:54 +0200, Steffen Klassert wrote:
> > We might extend the used aera of a skb beyond the total
> > headroom when we install the ipip header. Fix this by
> > calling skb_cow_head() unconditionally.
> > 
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > ---
> >  net/ipv4/ip_tunnel.c |   12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> Do you know or can we find when was the bug introduced ?

It was commit c544193214 (GRE: Refactor GRE tunneling code.)
when net/ipv4/ip_tunnel.c was created.

> 
> (commit id and title in your changelog would be really nice)
> 

I can send a v2 with these informations included if you want
that.

^ permalink raw reply

* Re: [PATCH RFC] random: introduce get_random_bytes_busy_wait_initialized
From: Eric Dumazet @ 2013-09-25 12:06 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: Tom Herbert, davem, netdev, jesse.brandeburg, tytso, linux-kernel
In-Reply-To: <20130925090034.GC4904@order.stressinduktion.org>

On Wed, 2013-09-25 at 11:00 +0200, Hannes Frederic Sowa wrote:

>  /*
> + * Busy loop until the nonblocking_pool is intialized and return
> + * random data in buf of size nbytes.
> + *
> + * This is used by the network stack to defer the extraction of
> + * entropy from the nonblocking_pool until the pool is initialized.
> + *
> + * We need to busy loop here, because we could be called from an
> + * atomic section.
> + */
> +void get_random_bytes_busy_wait_initialized(void *buf, int nbytes)
> +{
> +	while (!nonblocking_pool.initialized)
> +		cpu_relax();
> +	get_random_bytes(buf, nbytes);
> +}

No idea if this can work if called from IRQ context.

How is nonblocking_poll initialized if host has a single cpu ?

^ 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