Netdev List
 help / color / mirror / Atom feed
* Re: [patch] s2io: use snprintf() as a safety feature
From: David Miller @ 2015-01-20  0:45 UTC (permalink / raw)
  To: dan.carpenter; +Cc: jdmason, netdev, kernel-janitors
In-Reply-To: <20150119193451.GB32634@mwanda>

From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 19 Jan 2015 22:34:51 +0300

> "sp->desc[i]" has 25 characters.  "dev->name" has 15 characters.  If we
> used all 15 characters then the sprintf() would overflow.
> 
> I changed the "sprintf(sp->name, "%s Neterion %s"" to snprintf(), as
> well, even though it can't overflow just to be consistent.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied, thanks Dan.

^ permalink raw reply

* Re: [PATCH net-next] tipc: ratelimit network event traces
From: David Miller @ 2015-01-20  0:46 UTC (permalink / raw)
  To: erik.hugne; +Cc: richard.alpe, ying.xue, jon.maloy, netdev, tipc-discussion
In-Reply-To: <1421658164-26185-1-git-send-email-erik.hugne@ericsson.com>

From: <erik.hugne@ericsson.com>
Date: Mon, 19 Jan 2015 10:02:44 +0100

> From: Erik Hugne <erik.hugne@ericsson.com>
> 
> If a large number of namespaces is spawned on a node and TIPC is
> enabled in each of these, the excessive printk tracing of network
> events will cause the system to grind down to a near halt.
> We fix this by adding ratelimiting to the info/warning logs
> regarding link state and node availability.
> 
> Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
> Reviewed-by: Ying Xue <ying.xue@windriver.com>

Really these various state transition messages that are enabled
by default really should not be.

If you want to watch for every event like this, listen on a
netlink socket or set some debugging level param to some non-
default value.

^ permalink raw reply

* [GIT] Networking
From: David Miller @ 2015-01-20  1:07 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, linux-kernel


1) Socket addresses returned in the error queue need to be fully
   initialized before being passed on to userspace, fix from
   Willem de Bruijn.

2) Interrupt handling fixes to davinci_emac driver from Tony
   Lindgren.

3) Fix races between receive packet steering and cpu hotplug, from
   Eric Dumazet.

4) Allowing netlink sockets to subscribe to unknown multicast groups
   leads to crashes, don't allow it.  From Johannes Berg.

5) One to many socket races in SCTP fixed by Daniel Borkmann.

6) Put in a guard against the mis-use of ipv6 atomic fragments, from
   Hagen Paul Pfeifer.

7) Fix promisc mode and ethtool crashes in sh_eth driver, from Ben
   Hutchings.

8) Null deref and double kfree fix in sxgbe driver from Girish K.S
   and Byungho An.

9) cfg80211 deadlock fix from Arik Nemtsov.

Please pull, thanks a lot!

The following changes since commit a6391a924cf5a16761ccd6b45094a7d5b9aeebac:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-01-15 11:17:37 +1300)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 

for you to fetch changes up to a8c1d28ac3925b99b5a939617d3fef1644298ee8:

  s2io: use snprintf() as a safety feature (2015-01-19 19:42:21 -0500)

----------------------------------------------------------------
Ahmed S. Darwish (4):
      can: kvaser_usb: Don't free packets when tight on URBs
      can: kvaser_usb: Reset all URB tx contexts upon channel close
      can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels
      can: kvaser_usb: Don't dereference skb after a netif_rx()

Arik Nemtsov (1):
      cfg80211: fix deadlock during reg chan check

Ben Hutchings (2):
      sh_eth: Fix promiscuous mode on chips without TSU
      sh_eth: Fix ethtool operation crash when net device is down

Byungho An (1):
      net: sxgbe: Fix waring for double kfree()

Dan Carpenter (1):
      s2io: use snprintf() as a safety feature

Daniel Borkmann (1):
      net: sctp: fix race for one-to-many sockets in sendmsg's auto associate

David S. Miller (6):
      Merge tag 'mac80211-for-davem-2015-01-15' of git://git.kernel.org/.../jberg/mac80211
      Merge tag 'linux-can-fixes-for-3.19-20150115' of git://git.kernel.org/.../mkl/linux-can
      Merge branch 'davinci_emac'
      Merge branch 'sh_eth'
      Merge branch 'bgmac'
      Merge branch 'r8152'

Eric Dumazet (1):
      net: rps: fix cpu unplug

Geert Uytterhoeven (1):
      sh_eth: Fix addition of .trscer_err_mask to wrong SoC data

Girish K.S (1):
      net: sxgbe: Fix NULL dereferece when using DT

Hagen Paul Pfeifer (1):
      ipv6: stop sending PTB packets for MTU < 1280

Hauke Mehrtens (2):
      bgmac: register napi before the device
      bgmac: activate irqs only if there is nothing to poll

Johannes Berg (3):
      genetlink: document parallel_ops
      genetlink: disallow subscribing to unknown mcast groups
      genetlink: synchronize socket closing and family removal

John Linville (1):
      mac80211: uninitialized return val in __ieee80211_sta_handle_tspec_ac_params

Marc Kleine-Budde (1):
      MAINTAINERS: update linux-can git repositories

Mugunthan V N (1):
      drivers: net: cpsw: fix cpsw hung with add vlan using vconfig

Oliver Hartkopp (2):
      can: dev: fix crtlmode_supported check
      can: m_can: tag current CAN FD controllers as non-ISO

Or Gerlitz (1):
      net/mlx4: Don't disable vxlan offloads under DMFS-A0 optimized steering

Roger Quadros (1):
      can: c_can: use regmap_update_bits() to modify RAMINIT register

Sriharsha Basavapatna (1):
      be2net: Allow GRE to work concurrently while a VxLAN tunnel is configured

Tony Lindgren (6):
      net: davinci_emac: Fix hangs with interrupts
      net: davinci_emac: Fix runtime pm calls for davinci_emac
      net: davinci_emac: Free clock after checking the frequency
      net: davinci_emac: Fix incomplete code for getting the phy from device tree
      net: davinci_emac: Fix ioremap for devices with MDIO within the EMAC address space
      net: davinci_emac: Add support for emac on dm816x

Willem de Bruijn (1):
      ip: zero sockaddr returned on error queue

hayeswang (2):
      r8152: remove generic_ocp_read before writing
      r8152: remove sram_read

 Documentation/devicetree/bindings/net/davinci_emac.txt |  3 +-
 MAINTAINERS                                            |  6 ++--
 drivers/infiniband/hw/mlx4/main.c                      |  3 +-
 drivers/net/can/c_can/c_can_platform.c                 | 29 ++++++++++-------
 drivers/net/can/dev.c                                  |  8 +++--
 drivers/net/can/m_can/m_can.c                          |  5 +++
 drivers/net/can/usb/kvaser_usb.c                       | 31 +++++++++---------
 drivers/net/ethernet/broadcom/bgmac.c                  | 12 +++----
 drivers/net/ethernet/emulex/benet/be_main.c            | 41 +++++++++++++++++++++--
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c         |  3 +-
 drivers/net/ethernet/mellanox/mlx4/main.c              |  3 +-
 drivers/net/ethernet/neterion/s2io.c                   | 11 +++++--
 drivers/net/ethernet/renesas/sh_eth.c                  | 32 +++++++++++-------
 drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c        | 21 ++++++------
 drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c    |  8 ++---
 drivers/net/ethernet/ti/cpsw.c                         | 27 +++++++++------
 drivers/net/ethernet/ti/davinci_emac.c                 | 96 ++++++++++++++++++++++++++++++++++++++++++------------
 drivers/net/usb/r8152.c                                | 30 ++++-------------
 include/linux/genetlink.h                              |  4 +++
 include/net/genetlink.h                                |  7 +++-
 include/uapi/linux/can/netlink.h                       |  1 +
 net/core/dev.c                                         | 20 +++++++++---
 net/ipv4/ip_sockglue.c                                 |  8 ++---
 net/ipv6/datagram.c                                    | 10 ++----
 net/ipv6/route.c                                       |  7 ++--
 net/mac80211/mlme.c                                    |  2 +-
 net/netlink/af_netlink.c                               | 24 ++++++++++----
 net/netlink/af_netlink.h                               |  1 +
 net/netlink/genetlink.c                                | 18 +++++-----
 net/sctp/socket.c                                      |  8 ++++-
 net/wireless/reg.c                                     | 56 ++++++++++++++++++-------------
 31 files changed, 343 insertions(+), 192 deletions(-)

^ permalink raw reply

* Re: [PATCH] i40e: don't enable and init FCOE by default when do PF reset
From: ethan zhao @ 2015-01-20  2:05 UTC (permalink / raw)
  To: Dev, Vasu
  Cc: e1000-devel@lists.sourceforge.net, brian.maly@oracle.com,
	Allan, Bruce W, Brandeburg, Jesse, Parikh, Neerav, Linux NICS,
	Ronciak, John, netdev@vger.kernel.org, Ethan Zhao,
	linux-kernel@vger.kernel.org
In-Reply-To: <933BEC2E04D6A5458F4B0239FB547F9A34CCE42B@fmsmsx118.amr.corp.intel.com>


On 2015/1/20 5:10, Dev, Vasu wrote:
>> -----Original Message-----
>> From: ethan zhao [mailto:ethan.zhao@oracle.com]
>> Sent: Friday, January 16, 2015 7:01 PM
>> To: Kirsher, Jeffrey T
>> Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W;
>> Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
>> Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
>> devel@lists.sourceforge.net; netdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org; brian.maly@oracle.com
>> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
>> reset
>>
>> Vasu,
>>
>>     What' your idea about the v2, any suggestion ?  Jeff is looking forward to
>> see it.
>>
> Jeff was asking for v2 in response to your last comment as "disable FCOE as default configuration as a temporary step" but I think that is the fix and user should n't enable FCoE until they have FCoE enabled X710 FCoE with either fabric or VN2VN mode FCoE setup.
  As a Linux distro, we don't know users have FCoE capable X710 or not, 
so we couldn't disable the FCoE configuration
  by default in the released kernel except FCoE is officially not 
supported yet with X710, but if yes, fix those bugs I
  mentioned is the only choice.


Thanks,
Ethan


