Netdev List
 help / color / mirror / Atom feed
* pull request (net): ipsec 2017-08-29
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix dst_entry refcount imbalance when using socket policies.
   From Lorenzo Colitti.

2) Fix locking when adding the ESP trailers.

3) Fix tailroom calculation for the ESP trailer by using
   skb_tailroom instead of skb_availroom.

4) Fix some info leaks in xfrm_user.
   From Mathias Krause.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 2b33bc8aa236b75d6e86a8a79126fd9739e4a5bd:

  net: dsa: use consume_skb() (2017-08-23 22:13:34 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to 931e79d7a7ddee4709c56b39de169a36804589a1:

  xfrm_user: fix info leak in build_aevent() (2017-08-28 10:58:02 +0200)

----------------------------------------------------------------
Lorenzo Colitti (1):
      net: xfrm: don't double-hold dst when sk_policy in use.

Mathias Krause (4):
      xfrm_user: fix info leak in copy_user_offload()
      xfrm_user: fix info leak in xfrm_notify_sa()
      xfrm_user: fix info leak in build_expire()
      xfrm_user: fix info leak in build_aevent()

Steffen Klassert (2):
      esp: Fix locking on page fragment allocation
      esp: Fix skb tailroom calculation

 net/ipv4/esp4.c        | 7 ++++---
 net/ipv6/esp6.c        | 7 ++++---
 net/xfrm/xfrm_policy.c | 1 -
 net/xfrm/xfrm_user.c   | 6 +++++-
 4 files changed, 13 insertions(+), 8 deletions(-)

^ permalink raw reply

* [PATCH 1/7] net: xfrm: don't double-hold dst when sk_policy in use.
From: Steffen Klassert @ 2017-08-29 10:31 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1504002694-1931-1-git-send-email-steffen.klassert@secunet.com>

From: Lorenzo Colitti <lorenzo@google.com>

While removing dst_entry garbage collection, commit 52df157f17e5
("xfrm: take refcnt of dst when creating struct xfrm_dst bundle")
changed xfrm_resolve_and_create_bundle so it returns an xdst with
a refcount of 1 instead of 0.

However, it did not delete the dst_hold performed by xfrm_lookup
when a per-socket policy is in use. This means that when a
socket policy is in use, dst entries returned by xfrm_lookup have
a refcount of 2, and are not freed when no longer in use.

Cc: Wei Wang <weiwan@google.com>
Fixes: 52df157f17 ("xfrm: take refcnt of dst when creating struct xfrm_dst bundle")
Tested: https://android-review.googlesource.com/417481
Tested: https://android-review.googlesource.com/418659
Tested: https://android-review.googlesource.com/424463
Tested: https://android-review.googlesource.com/452776 passes on net-next
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Acked-by: Wei Wang <weiwan@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 6f5a0dad..69b16ee 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2226,7 +2226,6 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
 				goto no_transform;
 			}
 
-			dst_hold(&xdst->u.dst);
 			route = xdst->route;
 		}
 	}
-- 
2.7.4

^ permalink raw reply related

* Re: Fwd: DA850-evm MAC Address is random
From: Adam Ford @ 2017-08-29 10:23 UTC (permalink / raw)
  To: Sekhar Nori; +Cc: Tony Lindgren, Grygorii Strashko, linux-omap, netdev
In-Reply-To: <94c4cbb0-1df3-2f2f-69f3-f9a788e610fa@ti.com>

