Netdev List
 help / color / mirror / Atom feed
* Re: /proc/net/sockstat invalid memory accounting or memory leak in latest kernels? (trying to debug)
From: Denys Fedoryshchenko @ 2014-11-16  8:54 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Neal Cardwell, Yuchung Cheng, netdev
In-Reply-To: <1415813254.17262.7.camel@edumazet-glaptop2.roam.corp.google.com>

As latest findings, when servers are going crazy because of tcp memory 
invalid accounting.
First of all i upgraded kernel to latest version 3.17.3 and added also 
patch from upcoming kernel,
"12) Don't call sock_kfree_s() with NULL pointers, this function also 
has the side effect of adjusting
the socket memory usage.  From Cong Wang.", but it didnt helped.

I added printk_ratelimited to places where suspicious values might 
appear, and got some more information.
First, is not very suspicious, no idea if it is a problem:
[ 1413.031622] sk ffff8817184d8680 sk_mem_charge negative -10752 by 4352
[ 1413.032027] sk ffff8817184d8680 sk_mem_charge negative -15104 by 4352
[ 1415.768465] sk ffff881666842d80 sk_mem_charge negative -9984 by 4352
[ 1415.768868] sk ffff881666842d80 sk_mem_charge negative -14336 by 4352
[ 1415.769268] sk ffff881666842d80 sk_mem_charge negative -18688 by 4352
[ 1415.769681] sk ffff881666842d80 sk_mem_charge negative -9088 by 4352
[ 1418.933799] sk ffff8816dd640000 sk_mem_charge negative -9984 by 4352
[ 1418.934205] sk ffff8816dd640000 sk_mem_charge negative -14336 by 4352
[ 1418.934604] sk ffff8816dd640000 sk_mem_charge negative -18688 by 4352
[ 1427.131310] sk ffff881731801a00 sk_mem_charge negative -11776 by 4352
[ 1428.564640] sk ffff881731801a00 sk_mem_charge negative -11008 by 4352
[ 1429.134279] sk ffff881731801a00 sk_mem_charge negative -11776 by 4352
[ 1429.134691] sk ffff881731801a00 sk_mem_charge negative -16128 by 4352
[ 1430.666541] sk ffff881731801a00 sk_mem_charge negative -10496 by 4352
[ 1431.395099] sk ffff881731801a00 sk_mem_charge negative -12032 by 4352
[ 1431.395506] sk ffff881731801a00 sk_mem_charge negative -16384 by 4352
[ 1431.877862] sk ffff881731801a00 sk_mem_charge negative -11648 by 4352
Second is always linked with crashes, it is sk_mem_uncharge and 
sk_forward_alloc goes negative. Patch to show message
for sk_mem_uncharge in sock.h is very simple:

  static inline void sk_mem_uncharge(struct sock *sk, int size)
@@ -1480,6 +1485,8 @@
         if (!sk_has_account(sk))
                 return;
         sk->sk_forward_alloc += size;
+       if (sk->sk_forward_alloc < -8192)
+           printk_ratelimited(KERN_WARNING"sk %p sk_mem_uncharge 
negative %d by %d\n", sk, sk->sk_forward_alloc, size);
  }


This is what i am usually setting before box are rebooted:
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.363437] sk ffff88155e904100 
sk_mem_uncharge negative -2147482496 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.363837] sk ffff88155e904100 
sk_mem_uncharge negative -2147480192 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.364232] sk ffff88155e904100 
sk_mem_uncharge negative -2147477888 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.364627] sk ffff88155e904100 
sk_mem_uncharge negative -2147475584 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.365022] sk ffff88155e904100 
sk_mem_uncharge negative -2147473280 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.365416] sk ffff88155e904100 
sk_mem_uncharge negative -2147470976 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.365811] sk ffff88155e904100 
sk_mem_uncharge negative -2147468672 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.366214] sk ffff88155e904100 
sk_mem_uncharge negative -2147466368 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.366611] sk ffff88155e904100 
sk_mem_uncharge negative -2147464064 by 2304
Nov 15 13:20:53 g1 user.warn kernel: [ 2002.367004] sk ffff88155e904100 
sk_mem_uncharge negative -2147461760 by 2304
Nov 15 13:20:58 g1 user.warn kernel: [ 2007.363601] sk_mem_uncharge: 
576170 callbacks suppressed
Nov 15 13:20:58 g1 user.warn kernel: [ 2007.364001] sk ffff88155e904100 
sk_mem_uncharge negative -1517794432 by 2304
.....
until
...
Nov 15 13:31:58 g1 user.warn kernel: [ 2666.393667] sk ffff88170141ad80 
sk_mem_uncharge negative -169088 by 2944
Nov 15 13:31:58 g1 user.warn kernel: [ 2666.394064] sk ffff88170141ad80 
sk_mem_uncharge negative -166144 by 2944
Nov 15 13:31:58 g1 user.warn kernel: [ 2666.394472] sk ffff88170141ad80 
sk_mem_uncharge negative -163200 by 2944
Nov 15 13:31:58 g1 user.warn kernel: [ 2666.394871] sk ffff88170141ad80 
sk_mem_uncharge negative -158208 by 4992
<reboot>

Or:
Nov 15 14:55:01 g1 user.warn kernel: [ 1965.758129] sk ffff8815f8014e00 
sk_mem_uncharge negative -2147483520 by 2304
Nov 15 14:55:01 g1 user.warn kernel: [ 1965.758536] sk ffff8815f8014e00 
sk_mem_uncharge negative -2147481216 by 2304
Nov 15 14:55:01 g1 user.warn kernel: [ 1965.758935] sk ffff8815f8014e00 
sk_mem_uncharge negative -2147478912 by 2304
Nov 15 14:55:01 g1 user.warn kernel: [ 1965.759332] sk ffff8815f8014e00 
sk_mem_uncharge negative -2147476608 by 2304
Nov 15 14:55:01 g1 user.warn kernel: [ 1965.759728] sk ffff8815f8014e00 
sk_mem_uncharge negative -2147474304 by 2304
Nov 15 14:55:01 g1 user.warn kernel: [ 1965.760124] sk ffff8815f8014e00 
sk_mem_uncharge negative -2147472000 by 2304
Nov 15 14:55:01 g1 user.warn kernel: [ 1965.760518] sk ffff8815f8014e00 
sk_mem_uncharge negative -2147469696 by 2304
...
Nov 15 15:03:27 g1 user.warn kernel: [ 2472.170857] sk ffff88163f429a00 
sk_mem_uncharge negative -277169024 by 896
Nov 15 15:03:30 g1 user.warn kernel: [ 2474.611109] sk ffff88163f429a00 
sk_mem_uncharge negative -277164928 by 896
Nov 15 15:03:30 g1 user.warn kernel: [ 2474.611511] sk ffff88163f429a00 
sk_mem_uncharge negative -277164032 by 896
Nov 15 15:03:30 g1 user.warn kernel: [ 2474.706237] sk ffff88163f429a00 
sk_mem_uncharge negative -277159936 by 2304
Nov 15 15:03:34 g1 user.warn kernel: [ 2478.889110] sk ffff88163f429a00 
sk_mem_uncharge negative -277155840 by 2304
Nov 15 15:03:34 g1 user.warn kernel: [ 2478.911516] sk ffff88163f429a00 
sk_mem_uncharge negative -277147648 by 1536
Nov 15 15:03:35 g1 user.warn kernel: [ 2479.320851] sk ffff88163f429a00 
sk_mem_uncharge negative -277143552 by 2304
Nov 15 15:04:33 g1 user.warn kernel: [ 2537.408184] sk ffff88163f429a00 
sk_mem_uncharge negative -277139456 by 2304
Nov 15 15:04:33 g1 user.warn kernel: [ 2537.409790] sk ffff88163f429a00 
sk_mem_uncharge negative -277135360 by 896
(here i can see two sk at same time are like this)


Or