>
> Thanks,
> Vasu
>
>> Thanks,
>> Ethan
>>
>>
>> On 2015/1/16 22:47, Jeff Kirsher wrote:
>>> On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
>>>> Vasu,
>>>>
>>>>       OK, disable FCOE as default configuration as a temporary step to
>>>> make it  work.
>>> Sounds like I should expect a v2 coming, correct?
>>>
>>>> Thanks,
>>>> Ethan
>>>>
>>>> On 2015/1/16 7:45, Dev, Vasu wrote:
>>>>>> -----Original Message-----
>>>>>> From: ethan zhao [mailto:ethan.zhao@oracle.com]
>>>>>> Sent: Tuesday, January 13, 2015 6:41 PM
>>>>>> To: Dev, Vasu
>>>>>> Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
>>>>>> Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose,
>>>>>> Gregory V; Vick, Matthew; Williams, Mitch A; Parikh, Neerav; Linux
>>>>>> NICS; e1000- devel@lists.sourceforge.net; netdev@vger.kernel.org;
>>>>>> linux- kernel@vger.kernel.org; brian.maly@oracle.com
>>>>>> Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
>>>>>> when do PF reset
>>>>>>
>>>>>> Vasu,
>>>>>>
>>>>>> On 2015/1/14 3:38, Dev, Vasu wrote:
>>>>>>>> -----Original Message-----
>>>>>>>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>>>>>>>>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>>>>>>>>> index a5f2660..a2572cc 100644
>>>>>>>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>>>>>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>>>>>>>>> @@ -6180,9 +6180,12 @@ static void
>>>>>>>>>>> i40e_reset_and_rebuild(struct i40e_pf *pf, bool reinit)
>>>>>>>>>>>         }
>>>>>>>>>>>      #endif /* CONFIG_I40E_DCB */
>>>>>>>>>>>      #ifdef I40E_FCOE
>>>>>>>>>>> -   ret = i40e_init_pf_fcoe(pf);
>>>>>>>>>>> -   if (ret)
>>>>>>>>>>> -           dev_info(&pf->pdev->dev, "init_pf_fcoe failed: %d\n", ret);
>>>>>>>>>>> +   if (pf->flags & I40E_FLAG_FCOE_ENABLED) {
>>>>>>>>>>> +           ret = i40e_init_pf_fcoe(pf);
>>>>>>>>> Calling i40e_init_pf_fcoe() here conflicts with its
>>>>>>>> I40E_FLAG_FCOE_ENABLED pre-condition since
>>>>>> I40E_FLAG_FCOE_ENABLED is
>>>>>>>> set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
>>>>>>>> will never get called.
>>>>>>>>
>>>>>>>> I don't think so,  here ,i40e_reset_and_rebuild()  is not the
>>>>>>>> only and the first place that  i40e_init_pf_fcoe() is called, see
>>>>>>>> i40e_probe(), that is the first chance.
>>>>>>>>
>>>>>>>> i40e_probe()
>>>>>>>> -->i40e_sw_init()
>>>>>>>>          -->i40e_init_pf_fcoe()
>>>>>>>>
>>>>>>>> And the I40E_FLAG_FCOE_ENABLED is possible be set by
>>>>>>>> i40e_fcoe_enable() or i40e_fcoe_disable() interface before the
>>>>>>>> reset action is to be done.
>>>>>>>>
>>>>>>> It is set by i40e_init_pf_fcoe() and you are right that the
>>>>>>> modified call flow
>>>>>> by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED
>> anyway
>>>>>> which could have prevented calling i40e_init_pf_fcoe() as I
>>>>>> described above, so this is not an issue with the patch.
>>>>>>>> BTW, the reason I post this patch is that we hit a bug, after
>>>>>>>> setup vlan, the PF is enabled to FCOE.
>>>>>>>>
>>>>>>> Then that BUG would still remain un-fixed and calling
>>>>>>> i40e_init_pf_fcoe()
>>>>>> under I40E_FLAG_FCOE_ENABLED  flag really won't affect call flow to
>>>>>> fix anything. I mean I40E_FLAG_FCOE_ENABLED  condition will be true
>>>>>> with "pf-
>>>>>>> hw.func_caps.fcoe == true" and otherwise calling
>>>>>>> i40e_init_pf_fcoe() simply
>>>>>> returns back early on after checking "pf->hw.func_caps.fcoe ==
>>>>>> false", so how that bug is fixed here by added
>> I40E_FLAG_FCOE_ENABLED  condition ?
>>>>>> What is the bug ?
>>>>>>      The func_caps.fcoe is assigned by following call path, under
>>>>>> our test environment,
>>>>>>
>>>>>>      i40e_probe()
>>>>>>       ->i40e_get_capabilities()
>>>>>>          ->i40e_aq_discover_capabilities()
>>>>>>             ->i40e_parse_discover_capabilities()
>>>>>>
>>>>>>      Or
>>>>>>
>>>>>>      i40e_reset_and_rebuild()
>>>>>>       ->i40e_get_capabilities()
>>>>>>         ->i40e_aq_discover_capabilities()
>>>>>>           ->i40e_parse_discover_capabilities()
>>>>>>
>>>>>>      Under our test environment, the "pf->hw.func_caps.fcoe" is
>>>>>> true. so if
>>>>>> i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic
>> test.
>>>>>>      And then i40e_init_pf_fcoe() is to be called,
>>>>>>
>>>>>>      While if (!pf->hw.func_caps.fcoe) wouldn't return,
>>>>>>
>>>>> I said it would return with "pf->hw.func_caps.fcoe == false" in my last
>> response, more details below.
>>>>>>      So  pf->flags is set to I40E_FLAG_FCOE_ENABLED.
>>>>>>
>>>>>>      With my patch,  i40e_init_pf_fcoe() is only called after
>>>>>> I40E_FLAG_FCOE_ENABLED is set before reset.
>>>>>>
>>>>>> Enable FCOE in i40e_probe() or not is another issue.
>>>>>>
>>>>> Nope since both cases we should do i40e_init_pf_fcoe() or don't based
>> on fcoe cap true or false.
>>>>> I don't have much to add as I described before with the your patch that
>> "calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED  flag really
>> won't affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED
>> condition will be true with "pf->hw.func_caps.fcoe == true" and otherwise
>> calling i40e_init_pf_fcoe() simply returns back early on after checking "pf-
>>> hw.func_caps.fcoe == false".
>>>>> May be I'm missing something, I guess next either go with
>> CONFIG_I40E_FCOE disable as I suggested before and now it in upstream
>> kernel or we can have further off list discussion to fix the issue you are trying
>> to fix with the patch.
>>>>> Thanks,
>>>>> Vasu
>>>>>
>>>>>> Thanks,
>>>>>> Ethan
>>>>>>
>>>>>>
>>>>>>>>> Jeff Kirsher should be getting out a patch queued by me which
>>>>>>>>> adds
>>>>>>>> I40E_FCoE Kbuild option, in that FCoE is disabled by default and
>>>>>>>> user could enable FCoE only if needed, that patch would do same
>>>>>>>> of skipping
>>>>>>>> i40e_init_pf_fcoe() whether FCoE capability in device enabled or
>>>>>>>> not in default config.
>>>>>>>> The following patch will not fix the above issue -- configuration
>>>>>>>> of PF will be changed via reset.
>>>>>>>> How about the FCOE is configured and disabled by
>>>>>>>> i40e_fcoe_disable() , then reset happens ?
>>>>>>>>
>>>>>>> May be but if the BUG is due to FCoE being enabled then having it
>>>>>>> disabled
>>>>>> in config will avoid the bug for non FCoE config option and once
>>>>>> bug is understood then that has to be fixed for FCoE enabled config
>>>>>> also as I asked above.
>>>>>>> Thanks Ethan for detailed response.
>>>>>>> Vasu
>>>>>>>
>>>>>>>>>     From patchwork Wed Oct  2 23:26:08 2013
>>>>>>>>> Content-Type: text/plain; charset="utf-8"
>>>>>>>>> MIME-Version: 1.0
>>>>>>>>> Content-Transfer-Encoding: 7bit
>>>>>>>>> Subject: [net] i40e: adds FCoE configure option
>>>>>>>>> Date: Thu, 03 Oct 2013 07:26:08 -0000
>>>>>>>>> From: Vasu Dev <vasu.dev@intel.com>
>>>>>>>>> X-Patchwork-Id: 11797
>>>>>>>>>
>>>>>>>>> Adds FCoE config option I40E_FCOE, so that FCoE can be enabled
>>>>>>>>> as needed but otherwise have it disabled by default.
>>>>>>>>>
>>>>>>>>> This also eliminate multiple FCoE config checks, instead now
>>>>>>>>> just one config check for CONFIG_I40E_FCOE.
>>>>>>>>>
>>>>>>>>> The I40E FCoE was added with 3.17 kernel and therefore this
>>>>>>>>> patch shall be applied to stable 3.17 kernel also.
>>>>>>>>>
>>>>>>>>> CC: <stable@vger.kernel.org>
>>>>>>>>> Signed-off-by: Vasu Dev <vasu.dev@intel.com>
>>>>>>>>> Tested-by: Jim Young <jamesx.m.young@intel.com>
>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>> drivers/net/ethernet/intel/Kconfig           |   11 +++++++++++
>>>>>>>>>      drivers/net/ethernet/intel/i40e/Makefile     |    2 +-
>>>>>>>>>      drivers/net/ethernet/intel/i40e/i40e_osdep.h |    4 ++--
>>>>>>>>>      3 files changed, 14 insertions(+), 3 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/net/ethernet/intel/Kconfig
>>>>>>>>> b/drivers/net/ethernet/intel/Kconfig
>>>>>>>>> index 5b8300a..4d61ef5 100644
>>>>>>>>> --- a/drivers/net/ethernet/intel/Kconfig
>>>>>>>>> +++ b/drivers/net/ethernet/intel/Kconfig
>>>>>>>>> @@ -281,6 +281,17 @@ config I40E_DCB
>>>>>>>>>
>>>>>>>>>               If unsure, say N.
>>>>>>>>>
>>>>>>>>> +config I40E_FCOE
>>>>>>>>> +       bool "Fibre Channel over Ethernet (FCoE)"
>>>>>>>>> +       default n
>>>>>>>>> +       depends on I40E && DCB && FCOE
>>>>>>>>> +       ---help---
>>>>>>>>> +         Say Y here if you want to use Fibre Channel over Ethernet
>> (FCoE)
>>>>>>>>> +         in the driver. This will create new netdev for exclusive FCoE
>>>>>>>>> +         use with XL710 FCoE offloads enabled.
>>>>>>>>> +
>>>>>>>>> +         If unsure, say N.
>>>>>>>>> +
>>>>>>>>>      config I40EVF
>>>>>>>>>             tristate "Intel(R) XL710 X710 Virtual Function Ethernet
>> support"
>>>>>>>>>             depends on PCI_MSI
>>>>>>>>> diff --git a/drivers/net/ethernet/intel/i40e/Makefile
>>>>>>>>> b/drivers/net/ethernet/intel/i40e/Makefile
>>>>>>>>> index 4b94ddb..c405819 100644
>>>>>>>>> --- a/drivers/net/ethernet/intel/i40e/Makefile
>>>>>>>>> +++ b/drivers/net/ethernet/intel/i40e/Makefile
>>>>>>>>> @@ -44,4 +44,4 @@ i40e-objs := i40e_main.o \
>>>>>>>>>             i40e_virtchnl_pf.o
>>>>>>>>>
>>>>>>>>>      i40e-$(CONFIG_I40E_DCB) += i40e_dcb.o i40e_dcb_nl.o
>>>>>>>>> -i40e-$(CONFIG_FCOE:m=y) += i40e_fcoe.o
>>>>>>>>> +i40e-$(CONFIG_I40E_FCOE) += i40e_fcoe.o
>>>>>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_osdep.h
>>>>>>>>> b/drivers/net/ethernet/intel/i40e/i40e_osdep.h
>>>>>>>>> index 045b5c4..ad802dd 100644
>>>>>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_osdep.h
>>>>>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_osdep.h
>>>>>>>>> @@ -78,7 +78,7 @@ do {                                                            \
>>>>>>>>>      } while (0)
>>>>>>>>>
>>>>>>>>>      typedef enum i40e_status_code i40e_status; -#if
>>>>>>>>> defined(CONFIG_FCOE)
>>>>>>>>> || defined(CONFIG_FCOE_MODULE)
>>>>>>>>> +#ifdef CONFIG_I40E_FCOE
>>>>>>>>>      #define I40E_FCOE
>>>>>>>>> -#endif /* CONFIG_FCOE or CONFIG_FCOE_MODULE */
>>>>>>>>> +#endif
>>>>>>>>>      #endif /* _I40E_OSDEP_H_ */
>>>>>>>>>
>>>>>>>>>>> +           if (ret)
>>>>>>>>>>> +                   dev_info(&pf->pdev->dev,
>>>>>>>>>>> +                            "init_pf_fcoe failed: %d\n", ret);
>>>>>>>>>>> +   }
>>>>>>>>>>>
>>>>>>>>>>>      #endif
>>>>>>>>>>>         /* do basic switch setup */
>>>>>>>>>>> --
>>>>>>>>>>> 1.8.3.1
>>>>>>>> Thanks,
>>>>>>>> Ethan


------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH net-next v13 3/3] net: hisilicon: new hip04 ethernet driver
From: Ding Tianhong @ 2015-01-20  2:15 UTC (permalink / raw)
  To: Arnd Bergmann, Alexander Graf
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
	zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-lFZ/pmaqli7XmaaqVzeoHQ
In-Reply-To: <9314032.C8yR0htxiP@wuerfel>

On 2015/1/20 4:34, Arnd Bergmann wrote:
> On Monday 19 January 2015 19:11:11 Alexander Graf wrote:
>>
>> After hammering on the box a bit again, I'm in a situation where I get 
>> lots of
>>
>> [302398.232603] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302398.377309] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302398.395198] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302398.466118] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302398.659009] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302399.053389] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302399.122067] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302399.268192] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302399.286081] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302399.594201] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302399.683416] hip04-ether e28b0000.ethernet eth0: rx drop
>> [302399.701307] hip04-ether e28b0000.ethernet eth0: rx drop
>>
>> and I really am getting a lot of drops - I can't even ping the machine 
>> anymore.
>>
>> However, as it is there's a good chance the machine is simply 
>> unreachable because it's busy writing to the UART, and even if not all 
>> useful messages indicating anything have scrolled out. I really don't 
>> think you should emit any message over and over again to the user. Once 
>> or twice is enough.
>>
>> Please make sure to rate limit it.
> 
> I would argue that packet loss is not an error condition at all
> and you should not print this at netdev_err() level. You could make
> this a netdev_dbg(), or just make it silent because it's already
> counted in the statistics.
> 