On Tue, Aug 29, 2017 at 3:23 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Tuesday 29 August 2017 02:42 AM, Tony Lindgren wrote:
>> * Adam Ford <aford173@gmail.com> [170828 13:33]:
>>> On Mon, Aug 28, 2017 at 1:54 PM, Grygorii Strashko
>>> <grygorii.strashko@ti.com> wrote:
>>>> Cc: Sekhar
>>>>
>>>> On 08/28/2017 10:32 AM, Adam Ford wrote:
>>>>>
>>>>> The davinvi_emac MAC address seems to attempt a call to
>>>>> ti_cm_get_macid in cpsw-common.c but it returns the message
>>>>> 'davinci_emac davinci_emac.1: incompatible machine/device type for
>>>>> reading mac address ' and then generates a random MAC address.
>>>>>
>>>>> The function appears to lookup varions boards using
>>>>> 'of_machine_is_compaible' and supports dm8148, am33xx, am3517, dm816,
>>>>> am4372 and dra7.  I don't see the ti,davinci-dm6467-emac which is
>>>>> what's shown in the da850 device tree.
>>>>>
>>>>> Is there a patch somewhere for supporting the da850-evm?
>>>>
>>>>
>>>> Not sure if MAC address can be read from Control module.
>>>> May be Sekhar can say more?
>>>
>>> My understanding is that the MAC address is programmed by Logic PD
>>> into the SPI flash.  The Bootloader reads this from either SPI or its
>>> env variables.  Looking at the partition info listed in the
>>> da850-evm.dts file, it appears as if they've reserved space for it.
>>> Unfortunately, I don't see any code that reads it out.  I was hoping
>
> This code is present in U-Boot sources at
> board/davinci/da8xxevm/da850evm.c. See the function get_mac_addr() and
> its usage in misc_init_r().
>
>>> there might be a way to just pass cmdline parameter from the
>>> bootloader to the kernel to accept the MAC address.
>>>
>>>>
>>>>>
>>>>> If not, is there a way to pass the MAC address from U-Boot to the
>>>>> driver so it doesn't generate a random MAC?
>>>>
>>>>
>>>> "local-mac-address" dt porp
>>>
>>> The downside here, is that we'd have to have the Bootloader modify the
>>> device tree.
>>
>> That piece of code exists somewhere in u-boot already. Note how
>
> Yes, it is fdt_fixup_ethernet() and its usage is in common/image-fdt.c.
>
>> we are populating the mac address for USB Ethernet drivers in
>> u-boot and then the Ethernet driver code parses it. See commit
>> 055d31de7158 ("ARM: omap3: beagleboard-xm: dt: Add ethernet to
>> the device tree") for some more information.
>>
>> I think u-boot needs the ethernet alias for finding the interface.
>
> That's exactly what was missing. I have sent a patch for fixing that and
> copied you there.

Thanks for doing that.

>
> Adam, if I can get your Tested-by, I will make an attempt to send it for
> v4.13 itself.

I will test it.  Do need to run some instruction or do something
special in U-Boot to pass this in the proper place for the kernel to
pull it?  Tony's patch reference showed
command for fdt set, but I am not sure I fully understand the
parameters that went along with that.

adam
>
> Thanks,
> Sekhar

^ permalink raw reply

* NFCT writing localhost conntrack entries too
From: Akshat Kakkar @ 2017-08-29 10:23 UTC (permalink / raw)
  To: netdev

With ulog/nflog, NFCT plugin, is it possible to not log entries with src ip
as 127.0.0.1

I can see following options in ulogd.conf
#accept_src_filter=192.168.1.0/24,1:2::/64 # source ip of connection
must belong to these networks
#accept_dst_filter=192.168.1.0/24 # destination ip of connection must
belong to these networks
#accept_proto_filter=tcp,sctp # layer 4 proto of connections


... but what I am looking for is like deny_src_filter.

Is it possible?

^ permalink raw reply

* Re: [PATCH net-next v2] bridge: fdb add and delete tracepoints
From: Nikolay Aleksandrov @ 2017-08-29 10:13 UTC (permalink / raw)
  To: Roopa Prabhu, davem; +Cc: netdev, f.fainelli, bridge
In-Reply-To: <1503980568-35240-1-git-send-email-roopa@cumulusnetworks.com>

On 29/08/17 07:22, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> A few useful tracepoints to trace bridge forwarding
> database updates.
> 
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> v2 - address comments from Florian
> 
>  include/trace/events/bridge.h |   98 +++++++++++++++++++++++++++++++++++++++++
>  net/bridge/br_fdb.c           |    7 +++
>  net/core/net-traces.c         |    6 +++
>  3 files changed, 111 insertions(+)
>  create mode 100644 include/trace/events/bridge.h
> 

Very nice and useful, thanks!

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

^ permalink raw reply

* Re: [PATCH net-next 1/4] net: Add SRIOV VGT+ support
From: Saeed Mahameed @ 2017-08-29 10:13 UTC (permalink / raw)
  To: Sabrina Dubroca
  Cc: Saeed Mahameed, David S. Miller, Linux Netdev List,
	Eugenia Emantayev, Mohamad Haj Yahia, Hannes Frederic Sowa
In-Reply-To: <20170828155217.GA8399@bistromath.localdomain>

On Mon, Aug 28, 2017 at 6:52 PM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> 2017-08-27, 14:06:15 +0300, Saeed Mahameed wrote:
> [...]
>> +#define VF_VLAN_BITMAP       DIV_ROUND_UP(VF_VLAN_N_VID, sizeof(__u64) * BITS_PER_BYTE)
>> +struct ifla_vf_vlan_trunk {
>> +     __u32 vf;
>> +     __u64 allowed_vlans_8021q_bm[VF_VLAN_BITMAP];
>> +     __u64 allowed_vlans_8021ad_bm[VF_VLAN_BITMAP];
>> +};
>
> This is huge (1032B). And you put one of these in the netlink message
> for each VF.  This means that with 51 VF (at least in my environment,
> where each VF takes 1296B), you're going to overflow the u16 size of a
> single attribute (IFLA_VFINFO_LIST), and you cannot dump the device
> anymore. I'm afraid this is going to break existing setups.
>

Yes ! We will fix this,
we are considering to report only a boolean in VFINFO which indecates
if VGT+ is enable or not
and provide a new attribute per VF to report only the vlan list of specific VF.

Thanks for the input,
we will handle this.


> --
> Sabrina

^ permalink raw reply

* [PATCH net-next] staging: irda: force to be a kernel module
From: Greg Kroah-Hartman @ 2017-08-29  9:14 UTC (permalink / raw)
  To: davem, samuel; +Cc: netdev, linux-kernel, devel
In-Reply-To: <20170829070929.GA15824@kroah.com>

Now that the IRDA networking code has moved into drivers/staging/, the
link order is changed for when it is initialized if built into the
system.  This can cause a crash when initializing as the netfilter core
hasn't been initialized yet.

So force the IRDA code to be built as a module, preventing the crash.

Reported-by: kernel test robot <fengguang.wu@intel.com>
Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
---
 drivers/staging/irda/net/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/irda/net/Kconfig b/drivers/staging/irda/net/Kconfig
index 6abeae6c666a..9c6489bcb596 100644
--- a/drivers/staging/irda/net/Kconfig
+++ b/drivers/staging/irda/net/Kconfig
@@ -3,7 +3,7 @@
 #
 
 menuconfig IRDA
-	depends on NET && !S390
+	depends on NET && !S390 && m
 	tristate "IrDA (infrared) subsystem support"
 	select CRC_CCITT
 	---help---
-- 
2.14.1

^ permalink raw reply related

* Atualize o zimbra agora
From: ZIMBRA @ 2017-08-29  8:32 UTC (permalink / raw)


Você deve atualizar agora para o mais recente ZIMBRA! Correio para evitar o encerramento. Uma vez que sua conta foi atualizada, restauraremos seu estado normal.
 
Atualize o Zimbra Agora:-- http://a324543-001-55322111.tripod.com/
 
Saudações,
ZIMBRA! Serviços de correio

^ permalink raw reply

* Re: [PATCH 6/6] power: supply: make device_attribute const
From: Sebastian Reichel @ 2017-08-29  8:44 UTC (permalink / raw)
  To: Bhumika Goyal
  Cc: julia.lawall, rjw, lenb, jbacik, jikos, benjamin.tissoires,
	manish.chopra, rahul.verma, Dept-GELinuxNICDev, harish.patil,
	cascardo, don, dvhart, andy, linux-acpi, linux-kernel,
	linux-block, nbd-general, linux-input, netdev,
	platform-driver-x86, linux-pm
In-Reply-To: <1503315792-14837-7-git-send-email-bhumirks@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

Hi,

On Mon, Aug 21, 2017 at 05:13:12PM +0530, Bhumika Goyal wrote:
> Make these const as they are only passed as an argument to the
> function device_create_file and device_remove_file and the corresponding
> arguments are of type const.
> Done using Coccinelle.
> 
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/olpc_battery.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/olpc_battery.c b/drivers/power/supply/olpc_battery.c
> index fc20ca3..3bc2eea 100644
> --- a/drivers/power/supply/olpc_battery.c
> +++ b/drivers/power/supply/olpc_battery.c
> @@ -559,7 +559,7 @@ static ssize_t olpc_bat_error_read(struct device *dev,
>  	return sprintf(buf, "%d\n", ec_byte);
>  }
>  
> -static struct device_attribute olpc_bat_error = {
> +static const struct device_attribute olpc_bat_error = {
>  	.attr = {
>  		.name = "error",
>  		.mode = S_IRUGO,
> -- 
> 1.9.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v4 4/5] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated
From: Corentin Labbe @ 2017-08-29  8:34 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
	peppe.cavallaro, alexandre.torgue, f.fainelli, icenowy, netdev,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170826212051.GA10418@lunn.ch>

On Sat, Aug 26, 2017 at 11:20:51PM +0200, Andrew Lunn wrote:
> Hi Corentin
> 
> I think we have now all agreed this is an mdio-mux, plus it is also an
> MII mux. We should represent that in device tree. This patchset does
> this. However, as it is now, the mux structure in DT is ignored. All
> it does is search for the phy-is-integrated flags and goes on that.
> 
> I made the comment that the device tree representation cannot be
> implemented using an MDIO mux driver, because of driver loading
> issues.  However, the core of the MDIO mux code is just a library,
> symbols exported as GPL, free for anything to use.
> 
> What i think should happen is the mdio-mux is implemented inside the
> MAC driver, using the mux-core as a library. The device tree structure
> of a mix is then reflected within Linux. The mux switch callback is
> implemented within the MAC driver. So it can reset the MAC when the
> mux is switched. The 'phy-is-integrated' property is then no longer
> needed.

It is stilll needed because some settings (allwinner,leds-active-low for example) are only for integrated phy.

> 
> I would suggest a binding something like:
> 
> emac: ethernet@1c0b000 {
>         compatible = "allwinner,sun8i-h3-emac";
>         syscon = <&syscon>;
>         reg = <0x01c0b000 0x104>;
>         interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
>         interrupt-names = "macirq";
>         resets = <&ccu RST_BUS_EMAC>;
>         reset-names = "stmmaceth";
>         clocks = <&ccu CLK_BUS_EMAC>;
>         clock-names = "stmmaceth";
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         phy-handle = <&int_mii_phy>;
>         phy-mode = "mii";
>         allwinner,leds-active-low;
> 
>         mdio: mdio {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 	}
> 
> 	mdio-mux {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
> 		mdio@0 {
> 			reg = <0>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         int_mii_phy: ethernet-phy@1 {
>                                 reg = <1>;
>                                 clocks = <&ccu CLK_BUS_EPHY>;
>                                 resets = <&ccu RST_BUS_EPHY>;
>                         };
>                 };
>                 ext_mdio: mdio@0 {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         ext_rgmii_phy: ethernet-phy@1 {
>                                 reg = <1>;
>                         };
>                 };
>        };
> };
> 

I am trying to do that but I get:
dwmac-sun8i 1c30000.ethernet: Error: Failed to find reg for child /soc/ethernet@1c30000/mdio
dwmac-sun8i 1c30000.ethernet: Error: Failed to find reg for child /soc/ethernet@1c30000/mdio-mux
dwmac-sun8i 1c30000.ethernet: Error: No acceptable child buses found

So it seems that mdio_mux_init() must be run on mdio-mux and not on emac node.
But in the current state it cannot be done.

Do you agree that another mdio_mux_init() must be written ? (taking a of_node (in our case: mdio-mux) instead of a device)
Or do I miss something ?

Regards

^ permalink raw reply

* Re: Fwd: DA850-evm MAC Address is random
From: Sekhar Nori @ 2017-08-29  8:23 UTC (permalink / raw)
  To: Tony Lindgren, Adam Ford; +Cc: Grygorii Strashko, linux-omap, netdev
In-Reply-To: <20170828211217.GO6008@atomide.com>

On Tuesday 29 August 2017 02:42 AM, Tony Lindgren wrote:
> * Adam Ford <aford173@gmail.com> [170828 13:33]:
>> On Mon, Aug 28, 2017 at 1:54 PM, Grygorii Strashko
>> <grygorii.strashko@ti.com> wrote:
>>> Cc: Sekhar
>>>
>>> On 08/28/2017 10:32 AM, Adam Ford wrote:
>>>>
>>>> The davinvi_emac MAC address seems to attempt a call to
>>>> ti_cm_get_macid in cpsw-common.c but it returns the message
>>>> 'davinci_emac davinci_emac.1: incompatible machine/device type for
>>>> reading mac address ' and then generates a random MAC address.
>>>>
>>>> The function appears to lookup varions boards using
>>>> 'of_machine_is_compaible' and supports dm8148, am33xx, am3517, dm816,
>>>> am4372 and dra7.  I don't see the ti,davinci-dm6467-emac which is
>>>> what's shown in the da850 device tree.
>>>>
>>>> Is there a patch somewhere for supporting the da850-evm?
>>>
>>>
>>> Not sure if MAC address can be read from Control module.
>>> May be Sekhar can say more?
>>
>> My understanding is that the MAC address is programmed by Logic PD
>> into the SPI flash.  The Bootloader reads this from either SPI or its
>> env variables.  Looking at the partition info listed in the
>> da850-evm.dts file, it appears as if they've reserved space for it.
>> Unfortunately, I don't see any code that reads it out.  I was hoping

This code is present in U-Boot sources at
board/davinci/da8xxevm/da850evm.c. See the function get_mac_addr() and
its usage in misc_init_r().

>> there might be a way to just pass cmdline parameter from the
>> bootloader to the kernel to accept the MAC address.
>>
>>>
>>>>
>>>> If not, is there a way to pass the MAC address from U-Boot to the
>>>> driver so it doesn't generate a random MAC?
>>>
>>>
>>> "local-mac-address" dt porp
>>
>> The downside here, is that we'd have to have the Bootloader modify the
>> device tree.
> 
> That piece of code exists somewhere in u-boot already. Note how

Yes, it is fdt_fixup_ethernet() and its usage is in common/image-fdt.c.

> we are populating the mac address for USB Ethernet drivers in
> u-boot and then the Ethernet driver code parses it. See commit
> 055d31de7158 ("ARM: omap3: beagleboard-xm: dt: Add ethernet to
> the device tree") for some more information.
> 
> I think u-boot needs the ethernet alias for finding the interface.

That's exactly what was missing. I have sent a patch for fixing that and
copied you there.

Adam, if I can get your Tested-by, I will make an attempt to send it for
v4.13 itself.

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH 1/4] sgiseeq: switch to dma_alloc_attrs
From: Christoph Hellwig @ 2017-08-29  8:02 UTC (permalink / raw)
  To: David Miller; +Cc: hch, netdev, ralf, linux-mips, linux-parisc, linux-kernel
In-Reply-To: <20170828.154151.502026172655565151.davem@davemloft.net>

On Mon, Aug 28, 2017 at 03:41:51PM -0700, David Miller wrote:
> From: Christoph Hellwig <hch@lst.de>
> Date: Sat, 26 Aug 2017 09:21:22 +0200
> 
> > Use dma_alloc_attrs directly instead of the dma_alloc_noncoherent wrapper.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

I take the Acks as an ok to merges these patches through the dma-mapping
tree.  Thanks Dave!

^ permalink raw reply

* RE: [patch net-next 1/3] idr: Add new APIs to support unsigned long
From: Chris Mi @ 2017-08-29  8:00 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Hannes Frederic Sowa, netdev@vger.kernel.org, jhs@mojatatu.com,
	xiyou.wangcong@gmail.com, davem@davemloft.net,
	mawilcox@microsoft.com
In-Reply-To: <20170829075711.GE1977@nanopsycho.orion>



> -----Original Message-----
> From: Jiri Pirko [mailto:jiri@resnulli.us]
> Sent: Tuesday, August 29, 2017 3:57 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>;
> netdev@vger.kernel.org; jhs@mojatatu.com; xiyou.wangcong@gmail.com;
> davem@davemloft.net; mawilcox@microsoft.com
> Subject: Re: [patch net-next 1/3] idr: Add new APIs to support unsigned long
> 
> Tue, Aug 29, 2017 at 09:34:47AM CEST, chrism@mellanox.com wrote:
> >Hi,
> >
> >> -----Original Message-----
> >> From: Hannes Frederic Sowa [mailto:hannes@stressinduktion.org]
> >> Sent: Tuesday, August 29, 2017 3:14 PM
> >> To: Chris Mi <chrism@mellanox.com>
> >> Cc: netdev@vger.kernel.org; jhs@mojatatu.com;
> >> xiyou.wangcong@gmail.com; jiri@resnulli.us; davem@davemloft.net;
> >> mawilcox@microsoft.com
> >> Subject: Re: [patch net-next 1/3] idr: Add new APIs to support
> >> unsigned long
> >>
> >> Hello,
> >>
> >> Chris Mi <chrism@mellanox.com> writes:
> >>
> >> > The following new APIs are added:
> >> >
> >> > int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
> >> >                   unsigned long start, unsigned long end, gfp_t
> >> > gfp); static inline void *idr_remove_ext(struct idr *idr, unsigned
> >> > long id); static inline void *idr_find_ext(const struct idr *idr,
> >> > unsigned long id); void *idr_replace_ext(struct idr *idr, void
> >> > *ptr, unsigned long id); void *idr_get_next_ext(struct idr *idr,
> >> > unsigned long *nextid);
> >> >
> >> > Signed-off-by: Chris Mi <chrism@mellanox.com>
> >> > Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> >> > ---
> >> >  include/linux/idr.h        | 16 ++++++++++
> >> >  include/linux/radix-tree.h |  3 ++
> >> >  lib/idr.c                  | 56 +++++++++++++++++++++++++++++++++++
> >> >  lib/radix-tree.c           | 73
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >> >  4 files changed, 148 insertions(+)
> >> >
> >>
> >> [...]
> >>
> >> > +int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
> >> > +		  unsigned long start, unsigned long end, gfp_t gfp) {
> >> > +	void __rcu **slot;
> >> > +	struct radix_tree_iter iter;
> >> > +
> >> > +	if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
> >> > +		return -EINVAL;
> >> > +
> >> > +	radix_tree_iter_init(&iter, start);
> >> > +	slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
> >> > +	if (IS_ERR(slot))
> >> > +		return PTR_ERR(slot);
> >> > +
> >> > +	radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
> >> > +	radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
> >> > +
> >> > +	if (index)
> >> > +		*index = iter.index;
> >> > +	return 0;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(idr_alloc_ext);
> >>
> >> Can you express idr_alloc in terms of idr_alloc_ext? Same for most of
> >> the other functions (it seems that signed int was used as return
> >> value to indicate error cases, thus it should be easy to map those).
> >In idr_alloc(), we have the following check:
> >
> >        if (WARN_ON_ONCE(start < 0))
> >                return -EINVAL;
> >
> >But in idr_alloc_ext(), since we are using unsigned long, we needn't such
> check.
> 
> You can just check and call idr_alloc_ext then to do the actual work.
OK, will fix it.

^ permalink raw reply

* Re: [patch net-next 1/3] idr: Add new APIs to support unsigned long
From: Jiri Pirko @ 2017-08-29  7:57 UTC (permalink / raw)
  To: Chris Mi
  Cc: Hannes Frederic Sowa, netdev@vger.kernel.org, jhs@mojatatu.com,
	xiyou.wangcong@gmail.com, davem@davemloft.net,
	mawilcox@microsoft.com
In-Reply-To: <VI1PR0501MB214343F199709BB6EF6EB9B2AB9F0@VI1PR0501MB2143.eurprd05.prod.outlook.com>

Tue, Aug 29, 2017 at 09:34:47AM CEST, chrism@mellanox.com wrote:
>Hi,
>
>> -----Original Message-----
>> From: Hannes Frederic Sowa [mailto:hannes@stressinduktion.org]
>> Sent: Tuesday, August 29, 2017 3:14 PM
>> To: Chris Mi <chrism@mellanox.com>
>> Cc: netdev@vger.kernel.org; jhs@mojatatu.com;
>> xiyou.wangcong@gmail.com; jiri@resnulli.us; davem@davemloft.net;
>> mawilcox@microsoft.com
>> Subject: Re: [patch net-next 1/3] idr: Add new APIs to support unsigned long
>> 
>> Hello,
>> 
>> Chris Mi <chrism@mellanox.com> writes:
>> 
>> > The following new APIs are added:
>> >
>> > int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
>> >                   unsigned long start, unsigned long end, gfp_t gfp);
>> > static inline void *idr_remove_ext(struct idr *idr, unsigned long id);
>> > static inline void *idr_find_ext(const struct idr *idr, unsigned long
>> > id); void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long
>> > id); void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
>> >
>> > Signed-off-by: Chris Mi <chrism@mellanox.com>
>> > Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> > ---
>> >  include/linux/idr.h        | 16 ++++++++++
>> >  include/linux/radix-tree.h |  3 ++
>> >  lib/idr.c                  | 56 +++++++++++++++++++++++++++++++++++
>> >  lib/radix-tree.c           | 73
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> >  4 files changed, 148 insertions(+)
>> >
>> 
>> [...]
>> 
>> > +int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
>> > +		  unsigned long start, unsigned long end, gfp_t gfp) {
>> > +	void __rcu **slot;
>> > +	struct radix_tree_iter iter;
>> > +
>> > +	if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
>> > +		return -EINVAL;
>> > +
>> > +	radix_tree_iter_init(&iter, start);
>> > +	slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
>> > +	if (IS_ERR(slot))
>> > +		return PTR_ERR(slot);
>> > +
>> > +	radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
>> > +	radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
>> > +
>> > +	if (index)
>> > +		*index = iter.index;
>> > +	return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(idr_alloc_ext);
>> 
>> Can you express idr_alloc in terms of idr_alloc_ext? Same for most of the
>> other functions (it seems that signed int was used as return value to indicate
>> error cases, thus it should be easy to map those).
>In idr_alloc(), we have the following check:
>
>        if (WARN_ON_ONCE(start < 0))
>                return -EINVAL;
>
>But in idr_alloc_ext(), since we are using unsigned long, we needn't such check.

You can just check and call idr_alloc_ext then to do the actual work.

^ permalink raw reply

* Re: Question about ip_defrag
From: Florian Westphal @ 2017-08-29  7:53 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Florian Westphal, liujian (CE), davem@davemloft.net,
	kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
	elena.reshetova@intel.com, edumazet@google.com,
	netdev@vger.kernel.org, Wangkefeng (Kevin), weiyongjun (A)
In-Reply-To: <20170829092021.0a46fffa@redhat.com>

Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Mon, 28 Aug 2017 16:00:32 +0200
> Florian Westphal <fw@strlen.de> wrote:
> 
> > liujian (CE) <liujian56@huawei.com> wrote:
> > > Hi
> > > 
> > > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
> > > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> > >   
> > > > > > > the issue:
> > > > > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > > > > At this moment,sum_frag_mem_limit is about 10K.  
> > > 
> > > So should we change ipfrag high/low thresh to a reasonable value ? 
> > > And if it is, is there a standard to change the value?  
> > 
> > Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> > about so with 64 cpus that is ~8 mbyte.
> > 
> > possible solutions:
> > 1. reduce frag_percpu_counter_batch to 16k or so
> > 2. make both low and high thresh depend on NR_CPUS

I take 2) back.  Its wrong to do this, for large NR_CPU values it
would even overflow.