Nov 15 23:37:19 g1 user.warn kernel: [14137.049570] sk ffff8816b3312700 
sk_mem_uncharge negative -2147481856 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.049973] sk ffff8816b3312700 
sk_mem_uncharge negative -2147479552 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.050372] sk ffff8816b3312700 
sk_mem_uncharge negative -2147477248 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.050770] sk ffff8816b3312700 
sk_mem_uncharge negative -2147474944 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.051170] sk ffff8816b3312700 
sk_mem_uncharge negative -2147472640 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.051572] sk ffff8816b3312700 
sk_mem_uncharge negative -2147470336 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.051971] sk ffff8816b3312700 
sk_mem_uncharge negative -2147468032 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.052371] sk ffff8816b3312700 
sk_mem_uncharge negative -2147465728 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.052771] sk ffff8816b3312700 
sk_mem_uncharge negative -2147463424 by 2304
Nov 15 23:37:19 g1 user.warn kernel: [14137.053169] sk ffff8816b3312700 
sk_mem_uncharge negative -2147461120 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.526984] sk_mem_uncharge: 
1083519 callbacks suppressed
Nov 15 23:37:32 g1 user.warn kernel: [14150.527384] sk ffff8816b3312700 
sk_mem_uncharge negative -2147483392 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.527782] sk ffff8816b3312700 
sk_mem_uncharge negative -2147481088 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.528179] sk ffff8816b3312700 
sk_mem_uncharge negative -2147478784 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.528576] sk ffff8816b3312700 
sk_mem_uncharge negative -2147476480 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.528975] sk ffff8816b3312700 
sk_mem_uncharge negative -2147474176 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.529370] sk ffff8816b3312700 
sk_mem_uncharge negative -2147471872 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.529770] sk ffff8816b3312700 
sk_mem_uncharge negative -2147469568 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.530167] sk ffff8816b3312700 
sk_mem_uncharge negative -2147467264 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.530572] sk ffff8816b3312700 
sk_mem_uncharge negative -2147464960 by 2304
Nov 15 23:37:32 g1 user.warn kernel: [14150.530973] sk ffff8816b3312700 
sk_mem_uncharge negative -2147462656 by 2304
<here it reboots much earlier, at this point>

I will try to get more info, what kind of socket it is.

On 2014-11-12 19:27, Eric Dumazet wrote:
> On Wed, 2014-11-12 at 19:07 +0200, Denys Fedoryshchenko wrote:
>> Hi
>> 
>> 
>> I've been able to trace invalid memory shown for sk_forward_alloc. Not
>> sure if it is related to bug when all tcp stack are wrecked on 
>> highload
>> servers (i had even such crash on server with plain torrents, not that
>> much of them).
>> First they were appearing in ss output as f4294966016, but in tc it
>> shows as unsigned integer, while in kernel it is signed integer. 
>> Should
>> i provide patch for iproute2?
>> After changing value to correct one, here is what is got:
>>           skmem:(r0,rb359040,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb349440,t0,tb46080,f-1280,w1280,o0,bl0)
>>           skmem:(r0,rb357120,t0,tb46080,f-1280,w1280,o0,bl0)
>> 
>> So as it is signed integer, it is actually appears a lot as -1280 
>> bytes.
>> After placing several WARN_ON_ONCE on fall functions involving
>> sk_forward_alloc, where negative value may
>> appear i traced down at least to tcp_connect_queue_skb().
>> [   13.094561] WARNING: CPU: 4 PID: 2855 at include/net/sock.h:1476
>> tcp_connect_queue_skb+0x9f/0xd0()
>> After adding some debug values, it was confirmed:
>> 
>> 
>> skb_header_release(skb);
>> __tcp_add_write_queue_tail(sk, skb);
>> sk->sk_wmem_queued += skb->truesize;
>> sk_mem_charge(sk, skb->truesize); <<<< sk->sk_forward_alloc often is 
>> 0,
>> and skb->truesize is 1280, so -1280 is result
>> tp->write_seq = tcb->end_seq;
>> tp->packets_out += tcp_skb_pcount(skb);
>> 
>> Maybe it should not be subtracted here? Or maybe logic are inverted 
>> all
>> over the code?
>> Please help :)
>> 
> 
> Thanks a lot for the report, I am CCing other experts ;)
> 
> AFAIK, a negative forward alloc should not be a problem, a long as the
> incursion is bound.

^ permalink raw reply

* Re: [PATCH v1 net-next 1/2] bonding: Expand speed type bits of the AD Port Key
From: Jianhua Xie @ 2014-11-16  8:45 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: Jay Vosburgh, David Miller, netdev, andy, Jianhua Xie
In-Reply-To: <20141112112002.GA27653@raspberrypi>


在 2014年11月12日 19:20, Veaceslav Falico 写道:
> On Wed, Nov 12, 2014 at 05:53:41PM +0800, Jianhua Xie wrote:
>> Thanks you two for the valuable comments.
>>
>> If my understanding is right,  it is encouraged to use a counter
>> rather than a bitmask for the speed field, right?
>>
>> if yes, how many bits are better to use for current speed and
>> future speed (like 100Gbps/400Gbps and etc.)?  I am not sure
>> that 5 bits are enough (2**5=32) or not. And I am clear to keep
>> "the duplex bit in the key " in my mind.
>>
>> if not, what's your recommendation please?
>
> As it's visible to bonding only, I guess a simple enum should do the 
> trick.
> No need to invent something special, and it'll fit nicely with other 
> enums
> from AD.
Thanks comments from Jay Vosburgh and Veaceslav Falico.  However, my
method can also work, and also compatible with current bonding driver.
But Veaceslav Falico's method is better than mine.  I am glad to take his
advice. I will use an enum to instead of AD_LINK_SPEED_BITMASK micros
which are based on bitmask.

I also thank Miller for the kindly reminder on "please don't top-post".

Thank & Best Regards,
Jianhua
>
>>
>> Thanks & Best Regards,
>> Jianhua
>>
>> 在 2014年11月12日 03:47, Jay Vosburgh 写道:
>>> David Miller <davem@davemloft.net> wrote:
>>>
>>>> From: Xie Jianhua <Jianhua.Xie@freescale.com>
>>>> Date: Mon, 10 Nov 2014 15:16:40 +0800
>>>>
>>>>> From: Jianhua Xie <Jianhua.Xie@freescale.com>
>>>>>
>>>>> Port Key was determined as 16 bits according to the link speed,
>>>>> duplex and user key (which is yet not supported), in which key
>>>>> speed was 5 bits for 1Mbps/10Mbps/100Mbps/1Gbps/10Gbps as below:
>>>>> --------------------------------------------------------------
>>>>> Port key :|    User key    | Speed        |    Duplex|
>>>>> --------------------------------------------------------------
>>>>> 16            6        1        0
>>>>> This patch is expanding speed type from 5 bits to 9 bits for other
>>>>> speed 2.5Gbps/20Gbps/40Gbps/56Gbps and shrinking user key from 10
>>>>> bits to 6 bits.  New Port Key looks like below:
>>>>> --------------------------------------------------------------
>>>>> Port key :|    User key    | Speed        |    Duplex|
>>>>> --------------------------------------------------------------
>>>>> 16            10        1        0
>>>>>
>>>> Do we determine the layout of this value all ourselves?
>>>     Yes, we do.  The precise format of the port key is not defined
>>> by the standard; IEEE 802.1AX 5.3.5, "Capability identification":
>>>
>>> "A given Key value is meaningful only in the context of the System that
>>> allocates it; there is no global significance to Key values."
>>>
>>>     and
>>>
>>> "When a System assigns an operational Key value to a set of ports, it
>>> signifies that, in the absence of other constraints, the current
>>> operational state of the set of ports allows any subset of that set of
>>> ports (including the entire set) to be aggregated together from the
>>> perspective of the System making the assignment."
>>>
>>>     So, basically, it's a magic cookie that indicates that all ports
>>> on a particular system with the same key value are suitable to be
>>> aggregated together.
>>>
>>>> If not, then is it exported to anything user-visible that we
>>>> might be breaking?
>>>     The key values are not user-visible, and the "user" settable
>>> portion of the key has never been implemented.
>>>
>>>> If it is private, it makes no sense to use a bitmask for the speed.
>>>> We should instead change the field to be some numerically increasing
>>>> value.
>>>>
>>>> Otherwise we'll run out of bits again and keep having to adjust the
>>>> field layout more often than we really need to.
>>>     Agreed.
>>>
>>>     Also note that there are some internal dependencies within
>>> bonding on the format; in particular the duplex bit in the key is used
>>> to determine if a port is LACP-capable, and that functionality needs to
>>> be preserved.
>>>
>>>     -J
>>>
>>> ---
>>>     -Jay Vosburgh, jay.vosburgh@canonical.com
>>

^ permalink raw reply

* [PATCH] ip-link: Document IPoIB link type in the man page
From: Or Gerlitz @ 2014-11-16  7:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Haggai Eran, Shachar Raindel, Or Gerlitz

Add documentation on how to create devices of type IP-over-Infiniband
in the man page.

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
 man/man8/ip-link.8.in |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 6d32f5e..a05633f 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -449,6 +449,23 @@ when tunneling non-IP packets. The default value is 00.
 
 .in -8
 
+.TP
+IPoIB Type Support
+For a link of type
+.I IPoIB
+the following additional arguments are supported:
+
+.BI "ip link add " DEVICE " name " NAME
+.BI type " ipoib [ " pkey " PKEY ] [" mode " MODE " ]
+
+.in +8
+.sp
+.BI  pkey " PKEY "
+- specifies the IB P-Key to use.
+
+.BI  mode " MODE "
+- specifies the mode (datagram or connected) to use.
+
 .SS ip link delete - delete virtual link
 .I DEVICE
 specifies the virtual  device to act operate on.
-- 
1.7.1

^ permalink raw reply related

* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Sedat Dilek @ 2014-11-16  2:48 UTC (permalink / raw)
  To: Greg KH
  Cc: Dan Williams, David S. Miller, netdev@vger.kernel.org, linux-usb,
	Aleksander Morgado