I think something wrong with Graf's board, I will try to make it happen on my board, and 
in any case I will add rate limit to xx_drop and change to dbg log level.

Thanks 
Ding

> 	Arnd
> 
> .
> 


--
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: BW regression after "tcp: refine TSO autosizing"
From: Eric Dumazet @ 2015-01-20  2:16 UTC (permalink / raw)
  To: Eyal Perry, Yuchung Cheng, Neal Cardwell
  Cc: Eyal Perry, Or Gerlitz, Linux Netdev List, Amir Vadai,
	Yevgeny Petrilin, Saeed Mahameed, Ido Shamay, Amir Ancel
In-Reply-To: <54BC286B.8000605@mellanox.com>

On Sun, 2015-01-18 at 23:40 +0200, Eyal Perry wrote:

> So indeed, interrupt mitigation (tx-usecs 1 tx-frames 1) improves things up
> for the "refined TSO autosizing" kernel (from 18.4Gbps to 19.7Gbps). but
> in the
> other kernel, the BW is remains the same with and without the coalescing.

OK thanks for testing.

I believe the regression comes from inability for cc to cope with
stretch acks.

Nowadays on fast networks, each ACK packet acknowledges ~45 MSS, but
CUBIC (and others cc) got support for this only during slow start, with
commit 9f9843a751d0a2057f9f3d313886e7e5e6ebaac9
("tcp: properly handle stretch acks in slow start")

I guess it is time to also handle congestion avoidance phase.

With following patch (very close to what we use here at Google) I
reached 37Gbps instead of 20Gbps :

ethtool -C eth1 tx-usecs 4 tx-frames 4

DUMP_TCP_INFO=1 ./netperf -H remote -T2,2 -t TCP_STREAM -l 20
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to remote () port 0 AF_INET : cpu bind
rto=201000 ato=0 pmtu=1500 rcv_ssthresh=29200 rtt=67 rttvar=6 snd_ssthresh=263 cwnd=265 reordering=3 total_retrans=4569 ca_state=0
Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec  

 87380  16384  16384    20.00    37213.05   

I guess this is a world record, my previous one was 34Gbps.


 include/net/tcp.h    |    2 
 net/ipv4/tcp_cong.c  |    4 +
 net/ipv4/tcp_cubic.c |   91 +++++++++++++++++++----------------------
 3 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index b8fdc6bab3f3..05815fbb490f 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -843,7 +843,7 @@ void tcp_get_available_congestion_control(char *buf, size_t len);
 void tcp_get_allowed_congestion_control(char *buf, size_t len);
 int tcp_set_allowed_congestion_control(char *allowed);
 int tcp_set_congestion_control(struct sock *sk, const char *name);
-void tcp_slow_start(struct tcp_sock *tp, u32 acked);
+int tcp_slow_start(struct tcp_sock *tp, u32 acked);
 void tcp_cong_avoid_ai(struct tcp_sock *tp, u32 w);
 
 u32 tcp_reno_ssthresh(struct sock *sk);
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 63c29dba68a8..f0fc696b9333 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -360,13 +360,15 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
  * ABC caps N to 2. Slow start exits when cwnd grows over ssthresh and
  * returns the leftover acks to adjust cwnd in congestion avoidance mode.
  */
-void tcp_slow_start(struct tcp_sock *tp, u32 acked)
+int tcp_slow_start(struct tcp_sock *tp, u32 acked)
 {
 	u32 cwnd = tp->snd_cwnd + acked;
 
 	if (cwnd > tp->snd_ssthresh)
 		cwnd = tp->snd_ssthresh + 1;
+	acked -= cwnd - tp->snd_cwnd;
 	tp->snd_cwnd = min(cwnd, tp->snd_cwnd_clamp);
+	return acked;
 }
 EXPORT_SYMBOL_GPL(tcp_slow_start);
 
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 6b6002416a73..c0e048929b74 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -81,7 +81,6 @@ MODULE_PARM_DESC(hystart_ack_delta, "spacing between ack's indicating train (mse
 
 /* BIC TCP Parameters */
 struct bictcp {
-	u32	cnt;		/* increase cwnd by 1 after ACKs */
 	u32	last_max_cwnd;	/* last maximum snd_cwnd */
 	u32	loss_cwnd;	/* congestion window at last loss */
 	u32	last_cwnd;	/* the last snd_cwnd */
@@ -93,20 +92,18 @@ struct bictcp {
 	u32	epoch_start;	/* beginning of an epoch */
 	u32	ack_cnt;	/* number of acks */
 	u32	tcp_cwnd;	/* estimated tcp cwnd */
-#define ACK_RATIO_SHIFT	4
-#define ACK_RATIO_LIMIT (32u << ACK_RATIO_SHIFT)
-	u16	delayed_ack;	/* estimate the ratio of Packets/ACKs << 4 */
 	u8	sample_cnt;	/* number of samples to decide curr_rtt */
 	u8	found;		/* the exit point is found? */
 	u32	round_start;	/* beginning of each round */
 	u32	end_seq;	/* end_seq of the round */
 	u32	last_ack;	/* last time when the ACK spacing is close */
 	u32	curr_rtt;	/* the minimum rtt of current round */
+	u32	last_bic_target;/* last target cwnd computed by cubic
+				 * (not tcp_friendliness mode) */
 };
 
 static inline void bictcp_reset(struct bictcp *ca)
 {
-	ca->cnt = 0;
 	ca->last_max_cwnd = 0;
 	ca->last_cwnd = 0;
 	ca->last_time = 0;
@@ -114,7 +111,6 @@ static inline void bictcp_reset(struct bictcp *ca)
 	ca->bic_K = 0;
 	ca->delay_min = 0;
 	ca->epoch_start = 0;
-	ca->delayed_ack = 2 << ACK_RATIO_SHIFT;
 	ca->ack_cnt = 0;
 	ca->tcp_cwnd = 0;
 	ca->found = 0;
@@ -205,12 +201,14 @@ static u32 cubic_root(u64 a)
 /*
  * Compute congestion window to use.
  */
-static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
+static inline void bictcp_update(struct bictcp *ca, u32 pkts_acked, u32 cwnd)
 {
-	u32 delta, bic_target, max_cnt;
+	u32 delta, bic_target;
 	u64 offs, t;
 
-	ca->ack_cnt++;	/* count the number of ACKs */
+	ca->ack_cnt += pkts_acked;	/* count the number of packets that
+					 * have been ACKed
+					 */
 
 	if (ca->last_cwnd == cwnd &&
 	    (s32)(tcp_time_stamp - ca->last_time) <= HZ / 32)
@@ -221,7 +219,7 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
 
 	if (ca->epoch_start == 0) {
 		ca->epoch_start = tcp_time_stamp;	/* record beginning */
-		ca->ack_cnt = 1;			/* start counting */
+		ca->ack_cnt = pkts_acked;		/* start counting */
 		ca->tcp_cwnd = cwnd;			/* syn with cubic */
 
 		if (ca->last_max_cwnd <= cwnd) {
@@ -269,19 +267,7 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
 	else                                          /* above origin*/
 		bic_target = ca->bic_origin_point + delta;
 
-	/* cubic function - calc bictcp_cnt*/
-	if (bic_target > cwnd) {
-		ca->cnt = cwnd / (bic_target - cwnd);
-	} else {
-		ca->cnt = 100 * cwnd;              /* very small increment*/
-	}
-
-	/*
-	 * The initial growth of cubic function may be too conservative
-	 * when the available bandwidth is still unknown.
-	 */
-	if (ca->last_max_cwnd == 0 && ca->cnt > 20)
-		ca->cnt = 20;	/* increase cwnd 5% per RTT */
+	ca->last_bic_target = bic_target;
 
 	/* TCP Friendly */
 	if (tcp_friendliness) {
@@ -292,18 +278,7 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
 			ca->ack_cnt -= delta;
 			ca->tcp_cwnd++;
 		}
-
-		if (ca->tcp_cwnd > cwnd) {	/* if bic is slower than tcp */
-			delta = ca->tcp_cwnd - cwnd;
-			max_cnt = cwnd / delta;
-			if (ca->cnt > max_cnt)
-				ca->cnt = max_cnt;
-		}
 	}
-
-	ca->cnt = (ca->cnt << ACK_RATIO_SHIFT) / ca->delayed_ack;
-	if (ca->cnt == 0)			/* cannot be zero */
-		ca->cnt = 1;
 }
 
 static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked)
@@ -314,13 +289,43 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked)
 	if (!tcp_is_cwnd_limited(sk))
 		return;
 
+	/* cwnd may first advance in slow start then move on to congestion
+	 * control mode on a stretch ACK.
+	 */
 	if (tp->snd_cwnd <= tp->snd_ssthresh) {
 		if (hystart && after(ack, ca->end_seq))
 			bictcp_hystart_reset(sk);
-		tcp_slow_start(tp, acked);
-	} else {
-		bictcp_update(ca, tp->snd_cwnd);
-		tcp_cong_avoid_ai(tp, ca->cnt);
+		acked = tcp_slow_start(tp, acked);
+	}
+
+	if (acked && tp->snd_cwnd > tp->snd_ssthresh) {
+		u32 target, cnt;
+
+		bictcp_update(ca, acked, tp->snd_cwnd);
+		/* Compute target cwnd based on bic_target and tcp_cwnd
+		 * (whichever is faster)
+		 */
+		target = (ca->last_bic_target >= ca->tcp_cwnd) ?
+				ca->last_bic_target : ca->tcp_cwnd;
+		while (acked > 0) {
+			if (target > tp->snd_cwnd)
+				cnt = tp->snd_cwnd / (target - tp->snd_cwnd);
+			else
+				cnt = 100 * tp->snd_cwnd;
+
+			/* The initial growth of cubic function may be
+			 * too conservative when the available
+			 * bandwidth is still unknown.
+			 */
+			if (ca->last_max_cwnd == 0 && cnt > 20)
+				cnt = 20;   /* increase cwnd 5% per RTT */
+
+			if (cnt == 0)		/* cannot be zero */
+				cnt = 1;
+
+			tcp_cong_avoid_ai(tp, cnt);
+			acked--;
+		}
 	}
 }
 
@@ -411,20 +416,10 @@ static void hystart_update(struct sock *sk, u32 delay)
  */
 static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt_us)
 {
-	const struct inet_connection_sock *icsk = inet_csk(sk);
 	const struct tcp_sock *tp = tcp_sk(sk);
 	struct bictcp *ca = inet_csk_ca(sk);
 	u32 delay;
 
-	if (icsk->icsk_ca_state == TCP_CA_Open) {
-		u32 ratio = ca->delayed_ack;
-
-		ratio -= ca->delayed_ack >> ACK_RATIO_SHIFT;
-		ratio += cnt;
-
-		ca->delayed_ack = clamp(ratio, 1U, ACK_RATIO_LIMIT);
-	}
-
 	/* Some calls are for duplicates without timetamps */
 	if (rtt_us < 0)
 		return;

^ permalink raw reply related

* Re: net: prevent of emerging cross-namespace symlinks patches for 3.14?
From: David Miller @ 2015-01-20  2:19 UTC (permalink / raw)
  To: mikevs; +Cc: git.user, netdev, stable
In-Reply-To: <54BC3EF2.5080403@xs4all.net>

From: Miquel van Smoorenburg <mikevs@xs4all.net>
Date: Mon, 19 Jan 2015 00:17:06 +0100

> On 15/01/15 19:39, Alexander Y. Fomichev wrote:
>> On Thu, Jan 15, 2015 at 12:45 AM, Miquel van Smoorenburg
>> mikevs@xs4all.net> wrote:
>>> [first sent to lkml, now to netdev and the original patch author]
>>>
>>> When running 'lxc' on the latest -stable kernel, 3.14.28, I'm seeing
>>> these
>>> errors:
>>>
>>> Jan 14 17:47:16 lxc2 kernel: [ 10.704892] sysfs: cannot create
>>> duplicate
>>> filename '/devices/virtual/net/eth0.104/upper_eth0'
>>> I did not see these errors in 3.12. This was fixed in 3.17
>>
>> Hi,
>>
>> no objections of course,
>> actually it was written and tested with 3.14 in mind.
> 
> David, could you have a quick look and ack this if you agree this
> should go in 3.14-stable ?

Yes, this is fine, thanks for asking.

^ permalink raw reply

* Re: [PATCH] net: ipv4: Fix incorrect free in ICMP receive
From: subashab @ 2015-01-20  2:34 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20150116.164830.172588830692811414.davem@davemloft.net>

Thanks David and Eric for the insights. In order for me to steer this
debug in the right direction, can you please help me? Based on your input
I looked into this a little deeper to understand the refcnts for sockets
and skb's in this ping receive path.

from ping_rcv()

                sk = ping_lookup(net, skb, ntohs(icmph->un.echo.id));
                if (sk != NULL) {
                                pr_debug("rcv on socket %p\n", sk);
                                ping_queue_rcv_skb(sk, skb_get(skb));
                                sock_put(sk);
                                return;
                }

>From my understanding I have made the following analysis, please correct
if I am wrong.

1) There is no guarantee that sock_put() in the above code snippet
will not drop the socket refcount to 0 and free the socket. This can
hypothetically happen if say
sock_close()->ping_close()->*->ping_unhash()->sock_put()
can  happen between in a different context between ping_lookup() and
sock_put() in the above code snippet. Is this observation accurate?