> To me it looks like we/I have been using the wrong API for comparing
> against percpu_counters.  I guess we should have used __percpu_counter_compare().

Are you sure?  For liujian use case (64 cores) it looks like we would
always fall through to percpu_counter_sum() so we eat spinlock_irqsave
cost for all compares.

Before we entertain this we should consider reducing frag_percpu_counter_batch
to a smaller value.

^ permalink raw reply

* Re: [patch net-next 1/3] idr: Add new APIs to support unsigned long
From: Jiri Pirko @ 2017-08-29  7:56 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: Chris Mi, netdev, jhs, xiyou.wangcong, davem, mawilcox
In-Reply-To: <87y3q27sn7.fsf@stressinduktion.org>

Tue, Aug 29, 2017 at 09:14:04AM CEST, hannes@stressinduktion.org wrote:
>Hello,
>
>Chris Mi <chrism@mellanox.com> writes:
>
>> The following new APIs are added:
>>
>> int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
>>                   unsigned long start, unsigned long end, gfp_t gfp);
>> static inline void *idr_remove_ext(struct idr *idr, unsigned long id);
>> static inline void *idr_find_ext(const struct idr *idr, unsigned long id);
>> void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
>> void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
>>
>> Signed-off-by: Chris Mi <chrism@mellanox.com>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>  include/linux/idr.h        | 16 ++++++++++
>>  include/linux/radix-tree.h |  3 ++
>>  lib/idr.c                  | 56 +++++++++++++++++++++++++++++++++++
>>  lib/radix-tree.c           | 73 ++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 148 insertions(+)
>>
>
>[...]
>
>> +int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
>> +		  unsigned long start, unsigned long end, gfp_t gfp)
>> +{
>> +	void __rcu **slot;
>> +	struct radix_tree_iter iter;
>> +
>> +	if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
>> +		return -EINVAL;
>> +
>> +	radix_tree_iter_init(&iter, start);
>> +	slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
>> +	if (IS_ERR(slot))
>> +		return PTR_ERR(slot);
>> +
>> +	radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
>> +	radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
>> +
>> +	if (index)
>> +		*index = iter.index;
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(idr_alloc_ext);
>
>Can you express idr_alloc in terms of idr_alloc_ext? Same for most of
>the other functions (it seems that signed int was used as return value
>to indicate error cases, thus it should be easy to map those).

Agreed. Same for free function.


>
>[...]
>
>Thanks,
>Hannes

^ permalink raw reply

* Re: [patch net-next 11/12] mlxsw: spectrum_dpipe: Add support for IPv4 host table dump
From: Jiri Pirko @ 2017-08-29  7:55 UTC (permalink / raw)
  To: David Ahern
  Cc: Arkadi Sharshevsky, netdev, davem, idosch, mlxsw, roopa,
	Shrijeet Mukherjee
In-Reply-To: <f5bd669b-6a0f-46e9-97b9-7abe73a228a5@gmail.com>

Tue, Aug 29, 2017 at 04:57:12AM CEST, dsahern@gmail.com wrote:
>On 8/27/17 2:31 AM, Arkadi Sharshevsky wrote:
>>> Also, this dpipe capability seems to be just dumping data structures
>>> maintained by the driver. ie., you can compare the mlxsw view of
>>> networking state to IPv4 and IPv6 level tables. Any plans to offer a
>>> command that reads data from the h/w and passes that back to the user?
>>> i.e, a command to compare kernel tables to h/w state?
>>>
>> 
>> So this infra should provide several things-
>> 
>> 1) Reveal the interactions between various hardware tables
>> 2) Counters for this tables
>> 3) Debugabillity
>> 
>> The first two can be achieved right now. Regarding debugabillity, which
>> is a bit vague, the current assumption is that the drivers internal data
>> structures are synced with hardware (which is no always true), and maybe
>> are not synced with the kernel, so this can be achieved right now by
>> dumping the internal state of the driver. Furthermore, the counters are
>> dumped from the hardware and give the user additional indication.
>> 
>> I completely agree that the hardware should be dumped in order to
>> validate the internal data structures are really synced with HW. This
>> could be usable for observing data corruptions inside the ASIC and
>> various complex bugs.
>> 
>> In order to address that I though about maybe add a flag called
>> "validate_hw" so that during the dump the driver<-->hw state could be
>> validated.
>> 
>> What do you think about it?
>
>It is not just a matter of dumping hardware state. The data returned by
>dump needs to be consistent across platforms and vendors.
>
>If the intent is validating hardware state matches kernel state (ie.,

Nope, that is definitelly not the intent. The intent is to provide user
some more information about how the actual tables in hw look like, so he
knows exactly what is going on there and eventually can optimize things
if needed (resource allocations for example)


>h/w forwarding matches s/w forwarding), then the hardware state should
>be dumped by the driver in a form that parallels kernel state. e.g.,
>dump h/w routes, neighbor entries, fdb's in a form and granularity
>similar to what is done for kernel tables.
>
>With the recent dpipe changes that allows kernel to driver cache and
>kernel to h/w state comparisons.