In-Reply-To: <20141116023423.GA31016@kroah.com>

On Sun, Nov 16, 2014 at 3:34 AM, Greg KH <greg@kroah.com> wrote:
> On Sun, Nov 16, 2014 at 02:40:15AM +0100, Sedat Dilek wrote:
>> On Sat, Nov 15, 2014 at 9:07 PM, Greg KH <greg@kroah.com> wrote:
>> > On Sat, Nov 15, 2014 at 10:23:55AM +0100, Sedat Dilek wrote:
>> >> On Fri, Nov 14, 2014 at 5:18 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> > On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
>> >> >> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> >> >> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw@redhat.com> wrote:
>> >> >> >> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
>> >> >> >>> Hi,
>> >> >> >>>
>> >> >> >>> I wanted to understand what is going on the kernel-side when
>> >> >> >>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
>> >> >> >>> Generation: UMTS / HSPA).
>> >> >> >>>
>> >> >> >>> Especially the correlation between the diverse USB/NET kernel-drivers
>> >> >> >>> and how the networking is setup.
>> >> >> >>
>> >> >> >
>> >> >> > [ Sitting in front of a foreign Windows machine ]
>> >> >> >
>> >> >> > [ CC Aleksander ]
>> >> >> >
>> >> >> > Hi Dan,
>> >> >> >
>> >> >> > sorry for the late (and short) response.
>> >> >> >
>> >> >> > AFAICS you have given a "skeleton" for a "usb-wwan-networking"
>> >> >> > documentation :-).
>> >> >> >
>> >> >> > Personally, I would like to take into account some kernel-config
>> >> >> > options and some more things.
>> >> >> >
>> >> >>
>> >> >> I started with documenting...
>> >> >>
>> >> >> I have still some difficulties in understanding USB WWAN Networking.
>> >> >> So, this is what I revealed...
>> >> >>
>> >> >> ##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK
>> >> >>
>> >> >> ### USB-NETWORKING AND WWAN SETUP
>> >> >> CONFIG_USB_USBNET=m        <--- usb networking
>> >> >> CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
>> >> >> CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
>> >> >> CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"
>> >> >
>> >> > Most WWAN devices actually require option, because most WWAN devices
>> >> > have "serial" ports (even if they aren't used for PPP), and 'option' is
>> >> > the driver that handles this.  The 'option' name is historic, but the
>> >> > driver should really be called something like 'wwan-serial-generic' or
>> >> > something like that.
>> >> >ö"
>> >>
>> >> Is there sth. against renaming the "option" driver to "wwan-serial-generic"?
>> >
>> > Yes, people's scripts might break that are hard-coded to use the
>> > "option" driver.
>> >
>>
>> As far as I read on LKML... breaking userspace is a reason not to do
>> such changes.
>
> Exactly.
>
>> That's really a reason not to break "handmade" scripts on some machines?
>
> Yes.
>
>> As this is new to me... is that documented?
>
> It's our "culture" :)
>

OK.

>> Surely, it's fretful to change scripts, but life is change.
>> For me there is a more reasonable thing... Did you grep for "option"
>> pattern in the kernel sources?
>> Try.... :-).
>
> Oh I know, I wrote the first version of this driver and named it this :)
>

Ah, IIRC the company was called so.

>> > greg "here, have a vowel, they are cheap" k-h
>>
>> Hmm, being a non-English native, I am not sure to get this...
>> What about languages from mostly Eastern countries having so much
>> consonants in a single word like Russian, Polish, etc.
>> Not every language is rich like German which has WOWels like "ä" (ae)
>> "ö" (oe) "ü" (ue).
>
> I was referring to your "sth." abbreviation above.
>

Hmm, I thought this is a "normal" abbreviation.
Maybe I should not mix IRC and Email writing styles.

Thanks for your comments.

- Sedat -

P.S.: /me was reading about the systemd transition (now default
init-system) and reading about Joey Hess leaving Debian, Damn and I
initiated the Debian systemd wiki. If I ever knew... Life is change
and neat and polite people died (I lost my parents the last two
years).

> thanks,
>
> greg k-h

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Josh Triplett @ 2014-11-16  2:37 UTC (permalink / raw)
  To: Herbert Xu; +Cc: tgraf, netdev, eric.dumazet, paulmck
In-Reply-To: <20141116022227.GA24541@gondor.apana.org.au>

On November 15, 2014 6:22:27 PM PST, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>Josh Triplett <josh@joshtriplett.org> wrote:
>>
>> - Set up the new empty table with the new set of hash parameters.
>> - synchronize_rcu().  Readers will now search both old and new
>tables.
>> - Peel nodes off the ends of the old hash table and add them to the
>new
>
>We currently use a singly linked list in rhashtable.  Peeling nodes
>off the end would mean upgrading to a doubly linked list, which is
>no different than keeping two lists in terms of cache footprint, no?

No, since each pass just handles one set of nodes from each bucket anyway, you can just do a bit more work in the rehasher instead.

^ permalink raw reply

* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Greg KH @ 2014-11-16  2:34 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Dan Williams, David S. Miller, netdev@vger.kernel.org, linux-usb,
	Aleksander Morgado
In-Reply-To: <CA+icZUXFYYtvpKV-pOfj-pqhYD+TWBDB3SKKz_aBirrJbxvQkA@mail.gmail.com>

On Sun, Nov 16, 2014 at 02:40:15AM +0100, Sedat Dilek wrote:
> On Sat, Nov 15, 2014 at 9:07 PM, Greg KH <greg@kroah.com> wrote:
> > On Sat, Nov 15, 2014 at 10:23:55AM +0100, Sedat Dilek wrote:
> >> On Fri, Nov 14, 2014 at 5:18 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> > On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
> >> >> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >> >> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> >> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
> >> >> >>> Hi,
> >> >> >>>
> >> >> >>> I wanted to understand what is going on the kernel-side when
> >> >> >>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
> >> >> >>> Generation: UMTS / HSPA).
> >> >> >>>
> >> >> >>> Especially the correlation between the diverse USB/NET kernel-drivers
> >> >> >>> and how the networking is setup.
> >> >> >>
> >> >> >
> >> >> > [ Sitting in front of a foreign Windows machine ]
> >> >> >
> >> >> > [ CC Aleksander ]
> >> >> >
> >> >> > Hi Dan,
> >> >> >
> >> >> > sorry for the late (and short) response.
> >> >> >
> >> >> > AFAICS you have given a "skeleton" for a "usb-wwan-networking"
> >> >> > documentation :-).
> >> >> >
> >> >> > Personally, I would like to take into account some kernel-config
> >> >> > options and some more things.
> >> >> >
> >> >>
> >> >> I started with documenting...
> >> >>
> >> >> I have still some difficulties in understanding USB WWAN Networking.
> >> >> So, this is what I revealed...
> >> >>
> >> >> ##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK
> >> >>
> >> >> ### USB-NETWORKING AND WWAN SETUP
> >> >> CONFIG_USB_USBNET=m        <--- usb networking
> >> >> CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
> >> >> CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
> >> >> CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"
> >> >
> >> > Most WWAN devices actually require option, because most WWAN devices
> >> > have "serial" ports (even if they aren't used for PPP), and 'option' is
> >> > the driver that handles this.  The 'option' name is historic, but the
> >> > driver should really be called something like 'wwan-serial-generic' or
> >> > something like that.
> >> >ö"
> >>
> >> Is there sth. against renaming the "option" driver to "wwan-serial-generic"?
> >
> > Yes, people's scripts might break that are hard-coded to use the
> > "option" driver.
> >
> 
> As far as I read on LKML... breaking userspace is a reason not to do
> such changes.

Exactly.

> That's really a reason not to break "handmade" scripts on some machines?

Yes.

> As this is new to me... is that documented?

It's our "culture" :)

> Surely, it's fretful to change scripts, but life is change.
> For me there is a more reasonable thing... Did you grep for "option"
> pattern in the kernel sources?
> Try.... :-).

Oh I know, I wrote the first version of this driver and named it this :)

> > greg "here, have a vowel, they are cheap" k-h
> 
> Hmm, being a non-English native, I am not sure to get this...
> What about languages from mostly Eastern countries having so much
> consonants in a single word like Russian, Polish, etc.
> Not every language is rich like German which has WOWels like "ä" (ae)
> "ö" (oe) "ü" (ue).

I was referring to your "sth." abbreviation above.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-16  2:22 UTC (permalink / raw)
  To: Josh Triplett; +Cc: tgraf, netdev, eric.dumazet, paulmck
In-Reply-To: <20141115191219.GA19060@thin>

Josh Triplett <josh@joshtriplett.org> wrote:
>
> - Set up the new empty table with the new set of hash parameters.
> - synchronize_rcu().  Readers will now search both old and new tables.
> - Peel nodes off the ends of the old hash table and add them to the new