2)  Now since this socket is being freed in the ping receive path, I think
the following is what is happening with the skb.
alloc_skb()[skb->users=1]   -> deliver_skb()[skb->users=2]  -> * ->
icmp_rcv() -> ping_rcv() -> sk_free --> inet_sock_destruct()->
__skb_queue_purge()->kfree_skb()[dec ref cnt, skb->users=1]

when stack unwinds to icmp_rcv(), refcnt actually hits zero and packet is
freed calling the destructor which tries to access the freed socket.

If these observations are right, Can you please tell me what is the call
flow that is not supposed to happen but is happening in this issue? I am
trying to understand better to identify next steps to tackle this issue.

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project

> From: subashab@codeaurora.org
> Date: Fri, 16 Jan 2015 20:59:14 -0000
>
>>>skb_queue_purge() is also calling skb_orphan() on all skb
>> From my reading, it looked like skb_queue_purge() is dequeuing and
>> calling
>> kfree_skb() which will release a reference. I did not see skb_orphan()
>> being called directly. Am I missing something?
>> I think that if it had really orphaned the skb, then this crash would
>> not
>> be seen in the first place.
>
> The calls to skb->destructor(), done by skb_queue_purge() (via
> kfree_skb()) do this.
>
> But even if it didn't, the fact remains that we are operating on the
> socket right here in the destructor.  It still exists and has not been
> freed yet.
>
> And furthermore, exactly what skb_orphan() does is call skb->destructor(),
> _JUST_LIKE_ skb_queue_purge() will via kfree_skb().
>
> So either sock_rfree() is safe to call here, or it isn't.  You are not
> eliminating the calls to sock_rfree() which operate on this socket at
> all.  If you did, then the socket memory counters would end up being
> corrupts and the warnings would trigger:
>
> 	WARN_ON(atomic_read(&sk->sk_rmem_alloc));
> 	WARN_ON(atomic_read(&sk->sk_wmem_alloc));
>
> You're just moving the skb->destructor() call up a few lines in the
> same function, it makes therefore no sense why this would fix a bug
> or not.
> --
> 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: BW regression after "tcp: refine TSO autosizing"
From: Dave Taht @ 2015-01-20  2:37 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Eyal Perry, Yuchung Cheng, Neal Cardwell, Eyal Perry, Or Gerlitz,
	Linux Netdev List, Amir Vadai, Yevgeny Petrilin, Saeed Mahameed,
	Ido Shamay, Amir Ancel
In-Reply-To: <1421720216.11734.188.camel@edumazet-glaptop2.roam.corp.google.com>

On Mon, Jan 19, 2015 at 6:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sun, 2015-01-18 at 23:40 +0200, Eyal Perry wrote:
>
>> So indeed, interrupt mitigation (tx-usecs 1 tx-frames 1) improves things up
>> for the "refined TSO autosizing" kernel (from 18.4Gbps to 19.7Gbps). but
>> in the
>> other kernel, the BW is remains the same with and without the coalescing.
>
> OK thanks for testing.
>
> I believe the regression comes from inability for cc to cope with
> stretch acks.
>
> Nowadays on fast networks, each ACK packet acknowledges ~45 MSS, but
> CUBIC (and others cc) got support for this only during slow start, with
> commit 9f9843a751d0a2057f9f3d313886e7e5e6ebaac9
> ("tcp: properly handle stretch acks in slow start")
>
> I guess it is time to also handle congestion avoidance phase.

Are you saying that at long last, delayed acks as we knew them are
dead, dead, dead?

> With following patch (very close to what we use here at Google) I
> reached 37Gbps instead of 20Gbps :
>
> ethtool -C eth1 tx-usecs 4 tx-frames 4

What is the default here?

What happens with the default here?

>
> DUMP_TCP_INFO=1 ./netperf -H remote -T2,2 -t TCP_STREAM -l 20
> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to remote () port 0 AF_INET : cpu bind
> rto=201000 ato=0 pmtu=1500 rcv_ssthresh=29200 rtt=67 rttvar=6 snd_ssthresh=263 cwnd=265 reordering=3 total_retrans=4569 ca_state=0

The above statistics are not dumped by my netperf, and look extremely
desirable to capture in netperf-wrapper. This is a script parsing some
other kernel data at the conclusion of the run? or a better netperf?

If ECN was on the bottleneck link, I imagine total_retrans would be 0,
or are packets getting dropped in the kernel?