^ permalink raw reply

* Re: [ethtool] ethtool: Remove UDP Fragmentation Offload use from ethtool
From: Tariq Toukan @ 2017-08-29  7:50 UTC (permalink / raw)
  To: John W. Linville, Eric Dumazet, David Miller
  Cc: netdev, Eran Ben Elisha, Shaker Daibes
In-Reply-To: <20170828182251.GC3092@tuxdriver.com>


On 28/08/2017 9:22 PM, John W. Linville wrote:
> On Mon, Aug 28, 2017 at 08:00:11AM -0700, Eric Dumazet wrote:
>> On Mon, 2017-08-28 at 15:38 +0300, Tariq Toukan wrote:
>>> From: Shaker Daibes <shakerd@mellanox.com>
>>>
>>> UFO was removed in kernel, here we remove it in ethtool app.
>>>
>>> Fixes the following issue:
>>> Features for ens8:
>>> Cannot get device udp-fragmentation-offload settings: Operation not supported
>>>
>>> Tested with "make check"
>>>
>>> Signed-off-by: Shaker Daibes <shakerd@mellanox.com>
>>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>>> ---
>>
>>
>> Hi guys
>>
>> I would rather remove the warning, but leave the ability to switch UFO
>> on machines running old kernel but a recent ethtool.
>>
>> ethtool does not need to be downgraded every time we boot an old
>> kernel ;)