We currently use a singly linked list in rhashtable.  Peeling nodes
off the end would mean upgrading to a doubly linked list, which is
no different than keeping two lists in terms of cache footprint, no?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Sedat Dilek @ 2014-11-16  1:40 UTC (permalink / raw)
  To: Greg KH
  Cc: Dan Williams, David S. Miller,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Aleksander Morgado
In-Reply-To: <20141115200711.GA24643-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

On Sat, Nov 15, 2014 at 9:07 PM, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
> On Sat, Nov 15, 2014 at 10:23:55AM +0100, Sedat Dilek wrote:
>> On Fri, Nov 14, 2014 at 5:18 PM, Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
>> >> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> >> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> >> >> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
>> >> >>> Hi,
>> >> >>>
>> >> >>> I wanted to understand what is going on the kernel-side when
>> >> >>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
>> >> >>> Generation: UMTS / HSPA).
>> >> >>>
>> >> >>> Especially the correlation between the diverse USB/NET kernel-drivers
>> >> >>> and how the networking is setup.
>> >> >>
>> >> >
>> >> > [ Sitting in front of a foreign Windows machine ]
>> >> >
>> >> > [ CC Aleksander ]
>> >> >
>> >> > Hi Dan,
>> >> >
>> >> > sorry for the late (and short) response.
>> >> >
>> >> > AFAICS you have given a "skeleton" for a "usb-wwan-networking"
>> >> > documentation :-).
>> >> >
>> >> > Personally, I would like to take into account some kernel-config
>> >> > options and some more things.
>> >> >
>> >>
>> >> I started with documenting...
>> >>
>> >> I have still some difficulties in understanding USB WWAN Networking.
>> >> So, this is what I revealed...
>> >>
>> >> ##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK
>> >>
>> >> ### USB-NETWORKING AND WWAN SETUP
>> >> CONFIG_USB_USBNET=m        <--- usb networking
>> >> CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
>> >> CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
>> >> CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"
>> >
>> > Most WWAN devices actually require option, because most WWAN devices
>> > have "serial" ports (even if they aren't used for PPP), and 'option' is
>> > the driver that handles this.  The 'option' name is historic, but the
>> > driver should really be called something like 'wwan-serial-generic' or
>> > something like that.
>> >ö"
>>
>> Is there sth. against renaming the "option" driver to "wwan-serial-generic"?
>
> Yes, people's scripts might break that are hard-coded to use the
> "option" driver.
>

As far as I read on LKML... breaking userspace is a reason not to do
such changes.
That's really a reason not to break "handmade" scripts on some machines?
As this is new to me... is that documented?
Surely, it's fretful to change scripts, but life is change.
For me there is a more reasonable thing... Did you grep for "option"
pattern in the kernel sources?
Try.... :-).

> thanks,
>
> greg "here, have a vowel, they are cheap" k-h

Hmm, being a non-English native, I am not sure to get this...
What about languages from mostly Eastern countries having so much
consonants in a single word like Russian, Polish, etc.
Not every language is rich like German which has WOWels like "ä" (ae)
"ö" (oe) "ü" (ue).

- Sedat -
--
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

* sysctl to clamp mtu on all application-initiated sockets?
From: oskar @ 2014-11-16  0:28 UTC (permalink / raw)
  To: netdev

Hi!

I'm trying to setup routers that should be able to forward 
jumbo-packets, but applications should not get jumbo-capable sockets.

The major brands of routers support something like this. You set mtu 
for forwarding to something big (8192, 9000) but don't touch the system 
mtu of 1500.
Management-traffic, routing protocols and such communicate with 
<=1500byte packets, but the router can forward frames up to <big> mtu 
size if other machines feel like using them.

The only ways I've found to do this is either through mss-clamping in 
iptables (+ some iptables rule that drop other packets and return ICMP 
Packet too big), or update each route on the router with an MTU 
argument, but it feels clunky and i believe this will cause quite some 
overhead which would impact forwarding-performance of the router 
negatively, It's also not nice to change tcp packet values on the fly. 
(The MTU on routes-path is quite a lot of work when running IPv4 DHCP 
client + IPv6 RA-learned routes + full BGP table on multiple boxes, it's 
easy to miss something)

So, is it possible to create something like 
/proc/sys/net/ipv*/max_socket_mtu that would clamp the MTU of all 
created sockets to some nice value?

It should also be possible to override this clamping on a per-socket 
basis (and use up to link-layer MTU size) by setting a flag or such on 
the socket, for example tunnels.

I need to solve this some how for forwarding + VXLAN, but I believe 
this would also apply to and simplify management of other tunnel 
techniques. The tunnel interface should be able to transmit encapsulated 
packets using up to link-layer MTU size inside my datacenter/network, 
but applications on the same machine shouldn't when sending data 
somewhere since it would cause extra roundtrips / broken sessions when 
oversize packets are being dropped somewhere else in the network / on 
the Internet.

Best regards
Oskar Stenman

^ permalink raw reply

* [PATCH] qmi_wwan: Add support for HP lt4112 LTE/HSPA+ Gobi 4G Modem
From: Martin Hauke @ 2014-11-16  0:03 UTC (permalink / raw)
  To: netdev; +Cc: bjorn

Added the USB VID/PID for the HP lt4112 LTE/HSPA+ Gobi 4G Modem (Huawei me906e)
---
 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 22756db..b8a82b8 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -780,6 +780,7 @@ static const struct usb_device_id products[] = {
 	{QMI_FIXED_INTF(0x413c, 0x81a4, 8)},	/* Dell Wireless 5570e HSPA+ (42Mbps) Mobile Broadband Card */
 	{QMI_FIXED_INTF(0x413c, 0x81a8, 8)},	/* Dell Wireless 5808 Gobi(TM) 4G LTE Mobile Broadband Card */
 	{QMI_FIXED_INTF(0x413c, 0x81a9, 8)},	/* Dell Wireless 5808e Gobi(TM) 4G LTE Mobile Broadband Card */
+	{QMI_FIXED_INTF(0x03f0, 0x581d, 4)},	/* HP lt4112 LTE/HSPA+ Gobi 4G Module (Huawei me906e) */
 
 	/* 4. Gobi 1000 devices */
 	{QMI_GOBI1K_DEVICE(0x05c6, 0x9212)},	/* Acer Gobi Modem Device */
-- 
2.1.2

^ permalink raw reply related

* [PATCH net-next] device: Add dev_<level>_once variants
From: Joe Perches @ 2014-11-15 22:38 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Jeff Kirsher, davem, Shannon Nelson, netdev, nhorman, sassmann,
	jogreene
In-Reply-To: <5467C3BA.6090003@gmail.com>

Add the equivalents to pr_<level>_once.

Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
 include/linux/device.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index ce1f2160..a1ee071 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1118,6 +1118,41 @@ do {						     \
 })
 #endif
 