> Recv   Send    Send
> Socket Socket  Message  Elapsed
> Size   Size    Size     Time     Throughput
> bytes  bytes   bytes    secs.    10^6bits/sec
>
>  87380  16384  16384    20.00    37213.05
>
> I guess this is a world record, my previous one was 34Gbps.
>
>
>  include/net/tcp.h    |    2
>  net/ipv4/tcp_cong.c  |    4 +
>  net/ipv4/tcp_cubic.c |   91 +++++++++++++++++++----------------------
>  3 files changed, 47 insertions(+), 50 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index b8fdc6bab3f3..05815fbb490f 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -843,7 +843,7 @@ void tcp_get_available_congestion_control(char *buf, size_t len);
>  void tcp_get_allowed_congestion_control(char *buf, size_t len);
>  int tcp_set_allowed_congestion_control(char *allowed);
>  int tcp_set_congestion_control(struct sock *sk, const char *name);
> -void tcp_slow_start(struct tcp_sock *tp, u32 acked);
> +int tcp_slow_start(struct tcp_sock *tp, u32 acked);
>  void tcp_cong_avoid_ai(struct tcp_sock *tp, u32 w);
>
>  u32 tcp_reno_ssthresh(struct sock *sk);
> diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
> index 63c29dba68a8..f0fc696b9333 100644
> --- a/net/ipv4/tcp_cong.c
> +++ b/net/ipv4/tcp_cong.c
> @@ -360,13 +360,15 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
>   * ABC caps N to 2. Slow start exits when cwnd grows over ssthresh and
>   * returns the leftover acks to adjust cwnd in congestion avoidance mode.
>   */
> -void tcp_slow_start(struct tcp_sock *tp, u32 acked)
> +int tcp_slow_start(struct tcp_sock *tp, u32 acked)
>  {
>         u32 cwnd = tp->snd_cwnd + acked;
>
>         if (cwnd > tp->snd_ssthresh)
>                 cwnd = tp->snd_ssthresh + 1;
> +       acked -= cwnd - tp->snd_cwnd;
>         tp->snd_cwnd = min(cwnd, tp->snd_cwnd_clamp);
> +       return acked;
>  }
>  EXPORT_SYMBOL_GPL(tcp_slow_start);
>
> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 6b6002416a73..c0e048929b74 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -81,7 +81,6 @@ MODULE_PARM_DESC(hystart_ack_delta, "spacing between ack's indicating train (mse
>
>  /* BIC TCP Parameters */
>  struct bictcp {
> -       u32     cnt;            /* increase cwnd by 1 after ACKs */
>         u32     last_max_cwnd;  /* last maximum snd_cwnd */
>         u32     loss_cwnd;      /* congestion window at last loss */
>         u32     last_cwnd;      /* the last snd_cwnd */
> @@ -93,20 +92,18 @@ struct bictcp {
>         u32     epoch_start;    /* beginning of an epoch */
>         u32     ack_cnt;        /* number of acks */
>         u32     tcp_cwnd;       /* estimated tcp cwnd */
> -#define ACK_RATIO_SHIFT        4
> -#define ACK_RATIO_LIMIT (32u << ACK_RATIO_SHIFT)
> -       u16     delayed_ack;    /* estimate the ratio of Packets/ACKs << 4 */
>         u8      sample_cnt;     /* number of samples to decide curr_rtt */
>         u8      found;          /* the exit point is found? */
>         u32     round_start;    /* beginning of each round */
>         u32     end_seq;        /* end_seq of the round */
>         u32     last_ack;       /* last time when the ACK spacing is close */
>         u32     curr_rtt;       /* the minimum rtt of current round */
> +       u32     last_bic_target;/* last target cwnd computed by cubic
> +                                * (not tcp_friendliness mode) */
>  };
>
>  static inline void bictcp_reset(struct bictcp *ca)
>  {
> -       ca->cnt = 0;
>         ca->last_max_cwnd = 0;
>         ca->last_cwnd = 0;
>         ca->last_time = 0;
> @@ -114,7 +111,6 @@ static inline void bictcp_reset(struct bictcp *ca)
>         ca->bic_K = 0;
>         ca->delay_min = 0;
>         ca->epoch_start = 0;
> -       ca->delayed_ack = 2 << ACK_RATIO_SHIFT;
>         ca->ack_cnt = 0;
>         ca->tcp_cwnd = 0;
>         ca->found = 0;
> @@ -205,12 +201,14 @@ static u32 cubic_root(u64 a)
>  /*
>   * Compute congestion window to use.
>   */
> -static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
> +static inline void bictcp_update(struct bictcp *ca, u32 pkts_acked, u32 cwnd)
>  {
> -       u32 delta, bic_target, max_cnt;
> +       u32 delta, bic_target;
>         u64 offs, t;
>
> -       ca->ack_cnt++;  /* count the number of ACKs */
> +       ca->ack_cnt += pkts_acked;      /* count the number of packets that
> +                                        * have been ACKed
> +                                        */
>
>         if (ca->last_cwnd == cwnd &&
>             (s32)(tcp_time_stamp - ca->last_time) <= HZ / 32)
> @@ -221,7 +219,7 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
>
>         if (ca->epoch_start == 0) {
>                 ca->epoch_start = tcp_time_stamp;       /* record beginning */
> -               ca->ack_cnt = 1;                        /* start counting */
> +               ca->ack_cnt = pkts_acked;               /* start counting */
>                 ca->tcp_cwnd = cwnd;                    /* syn with cubic */
>
>                 if (ca->last_max_cwnd <= cwnd) {
> @@ -269,19 +267,7 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
>         else                                          /* above origin*/
>                 bic_target = ca->bic_origin_point + delta;
>
> -       /* cubic function - calc bictcp_cnt*/
> -       if (bic_target > cwnd) {
> -               ca->cnt = cwnd / (bic_target - cwnd);
> -       } else {
> -               ca->cnt = 100 * cwnd;              /* very small increment*/
> -       }
> -
> -       /*
> -        * The initial growth of cubic function may be too conservative
> -        * when the available bandwidth is still unknown.
> -        */
> -       if (ca->last_max_cwnd == 0 && ca->cnt > 20)
> -               ca->cnt = 20;   /* increase cwnd 5% per RTT */
> +       ca->last_bic_target = bic_target;
>
>         /* TCP Friendly */
>         if (tcp_friendliness) {
> @@ -292,18 +278,7 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
>                         ca->ack_cnt -= delta;
>                         ca->tcp_cwnd++;
>                 }
> -
> -               if (ca->tcp_cwnd > cwnd) {      /* if bic is slower than tcp */
> -                       delta = ca->tcp_cwnd - cwnd;
> -                       max_cnt = cwnd / delta;
> -                       if (ca->cnt > max_cnt)
> -                               ca->cnt = max_cnt;
> -               }
>         }
> -
> -       ca->cnt = (ca->cnt << ACK_RATIO_SHIFT) / ca->delayed_ack;
> -       if (ca->cnt == 0)                       /* cannot be zero */
> -               ca->cnt = 1;
>  }
>
>  static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked)
> @@ -314,13 +289,43 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked)
>         if (!tcp_is_cwnd_limited(sk))
>                 return;
>
> +       /* cwnd may first advance in slow start then move on to congestion
> +        * control mode on a stretch ACK.
> +        */
>         if (tp->snd_cwnd <= tp->snd_ssthresh) {
>                 if (hystart && after(ack, ca->end_seq))
>                         bictcp_hystart_reset(sk);
> -               tcp_slow_start(tp, acked);
> -       } else {
> -               bictcp_update(ca, tp->snd_cwnd);
> -               tcp_cong_avoid_ai(tp, ca->cnt);
> +               acked = tcp_slow_start(tp, acked);
> +       }
> +
> +       if (acked && tp->snd_cwnd > tp->snd_ssthresh) {
> +               u32 target, cnt;
> +
> +               bictcp_update(ca, acked, tp->snd_cwnd);
> +               /* Compute target cwnd based on bic_target and tcp_cwnd
> +                * (whichever is faster)
> +                */
> +               target = (ca->last_bic_target >= ca->tcp_cwnd) ?
> +                               ca->last_bic_target : ca->tcp_cwnd;
> +               while (acked > 0) {
> +                       if (target > tp->snd_cwnd)
> +                               cnt = tp->snd_cwnd / (target - tp->snd_cwnd);
> +                       else
> +                               cnt = 100 * tp->snd_cwnd;
> +
> +                       /* The initial growth of cubic function may be
> +                        * too conservative when the available
> +                        * bandwidth is still unknown.
> +                        */
> +                       if (ca->last_max_cwnd == 0 && cnt > 20)
> +                               cnt = 20;   /* increase cwnd 5% per RTT */
> +
> +                       if (cnt == 0)           /* cannot be zero */
> +                               cnt = 1;
> +
> +                       tcp_cong_avoid_ai(tp, cnt);
> +                       acked--;
> +               }
>         }
>  }
>
> @@ -411,20 +416,10 @@ static void hystart_update(struct sock *sk, u32 delay)
>   */
>  static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt_us)
>  {
> -       const struct inet_connection_sock *icsk = inet_csk(sk);
>         const struct tcp_sock *tp = tcp_sk(sk);
>         struct bictcp *ca = inet_csk_ca(sk);
>         u32 delay;
>
> -       if (icsk->icsk_ca_state == TCP_CA_Open) {
> -               u32 ratio = ca->delayed_ack;
> -
> -               ratio -= ca->delayed_ack >> ACK_RATIO_SHIFT;
> -               ratio += cnt;
> -
> -               ca->delayed_ack = clamp(ratio, 1U, ACK_RATIO_LIMIT);
> -       }
> -
>         /* Some calls are for duplicates without timetamps */
>         if (rtt_us < 0)
>                 return;
>
>
> --
> 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



-- 
Dave Täht

http://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks

^ permalink raw reply

* RE: [PATCH net-next 1/7] r8152: adjust rx_bottom
From: Hayes Wang @ 2015-01-20  2:48 UTC (permalink / raw)
  To: David Miller, sfeldma@gmail.com
  Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org
In-Reply-To: <20150119.161333.1471925264489559119.davem@davemloft.net>

 David Miller [mailto:davem@davemloft.net] 
> Sent: Tuesday, January 20, 2015 5:14 AM
[...]
> >> -               r8152_submit_rx(tp, agg, GFP_ATOMIC);
> >> +               if (!ret) {
> >> +                       ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
> >> +               } else {
> >> +                       urb->actual_length = 0;
> >> +                       list_add_tail(&agg->list, next);
> > 
> > Do you need a spin_lock_irqsave(&tp->rx_lock, flags) around this?
> 
> Indeed, and rtl_start_rx() seems to also access agg->list without
> proper locking.

It is unnecessary because I deal with them in a local list_head. My steps are
   1. Move the whole list from tp->rx_done to local rx_queue. (with spin lock)
   2. dequeue/enqueue the lists in rx_queue.
   3. Move the lists in rx_queue to tp->rx_done if it is necessary. (spin lock)
For step 2, it wouldn't have race, because the list_head is local and no other
function would change it. Therefore, I don't think it needs the spin lock.

The rtl_start_rx() also uses the similar way.
 
Best Regards,
Hayes

^ permalink raw reply

* Re: [PATCH net-next 1/7] r8152: adjust rx_bottom
From: David Miller @ 2015-01-20  2:52 UTC (permalink / raw)
  To: hayeswang; +Cc: sfeldma, netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2EE6E76@RTITMBSV03.realtek.com.tw>

From: Hayes Wang <hayeswang@realtek.com>
Date: Tue, 20 Jan 2015 02:48:50 +0000

>> >> +                       urb->actual_length = 0;
>> >> +                       list_add_tail(&agg->list, next);
>> > 
>> > Do you need a spin_lock_irqsave(&tp->rx_lock, flags) around this?
>> 
>> Indeed, and rtl_start_rx() seems to also access agg->list without
>> proper locking.
> 
> It is unnecessary because I deal with them in a local list_head. My steps are
>    1. Move the whole list from tp->rx_done to local rx_queue. (with spin lock)
>    2. dequeue/enqueue the lists in rx_queue.
>    3. Move the lists in rx_queue to tp->rx_done if it is necessary. (spin lock)
> For step 2, it wouldn't have race, because the list_head is local and no other
> function would change it. Therefore, I don't think it needs the spin lock.
> 
> The rtl_start_rx() also uses the similar way.

agg->list is not local, you have to use a spinlock to protect
modifications to it, some other sites which modify agg->list do take
the lock properly.

You cannot modify a list like agg->list without proper locking.

^ permalink raw reply

* Re: [3.19.0-rc4+] rhashtable: BUG kmalloc-2048 (Not tainted): Poison overwritten
From: Ying Xue @ 2015-01-20  2:53 UTC (permalink / raw)
  To: Thomas Graf; +Cc: tipc-discussion, Netdev
In-Reply-To: <20150119125928.GB7672@casper.infradead.org>

On 01/19/2015 08:59 PM, Thomas Graf wrote:
> On 01/19/15 at 04:03pm, Ying Xue wrote:
>> On 3.19.0-rc4+, I encountered below error with attached test
>> case(bind_netlink.c). Please execute the following commands to reproduce
>> the error:
>>
>> gcc -Wall -o bind_netlink bind_netlink.c
>> ./bind_netlink 1000
>>
>> By the way, if we run another test case(bind_tipc.c), the similar issue
>> will happen on TIPC socket.
>>
>> Therefore, it seems that the issue is closely associated with rhashtable
>> instead of specific stacks like netlink or tipc.
> 
> Looks like a RCU read side critical section was missed. Does the TIPC
> poision warning look the same? offset 2048?
> 
> 

TIPC panic log is as below, and it's very similar to netlink.

Regards,
Ying

[   16.825649]
=============================================================================
[   16.826471] BUG TIPC (Not tainted): Poison overwritten
[   16.826941]
-----------------------------------------------------------------------------
[   16.826941]
[   16.827877] INFO: 0xffff880010c805a0-0xffff880010c805a7. First byte
0x33 instead of 0x6b
[   16.828682] INFO: Allocated in sk_prot_alloc+0x48/0x1b0 age=458 cpu=2
pid=4511
[   16.829383] 	__slab_alloc+0x453/0x4ba
[   16.829763] 	kmem_cache_alloc+0x225/0x340
[   16.830161] 	sk_prot_alloc+0x48/0x1b0
[   16.830508] 	sk_alloc+0x39/0x120
[   16.830817] 	tipc_sk_create+0x96/0x420 [tipc]
[   16.831239] 	__sock_create+0x299/0x390
[   16.831593] 	sock_create+0x30/0x40
[   16.831911] 	SyS_socket+0x36/0xb0
[   16.832238] 	compat_SyS_socketcall+0x6b/0x200
[   16.832648] 	sysenter_dispatch+0x7/0x1f
[   16.833028] INFO: Freed in __sk_free+0xdf/0x160 age=7 cpu=0 pid=3
[   16.833607] 	__slab_free+0x32/0x1f1
[   16.833929] 	kmem_cache_free+0x274/0x380
[   16.834319] 	__sk_free+0xdf/0x160
[   16.834634] 	sk_free+0x1d/0x30
[   16.834924] 	tipc_sk_callback+0x24/0x30 [tipc]
[   16.835359] 	rcu_process_callbacks+0x3e2/0x1320
[   16.835786] 	__do_softirq+0xdc/0x650
[   16.836134] 	run_ksoftirqd+0x2d/0x60
[   16.836514] 	smpboot_thread_fn+0x12e/0x1f0
[   16.837035] 	kthread+0xef/0x110
[   16.837351] 	ret_from_fork+0x7c/0xb0
[   16.837691] INFO: Slab 0xffffea0000432000 objects=18 used=0
fp=0xffff880010c83f00 flags=0x100000000004080
[   16.838611] INFO: Object 0xffff880010c80000 @offset=0 fp=0x
(null)
[   16.838611]
[   16.839400] Object ffff880010c80000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.840267] Object ffff880010c80010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.841130] Object ffff880010c80020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.841998] Object ffff880010c80030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.842872] Object ffff880010c80040: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.843752] Object ffff880010c80050: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.844624] Object ffff880010c80060: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.845487] Object ffff880010c80070: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.846506] Object ffff880010c80080: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.847455] Object ffff880010c80090: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.848360] Object ffff880010c800a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.849242] Object ffff880010c800b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.850148] Object ffff880010c800c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.851046] Object ffff880010c800d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.851962] Object ffff880010c800e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.852911] Object ffff880010c800f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.853791] Object ffff880010c80100: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.854681] Object ffff880010c80110: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.855545] Object ffff880010c80120: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.856414] Object ffff880010c80130: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.857280] Object ffff880010c80140: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.858201] Object ffff880010c80150: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.859068] Object ffff880010c80160: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.859928] Object ffff880010c80170: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.860800] Object ffff880010c80180: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.861672] Object ffff880010c80190: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.862579] Object ffff880010c801a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.863480] Object ffff880010c801b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.864357] Object ffff880010c801c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.865234] Object ffff880010c801d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.866146] Object ffff880010c801e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.867023] Object ffff880010c801f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.867904] Object ffff880010c80200: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.868781] Object ffff880010c80210: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.869648] Object ffff880010c80220: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.870556] Object ffff880010c80230: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.871429] Object ffff880010c80240: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.872302] Object ffff880010c80250: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.873179] Object ffff880010c80260: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.874085] Object ffff880010c80270: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.874966] Object ffff880010c80280: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.875862] Object ffff880010c80290: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.876741] Object ffff880010c802a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.877656] Object ffff880010c802b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.878562] Object ffff880010c802c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.879461] Object ffff880010c802d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.880355] Object ffff880010c802e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.881250] Object ffff880010c802f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.882160] Object ffff880010c80300: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.883041] Object ffff880010c80310: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.883999] Object ffff880010c80320: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.884895] Object ffff880010c80330: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.885770] Object ffff880010c80340: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.886673] Object ffff880010c80350: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.887573] Object ffff880010c80360: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.888487] Object ffff880010c80370: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.889384] Object ffff880010c80380: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.890285] Object ffff880010c80390: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.891182] Object ffff880010c803a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.892058] Object ffff880010c803b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.892925] Object ffff880010c803c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.893817] Object ffff880010c803d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.894698] Object ffff880010c803e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.895625] Object ffff880010c803f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.896528] Object ffff880010c80400: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.897424] Object ffff880010c80410: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.898329] Object ffff880010c80420: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.899226] Object ffff880010c80430: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.900155] Object ffff880010c80440: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.901043] Object ffff880010c80450: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.901912] Object ffff880010c80460: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.902830] Object ffff880010c80470: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.903714] Object ffff880010c80480: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.904630] Object ffff880010c80490: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.905526] Object ffff880010c804a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.906428] Object ffff880010c804b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.907321] Object ffff880010c804c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.908221] Object ffff880010c804d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.909109] Object ffff880010c804e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.909991] Object ffff880010c804f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.910877] Object ffff880010c80500: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.911774] Object ffff880010c80510: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.912689] Object ffff880010c80520: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.913601] Object ffff880010c80530: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.914547] Object ffff880010c80540: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.915441] Object ffff880010c80550: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.916334] Object ffff880010c80560: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.917229] Object ffff880010c80570: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.918126] Object ffff880010c80580: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.919009] Object ffff880010c80590: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[   16.919887] Object ffff880010c805a0: 33 02 00 00 00 00 00 00 6b 6b 6b
6b 6b 6b 6b 6b  3.......kkkkkkkk
[   16.920779] Object ffff880010c805b0: 6b 6b 6b 6b 6b 6b 6b a5
                 kkkkkkk.