Thanks all for your quick replies.

We thought about the backward compatibility issue before getting to 
writing this patch.
But, as the feature has very few device support, and is not that useful,
we thought it would be best to just totally remove it from ethtool.

We can re-work this so the feature would still be available on old kernels.

But I wonder how the warning removal should be done??

I have some suggestions in mind:
1) Have a special condition that does not print a warning only in the 
case of UFO?
2) Remove the warning totally? I don't like this option.
3) Add a max_kernel_ver field in struct off_flag_def, and use it to not 
print the warning, or to mark the feature 'off [fixed]'.

Please let me know what you think.

> 
> No, definitely not.
>   
>> Thanks !
> 
> Tariq, will you be reworking this as Eric suggests?

Yes. Once we decide what is the correct way to keep it backward compatible.

> 
> John
> 

Regards,
Tariq Toukan

^ permalink raw reply

* Re: [PATCH] DSA support for Micrel KSZ8895
From: Pavel Machek @ 2017-08-29  7:45 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Woojung.Huh, nathan.leigh.conrad, vivien.didelot, f.fainelli,
	netdev, linux-kernel, Tristram.Ha
In-Reply-To: <20170828140927.GD10418@lunn.ch>

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

On Mon 2017-08-28 16:09:27, Andrew Lunn wrote:
> > I may be confused here, but AFAICT:
> > 
> > 1) Yes, it has standard layout when accessed over MDIO. 
> 
> 
> Section 4.8 of the datasheet says:
> 
> 	All the registers defined in this section can be also accessed
> 	via the SPI interface.
> 
> Meaning all PHY registers can be access via the SPI interface. So you
> should be able to make a standard Linux MDIO bus driver which performs
> SPI reads.