+#ifdef CONFIG_PRINTK
+#define dev_level_once(dev_level, dev, fmt, ...)			\
+do {									\
+	static bool __print_once __read_mostly;				\
+									\
+	if (!__print_once) {						\
+		__print_once = true;					\
+		dev_level(dev, fmt, ##__VA_ARGS__);			\
+	}								\
+} while (0)
+#else
+#define dev_level_once(dev_level, dev, fmt, ...)			\
+do {									\
+	if (0)								\
+		dev_level(dev, fmt, ##__VA_ARGS__);			\
+} while (0)
+#endif
+
+#define dev_emerg_once(dev, fmt, ...)					\
+	dev_level_once(dev_emerg, dev, fmt, ##__VA_ARGS__)
+#define dev_alert_once(dev, fmt, ...)					\
+	dev_level_once(dev_alert, dev, fmt, ##__VA_ARGS__)
+#define dev_crit_once(dev, fmt, ...)					\
+	dev_level_once(dev_crit, dev, fmt, ##__VA_ARGS__)
+#define dev_err_once(dev, fmt, ...)					\
+	dev_level_once(dev_err, dev, fmt, ##__VA_ARGS__)
+#define dev_warn_once(dev, fmt, ...)					\
+	dev_level_once(dev_warn, dev, fmt, ##__VA_ARGS__)
+#define dev_notice_once(dev, fmt, ...)					\
+	dev_level_once(dev_notice, dev, fmt, ##__VA_ARGS__)
+#define dev_info_once(dev, fmt, ...)					\
+	dev_level_once(dev_info, dev, fmt, ##__VA_ARGS__)
+#define dev_dbg_once(dev, fmt, ...)					\
+	dev_level_once(dev_info, dev, fmt, ##__VA_ARGS__)
+
 #define dev_level_ratelimited(dev_level, dev, fmt, ...)			\
 do {									\
 	static DEFINE_RATELIMIT_STATE(_rs,				\

^ permalink raw reply related

* [PATCH 1/3] net: dsa: Centralise code for reading the temperature sensor
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn
In-Reply-To: <1416086694-15790-1-git-send-email-andrew@lunn.ch>

The method to read the temperature used in the mve6123_61_65 driver
can also be used for other chips. Move the code into the shared code
base of mv88e6xxx.c.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6123_61_65.c | 50 +--------------------------------------
 drivers/net/dsa/mv88e6xxx.c       | 48 +++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |  1 +
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c
index 9a3f9e0b0532..e9c736e1cef3 100644
--- a/drivers/net/dsa/mv88e6123_61_65.c
+++ b/drivers/net/dsa/mv88e6123_61_65.c
@@ -291,54 +291,6 @@ static int mv88e6123_61_65_setup_port(struct dsa_switch *ds, int p)
 	return 0;
 }
 
-#ifdef CONFIG_NET_DSA_HWMON
-
-static int  mv88e6123_61_65_get_temp(struct dsa_switch *ds, int *temp)
-{
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int ret;
-	int val;
-
-	*temp = 0;
-
-	mutex_lock(&ps->phy_mutex);
-
-	ret = mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x6);
-	if (ret < 0)
-		goto error;
-
-	/* Enable temperature sensor */
-	ret = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
-	if (ret < 0)
-		goto error;
-
-	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret | (1 << 5));
-	if (ret < 0)
-		goto error;
-
-	/* Wait for temperature to stabilize */
-	usleep_range(10000, 12000);
-
-	val = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
-	if (val < 0) {
-		ret = val;
-		goto error;
-	}
-
-	/* Disable temperature sensor */
-	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret & ~(1 << 5));
-	if (ret < 0)
-		goto error;
-
-	*temp = ((val & 0x1f) - 5) * 5;
-
-error:
-	mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x0);
-	mutex_unlock(&ps->phy_mutex);
-	return ret;
-}
-#endif /* CONFIG_NET_DSA_HWMON */
-
 static int mv88e6123_61_65_setup(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
@@ -471,7 +423,7 @@ struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
 	.get_ethtool_stats	= mv88e6123_61_65_get_ethtool_stats,
 	.get_sset_count		= mv88e6123_61_65_get_sset_count,
 #ifdef CONFIG_NET_DSA_HWMON
-	.get_temp		= mv88e6123_61_65_get_temp,
+	.get_temp		= mv88e6xxx_get_temp,
 #endif
 	.get_regs_len		= mv88e6xxx_get_regs_len,
 	.get_regs		= mv88e6xxx_get_regs,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index da558d887dad..cd6807c6b4ed 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -539,6 +539,54 @@ void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 	}
 }
 
+#ifdef CONFIG_NET_DSA_HWMON
+
+int  mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+	int val;
+
+	*temp = 0;
+
+	mutex_lock(&ps->phy_mutex);
+
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x6);
+	if (ret < 0)
+		goto error;
+
+	/* Enable temperature sensor */
+	ret = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
+	if (ret < 0)
+		goto error;
+
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret | (1 << 5));
+	if (ret < 0)
+		goto error;
+
+	/* Wait for temperature to stabilize */
+	usleep_range(10000, 12000);
+
+	val = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
+	if (val < 0) {
+		ret = val;
+		goto error;
+	}
+
+	/* Disable temperature sensor */
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret & ~(1 << 5));
+	if (ret < 0)
+		goto error;
+
+	*temp = ((val & 0x1f) - 5) * 5;
+
+error:
+	mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x0);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
+}
+#endif /* CONFIG_NET_DSA_HWMON */
+
 static int __init mv88e6xxx_init(void)
 {
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6131)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index a0780b08bb4c..03e397efde36 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -81,6 +81,7 @@ void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds,
 int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port);
 void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 			struct ethtool_regs *regs, void *_p);
+int  mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp);
 
 extern struct dsa_switch_driver mv88e6131_switch_driver;
 extern struct dsa_switch_driver mv88e6123_61_65_switch_driver;
-- 
2.1.3

^ permalink raw reply related

* [PATCH 3/3] net: dsa: mv88e6171: Add support for reading switch registers
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn
In-Reply-To: <1416086694-15790-1-git-send-email-andrew@lunn.ch>

The infrastructure can now report switch registers to ethtool.
Add support for it to the mv88e6171 driver.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6171.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index ed4c79778823..aa33d16f2e22 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -423,6 +423,8 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 #ifdef CONFIG_NET_DSA_HWMON
 	.get_temp               = mv88e6xxx_get_temp,
 #endif
+	.get_regs_len		= mv88e6xxx_get_regs_len,
+	.get_regs		= mv88e6xxx_get_regs,
 };
 
 MODULE_ALIAS("platform:mv88e6171");
-- 
2.1.3

^ permalink raw reply related

* [PATCH 2/3] net: dsa: mv88e6171: Add support for reading the temperature
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn
In-Reply-To: <1416086694-15790-1-git-send-email-andrew@lunn.ch>

This chip also has a temperature sensor which can be read using the
common code. In order to use it, add the needed mutex protection for
accessing registers via the shared code.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6171.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 537eeedece21..ed4c79778823 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -316,6 +316,8 @@ static int mv88e6171_setup(struct dsa_switch *ds)
 			return ret;
 	}
 
+	mutex_init(&ps->phy_mutex);
+
 	return 0;
 }
 
@@ -329,18 +331,28 @@ static int mv88e6171_port_to_phy_addr(int port)
 static int
 mv88e6171_phy_read(struct dsa_switch *ds, int port, int regnum)
 {
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int addr = mv88e6171_port_to_phy_addr(port);
+	int ret;
 
-	return mv88e6xxx_phy_read(ds, addr, regnum);
+	mutex_lock(&ps->phy_mutex);
+	ret = mv88e6xxx_phy_read(ds, addr, regnum);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
 }
 
 static int
 mv88e6171_phy_write(struct dsa_switch *ds,
 		    int port, int regnum, u16 val)
 {
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int addr = mv88e6171_port_to_phy_addr(port);
+	int ret;
 
-	return mv88e6xxx_phy_write(ds, addr, regnum, val);
+	mutex_lock(&ps->phy_mutex);
+	ret = mv88e6xxx_phy_write(ds, addr, regnum, val);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
 }
 
 static struct mv88e6xxx_hw_stat mv88e6171_hw_stats[] = {
@@ -408,6 +420,9 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 	.get_strings		= mv88e6171_get_strings,
 	.get_ethtool_stats	= mv88e6171_get_ethtool_stats,
 	.get_sset_count		= mv88e6171_get_sset_count,
+#ifdef CONFIG_NET_DSA_HWMON
+	.get_temp               = mv88e6xxx_get_temp,
+#endif
 };
 
 MODULE_ALIAS("platform:mv88e6171");
-- 
2.1.3

^ permalink raw reply related

* [PATCH 0/3] Add temperature reading and registers dump to mv88e6171
From: Andrew Lunn @ 2014-11-15 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux, Andrew Lunn

These patches centralize the temperature sensor reading code, and then
make use of it with the mv88e6171 which has a compatible
sensor. Additionally, support is added for reading the mv88e6171 via
ethtool.

Andrew Lunn (3):
  net: dsa: Centralise code for reading the temperature sensor
  net: dsa: mv88e6171: Add support for reading the temperature
  net: dsa: mv88e6171: Add support for reading switch registers

 drivers/net/dsa/mv88e6123_61_65.c | 50 +--------------------------------------
 drivers/net/dsa/mv88e6171.c       | 21 ++++++++++++++--
 drivers/net/dsa/mv88e6xxx.c       | 48 +++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |  1 +
 4 files changed, 69 insertions(+), 51 deletions(-)

-- 
2.1.3

^ permalink raw reply

* Re: [net-next 01/12] i40e: only warn once of PTP nonsupport in 100Mbit speed
From: Florian Fainelli @ 2014-11-15 21:20 UTC (permalink / raw)
  To: Jeff Kirsher, davem
  Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene, joe
In-Reply-To: <1416031715-32498-2-git-send-email-jeffrey.t.kirsher@intel.com>