[   16.921595] Redzone ffff880010c805b8: bb bb bb bb bb bb bb bb
                  ........
[   16.922450] Padding ffff880010c806f8: 5a 5a 5a 5a 5a 5a 5a 5a
                  ZZZZZZZZ
[   16.923380] FIX TIPC: Restoring
0xffff880010c805a0-0xffff880010c805a7=0x6b
[   16.923380]
[   17.033416] BUG: unable to handle kernel paging request at
ffffffffffffff77
[   17.034009] IP: [<ffffffffa0019b8c>] jhash+0xec/0x160 [tipc]
[   17.034009] PGD 2212067 PUD 2214067 PMD 0
[   17.034009] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[   17.034009] Modules linked in: tipc
[   17.034009] CPU: 2 PID: 411 Comm: kworker/2:1 Tainted: G    B
  3.19.0-rc4+ #110
[   17.034009] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[   17.034009] Workqueue: events rht_deferred_worker
[   17.034009] task: ffff880015b8cae0 ti: ffff880015440000 task.ti:
ffff880015440000
[   17.034009] RIP: 0010:[<ffffffffa0019b8c>]  [<ffffffffa0019b8c>]
jhash+0xec/0x160 [tipc]
[   17.034009] RSP: 0018:ffff880015443c18  EFLAGS: 00010293
[   17.034009] RAX: 00000000c910c82d RBX: ffff88000d844000 RCX:
00000000c910c82d
[   17.034009] RDX: 00000000ea63093a RSI: 0000000000000004 RDI:
ffffffffffffff74
[   17.034009] RBP: ffff880015443c18 R08: 00000000c910c82d R09:
ffffffffffffff74
[   17.034009] R10: 0000000000000000 R11: 0000000000000001 R12:
ffff880012b7b6a0
[   17.034009] R13: ffff8800100d31e8 R14: 0000000000000245 R15:
ffff88000d844000
[   17.034009] FS:  0000000000000000(0000) GS:ffff880016200000(0000)
knlGS:0000000000000000
[   17.034009] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   17.034009] CR2: ffffffffffffff77 CR3: 0000000002211000 CR4:
00000000000006e0
[   17.034009] Stack:
[   17.034009]  ffff880015443c38 ffffffff813b38e9 0000000000000175
0000000000000000
[   17.034009]  ffff880015443cc8 ffffffff813b475b ffff88000dc628a0
0000000000000001
[   17.034009]  ffffc90000955368 0000000000000175 ffff88000d844000
ffff8800100d31e8
[   17.034009] Call Trace:
[   17.034009]  [<ffffffff813b38e9>] head_hashfn+0x29/0x50
[   17.034009]  [<ffffffff813b475b>] rhashtable_expand+0x37b/0x680
[   17.034009]  [<ffffffff813b4b38>] rht_deferred_worker+0xd8/0xe0
[   17.034009]  [<ffffffff81074d10>] process_one_work+0x200/0x800
[   17.034009]  [<ffffffff81074c71>] ? process_one_work+0x161/0x800
[   17.034009]  [<ffffffff81074c71>] ? process_one_work+0x161/0x800
[   17.034009]  [<ffffffff8107533c>] process_scheduled_works+0x2c/0x40
[   17.034009]  [<ffffffff810758d3>] worker_thread+0x253/0x4b0
[   17.034009]  [<ffffffff81075680>] ? rescuer_thread+0x330/0x330
[   17.034009]  [<ffffffff8107bb6f>] kthread+0xef/0x110
[   17.034009]  [<ffffffff8107ba80>] ? flush_kthread_work+0x170/0x170
[   17.034009]  [<ffffffff81a44a2c>] ret_from_fork+0x7c/0xb0
[   17.034009]  [<ffffffff8107ba80>] ? flush_kthread_work+0x170/0x170
[   17.034009] Code: 01 d0 41 0f b6 51 07 c1 e2 18 41 01 d0 41 0f b6 51
06 c1 e2 10 41 01 d0 41 0f b6 51 05 c1 e2 08 41 01 d0 41 0f b6 51 04 41
01 d0 <41> 0f b6 51 03 c1 e2 18 01 d1 41 0f b6 51 02 c1 e2 10 01 d1 41
[   17.034009] RIP  [<ffffffffa0019b8c>] jhash+0xec/0x160 [tipc]
[   17.034009]  RSP <ffff880015443c18>
[   17.034009] CR2: ffffffffffffff77
[   17.034009] ---[ end trace 5abb1862651dca6a ]---
[   17.034009] Kernel panic - not syncing: Fatal exception in interrupt
[   17.034009] Kernel Offset: 0x0 from 0xffffffff81000000 (relocation
range: 0xffffffff80000000-0xffffffff9fffffff)
[   17.034009] ---[ end Kernel panic - not syncing: Fatal exception in
interrupt



------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet

^ permalink raw reply

* Re: Re: [PATCH tip 0/9] tracing: attach eBPF programs to tracepoints/syscalls/kprobe
From: Masami Hiramatsu @ 2015-01-20  2:58 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Ingo Molnar, Steven Rostedt, Namhyung Kim,
	Arnaldo Carvalho de Melo, Jiri Olsa, David S. Miller,
	Daniel Borkmann, Hannes Frederic Sowa, Brendan Gregg, Linux API,
	Network Development, LKML, zhangwei(Jovi),
	yrl.pp-manager.tt@hitachi.com
In-Reply-To: <CAMEtUuwzx-HZqEaTS30JFfF_RX5kGaZfsgE8wmogzcb2k5=k1g@mail.gmail.com>

(2015/01/20 5:48), Alexei Starovoitov wrote:
> On Mon, Jan 19, 2015 at 1:52 AM, Masami Hiramatsu
> <masami.hiramatsu.pt@hitachi.com> wrote:
>> If we can write the script as
>>
>> int bpf_prog4(s64 write_size)
>> {
>>    ...
>> }
>>
>> This will be much easier to play with.
> 
> yes. that's the intent for user space to do.
> 
>>>   The example of this arbitrary pointer walking is tracex1_kern.c
>>>   which does skb->dev->name == "lo" filtering.
>>
>> At least I would like to see this way on kprobes event too, since it should be
>> treated as a traceevent.
> 
> it's done already... one can do the same skb->dev->name logic
> in kprobe attached program... so from bpf program point of view,
> tracepoints and kprobes feature-wise are exactly the same.
> Only input is different.

No, I meant that the input should also be same, at least for the first step.
I guess it is easy to hook the ring buffer committing and fetch arguments
from the event entry.

>>> - kprobe programs are architecture dependent and need user scripting
>>>   language like ktap/stap/dtrace/perf that will dynamically generate
>>>   them based on debug info in vmlinux
>>
>> If we can use kprobe event as a normal traceevent, user scripting can be
>> architecture independent too. Only perf-probe fills the gap. All other
>> userspace tools can collaborate with perf-probe to setup the events.
>> If so, we can avoid redundant works on debuginfo. That is my point.
> 
> yes. perf already has infra to read debug info and it can be extended
> to understand C like script as:
> int kprobe:sys_write(int fd, char *buf, size_t count)
> {
>    // do stuff with 'count'
> }
> perf can be made to parse this text, recognize that it wants
> to create kprobe on 'sys_write' function. Then based on
> debuginfo figure out where 'count' is (either register or stack)
> and generate corresponding bpf program either
> using llvm/gcc backends or directly.

And what I expected scenario was

1. setup kprobe traceevent with fd, buf, count by using perf-probe.
2. load bpf module
3. the module processes given event arguments.

> perf facility of extracting debug info can be made into
> library too and used by ktap/dtrace tools for their
> languages.
> User space can innovate in many directions.
> and, yes, once we have a scripting language whether
> it's C like with perf or else, this language hides architecture
> depend things from users.
> Such scripting language will also hide the kernel
> side differences between tracepoint and kprobe.

Hmm, it sounds making another systemtap on top of tracepoint and kprobes.
Why don't you just reuse the existing facilities (perftools and ftrace)
instead of co-exist?

> Just look how ktap scripts look alike for kprobes and tracepoints.

Ktap is a good example, it provides only a language parser and a runtime engine.
Actually, currently it lacks a feature to execute "perf-probe" helper from
script, but it is easy to add such feature.

Jovi, if you hire perf-probe helper, you could do

trace probe:do_sys_open dfd fname flags mode {
...
}

instead of

trace probe:do_sys_open dfd=%di fname=%dx flags=%cx mode=+4($stack) {
...
}

For this usecase, I've made --output option for perf probe
https://lkml.org/lkml/2014/10/31/210

It currently stopped, but easy to resume on the latest perf.

Thank you,

> Whether ktap syntax becomes part of perf or perf invents
> its own language, it's going to be good for users regardless.
> The C examples here are just examples. Something
> users can play with already until more user friendly
> tools are being worked on.


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

^ permalink raw reply

* Re: [PATCH 1/6] selftests: Introduce minimal shared logic for running tests
From: Michael Ellerman @ 2015-01-20  3:08 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kernel, mmarek, gregkh, akpm, rostedt, mingo, davem,
	keescook, tranmanphong, cov, dh.herrmann, hughd, bobby.prani,
	serge.hallyn, ebiederm, tim.bird, josh, koct9i, linux-kbuild,
	linux-api, netdev
In-Reply-To: <54BD332D.6010907@osg.samsung.com>

On Mon, 2015-01-19 at 09:39 -0700, Shuah Khan wrote:
> On 01/18/2015 05:35 PM, Michael Ellerman wrote:
> > On Fri, 2015-01-16 at 10:53 -0700, Shuah Khan wrote:
> >> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
> >>> This adds a Make include file which most selftests can then include to
> >>> get the run_tests logic.
> >>>
> >>> On its own this has the advantage of some reduction in repetition, and
> >>> also means the pass/fail message is defined in fewer places.
> >>>
> >>> However the key advantage is it will allow us to implement install very
> >>> simply in a subsequent patch.
> >>>
> >>> The default implementation just executes each program in $(TEST_PROGS).
> >>>
> >>> We use a variable to hold the default implementation of $(RUN_TESTS)
> >>> because that gives us a clean way to override it if necessary, ie. using
> >>> override. The mount, memory-hotplug and mqueue tests use that to provide
> >>> a different implementation.
> >>>
> >>> Tests are not run via /bin/bash, so if they are scripts they must be
> >>> executable, we add u+x to several.
> >>>
> >>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> >>
> >> I like the shared logic approach in general provided it leaves the
> >> flexibility to not use the shared logic if a test have the need to
> >> do so.
> > 
> > Yes of course it does, it's entirely optional to include lib.mk.
> > 
> >> This series requires some patch planning. shared logic patch
> >> followed by individual test patches as opposed a single patch.
> > 
> > It could be a single patch too, but there's no reason to do it that way. The
> > series works fine as I sent it.
> > 
> >> I would like to see the shared logic work done on top of my patch v4
> >> series.
> > 
> > That's a waste of time. This series replaces your v4. Doing this "on top" of
> > your v4 would just mean reverting your v4 series and then applying this.
> 
> No necessarily if the work is done as evolutionary step. In any case,
> I want the first step install target support going into the upcoming
> release and then make improvements to it. Please send separate patch
> for the shared logic and individual test patches that use the shared
> logic if you would like to make the improvements.

No that's pointless.

My series does everything yours does, and more, and is less code.

It is ready to merge in the next release, you just need to remove your series
and merge it.

I'm happy to change the default install path or change other minor details, but
it's pointless to merge your series and then remove it all to merge mine.

cheers

^ permalink raw reply

* Re: [PATCH 2/6] selftests: Add install target
From: Michael Ellerman @ 2015-01-20  3:13 UTC (permalink / raw)
  To: Shuah Khan
  Cc: linux-kernel, mmarek, gregkh, akpm, rostedt, mingo, davem,
	keescook, tranmanphong, cov, dh.herrmann, hughd, bobby.prani,
	serge.hallyn, ebiederm, tim.bird, josh, koct9i, linux-kbuild,
	linux-api, netdev
In-Reply-To: <54BD326A.6030409@osg.samsung.com>

On Mon, 2015-01-19 at 09:35 -0700, Shuah Khan wrote:
> On 01/18/2015 05:35 PM, Michael Ellerman wrote:
> > On Fri, 2015-01-16 at 10:46 -0700, Shuah Khan wrote:
> >> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
> >>> This adds make install support to selftests. The basic usage is:
> >>>
> >>> $ cd tools/testing/selftests
> >>> $ make install
> >>>
> >>> That installs into tools/testing/selftests/install, which can then be
> >>> copied where ever necessary.
> >>>
> >>> The install destination is also configurable using eg:
> >>>
> >>> $ INSTALL_PATH=/mnt/selftests make install
> >>
> >> Please see my response to [PATCH 4/6] kbuild: add a new
> >> kselftest_install make target to install selftests
> >>
> >> These are addressed by the current approach to use existing
> >> INSTALL_MOD_PATH.
> > 
> > No that's a separate issue.
> > 
> > This patch adds install support for tools/testing/selftests, *completely
> > separate* from the kbuild infrastructure. 
> 
> What's the use-case for this feature? I don't see why we need multiple
> ways to do the install?

Exactly the use case I described in the sentence above.

Currently the selftests directory is usable on its own. You can copy the
selftests directory somewhere and it is functional. That is a useful feature,
and there's no reason to break it.

cheers

^ permalink raw reply

* Re: BW regression after "tcp: refine TSO autosizing"
From: Eric Dumazet @ 2015-01-20  3:14 UTC (permalink / raw)
  To: Dave Taht
  Cc: Eyal Perry, Yuchung Cheng, Neal Cardwell, Eyal Perry, Or Gerlitz,
	Linux Netdev List, Amir Vadai, Yevgeny Petrilin, Saeed Mahameed,
	Ido Shamay, Amir Ancel
In-Reply-To: <CAA93jw42i37s5BJO5QJvFc=JnQBT5JsOX4QDxT6yKY3Ha3J3rg@mail.gmail.com>

On Mon, 2015-01-19 at 18:37 -0800, Dave Taht wrote:
> On Mon, Jan 19, 2015 at 6:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Sun, 2015-01-18 at 23:40 +0200, Eyal Perry wrote:
> >
> >> So indeed, interrupt mitigation (tx-usecs 1 tx-frames 1) improves things up
> >> for the "refined TSO autosizing" kernel (from 18.4Gbps to 19.7Gbps). but
> >> in the
> >> other kernel, the BW is remains the same with and without the coalescing.
> >
> > OK thanks for testing.
> >
> > I believe the regression comes from inability for cc to cope with
> > stretch acks.
> >
> > Nowadays on fast networks, each ACK packet acknowledges ~45 MSS, but
> > CUBIC (and others cc) got support for this only during slow start, with
> > commit 9f9843a751d0a2057f9f3d313886e7e5e6ebaac9
> > ("tcp: properly handle stretch acks in slow start")
> >
> > I guess it is time to also handle congestion avoidance phase.
> 
> Are you saying that at long last, delayed acks as we knew them are
> dead, dead, dead?

Sorry, I can not parse what you are saying.

In case you missed it, it has nothing to do with delayed ACK but GRO on
receiver.


> 
> > With following patch (very close to what we use here at Google) I
> > reached 37Gbps instead of 20Gbps :
> >
> > ethtool -C eth1 tx-usecs 4 tx-frames 4
> 
> What is the default here?

16 & 16, see my prior answer in this thread.

> 
> What happens with the default here?

ethtool -C eth1 tx-usecs 16 tx-frames 16
DUMP_TCP_INFO=1 ./netperf -H remote -T2,2 -t TCP_STREAM -l 20
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to remote
() port 0 AF_INET : cpu bind
rto=201000 ato=0 pmtu=1500 rcv_ssthresh=29200 rtt=60 rttvar=2
snd_ssthresh=179 cwnd=243 reordering=3 total_retrans=23 ca_state=0
Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec  

 87380  16384  16384    20.00    22923.74   




> 
> >
> > DUMP_TCP_INFO=1 ./netperf -H remote -T2,2 -t TCP_STREAM -l 20
> > MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to remote () port 0 AF_INET : cpu bind
> > rto=201000 ato=0 pmtu=1500 rcv_ssthresh=29200 rtt=67 rttvar=6 snd_ssthresh=263 cwnd=265 reordering=3 total_retrans=4569 ca_state=0
> 
> The above statistics are not dumped by my netperf, and look extremely
> desirable to capture in netperf-wrapper. This is a script parsing some
> other kernel data at the conclusion of the run? or a better netperf?

Thats a 3 lines patch in netperf actually.

> 
> If ECN was on the bottleneck link, I imagine total_retrans would be 0,
> or are packets getting dropped in the kernel?

The receiver drops frames, because we are at the limit of what the NIC
can do on a single RX queue.

^ permalink raw reply

* RE: [PATCH net-next 1/7] r8152: adjust rx_bottom
From: Hayes Wang @ 2015-01-20  3:24 UTC (permalink / raw)
  To: David Miller
  Cc: sfeldma@gmail.com, netdev@vger.kernel.org, nic_swsd,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
In-Reply-To: <20150119.215220.720365670558757349.davem@davemloft.net>

 David Miller [mailto:davem@davemloft.net] 
> Sent: Tuesday, January 20, 2015 10:52 AM
[...]
> agg->list is not local, you have to use a spinlock to protect
> modifications to it, some other sites which modify agg->list do take
> the lock properly.
> 
> You cannot modify a list like agg->list without proper locking.

Excuse me. I don't understand.

Before step1
   tp_rx_done->listA->listB->listC->listD->...
   rx_queue->
Because the other function would chage tp->rx_done,
I need move the lists with spin lock.

After step1
   tp_rx_done->
   rx_queue->listA->listB->listC->listD->...

Now I dequeue one of the lists from the list_head and
deal with it.
   tp_rx_done->
   rx_queue->listA->listC->listD->...
                    listB

Then, if I want to put it back to rx_queue, I have to
use spin lock. Why? No other function would chage
rx_queue and the items in it.
 
Best Regards,
Hayes

^ permalink raw reply

* [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Fu, Zhonghui @ 2015-01-20  3:31 UTC (permalink / raw)
  To: brudley, Arend van Spriel, Franky Lin, meuleman, kvalo, linville,
	pieterpg, hdegoede, wens, linux-wireless, brcm80211-dev-list,
	netdev, linux-kernel@vger.kernel.org

>From 04d3fa673897ca4ccbea6c76836d0092dba2484a Mon Sep 17 00:00:00 2001
From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
Date: Tue, 20 Jan 2015 11:14:13 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger
twice suspend/resume operations for one WiFi chip to do
the same things. This patch avoid this case.

Acked-by: Arend van Spriel <arend@broadcom.com>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Acked-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 9880dae..618b545 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1139,11 +1139,17 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
 static int brcmf_ops_sdio_suspend(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
 	mmc_pm_flag_t sdio_flags;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
 
+	if (func->num == 2)
+		return 0;
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	atomic_set(&sdiodev->suspend, true);
 
 	if (sdiodev->wowl_enabled) {
@@ -1164,9 +1170,16 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 static int brcmf_ops_sdio_resume(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
+
+	if (func->num == 2)
+		return 0;
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	if (sdiodev->pdata && sdiodev->pdata->oob_irq_supported)
 		disable_irq_wake(sdiodev->pdata->oob_irq_nr);
 	brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1

^ permalink raw reply related

* Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Fu, Zhonghui @ 2015-01-20  3:34 UTC (permalink / raw)
  To: Kalle Valo, Arend van Spriel
  Cc: brudley, Franky Lin, meuleman, linville, pieterpg, hdegoede, wens,
	linux-wireless, brcm80211-dev-list, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <877fwotddy.fsf@kamboji.qca.qualcomm.com>


On 2015/1/15 20:52, Kalle Valo wrote:
> Arend van Spriel <arend@broadcom.com> writes:
>
>> On 01/12/15 07:41, Fu, Zhonghui wrote:
>>>  From 8685c3c2746b4275fc808d9db23c364b2f54b52a Mon Sep 17 00:00:00 2001
>>> From: Zhonghui Fu<zhonghui.fu@linux.intel.com>
>>> Date: Mon, 12 Jan 2015 14:25:46 +0800
>>> Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
>>>
>>> WiFi chip has 2 SDIO functions, and PM core will trigger
>>> twice suspend/resume operations for one WiFi chip to do
>>> the same things. This patch avoid this case.
>>>
>>> Acked-by: Arend van Spriel<arend@broadcom.com>
>>> Acked-by: Sergei Shtylyov<sergei.shtylyov@cogentembedded.com>
>>> Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com>
>> This patch needs to be rebased.
>>
>> Kalle,
>>
>> Please drop this one.
> Ok, dropped. I'll wait for the rebase.
I have re-based this patch and sent it in another mail with the same subject.

Thanks,
Zhonghui
>

^ permalink raw reply

* Re: [PATCH] net: ipv4: Fix incorrect free in ICMP receive
From: Eric Dumazet @ 2015-01-20  3:50 UTC (permalink / raw)
  To: subashab; +Cc: David Miller, netdev
In-Reply-To: <9dd57106ffab140ee31fb4f0390a87dd.squirrel@www.codeaurora.org>

On Tue, 2015-01-20 at 02:34 +0000, subashab@codeaurora.org wrote:
> Thanks David and Eric for the insights. In order for me to steer this
> debug in the right direction, can you please help me? Based on your input
> I looked into this a little deeper to understand the refcnts for sockets
> and skb's in this ping receive path.
> 
> from ping_rcv()
> 
>                 sk = ping_lookup(net, skb, ntohs(icmph->un.echo.id));
>                 if (sk != NULL) {
>                                 pr_debug("rcv on socket %p\n", sk);
>                                 ping_queue_rcv_skb(sk, skb_get(skb));
>                                 sock_put(sk);
>                                 return;
>                 }
> 
> From my understanding I have made the following analysis, please correct
> if I am wrong.
> 
> 1) There is no guarantee that sock_put() in the above code snippet
> will not drop the socket refcount to 0 and free the socket. This can
> hypothetically happen if say
> sock_close()->ping_close()->*->ping_unhash()->sock_put()
> can  happen between in a different context between ping_lookup() and
> sock_put() in the above code snippet. Is this observation accurate?
> 
> 2)  Now since this socket is being freed in the ping receive path, I think
> the following is what is happening with the skb.
> alloc_skb()[skb->users=1]   -> deliver_skb()[skb->users=2]  -> * ->
> icmp_rcv() -> ping_rcv() -> sk_free --> inet_sock_destruct()->
> __skb_queue_purge()->kfree_skb()[dec ref cnt, skb->users=1]
> 
> when stack unwinds to icmp_rcv(), refcnt actually hits zero and packet is
> freed calling the destructor which tries to access the freed socket.
> 
> If these observations are right, Can you please tell me what is the call
> flow that is not supposed to happen but is happening in this issue? I am
> trying to understand better to identify next steps to tackle this issue.

This is why skb_get() is very often a bug.

There is no guarantee the consume_skb() in icmp_rcv() is done before the
skb_queue_purge().

diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index c0d82f78d364..2a3720fb5a5f 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -966,8 +966,11 @@ bool ping_rcv(struct sk_buff *skb)
 
 	sk = ping_lookup(net, skb, ntohs(icmph->un.echo.id));
 	if (sk != NULL) {
+		struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
+
 		pr_debug("rcv on socket %p\n", sk);
-		ping_queue_rcv_skb(sk, skb_get(skb));
+		if (skb2)
+			ping_queue_rcv_skb(sk, skb2);
 		sock_put(sk);
 		return true;
 	}

^ permalink raw reply related

* Re: Re: [PATCH tip 0/9] tracing: attach eBPF programs to tracepoints/syscalls/kprobe
From: Alexei Starovoitov @ 2015-01-20  3:55 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, Namhyung Kim,
	Arnaldo Carvalho de Melo, Jiri Olsa, David S. Miller,
	Daniel Borkmann, Hannes Frederic Sowa, Brendan Gregg, Linux API,
	Network Development, LKML, zhangwei(Jovi),
	yrl.pp-manager.tt-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org

On Mon, Jan 19, 2015 at 6:58 PM, Masami Hiramatsu
<masami.hiramatsu.pt-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org> wrote:
>>
>> it's done already... one can do the same skb->dev->name logic
>> in kprobe attached program... so from bpf program point of view,
>> tracepoints and kprobes feature-wise are exactly the same.
>> Only input is different.
>
> No, I meant that the input should also be same, at least for the first step.
> I guess it is easy to hook the ring buffer committing and fetch arguments
> from the event entry.

No. That would be very slow. See my comment to Steven
and more detailed numbers below.
Allocating ring buffer takes too much time.

> And what I expected scenario was
>
> 1. setup kprobe traceevent with fd, buf, count by using perf-probe.
> 2. load bpf module
> 3. the module processes given event arguments.

from ring buffer? that's too slow.
It's not usable for high frequency events which
need this in-kernel aggregation.
If events are rare, then just dumping everything
into trace buffer is just fine. No in-kernel program is needed.

> Hmm, it sounds making another systemtap on top of tracepoint and kprobes.
> Why don't you just reuse the existing facilities (perftools and ftrace)
> instead of co-exist?

hmm. I don't think we're on the same page yet...
ring buffer and tracing interface is fully reused.
programs are run as soon as event triggers.
They can return non-zero and kernel will allocate ring
buffer which user space will consume.
Please take a look at tracex1

>> Just look how ktap scripts look alike for kprobes and tracepoints.
>
> Ktap is a good example, it provides only a language parser and a runtime engine.
> Actually, currently it lacks a feature to execute "perf-probe" helper from
> script, but it is easy to add such feature.
...
> For this usecase, I've made --output option for perf probe
> https://lkml.org/lkml/2014/10/31/210

you're proposing to call perf binary from ktap binary?
I think packaging headaches and error conditions
will make such approach very hard to use.
it would be much cleaner to have ktap as part of perf
generating bpf on the fly and feeding into kernel.
'perf probe' parsing and functions don't belong in kernel
when userspace can generate them in more efficient way.

Speaking of performance...
I've added temporary tracepoint like this:
TRACE_EVENT(sys_write,
        TP_PROTO(int count),
        TP_fast_assign(
                __entry->cnt = count;
        ),
and call it from SYSCALL_DEFINE3(write,..., count):
 trace_sys_write(count);

and run the following test:
dd if=/dev/zero of=/dev/null count=5000000

1.19343 s, 2.1 GB/s - raw base line
1.53301 s, 1.7 GB/s - echo 1 > enable
1.62742 s, 1.6 GB/s - echo cnt==1234 > filter
and profile looks like:
     6.23%  dd       [kernel.vmlinux]  [k] __clear_user
     6.19%  dd       [kernel.vmlinux]  [k] __srcu_read_lock
     5.94%  dd       [kernel.vmlinux]  [k] system_call
     4.54%  dd       [kernel.vmlinux]  [k] __srcu_read_unlock
     4.14%  dd       [kernel.vmlinux]  [k] system_call_after_swapgs
     3.96%  dd       [kernel.vmlinux]  [k] fsnotify
     3.74%  dd       [kernel.vmlinux]  [k] ring_buffer_discard_commit
     3.18%  dd       [kernel.vmlinux]  [k] rb_reserve_next_event
     1.69%  dd       [kernel.vmlinux]  [k] rb_add_time_stamp

the slowdown due to unconditional buffer allocation
is too high to use this in production for aggregation
of high frequency events.
There is little reason to run bpf program in kernel after
such penalty. User space can just read trace_pipe_raw
and process data there.

Now if program is run right after tracepoint fires
the profile will look like:
    10.01%  dd             [kernel.vmlinux]            [k] __clear_user
     7.50%  dd             [kernel.vmlinux]            [k] system_call
     6.95%  dd             [kernel.vmlinux]            [k] __srcu_read_lock
     6.02%  dd             [kernel.vmlinux]            [k] __srcu_read_unlock
...
     1.15%  dd             [kernel.vmlinux]            [k]
ftrace_raw_event_sys_write
     0.90%  dd             [kernel.vmlinux]            [k] __bpf_prog_run
this is much more usable.
For empty bpf program that does 'return 0':
1.23418 s, 2.1 GB/s
For full tracex4 example that does map[log2(count)]++
1.2589 s, 2.0 GB/s

so the cost of doing such in-kernel aggregation is
1.19/1.25 is ~ 5%
which makes the whole solution usable as live
monitoring/analytics tool.
We would only need good set of tracepoints.
kprobe via fentry overhead is also not cheap.
Same tracex4 example via kprobe (instead of tracepoint)
1.45673 s, 1.8 GB/s
So tracepoints are 1.45/1.25 ~ 15% faster than kprobes.
which is huge when the cost of running bpf program
is just 5%.

^ permalink raw reply

* Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280
From: Loganaden Velvindron @ 2015-01-20  4:02 UTC (permalink / raw)
  To: Hagen Paul Pfeifer
  Cc: Hannes Frederic Sowa, David Miller, netdev, stable, Fernando Gont
In-Reply-To: <CAPh34mc_5XPqC5_Xdx3gZScfnqtZC7b9JPGp4FfWVh+PvSfb3w@mail.gmail.com>

On Mon, Jan 19, 2015 at 10:05 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> On 19 January 2015 at 21:05, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>
>> Oh yes, although we never exposed an ip route knob for that, it is still
>> possible users did set manually, so we cannot get rid of that, agreed.
>
> I thought about that, sure but come to the conclusion that the code
> cleanup outweigh an potential user somewhere in space (well, the use
> case is broken and should be fixed). Additionally, I left the
> RTAX_FEATURE_ALLFRAG (as mentioned) and removed all related
> functionality - no visible API change (except no-op behavior).
>
> Davem, I will sent the patch anyway. Feel free to accept/ignore.
>
> Hagen
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi.

Last time I was inquiring about depracated atomic fragments, people
were concerned that there wasn't enough practical data to decide
whether to go forward or not.

Would a sysctl with it turned on by default be a good option, until we
are 100% sure ?

Kind regards,
//Logan
C-x-C-c


-- 
This message is strictly personal and the opinions expressed do not
represent those of my employers, either past or present.

^ permalink raw reply

* Re: [PATCH net] ipv6: stop sending PTB packets for MTU < 1280
From: Fernando Gont @ 2015-01-20  4:52 UTC (permalink / raw)
  To: Loganaden Velvindron, Hagen Paul Pfeifer
  Cc: Hannes Frederic Sowa, David Miller, netdev, stable
In-Reply-To: <CAOp4FwRdRai8tL5koFvHKr0P=dR6MgT5ZLtMg85drt6LTzYNrQ@mail.gmail.com>

Hi, Loganaden,

On 01/20/2015 01:02 AM, Loganaden Velvindron wrote:
> 
> Last time I was inquiring about depracated atomic fragments, people
> were concerned that there wasn't enough practical data to decide
> whether to go forward or not.

What kind of practical data?

FWIW,
<https://tools.ietf.org/id/draft-ietf-6man-deprecate-atomfrag-generation-00.txt>
seems to be good enough when it comes to reasons for deprecating them.


Besides, please check Section 5.2 of
<http://www.ietf.org/id/draft-gont-v6ops-ipv6-ehs-in-real-world-01.txt>
-- my "connection" to kernel.org was vulnerable to such attack.



> Would a sysctl with it turned on by default be a good option, until we
> are 100% sure ?

<https://tools.ietf.org/id/draft-ietf-6man-deprecate-atomfrag-generation-00.txt>
 was adopted by the 6man wg last November. While the I-D is not ready an
RFC, and there might be minor modifications, it seems that there's
agreement in not generating atomic fragments.

If you do want to have a sysctl for this, please make it default to "off".

Thanks!

Best regards,
-- 
Fernando Gont
SI6 Networks
e-mail: fgont@si6networks.com
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492

^ permalink raw reply

* [PATCH] wireless: p54: add handling of the signal case
From: Nicholas Mc Guire @ 2015-01-20  5:25 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Kalle Valo, linux-wireless, netdev, linux-kernel,
	Nicholas Mc Guire

if(!wait_for_completion_interruptible_timeout(...))
only handles the timeout case - this patch adds handling the
signal case the same as timeout.

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---

Only the timeout case was being handled, the signal case
(-ERESTARTSYS) was treated just like the case of successful
completion, which is most likely not reasonable.

p54_download_eeprom() is called in p54_read_eeprom() and will 
terminate if p54_download_eeprom() returned != 0 so the logic
should be correct - but this needs a check from someone who
knows the driver. Translating -ETIMEOUT to -EBUSY might be ok
not sure if -ERESTARTSYS also should be returned as -EBUSY ?

Patch was only compild tested with x86_64_defcofnig +
CONFIG_P54_COMMON=m, CONFIG_P54_PCI=m

Patch is against 3.19.0-rc5 -next-20150119

 drivers/net/wireless/p54/fwio.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/p54/fwio.c b/drivers/net/wireless/p54/fwio.c
index bc065e8..5367d51 100644
--- a/drivers/net/wireless/p54/fwio.c
+++ b/drivers/net/wireless/p54/fwio.c
@@ -220,6 +220,7 @@ int p54_download_eeprom(struct p54_common *priv, void *buf,
 	struct sk_buff *skb;
 	size_t eeprom_hdr_size;
 	int ret = 0;
+	long timeout;
 
 	if (priv->fw_var >= 0x509)
 		eeprom_hdr_size = sizeof(*eeprom_hdr);
@@ -249,9 +250,11 @@ int p54_download_eeprom(struct p54_common *priv, void *buf,
 
 	p54_tx(priv, skb);
 
-	if (!wait_for_completion_interruptible_timeout(
-	     &priv->eeprom_comp, HZ)) {
-		wiphy_err(priv->hw->wiphy, "device does not respond!\n");
+	timeout = wait_for_completion_interruptible_timeout(
+			&priv->eeprom_comp, HZ);
+	if (timeout <= 0) {
+		wiphy_err(priv->hw->wiphy,
+			"device does not respond or signal received!\n");
 		ret = -EBUSY;
 	}
 	priv->eeprom = NULL;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH] wireless: p54pci: add handling of signal case
From: Nicholas Mc Guire @ 2015-01-20  5:26 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Kalle Valo, linux-wireless, netdev, linux-kernel,
	Nicholas Mc Guire

if(!wait_for_completion_interruptible_timeout(...))
only handles the timeout case - this patch adds handling the
signal case the same as timeout.

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---

Only the timeout case was being handled, the signal case 
(-ERESTARTSYS) was treated just like the case of successful 
completion, which is most likely not reasonable.

The callsite for p54p_open() in p54p_firmware_step2() expect !=0 to 
indicate error so both -ERESTARTSYS and -ETIMEDOUT should be fine 
for the current handling.

Patch was only compild tested with x86_64_defcofnig +
CONFIG_P54_COMMON=m, CONFIG_P54_PCI=m

Patch is against 3.19.0-rc5 -next-20150119

 drivers/net/wireless/p54/p54pci.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index d4aee64..27a4906 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -431,6 +431,7 @@ static int p54p_open(struct ieee80211_hw *dev)
 {
 	struct p54p_priv *priv = dev->priv;
 	int err;
+	long timeout;
 
 	init_completion(&priv->boot_comp);
 	err = request_irq(priv->pdev->irq, p54p_interrupt,
@@ -468,10 +469,12 @@ static int p54p_open(struct ieee80211_hw *dev)
 	P54P_WRITE(dev_int, cpu_to_le32(ISL38XX_DEV_INT_RESET));
 	P54P_READ(dev_int);
 
-	if (!wait_for_completion_interruptible_timeout(&priv->boot_comp, HZ)) {
+	timeout = wait_for_completion_interruptible_timeout(
+			&priv->boot_comp, HZ);
+	if (timeout <= 0) {
 		wiphy_err(dev->wiphy, "Cannot boot firmware!\n");
 		p54p_stop(dev);
-		return -ETIMEDOUT;
+		return timeout ? -ERESTARTSYS : -ETIMEDOUT;
 	}
 
 	P54P_WRITE(int_enable, cpu_to_le32(ISL38XX_INT_IDENT_UPDATE));
-- 
1.7.10.4

^ permalink raw reply related


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