As far as I can tell (and their driver confirms) -- yes, all those
registers can be accessed over the SPI, they are just shuffled
around... hence MDIO emulation code. I copied it from their code (see
the copyrights) so no, I don't believe there's nicer solution.

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply

* RE: Question about ip_defrag
From: liujian (CE) @ 2017-08-29  7:44 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Florian Westphal
  Cc: davem@davemloft.net, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org, elena.reshetova@intel.com,
	edumazet@google.com, netdev@vger.kernel.org, Wangkefeng (Kevin),
	weiyongjun (A)
In-Reply-To: <20170829092021.0a46fffa@redhat.com>


> -----Original Message-----
> From: Jesper Dangaard Brouer [mailto:brouer@redhat.com]
> Sent: Tuesday, August 29, 2017 3:20 PM
> To: Florian Westphal
> Cc: liujian (CE); davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> yoshfuji@linux-ipv6.org; elena.reshetova@intel.com; edumazet@google.com;
> netdev@vger.kernel.org; Wangkefeng (Kevin); weiyongjun (A);
> brouer@redhat.com
> Subject: Re: Question about ip_defrag
> 
> On Mon, 28 Aug 2017 16:00:32 +0200
> Florian Westphal <fw@strlen.de> wrote:
> 
> > liujian (CE) <liujian56@huawei.com> wrote:
> > > Hi
> > >
> > > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in
> lib/percpu_counter.c and include/linux/percpu_counter.h.
> > > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> > >
> > > > > > > the issue:
> > > > > > > Ip_defrag fail caused by frag_mem_limit reached
> 4M(frags.high_thresh).
> > > > > > > At this moment,sum_frag_mem_limit is about 10K.
> > >
> > > So should we change ipfrag high/low thresh to a reasonable value ?
> > > And if it is, is there a standard to change the value?
> >
> > Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> > about so with 64 cpus that is ~8 mbyte.
> >
> > possible solutions:
> > 1. reduce frag_percpu_counter_batch to 16k or so 2. make both low and
> > high thresh depend on NR_CPUS
> 
> To me it looks like we/I have been using the wrong API for comparing against
> percpu_counters.  I guess we should have used
> __percpu_counter_compare().

Are you means?
Change 
if (frag_mem_limit(nf) > nf->low_thresh)
to
__percpu_counter_compare(&nf->mem, nf->low_thresh, frag_percpu_counter_batch)