Le 14/11/2014 22:08, Jeff Kirsher a écrit :
> From: Shannon Nelson <shannon.nelson@intel.com>
>
> Only warn once that PTP is not supported when linked at 100Mbit.
>
> Yes, using a static this way means that this once-only message is not
> port specific, but once only for the life of the driver, regardless of
> the number of ports.  That should be plenty.
>
> Change-ID: Ie6476530056df408452e195ef06afd4f57caa4b2
> Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index 537b621..f9151037 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -382,11 +382,17 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
>   		incval = I40E_PTP_1GB_INCVAL;
>   		break;
>   	case I40E_LINK_SPEED_100MB:
> -		dev_warn(&pf->pdev->dev,
> -			 "%s: 1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n",
> -			 __func__);
> +	{
> +		static int warn_once;
> +
> +		if (!warn_once) {
> +			dev_warn(&pf->pdev->dev,
> +				 "1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n");
> +			warn_once++;
> +		}

I am sure Joe could come up with dev_warn_once() and others pretty 
easily. As a temporary change you could use pr_warn_once("%s: ...\n", 
dev_name(&pf->pdev->dev) which is almost identical?

>   		incval = 0;
>   		break;
> +	}
>   	case I40E_LINK_SPEED_40GB:
>   	default:
>   		incval = I40E_PTP_40GB_INCVAL;
>

^ permalink raw reply

* Re: [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead
From: Francois Romieu @ 2014-11-15 21:13 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Alexander Duyck, linux-arch, netdev, linux-kernel, mikey,
	tony.luck, mathieu.desnoyers, donald.c.skidmore, peterz, benh,
	heiko.carstens, oleg, will.deacon, davem, michael, matthew.vick,
	nic_swsd, geert, jeffrey.t.kirsher, fweisbec, schwidefsky, linux,
	paulmck, torvalds, mingo, Adam Nielsen
In-Reply-To: <54653ABF.3080400@gmail.com>

Alexander Duyck <alexander.duyck@gmail.com> :
> On 11/13/2014 01:30 PM, Francois Romieu wrote:
> > Alexander Duyck <alexander.h.duyck@redhat.com> :
> > [...]
> >> In addition the r8169 uses a rmb() however I believe it is placed incorrectly
> >> as I assume it supposed to be ordering descriptor reads after the check for
> >> ownership.
> > Not exactly. It's a barrier against compiler optimization from 2004.
> > It should not matter.
> 
> Okay.  Do you recall the kind of problem it was you were seeing ?

Mildly, I had to grep the local archives.

The relevant code used to be included in the irq handler at that time
(napi support for this driver took place in may 2004). One did not want
a runaway loop in the Tx reaper.

Compiler optimization was suggested by Manfred Spraul in the thread below:
http://marc.info/?l=linux-kernel&m=108096868119004

> The origin of the rmb() for the Intel drivers was a PowerPC issue in
> which it was fetching the length of a buffer before it checked the DD
> bit (equivalent of DescOwn).  I'm wondering if the issue you were seeing
> was something similar where it had reordered reads in the descriptor to
> cause that type of result.

The problem was only reported on Intel 32 bit + slackware + gcc 3.2.3.

Adam Nielsen - Cc: added - did not return for this bug.

-- 
Ueimor

^ permalink raw reply

* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Greg KH @ 2014-11-15 20:07 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Dan Williams, David S. Miller, netdev@vger.kernel.org, linux-usb,
	Aleksander Morgado
In-Reply-To: <CA+icZUUWK8R0zFLBHfvfE6zmZ7=+yP_FJgkkeELVbwdhPUYfRA@mail.gmail.com>

On Sat, Nov 15, 2014 at 10:23:55AM +0100, Sedat Dilek wrote:
> On Fri, Nov 14, 2014 at 5:18 PM, Dan Williams <dcbw@redhat.com> wrote:
> > On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
> >> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> >> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
> >> >>> Hi,
> >> >>>
> >> >>> I wanted to understand what is going on the kernel-side when
> >> >>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
> >> >>> Generation: UMTS / HSPA).
> >> >>>
> >> >>> Especially the correlation between the diverse USB/NET kernel-drivers
> >> >>> and how the networking is setup.
> >> >>
> >> >
> >> > [ Sitting in front of a foreign Windows machine ]
> >> >
> >> > [ CC Aleksander ]
> >> >
> >> > Hi Dan,
> >> >
> >> > sorry for the late (and short) response.
> >> >
> >> > AFAICS you have given a "skeleton" for a "usb-wwan-networking"
> >> > documentation :-).
> >> >
> >> > Personally, I would like to take into account some kernel-config
> >> > options and some more things.
> >> >
> >>
> >> I started with documenting...
> >>
> >> I have still some difficulties in understanding USB WWAN Networking.
> >> So, this is what I revealed...
> >>
> >> ##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK
> >>
> >> ### USB-NETWORKING AND WWAN SETUP
> >> CONFIG_USB_USBNET=m        <--- usb networking
> >> CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
> >> CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
> >> CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"
> >
> > Most WWAN devices actually require option, because most WWAN devices
> > have "serial" ports (even if they aren't used for PPP), and 'option' is
> > the driver that handles this.  The 'option' name is historic, but the
> > driver should really be called something like 'wwan-serial-generic' or
> > something like that.
> >
> 
> Is there sth. against renaming the "option" driver to "wwan-serial-generic"?

Yes, people's scripts might break that are hard-coded to use the
"option" driver.

thanks,

greg "here, have a vowel, they are cheap" k-h

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Josh Triplett @ 2014-11-15 19:12 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Herbert Xu, netdev, eric.dumazet, paulmck
In-Reply-To: <20141115111626.GP19157@casper.infradead.org>

On Sat, Nov 15, 2014 at 11:16:26AM +0000, Thomas Graf wrote:
> On 11/15/14 at 11:25am, Herbert Xu wrote:
> > On Thu, Nov 13, 2014 at 06:43:43PM +0800, Herbert Xu wrote:
> > > On Thu, Nov 13, 2014 at 10:41:24AM +0000, Thomas Graf wrote:
> > > >
> > > > Never mind. You did fix it. I looked at the wrong patch.
> > > 
> > > OK.  Now comes the fun part of shoehorning the xfrm_policy bydst
> > > hash into rhashtable :)
> > 
> > Thomas, it appears that rhashtable as it stands cannot handle
> > changing the random seed because of the way it constructs the
> > new hash table without degrading into a linked list.  Is there
> > something I'm missing?
> > 
> > FWIW my hashtable in net/bridge/br_multicast.c handles rehashing
> > correctly.  Any objections to me converting rhashtable to use my
> > scheme instead?
> 
> Can you elaborate a bit?
> 
> The point of rhashtable is to not require two sets of linked list
> pointers as done by MDB or OVS flow tables to work around the
> increased cache footprint of that approach. The difference of the
> two algos is dicussed in this paper [0].
> 
> The disadvantage of rhashtable is that, AFAIK, the hash function
> cannot change while resizing as it would break the mutual linked
> lists.

You can handle hash function changes with rhashtable without needing a
second set of linked-list pointers, actually.  You'd have to add ~1
unlikely() conditional to the reader common case, and you'd make readers
*during* a hash algorithm change (which I'd hope happens as rarely as a
resize) somewhat less efficient, but you wouldn't use any more memory
than a resize currently does, and you wouldn't use the memory of extra
linked-list pointers in the common case.