> /*
>  * Compare counter against given value.
>  * Return 1 if greater, 0 if equal and -1 if less  */ int
> __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch) {
> 	s64	count;
> 
> 	count = percpu_counter_read(fbc);
> 	/* Check to see if rough count will be sufficient for comparison */
> 	if (abs(count - rhs) > (batch * num_online_cpus())) {
> 		if (count > rhs)
> 			return 1;
> 		else
> 			return -1;
> 	}
> 	/* Need to use precise count */
> 	count = percpu_counter_sum(fbc);
> 	if (count > rhs)
> 		return 1;
> 	else if (count < rhs)
> 		return -1;
> 	else
> 		return 0;
> }
> EXPORT_SYMBOL(__percpu_counter_compare);
> 
> 
> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH] DSA support for Micrel KSZ8895
From: Pavel Machek @ 2017-08-29  7:41 UTC (permalink / raw)
  To: Maxim Uvarov
  Cc: Andrew Lunn, Woojung.Huh, nathan.leigh.conrad, Vivien Didelot,
	Florian Fainelli, netdev, linux-kernel, Tristram.Ha
In-Reply-To: <CAJGZr0K46jBd9Sn4HNPgAeHTQtBuBHzX87GveAJf3_5b_WM69g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]

Hi!

> Micrel has some drivers on their web site to support some chips. For
> that chips they do virtual mdio over spi.
> And driver is available on download page:
> http://www.microchip.com/wwwproducts/en/KSZ8895
> 
> Documentation->Software library.
> 
> Both driver and DSA driver. Driver has to work with some minor fixups
> related to your kernel version. But I think they are don't care about
> up-streaming that code.
> So you can take their code as a reference.

"Minor fixups". Take a look at the driver.. I wanted to do a "minor
fixups". It turned out it was easier to start from scratch.

But the MDIO emaulation code is from their driver, after lots of
deletions.

								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply

* RE: Question about ip_defrag
From: liujian (CE) @ 2017-08-29  7:40 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Jesper Dangaard Brouer, davem@davemloft.net, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org, elena.reshetova@intel.com,
	edumazet@google.com, netdev@vger.kernel.org, Wangkefeng (Kevin),
	weiyongjun (A)
In-Reply-To: <20170828140032.GB12926@breakpoint.cc>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Florian Westphal
> Sent: Monday, August 28, 2017 10:01 PM
> To: liujian (CE)
> Cc: Jesper Dangaard Brouer; davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> yoshfuji@linux-ipv6.org; elena.reshetova@intel.com; edumazet@google.com;
> netdev@vger.kernel.org; Wangkefeng (Kevin); weiyongjun (A)
> Subject: Re: Question about ip_defrag
> 
> liujian (CE) <liujian56@huawei.com> wrote:
> > Hi
> >
> > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in
> lib/percpu_counter.c and include/linux/percpu_counter.h.
> > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> >
> > > > > > the issue:
> > > > > > Ip_defrag fail caused by frag_mem_limit reached
> 4M(frags.high_thresh).
> > > > > > At this moment,sum_frag_mem_limit is about 10K.
> >
> > So should we change ipfrag high/low thresh to a reasonable value ?
> > And if it is, is there a standard to change the value?
> 
> Each cpu can have frag_percpu_counter_batch bytes rest doesn't know about
> so with 64 cpus that is ~8 mbyte.
> 
> possible solutions:
> 1. reduce frag_percpu_counter_batch to 16k or so 2. make both low and high
> thresh depend on NR_CPUS
> 
Thank you for your reply.
 
> liujian, does this change help in any way?

I will have a try.

> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -123,6 +123,17 @@ static bool inet_fragq_should_evict(const struct
> inet_frag_queue *q)
>  	       frag_mem_limit(q->net) >= q->net->low_thresh;  }
> 
> +/* ->mem batch size is huge, this can cause severe discrepancies
> + * between actual value (sum of pcpu values) and the global estimate.
> + *
> + * Use a smaller batch to give an opportunity for the global estimate
> + * to more accurately reflect current state.
> + */
> +static void update_frag_mem_limit(struct netns_frags *nf, unsigned int
> +batch) {
> +	 percpu_counter_add_batch(&nf->mem, 0, batch); }
> +
>  static unsigned int
>  inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb)  { @@
> -146,8 +157,12 @@ inet_evict_bucket(struct inet_frags *f, struct
> inet_frag_bucket *hb)
> 
>  	spin_unlock(&hb->chain_lock);
> 
> -	hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
> +	hlist_for_each_entry_safe(fq, n, &expired, list_evictor) {
> +		struct netns_frags *nf = fq->net;
> +
>  		f->frag_expire((unsigned long) fq);
> +		update_frag_mem_limit(nf, 1);

> +	}
> 
>  	return evicted;
>  }
> @@ -396,8 +411,10 @@ struct inet_frag_queue *inet_frag_find(struct
> netns_frags *nf,
>  	struct inet_frag_queue *q;
>  	int depth = 0;
> 
> -	if (frag_mem_limit(nf) > nf->low_thresh)
> +	if (frag_mem_limit(nf) > nf->low_thresh) {
>  		inet_frag_schedule_worker(f);
> +		update_frag_mem_limit(nf, SKB_TRUESIZE(1500) * 16); 
> +	}
> 
>  	hash &= (INETFRAGS_HASHSZ - 1);
>  	hb = &f->hash[hash];
> @@ -416,6 +433,8 @@ struct inet_frag_queue *inet_frag_find(struct
> netns_frags *nf,
>  	if (depth <= INETFRAGS_MAXDEPTH)
>  		return inet_frag_create(nf, f, key);
> 
> +	update_frag_mem_limit(nf, 1);
> +
>  	if (inet_frag_may_rebuild(f)) {
>  		if (!f->rebuild)
>  			f->rebuild = true;

^ permalink raw reply

* RE: [patch net-next 1/3] idr: Add new APIs to support unsigned long
From: Chris Mi @ 2017-08-29  7:34 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
	xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
	mawilcox@microsoft.com
In-Reply-To: <87y3q27sn7.fsf@stressinduktion.org>

Hi,

> -----Original Message-----
> From: Hannes Frederic Sowa [mailto:hannes@stressinduktion.org]
> Sent: Tuesday, August 29, 2017 3:14 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: netdev@vger.kernel.org; jhs@mojatatu.com;
> xiyou.wangcong@gmail.com; jiri@resnulli.us; davem@davemloft.net;
> mawilcox@microsoft.com
> Subject: Re: [patch net-next 1/3] idr: Add new APIs to support unsigned long
> 
> Hello,
> 
> Chris Mi <chrism@mellanox.com> writes:
> 
> > The following new APIs are added:
> >
> > int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
> >                   unsigned long start, unsigned long end, gfp_t gfp);
> > static inline void *idr_remove_ext(struct idr *idr, unsigned long id);
> > static inline void *idr_find_ext(const struct idr *idr, unsigned long
> > id); void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long
> > id); void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
> >
> > Signed-off-by: Chris Mi <chrism@mellanox.com>
> > Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> > ---
> >  include/linux/idr.h        | 16 ++++++++++
> >  include/linux/radix-tree.h |  3 ++
> >  lib/idr.c                  | 56 +++++++++++++++++++++++++++++++++++
> >  lib/radix-tree.c           | 73
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 148 insertions(+)
> >
> 
> [...]
> 
> > +int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
> > +		  unsigned long start, unsigned long end, gfp_t gfp) {
> > +	void __rcu **slot;
> > +	struct radix_tree_iter iter;
> > +
> > +	if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
> > +		return -EINVAL;
> > +
> > +	radix_tree_iter_init(&iter, start);
> > +	slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
> > +	if (IS_ERR(slot))
> > +		return PTR_ERR(slot);
> > +
> > +	radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
> > +	radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
> > +
> > +	if (index)
> > +		*index = iter.index;
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(idr_alloc_ext);
> 
> Can you express idr_alloc in terms of idr_alloc_ext? Same for most of the
> other functions (it seems that signed int was used as return value to indicate
> error cases, thus it should be easy to map those).
In idr_alloc(), we have the following check:

        if (WARN_ON_ONCE(start < 0))
                return -EINVAL;

But in idr_alloc_ext(), since we are using unsigned long, we needn't such check.

In order to reuse several lines of code, I think it is not worth to express idr_alloc()
In terms of idr_alloc_ext. 

Thanks,
Chris
> 
> [...]
> 
> Thanks,
> Hannes

^ permalink raw reply

* Re: Question about ip_defrag
From: Jesper Dangaard Brouer @ 2017-08-29  7:20 UTC (permalink / raw)
  To: Florian Westphal
  Cc: liujian (CE), davem@davemloft.net, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org, elena.reshetova@intel.com,
	edumazet@google.com, netdev@vger.kernel.org, Wangkefeng (Kevin),
	weiyongjun (A), brouer
In-Reply-To: <20170828140032.GB12926@breakpoint.cc>

On Mon, 28 Aug 2017 16:00:32 +0200
Florian Westphal <fw@strlen.de> wrote:

> liujian (CE) <liujian56@huawei.com> wrote:
> > Hi
> > 
> > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
> > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> >   
> > > > > > the issue:
> > > > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > > > At this moment,sum_frag_mem_limit is about 10K.  
> > 
> > So should we change ipfrag high/low thresh to a reasonable value ? 
> > And if it is, is there a standard to change the value?  
> 
> Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> about so with 64 cpus that is ~8 mbyte.
> 
> possible solutions:
> 1. reduce frag_percpu_counter_batch to 16k or so
> 2. make both low and high thresh depend on NR_CPUS

To me it looks like we/I have been using the wrong API for comparing
against percpu_counters.  I guess we should have used __percpu_counter_compare().

/*
 * Compare counter against given value.
 * Return 1 if greater, 0 if equal and -1 if less
 */
int __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch)
{
	s64	count;

	count = percpu_counter_read(fbc);
	/* Check to see if rough count will be sufficient for comparison */
	if (abs(count - rhs) > (batch * num_online_cpus())) {
		if (count > rhs)
			return 1;
		else
			return -1;
	}
	/* Need to use precise count */
	count = percpu_counter_sum(fbc);
	if (count > rhs)
		return 1;
	else if (count < rhs)
		return -1;
	else
		return 0;
}
EXPORT_SYMBOL(__percpu_counter_compare);


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls of udelay to usleep_range
From: Neftin, Sasha @ 2017-08-29  7:19 UTC (permalink / raw)
  To: Matthew Tan, jeffrey.t.kirsher
  Cc: michael.kardonik, mitch.a.williams, linux-kernel, john.ronciak,
	intel-wired-lan, netdev
In-Reply-To: <1503503985-3869-1-git-send-email-matthew.tan_1@nxp.com>

On 8/23/2017 18:59, Matthew Tan wrote:
>      Calls to udelay are not preemtable by userspace so userspace
>      applications experience a large (~200us) latency when running on core
>      0. Instead usleep_range can be used to be more friendly to userspace
>      since it is preemtable. This is due to udelay using busy-wait loops
>      while usleep_rang uses hrtimers instead. It is recommended to use
>      udelay when the delay is <10us since at that precision overhead of
>      usleep_range hrtimer setup causes issues. However, the replaced calls
>      are for 50us and 100us so this should not be not an issue.
>
> Signed-off-by: Matthew Tan <matthew.tan_1@nxp.com>
> ---
>   drivers/net/ethernet/intel/e1000e/phy.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
> index de13aea..e318fdc 100644
> --- a/drivers/net/ethernet/intel/e1000e/phy.c
> +++ b/drivers/net/ethernet/intel/e1000e/phy.c
> @@ -158,7 +158,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
>   	 * the lower time out
>   	 */
>   	for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
> -		udelay(50);
> +		usleep_range(40, 60);
>   		mdic = er32(MDIC);
>   		if (mdic & E1000_MDIC_READY)
>   			break;
> @@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
>   	 * reading duplicate data in the next MDIC transaction.
>   	 */
>   	if (hw->mac.type == e1000_pch2lan)
> -		udelay(100);
> +		usleep_range(90, 100);
>   
>   	return 0;
>   }
> @@ -222,7 +222,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
>   	 * the lower time out
>   	 */
>   	for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
> -		udelay(50);
> +		usleep_range(40, 60);
>   		mdic = er32(MDIC);
>   		if (mdic & E1000_MDIC_READY)
>   			break;
> @@ -246,7 +246,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
>   	 * reading duplicate data in the next MDIC transaction.
>   	 */
>   	if (hw->mac.type == e1000_pch2lan)
> -		udelay(100);
> +		usleep_range(90, 110);
>   
>   	return 0;
>   }

Reasonable. Do you have any open bug or other reference describe this 
problem?

^ permalink raw reply


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