Rather than the current approach of switching out the hash table pointer
and having readers only search the new table (which will have
valid-but-crosslinked buckets during a resize), instead keep two hash
table pointers (each with their own hash parameters) and a single
toggleable old/new indicator.  Readers check both hash table pointers,
and if valid, check both tables, first old then new (order important to
make sure they don't miss a node).  Then, the rehashing algorithm can
incrementally move nodes from the old buckets to the new buckets,
*without* disrupting concurrent readers.

Rough rehashing algorithm sketch:

- Set up the new empty table with the new set of hash parameters.
- synchronize_rcu().  Readers will now search both old and new tables.
- Peel nodes off the ends of the old hash table and add them to the new
  hash table (similar to the unzip step in the resize algorithm).
  Because readers search old-then-new, and writers make each node appear
  in the new table before making it disappear from the old, you don't
  need a synchronize_rcu() here, just an smp_wmb() after linking the
  node into the new table and before unlinking the node from the old
  table.  Also, because you remove nodes from the *ends* of old-table
  buckets, you don't have to worry about a reader's linked-list walk
  getting dragged over to the new table and missing nodes from the old
  one.
- Once all nodes have moved to the new table, mark the pointer to the
  old table as NULL, synchronize_rcu(), and free the old table.
- Toggle the old/new flag.

Ordering principles in this algorithm:
- You want readers to see the changes made by the rehasher in order.
- If the reader reads location A then B, and the rehasher writes
  location B then A, the rehasher just needs an smp_wmb() in between.
- If the reader reads location A then B, and the rehasher writes
  location A then B, the rehasher needs a synchronize_rcu() in between.


- Josh Triplett

^ permalink raw reply

* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-15 18:47 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
	Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CA+mtBx-=fbA+pW35K8cjXTJpw30jhawy1mhs-MDEYv9SCyD2MQ@mail.gmail.com>

On Sat, Nov 15, 2014 at 10:41 AM, Tom Herbert <therbert@google.com> wrote:
> On Fri, Nov 14, 2014 at 4:50 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Nov 14, 2014 at 4:40 PM, Tom Herbert <therbert@google.com> wrote:
>>> On Fri, Nov 14, 2014 at 4:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Fri, Nov 14, 2014 at 4:06 PM, Tom Herbert <therbert@google.com> wrote:
>>>>> On Fri, Nov 14, 2014 at 2:10 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>> On Fri, Nov 14, 2014 at 1:36 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>> On Fri, Nov 14, 2014 at 12:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>>> On Fri, Nov 14, 2014 at 12:25 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>>> On Fri, Nov 14, 2014 at 12:16 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>>>>> On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>>>>> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>>>>>>>>>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>>>>>>>>>>>> question about this API:
>>>>>>>>>>>>>
>>>>>>>>>>>>> How does an application tell whether the socket represents a
>>>>>>>>>>>>> non-actively-steered flow?  If the flow is subject to RFS, then moving
>>>>>>>>>>>>> the application handling to the socket's CPU seems problematic, as the
>>>>>>>>>>>>> socket's CPU might move as well.  The current implementation in this
>>>>>>>>>>>>> patch seems to tell me which CPU the most recent packet came in on,
>>>>>>>>>>>>> which is not necessarily very useful.
>>>>>>>>>>>>
>>>>>>>>>>>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>>>>>>>>>>>> its cache. This is all that matters,
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Some possibilities:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>>>>>>>>>>>
>>>>>>>>>>>> Well, idea is to not use RFS at all. Otherwise, it is useless.
>>>>>>>>>>
>>>>>>>>>> Sure, but how do I know that it'll be the same CPU next time?
>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> Bear in mind this is only an interface to report RX CPU and in itself
>>>>>>>>>>> doesn't provide any functionality for changing scheduling, there is
>>>>>>>>>>> obviously logic needed in user space that would need to do something.
>>>>>>>>>>>
>>>>>>>>>>> If we track the interrupting CPU in skb, the interface could be easily
>>>>>>>>>>> extended to provide the interrupting CPU, the RPS CPU (calculated at
>>>>>>>>>>> reported time), and the CPU processing transport (post steering which
>>>>>>>>>>> is what is currently returned). That would provide the complete
>>>>>>>>>>> picture to control scheduling a flow from userspace, and an interface
>>>>>>>>>>> to selectively turn off RFS for a socket would make sense then.
>>>>>>>>>>
>>>>>>>>>> I think that a turn-off-RFS interface would also want a way to figure
>>>>>>>>>> out where the flow would go without RFS.  Can the network stack do
>>>>>>>>>> that (e.g. evaluate the rx indirection hash or whatever happens these
>>>>>>>>>> days)?
>>>>>>>>>>
>>>>>>>>> Yes,. We need the rxhash and the CPU that packets are received on from
>>>>>>>>> the device for the socket. The former we already have, the latter
>>>>>>>>> might be done by adding a field to skbuff to set received CPU. Given
>>>>>>>>> the L4 hash and interrupting CPU we can calculated the RPS CPU which
>>>>>>>>> is where packet would have landed with RFS off.
>>>>>>>>
>>>>>>>> Hmm.  I think this would be useful for me.  It would *definitely* be
>>>>>>>> useful for me if I could pin an RFS flow to a cpu of my choice.
>>>>>>>>
>>>>>>> Andy, can you elaborate a little more on your use case. I've thought
>>>>>>> several times about an interface to program the flow table from
>>>>>>> userspace, but never quite came up with a compelling use case and
>>>>>>> there is the security concern that a user could "steal" cycles from
>>>>>>> arbitrary CPUs.
>>>>>>
>>>>>> I have a bunch of threads that are pinned to various CPUs or groups of
>>>>>> CPUs.  Each thread is responsible for a fixed set of flows.  I'd like
>>>>>> those flows to go to those CPUs.
>>>>>>
>>>>>> RFS will eventually do it, but it would be nice if I could
>>>>>> deterministically ask for a flow to be routed to the right CPU.  Also,
>>>>>> if my thread bounces temporarily to another CPU, I don't really need
>>>>>> the flow to follow it -- I'd like it to stay put.
>>>>>>
>>>>> Okay, how about it we have a SO_RFS_LOCK_FLOW sockopt. When this is
>>>>> called on a socket we can lock the socket to CPU binding to the
>>>>> current CPU it is called from. It could be unlocked at a later point.
>>>>> Would this satisfy your requirements?
>>>>
>>>> Yes, I think.  Especially if it bypassed the hash table.
>>>
>>> Unfortunately we can't easily bypass the hash table. The only way I
>>> know of to to do that is to perform the socket lookup to do steering
>>> (I tried that early on, but it was pretty costly).
>>
>> What happens if you just call ndo_rx_flow_steer and do something to
>> keep the result from expiring?
>>
> Okay, I will look at that. Do you know how many flows we are talking
> about, both in the number you need and the number that can be put in
> the HW without collision?

I think I have on the order of 100 flows.  Maybe 400.  It's been a few
months since I checked, and this particular metric is much easier to
measure on a weekday.

I think the HW has several thousand slots.

--Andy

^ permalink raw reply

* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-15 18:41 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
	Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CALCETrU5Og8tfkFuA-rPY5evV5CkPsx=22f6mePHJPvfcRrBdA@mail.gmail.com>

On Fri, Nov 14, 2014 at 4:50 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Nov 14, 2014 at 4:40 PM, Tom Herbert <therbert@google.com> wrote:
>> On Fri, Nov 14, 2014 at 4:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Fri, Nov 14, 2014 at 4:06 PM, Tom Herbert <therbert@google.com> wrote:
>>>> On Fri, Nov 14, 2014 at 2:10 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> On Fri, Nov 14, 2014 at 1:36 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>> On Fri, Nov 14, 2014 at 12:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>> On Fri, Nov 14, 2014 at 12:25 PM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>> On Fri, Nov 14, 2014 at 12:16 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>>>>> On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert@google.com> wrote:
>>>>>>>>>> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>>>>>>>>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>>>>>>>>>>
>>>>>>>>>>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>>>>>>>>>>> question about this API:
>>>>>>>>>>>>
>>>>>>>>>>>> How does an application tell whether the socket represents a
>>>>>>>>>>>> non-actively-steered flow?  If the flow is subject to RFS, then moving
>>>>>>>>>>>> the application handling to the socket's CPU seems problematic, as the
>>>>>>>>>>>> socket's CPU might move as well.  The current implementation in this
>>>>>>>>>>>> patch seems to tell me which CPU the most recent packet came in on,
>>>>>>>>>>>> which is not necessarily very useful.
>>>>>>>>>>>
>>>>>>>>>>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>>>>>>>>>>> its cache. This is all that matters,
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Some possibilities:
>>>>>>>>>>>>
>>>>>>>>>>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>>>>>>>>>>
>>>>>>>>>>> Well, idea is to not use RFS at all. Otherwise, it is useless.
>>>>>>>>>
>>>>>>>>> Sure, but how do I know that it'll be the same CPU next time?
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> Bear in mind this is only an interface to report RX CPU and in itself
>>>>>>>>>> doesn't provide any functionality for changing scheduling, there is
>>>>>>>>>> obviously logic needed in user space that would need to do something.
>>>>>>>>>>
>>>>>>>>>> If we track the interrupting CPU in skb, the interface could be easily
>>>>>>>>>> extended to provide the interrupting CPU, the RPS CPU (calculated at
>>>>>>>>>> reported time), and the CPU processing transport (post steering which
>>>>>>>>>> is what is currently returned). That would provide the complete
>>>>>>>>>> picture to control scheduling a flow from userspace, and an interface
>>>>>>>>>> to selectively turn off RFS for a socket would make sense then.
>>>>>>>>>
>>>>>>>>> I think that a turn-off-RFS interface would also want a way to figure
>>>>>>>>> out where the flow would go without RFS.  Can the network stack do
>>>>>>>>> that (e.g. evaluate the rx indirection hash or whatever happens these
>>>>>>>>> days)?
>>>>>>>>>
>>>>>>>> Yes,. We need the rxhash and the CPU that packets are received on from
>>>>>>>> the device for the socket. The former we already have, the latter
>>>>>>>> might be done by adding a field to skbuff to set received CPU. Given
>>>>>>>> the L4 hash and interrupting CPU we can calculated the RPS CPU which
>>>>>>>> is where packet would have landed with RFS off.
>>>>>>>
>>>>>>> Hmm.  I think this would be useful for me.  It would *definitely* be
>>>>>>> useful for me if I could pin an RFS flow to a cpu of my choice.
>>>>>>>
>>>>>> Andy, can you elaborate a little more on your use case. I've thought
>>>>>> several times about an interface to program the flow table from
>>>>>> userspace, but never quite came up with a compelling use case and
>>>>>> there is the security concern that a user could "steal" cycles from
>>>>>> arbitrary CPUs.
>>>>>
>>>>> I have a bunch of threads that are pinned to various CPUs or groups of
>>>>> CPUs.  Each thread is responsible for a fixed set of flows.  I'd like
>>>>> those flows to go to those CPUs.
>>>>>
>>>>> RFS will eventually do it, but it would be nice if I could
>>>>> deterministically ask for a flow to be routed to the right CPU.  Also,
>>>>> if my thread bounces temporarily to another CPU, I don't really need
>>>>> the flow to follow it -- I'd like it to stay put.
>>>>>
>>>> Okay, how about it we have a SO_RFS_LOCK_FLOW sockopt. When this is
>>>> called on a socket we can lock the socket to CPU binding to the
>>>> current CPU it is called from. It could be unlocked at a later point.
>>>> Would this satisfy your requirements?
>>>
>>> Yes, I think.  Especially if it bypassed the hash table.
>>
>> Unfortunately we can't easily bypass the hash table. The only way I
>> know of to to do that is to perform the socket lookup to do steering
>> (I tried that early on, but it was pretty costly).
>
> What happens if you just call ndo_rx_flow_steer and do something to
> keep the result from expiring?
>
Okay, I will look at that. Do you know how many flows we are talking
about, both in the number you need and the number that can be put in
the HW without collision?

Thanks,
Tom

>>>
>>>> As I mentioned, there is no material functionality in this patch and
>>>> it should be independent of RFS. It simply returns the CPU where the
>>>> stack processed the packet. Whether or not this is meaningful
>>>> information to the algorithm being implemented in userspace is
>>>> completely up to the caller to decide.
>>>
>>> Agreed.
>>>
>>> My only concern is that writing that userspace algorithm might result
>>> in surprises if RFS is on.  Having the user program notice the problem
>>> early and alert the admin might help keep Murphy's Law at bay here.
>>>
>> By Murphy's law we'd also have to consider that the flow hash could
>> change after reading the results so that the scheduling done in
>> userspace is completely wrong until the CPU is read again.
>> Synchronizing kernel and device state with userspace state is not
>> always so easy. One way to mitigate is to use ancillary data which
>> would provide real time information and obviate the need for another
>> system call.
>
> Hmm.  That would work, too.  I don't know how annoyed user code would
> be at having to read ancillary data, though.
>
> The flow hash really shouldn't change much, though, right?
>
> --Andy

^ permalink raw reply

* Re: [net-next 03/12] i40e: Handle a single mss packet with more than 8 frags
From: Eric Dumazet @ 2014-11-15 18:21 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Serey Kong, netdev, nhorman, sassmann, jogreene,
	Shannon Nelson
In-Reply-To: <1416031715-32498-4-git-send-email-jeffrey.t.kirsher@intel.com>

On Fri, 2014-11-14 at 22:08 -0800, Jeff Kirsher wrote:
> From: Serey Kong <serey.kong@intel.com>
> 
> This handles the case where a single packet with more than 8 data
> descriptors triggers a Malicious Driver Detect event in the device.
> 
> Change-ID: I2bfd657fd036bed616863e121f6aa86a35d2194c
> Signed-off-by: Serey Kong <serey.kong@intel.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
> Tested-by: Jim Young <jamesx.m.young@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 14 ++++++++++----
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  1 +
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 14 ++++++++++----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.h |  1 +
>  4 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 3195d82..9731026 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2113,12 +2113,12 @@ static void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
>  			const u8 hdr_len, u32 td_cmd, u32 td_offset)
>  #endif
>  {
> -	unsigned int data_len = skb->data_len;
> -	unsigned int size = skb_headlen(skb);
>  	struct skb_frag_struct *frag;
>  	struct i40e_tx_buffer *tx_bi;
>  	struct i40e_tx_desc *tx_desc;
>  	u16 i = tx_ring->next_to_use;
> +	unsigned int data_len;
> +	unsigned int size;
>  	u32 td_tag = 0;
>  	dma_addr_t dma;
>  	u16 gso_segs;
> @@ -2129,10 +2129,16 @@ static void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
>  			 I40E_TX_FLAGS_VLAN_SHIFT;
>  	}
>  
> -	if (tx_flags & (I40E_TX_FLAGS_TSO | I40E_TX_FLAGS_FSO))
> +	if (tx_flags & (I40E_TX_FLAGS_TSO | I40E_TX_FLAGS_FSO)) {
>  		gso_segs = skb_shinfo(skb)->gso_segs;
> -	else
> +	} else {
>  		gso_segs = 1;
> +		if (skb_shinfo(skb)->nr_frags >= I40E_MAX_BUFFER_TXD)
> +			skb_linearize(skb);

What exactly happens if skb_linearize() fails ?

Is this "Malicious Driver Detect event" fatal or simply packet is
dropped without additional harm ?

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-15 15:51 UTC (permalink / raw)
  To: Thomas Graf; +Cc: netdev, eric.dumazet, paulmck, josh, David S. Miller
In-Reply-To: <20141115112313.GA20970@gondor.apana.org.au>

On Sat, Nov 15, 2014 at 07:23:13PM +0800, Herbert Xu wrote:
> 
> This is I think a show-stopper because for anything that can be
> influenced by remote parties we have to reseed and therefore
> be able to cope with hashes changing on the fly.  Since most
> hash tables in the network stack can be influenced by remote
> entities (including the xfrm policy bydst hash that I am currently
> working on), this is something rhashtable must be able to support
> if it is to be used throughout the network stack.

So I noticed that you got rid of the rehash when you converted
netlink over to rthashtable.  Was this aspect of the conversion
discussed anywhere? In particular, how do you protect against
a malicious user that's trying to attack the netlink hash table?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: phy/micrel: KSZ8031RNL RMII clock reconfiguration bug
From: Johan Hovold @ 2014-11-15 14:18 UTC (permalink / raw)
  To: Bruno Thomsen
  Cc: Johan Hovold, netdev@vger.kernel.org, f.fainelli@gmail.com,
	s.hauer@pengutronix.de, bruno.thomsen@gmail.com,
	linux-kernel@vger.kernel.org
In-Reply-To: <915054555B5659448ACF8A70E114824D01704B9131@Exchange2010.kamstrup.dk>

On Wed, Nov 12, 2014 at 12:17:57PM +0000, Bruno Thomsen wrote:
> Hi Johan,
> 
> > As you may have seen by now, I've been working on refactoring the
> > micrel phy driver to be able to use common initialisation code.
> >
> > Specifically, I've added generic support for disabling the broadcast
> > address, which is what the MII_KSZPHY_OMSO write above does.
> >
> > Generally you want this to be the first thing you do in order to
> > avoid unnecessary reconfigurations. If we ever were to allow
> > concurrent probing this would also be a requirement.
> >
> > Could you provide some detail about the setup were you find that the
> > PHY becomes unresponsive without your patch? Do you have more than
> > one PHY on the bus? Using what addresses? And using what clock modes
> > (i.e. 25 MHz or 50 MHz)?
> > 
> > Also, what exactly do you mean by "unresponsive"? Are you still able
> > to read the PHY registers for example?
>  
> I think it sounds like a good idea to refactor the init code.
> 
> My setup:
> iMX28 processor with dual Ethernet MAC; FEC0 (enabled) and FEC1 (disabled).
> There is a single KSZ8031 PHY that receives 50MHz RMII clock from the MAC.
> I am unable to read PHY registers from both user-land tools and extra
> debug PHY reads in driver code.

Did you specify a led-mode as well, or was the Operation Mode Strap
Override (OMSO) write the first access after the soft reset?

Did you try any other workarounds besides setting the clock mode before
doing the OMSO write?

And REF_CLK (pin 16) is not connected? 

> Boot trace:
> [   22.277785] fec 800f0000.ethernet eth0: Freescale FEC PHY driver [Micrel KSZ8031] (mii_bus:phy_addr=800f0000.etherne:00, irq=-1)
> [   22.292527] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> [   24.276217] libphy: 800f0000.etherne:00 - Link is Up - 100/Full
> [   24.285094] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready

Ok, so you use a single PHY strapped to address 0. 

Would you able to test my series on your setup, and possibly a couple of
diagnostic patches on top?

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-15 11:23 UTC (permalink / raw)
  To: Thomas Graf; +Cc: netdev, eric.dumazet, paulmck, josh, David S. Miller
In-Reply-To: <20141115111626.GP19157@casper.infradead.org>

On Sat, Nov 15, 2014 at 11:16:26AM +0000, Thomas Graf wrote:
>
> > Thomas, it appears that rhashtable as it stands cannot handle
> > changing the random seed because of the way it constructs the
> > new hash table without degrading into a linked list.  Is there
> > something I'm missing?
> > 
> > FWIW my hashtable in net/bridge/br_multicast.c handles rehashing
> > correctly.  Any objections to me converting rhashtable to use my
> > scheme instead?
> 
> Can you elaborate a bit?
> 
> The point of rhashtable is to not require two sets of linked list
> pointers as done by MDB or OVS flow tables to work around the
> increased cache footprint of that approach. The difference of the
> two algos is dicussed in this paper [0].
> 
> The disadvantage of rhashtable is that, AFAIK, the hash function
> cannot change while resizing as it would break the mutual linked
> lists.

Well it doesn't break so much as degenerate into a linked list (I'm
talking about the concept rather than what the current code does).

This is I think a show-stopper because for anything that can be
influenced by remote parties we have to reseed and therefore
be able to cope with hashes changing on the fly.  Since most
hash tables in the network stack can be influenced by remote
entities (including the xfrm policy bydst hash that I am currently
working on), this is something rhashtable must be able to support
if it is to be used throughout the network stack.

Unless there is a better solution then I think keeping two lists
will have to do.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ 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