Netdev List
 help / color / mirror / Atom feed
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: David Miller @ 2014-10-07 17:01 UTC (permalink / raw)
  To: alexei.starovoitov
  Cc: danny.zhou, willemb, john.fastabend, dborkman, fw, gerlitz.or,
	hannes, netdev, john.ronciak, amirv, eric.dumazet
In-Reply-To: <CAADnVQJf8jtRr3xBdSOt_X188Xkkg0EnxVd+gh6ruTMjc9jOHg@mail.gmail.com>

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Tue, 7 Oct 2014 09:33:04 -0700

> I guess that's great for DPDK users, but I don't think it's good for
> linux.

Any use of a piece of hardware is fine with me, personally, as long
as adequate protections are in place.

If it's just a descriptor ring in software and a doorbell to trigger
a refetch of the head and tail pointers, with appropriate protection
and control of the memory attached to the ring, I don't see how I
could object to such a facility.

^ permalink raw reply

* Re: [PATCH v2 net-next] net: phy: adjust fixed_phy_register() return value
From: David Miller @ 2014-10-07 17:02 UTC (permalink / raw)
  To: pgynther; +Cc: netdev, f.fainelli, thomas.petazzoni
In-Reply-To: <CAGXr9JF8fx6YH3szY=nwxTJeJgopBCW9Kzhcs33q4oFG5GNOBQ@mail.gmail.com>

From: Petri Gynther <pgynther@google.com>
Date: Tue, 7 Oct 2014 09:47:37 -0700

> I'm not sure if I understand your comment. The caller of
> fixed_phy_register() now gets the pointer to the phydev created by
> get_phy_device(). What other thread is aware of this pointer and how could
> they free it? Isn't the caller of fixed_phy_register() exclusively in
> charge of the created phydev?

If this is the case then my concerns are unfounded.

Thanks for clearing that up and I'll apply your patch, thanks!

^ permalink raw reply

* Re: randconfig build error with next-20141001, in drivers/i2c/algos/i2c-algo-bit.c
From: Randy Dunlap @ 2014-10-07 17:03 UTC (permalink / raw)
  To: Oliver Hartkopp, Stephane Grosjean
  Cc: Jim Davis, Stephen Rothwell, linux-next,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-can-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5433AB31.9090603-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

On 10/07/14 01:58, Oliver Hartkopp wrote:
> On 10/06/2014 08:09 PM, Randy Dunlap wrote:
>> On 10/06/14 10:39, Oliver Hartkopp wrote:
> 
>>> AFAICS there is 'just' a style problem as 'configs should not enable entire
>>> subsystems'. But it finally is a correct and valid Kconfig, right?
>>
>> Yes, right.
> 
> (..)
> 
>> In the unlikely case that I2C is not enabled, the user should have to enable
>> it instead of a solitary driver enabling it.  IOW, if a subsystem is disabled,
>> the user probably wanted it that way and a single driver should not override
>> that setting.
> 
> Due to the fact that a change to 'depends on I2C' would make the config option
> invisible (and therefore not selectable) in the case I2C was (unlikely)
> disabled I would finally vote to leave it as-is.
> 
> The current Kconfig entry already contains a description that points to the
> requirement to have I2C and I2C_ALGOBIT to be enabled to compile this driver:
> 
> config CAN_PEAK_PCIEC
> 	bool "PEAK PCAN-ExpressCard Cards"
> 	depends on CAN_PEAK_PCI
> 	select I2C
> 	select I2C_ALGOBIT
> 	default y
> 	---help---
> 	  Say Y here if you want to use a PCAN-ExpressCard from PEAK-System
> 	  Technik. This will also automatically select I2C and I2C_ALGO
> 	  configuration options.
> 
> AFAIK the PEAK PCAN-ExpressCard is usually used in x86 architecture Laptops,
> so it's near to an academic discussion as x86 usually selects I2C ;-)

Pray tell where does x86 usually select I2C?
Thanks.

> @Stephane: When updating the help text to introduce the PCAN-ExpressCard 34
> support anyway you might probably add some more information *why* the I2C
> support is needed (for CAN transceiver settings and status LED).
> 
> And /s/I2C_ALGO/I2C_ALGOBIT/ :-)


-- 
~Randy

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: David Miller @ 2014-10-07 17:05 UTC (permalink / raw)
  To: alexei.starovoitov
  Cc: therbert, jesse, gerlitz.or, alexander.h.duyck, john.r.fastabend,
	jeffrey.t.kirsher, netdev, tgraf, pshelar, azhou
In-Reply-To: <CAADnVQLkWrjN=MgtU_8zBrGFW1eJd01hd6BM_m9iOq=ypHvSLQ@mail.gmail.com>

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Tue, 7 Oct 2014 09:50:50 -0700

> CHECKSUM_COMPLETE is a burden on software.

I totally disagree, it's the most software friendly checksumming
offload mechanism possible.  I wish every card did it.

CHECKSUM_COMPLETE means that any sub-protocol or tunneling mechanism
can be trivially supported without any modifications to hardware, and
it therefore makes checksum offloading of new protocols require no
hardware changes whatsoever.

^ permalink raw reply

* Re: [PATCH v2 net-next] net: bcmgenet: fix Tx ring priority programming
From: Florian Fainelli @ 2014-10-07 17:07 UTC (permalink / raw)
  To: Petri Gynther, netdev; +Cc: davem
In-Reply-To: <20141007163001.85125100761@puck.mtv.corp.google.com>

On 10/07/2014 09:30 AM, Petri Gynther wrote:
> GENET MAC has three Tx ring priority registers:
> - GENET_x_TDMA_PRIORITY0 for queues 0-5
> - GENET_x_TDMA_PRIORITY1 for queues 6-11
> - GENET_x_TDMA_PRIORITY2 for queues 12-16
> 
> Fix bcmgenet_init_multiq() to program them correctly.

Looks good to me, the register layout is correct for GENETv1 to v4, thanks!

> 
> Signed-off-by: Petri Gynther <pgynther@google.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 42 +++++++++++++++-----------
>  drivers/net/ethernet/broadcom/genet/bcmgenet.h |  2 ++
>  2 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index e0a6238..fff2634 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -191,8 +191,9 @@ enum dma_reg {
>  	DMA_STATUS,
>  	DMA_SCB_BURST_SIZE,
>  	DMA_ARB_CTRL,
> -	DMA_PRIORITY,
> -	DMA_RING_PRIORITY,
> +	DMA_PRIORITY_0,
> +	DMA_PRIORITY_1,
> +	DMA_PRIORITY_2,
>  };
>  
>  static const u8 bcmgenet_dma_regs_v3plus[] = {
> @@ -201,8 +202,9 @@ static const u8 bcmgenet_dma_regs_v3plus[] = {
>  	[DMA_STATUS]		= 0x08,
>  	[DMA_SCB_BURST_SIZE]	= 0x0C,
>  	[DMA_ARB_CTRL]		= 0x2C,
> -	[DMA_PRIORITY]		= 0x30,
> -	[DMA_RING_PRIORITY]	= 0x38,
> +	[DMA_PRIORITY_0]	= 0x30,
> +	[DMA_PRIORITY_1]	= 0x34,
> +	[DMA_PRIORITY_2]	= 0x38,
>  };
>  
>  static const u8 bcmgenet_dma_regs_v2[] = {
> @@ -211,8 +213,9 @@ static const u8 bcmgenet_dma_regs_v2[] = {
>  	[DMA_STATUS]		= 0x08,
>  	[DMA_SCB_BURST_SIZE]	= 0x0C,
>  	[DMA_ARB_CTRL]		= 0x30,
> -	[DMA_PRIORITY]		= 0x34,
> -	[DMA_RING_PRIORITY]	= 0x3C,
> +	[DMA_PRIORITY_0]	= 0x34,
> +	[DMA_PRIORITY_1]	= 0x38,
> +	[DMA_PRIORITY_2]	= 0x3C,
>  };
>  
>  static const u8 bcmgenet_dma_regs_v1[] = {
> @@ -220,8 +223,9 @@ static const u8 bcmgenet_dma_regs_v1[] = {
>  	[DMA_STATUS]		= 0x04,
>  	[DMA_SCB_BURST_SIZE]	= 0x0C,
>  	[DMA_ARB_CTRL]		= 0x30,
> -	[DMA_PRIORITY]		= 0x34,
> -	[DMA_RING_PRIORITY]	= 0x3C,
> +	[DMA_PRIORITY_0]	= 0x34,
> +	[DMA_PRIORITY_1]	= 0x38,
> +	[DMA_PRIORITY_2]	= 0x3C,
>  };
>  
>  /* Set at runtime once bcmgenet version is known */
> @@ -1696,7 +1700,8 @@ static void bcmgenet_init_multiq(struct net_device *dev)
>  {
>  	struct bcmgenet_priv *priv = netdev_priv(dev);
>  	unsigned int i, dma_enable;
> -	u32 reg, dma_ctrl, ring_cfg = 0, dma_priority = 0;
> +	u32 reg, dma_ctrl, ring_cfg = 0;
> +	u32 dma_priority[3] = {0, 0, 0};
>  
>  	if (!netif_is_multiqueue(dev)) {
>  		netdev_warn(dev, "called with non multi queue aware HW\n");
> @@ -1721,22 +1726,25 @@ static void bcmgenet_init_multiq(struct net_device *dev)
>  
>  		/* Configure ring as descriptor ring and setup priority */
>  		ring_cfg |= 1 << i;
> -		dma_priority |= ((GENET_Q0_PRIORITY + i) <<
> -				(GENET_MAX_MQ_CNT + 1) * i);
>  		dma_ctrl |= 1 << (i + DMA_RING_BUF_EN_SHIFT);
> +
> +		dma_priority[DMA_PRIO_REG_INDEX(i)] |=
> +			((GENET_Q0_PRIORITY + i) << DMA_PRIO_REG_SHIFT(i));
>  	}
>  
> +	/* Set ring 16 priority and program the hardware registers */
> +	dma_priority[DMA_PRIO_REG_INDEX(DESC_INDEX)] |=
> +		((GENET_Q0_PRIORITY + priv->hw_params->tx_queues) <<
> +		 DMA_PRIO_REG_SHIFT(DESC_INDEX));
> +	bcmgenet_tdma_writel(priv, dma_priority[0], DMA_PRIORITY_0);
> +	bcmgenet_tdma_writel(priv, dma_priority[1], DMA_PRIORITY_1);
> +	bcmgenet_tdma_writel(priv, dma_priority[2], DMA_PRIORITY_2);
> +
>  	/* Enable rings */
>  	reg = bcmgenet_tdma_readl(priv, DMA_RING_CFG);
>  	reg |= ring_cfg;
>  	bcmgenet_tdma_writel(priv, reg, DMA_RING_CFG);
>  
> -	/* Use configured rings priority and set ring #16 priority */
> -	reg = bcmgenet_tdma_readl(priv, DMA_RING_PRIORITY);
> -	reg |= ((GENET_Q0_PRIORITY + priv->hw_params->tx_queues) << 20);
> -	reg |= dma_priority;
> -	bcmgenet_tdma_writel(priv, reg, DMA_PRIORITY);
> -
>  	/* Configure ring as descriptor ring and re-enable DMA if enabled */
>  	reg = bcmgenet_tdma_readl(priv, DMA_CTRL);
>  	reg |= dma_ctrl;
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> index 321b1db..dbf524e 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -401,6 +401,8 @@ struct bcmgenet_mib_counters {
>  #define DMA_ARBITER_MODE_MASK		0x03
>  #define DMA_RING_BUF_PRIORITY_MASK	0x1F
>  #define DMA_RING_BUF_PRIORITY_SHIFT	5
> +#define DMA_PRIO_REG_INDEX(q)		((q) / 6)
> +#define DMA_PRIO_REG_SHIFT(q)		(((q) % 6) * DMA_RING_BUF_PRIORITY_SHIFT)
>  #define DMA_RATE_ADJ_MASK		0xFF
>  
>  /* Tx/Rx Dma Descriptor common bits*/
> 

^ permalink raw reply

* Re: [PATCH net-next 1/1] tipc: fix bug in multicast congestion handling
From: David Miller @ 2014-10-07 17:07 UTC (permalink / raw)
  To: jon.maloy
  Cc: netdev, paul.gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion
In-Reply-To: <1412698715-9838-1-git-send-email-jon.maloy@ericsson.com>

From: Jon Maloy <jon.maloy@ericsson.com>
Date: Tue,  7 Oct 2014 12:18:35 -0400

> +	uint flags = node->action_flags;

Please use explicit "unsigned int"

^ permalink raw reply

* Re: [PATCH v2 net-next] net: bcmgenet: fix Tx ring priority programming
From: David Miller @ 2014-10-07 17:09 UTC (permalink / raw)
  To: pgynther; +Cc: netdev, f.fainelli
In-Reply-To: <20141007163001.85125100761@puck.mtv.corp.google.com>

From: Petri Gynther <pgynther@google.com>
Date: Tue,  7 Oct 2014 09:30:01 -0700 (PDT)

> GENET MAC has three Tx ring priority registers:
> - GENET_x_TDMA_PRIORITY0 for queues 0-5
> - GENET_x_TDMA_PRIORITY1 for queues 6-11
> - GENET_x_TDMA_PRIORITY2 for queues 12-16
> 
> Fix bcmgenet_init_multiq() to program them correctly.
> 
> Signed-off-by: Petri Gynther <pgynther@google.com>

Yeah this looks a lot nicer, applied, thanks!

^ permalink raw reply

* Re: [PATCH] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: David Miller @ 2014-10-07 17:15 UTC (permalink / raw)
  To: LW; +Cc: netdev, rmk+kernel, Frank.Li, fabio.estevam, linux-kernel
In-Reply-To: <1412687977-11742-1-git-send-email-LW@KARO-electronics.de>

From: Lothar Waßmann <LW@KARO-electronics.de>
Date: Tue,  7 Oct 2014 15:19:37 +0200

> commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> introduced a regression for i.MX28. The swap_buffer() function doing
> the endian conversion of the received data on i.MX28 may access memory
> beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> does not copy those bytes, so that the last bytes of a packet may be
> filled with invalid data after swapping.
> This will likely lead to checksum errors on received packets.
> E.g. when trying to mount an NFS rootfs:
> UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> 
> Do the byte swapping and copying to the new skb in one go if
> necessary.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>

Why don't you just round up the length fec_enet_copybreak() uses when
need_swap is true?  Then you will end up mimicking the original behavior
and not require this new helper function.

And in any case I agree with Sergei that if you do retain your approach,
the new 'swap' argument to fec_enet_copybreak() should be a 'bool'.

I'm really surprised there isn't a control register bit to adjust the
endianness of the data DMA'd to/from the network.

^ permalink raw reply

* Re: [PATCH RESEND v3] 3c59x: fix bad split of cpu_to_le32(pci_map_single())
From: David Miller @ 2014-10-07 17:16 UTC (permalink / raw)
  To: sylvain.hitier; +Cc: linux-kernel, nhorman, mroos, netdev, klassert
In-Reply-To: <20141007134034.GA8838@erable>

From: Sylvain 'ythier' Hitier <sylvain.hitier@gmail.com>
Date: Tue, 7 Oct 2014 13:40:34 +0000

> From: Sylvain "ythier" Hitier <sylvain.hitier@gmail.com>
> 
> In commit 6f2b6a3005b2c34c39f207a87667564f64f2f91a,
>   # 3c59x: Add dma error checking and recovery
> the intent is to split out the mapping from the byte-swapping in order to
> insert a dma_mapping_error() check.
> 
> Kinda this semantic patch:
> 
>     // See http://coccinelle.lip6.fr/
>     //
>     // Beware, grouik-and-dirty!
>     @@
>     expression DEV, X, Y, Z;
>     @@
>     -   cpu_to_le32(pci_map_single(DEV, X, Y, Z))
>     +   dma_addr_t addr = pci_map_single(DEV, X, Y, Z);
>     +   if (dma_mapping_error(&DEV->dev, addr))
>     +       /* snip */;
>     +   cpu_to_le32(addr)
> 
> However, the #else part (of the #if DO_ZEROCOPY test) is changed this way:
> 
>     -   cpu_to_le32(pci_map_single(DEV, X, Y, Z))
>     +   dma_addr_t addr = cpu_to_le32(pci_map_single(DEV, X, Y, Z));
>     //                    ^^^^^^^^^^^
>     //                    That mismatches the 3 other changes!
>     +   if (dma_mapping_error(&DEV->dev, addr))
>     +       /* snip */;
>     +   cpu_to_le32(addr)
> 
> Let's remove the leftover cpu_to_le32() for coherency.
> 
> v2: Better changelog.
> v3: Add Acked-by
> 
> Fixes: 6f2b6a3005b2c34c39f207a87667564f64f2f91a
>   # 3c59x: Add dma error checking and recovery
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Sylvain "ythier" Hitier <sylvain.hitier@gmail.com>

Applied and queued up for -stable, thanks!

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Alexei Starovoitov @ 2014-10-07 17:18 UTC (permalink / raw)
  To: David Miller
  Cc: Tom Herbert, Jesse Gross, gerlitz.or@gmail.com, Alexander Duyck,
	John Fastabend, Jeff Kirsher, netdev@vger.kernel.org, Thomas Graf,
	Pravin Shelar, Andy Zhou
In-Reply-To: <20141007.130504.1805673129740661479.davem@davemloft.net>

On Tue, Oct 7, 2014 at 10:05 AM, David Miller <davem@davemloft.net> wrote:
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Date: Tue, 7 Oct 2014 09:50:50 -0700
>
>> CHECKSUM_COMPLETE is a burden on software.
>
> I totally disagree, it's the most software friendly checksumming
> offload mechanism possible.  I wish every card did it.
>
> CHECKSUM_COMPLETE means that any sub-protocol or tunneling mechanism
> can be trivially supported without any modifications to hardware, and
> it therefore makes checksum offloading of new protocols require no
> hardware changes whatsoever.

yes, of course. My point is that if HW can parse the packet and validate
csum it should do that, since it's faster for the stack on top.
HW can fall back to CHECKSUM_COMPLETE if it fails to parse, for example.
I think some NICs do exactly that.

^ permalink raw reply

* Re: Quota in __qdisc_run()
From: David Miller @ 2014-10-07 17:19 UTC (permalink / raw)
  To: eric.dumazet
  Cc: hannes, brouer, netdev, therbert, fw, dborkman, jhs,
	alexander.duyck, john.r.fastabend, dave.taht, toke
In-Reply-To: <1412694080.11091.131.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 07 Oct 2014 08:01:20 -0700

> On Tue, 2014-10-07 at 16:43 +0200, Hannes Frederic Sowa wrote:
> 
>> This needs to be:
>> 
>> do
>>    ...
>> while ((iskb = iskb->next))
> 
> I do not feel needed to break the bulk dequeue at precise quota
> boundary. These quotas are advisory, and bql prefers to get its full
> budget for appropriate feedback from TX completion.
> 
> Quota was a packet quota, which was quite irrelevant if segmentation had
> to be done, so I would just let the dequeue be done so that we benefit
> from optimal xmit_more.

Yes, this makes sense, do a full qdisc_restart() cycle without boundaries,
then check how much quota was used afterwards to guard the outermost loop.

^ permalink raw reply

* Re: [PATCH net-next] net/mlx4_en: remove NETDEV_TX_BUSY
From: David Miller @ 2014-10-07 17:21 UTC (permalink / raw)
  To: amirv, amirv.mellanox
  Cc: eric.dumazet, edumazet, netdev, yevgenyp, ogerlitz, idos
In-Reply-To: <5433A48E.1040505@gmail.com>

From: Amir Vadai <amirv.mellanox@gmail.com>
Date: Tue, 07 Oct 2014 11:30:06 +0300

> On 10/6/2014 7:30 PM, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> 
>> Drivers should avoid NETDEV_TX_BUSY as much as possible.
>> 
>> They should stop the tx queue before qdisc even tries to push another
>> packet, to avoid requeues.
>> 
>> For a driver supporting skb->xmit_more, this is likely to be a prereq
>> anyway, otherwise we could have a tx deadlock : We need to force a
>> doorbell if TX ring is full.
>> 
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> ---
> 
> Reviewed. Also verified that it fixes the deadlock (by sending a large
> burst - larger than ring size). Before this fix, last packet of the
> burst wasn't sent, therefore no doorbell was rang, and the queue was
> stalled.
> 
> BTW, another nice optimization that we hope to send soon, is not to arm
> the CQ unless ringing the doorbell.
> 
> Acked-by: Amir Vadai <amirv@mellanox.com>

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH v2 net-next] net: better IFF_XMIT_DST_RELEASE support
From: David Miller @ 2014-10-07 17:22 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, ja
In-Reply-To: <1412633126.11091.89.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 06 Oct 2014 15:05:26 -0700

> We have a loop over team/bonding members, where we do :
> 
> dst_release_flag &= slave->dev->priv_flags;
> 
> So at the end of the loop, we check if any one of the member had one of
> the bit cleared.
> 
> if dst_release_flags has both bits set, then we are set and we allow the
> IFF_XMIT_DST_RELEASE being set on the master.

Oh I see, I mis-read the patch thinking dst_release_flags was a new variable
when in fact it's an existing one.

Patch applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Eric Dumazet @ 2014-10-07 17:23 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Tom Herbert, Jesse Gross, Or Gerlitz, Alexander Duyck,
	John Fastabend, Jeff Kirsher, David Miller, Linux Netdev List,
	Thomas Graf, Pravin Shelar, Andy Zhou
In-Reply-To: <CAADnVQLkWrjN=MgtU_8zBrGFW1eJd01hd6BM_m9iOq=ypHvSLQ@mail.gmail.com>

On Tue, 2014-10-07 at 09:50 -0700, Alexei Starovoitov wrote:

> it's definitely more difficult to properly implement
> CHECKSUM_UNNECESSARY in HW, but it's worth it.
> CHECKSUM_COMPLETE is a burden on software. Old NICs
> used to do that, but overhead of recomputing csum for every
> step of packet parsing and header modifications is too high.
> sw routers, bridges and < L4 networking devices are
> simpler and faster with CHECKSUM_UNNECESSARY.

Really this is wrong. Once you validated/pulled a header,
adjusting the complete checksum is in the order of 10 cycles or so,
ie less than 1% of the other costs.

UNNECESSARY usually requests complex NIC firmware, and usually the NIC
has fewer cores than the host.

It mostly worked for basic IP+TCP kind of traffic, but once you want
complex cloud models, it is a major pain.

If we need to optimize csum_partial() for short lengths, lets do it,
instead of pushing hardware vendors adding more and more schemes.

^ permalink raw reply

* Re: Quota in __qdisc_run()
From: Eric Dumazet @ 2014-10-07 17:32 UTC (permalink / raw)
  To: David Miller
  Cc: hannes, brouer, netdev, therbert, fw, dborkman, jhs,
	alexander.duyck, john.r.fastabend, dave.taht, toke
In-Reply-To: <20141007.131938.1410434352331637585.davem@davemloft.net>

On Tue, 2014-10-07 at 13:19 -0400, David Miller wrote:

> Yes, this makes sense, do a full qdisc_restart() cycle without boundaries,
> then check how much quota was used afterwards to guard the outermost loop.

I am testing this, and also am testing the xmit_more patch for I40E.

Will send patches today.

Thanks

^ permalink raw reply

* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Tejun Heo @ 2014-10-07 17:34 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, tiwai, arjan, teg,
	rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
	linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
	One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
	Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
	Abhijit Mahajan, Casey Leedom
In-Reply-To: <20141006231046.GD14081@wotan.suse.de>

Hello,

On Tue, Oct 07, 2014 at 01:10:46AM +0200, Luis R. Rodriguez wrote:
> On Mon, Oct 06, 2014 at 05:01:18PM -0400, Tejun Heo wrote:
> > For in-kernel stuff, we already have a clear
> > synchronization point where we already synchronize all async calls.
> > Shouldn't we be flushing these async probes there too?
> 
> This seems to be addressing if what I meant by prepared, "ready", so let
> me address this as I do think its important.
> 
> By async calls do you mean users of async_schedule()? I see it

Yes.

> also uses system_unbound_wq as well but I do not see anyone calling
> flush_workqueue(system_unbound_wq) on the kernel. We do use
> async_synchronize_full() on kernel_init() but that just waits.

But you can create a new workqueue and queue all the async probing
work items there and flush the workqueue right after
async_synchronize_full().

...
> bus.enable_kern_async=1 would still also serve as a helper for the driver core
> to figure out if it should use async probe then on modules if prefer_async_probe
> was enabled. Let me know if you figure out a way to avoid it.

Why do we need the choice at all?  It always should, no?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: macvlan: optimizing the receive path?
From: Jason Baron @ 2014-10-07 17:35 UTC (permalink / raw)
  To: Vlad Yasevich, David Miller; +Cc: netdev@vger.kernel.org, kaber@trash.net
In-Reply-To: <5432936D.7010906@gmail.com>

On 10/06/2014 09:04 AM, Vlad Yasevich wrote:
> On 10/04/2014 08:42 PM, David Miller wrote:
>> From: Jason Baron <jbaron@akamai.com>
>> Date: Thu, 02 Oct 2014 16:28:13 -0400
>>
>>> --- a/drivers/net/macvlan.c
>>> +++ b/drivers/net/macvlan.c
>>> @@ -321,8 +321,8 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
>>>         skb->dev = dev;
>>>         skb->pkt_type = PACKET_HOST;
>>>  
>>> -       ret = netif_rx(skb);
>>> -
>>> +      macvlan_count_rx(vlan, len, true, 0);
>>> +      return RX_HANDLER_ANOTHER;
>>>  out:
>>>         macvlan_count_rx(vlan, len, ret == NET_RX_SUCCESS, 0);
>>>         return RX_HANDLER_CONSUMED;
>>
>> That last argument to macvlan_count_rx() is a bool and thus should be
>> specified as "false".  Yes I know other areas of this file get it
>> wrong too.
>>

ok. I can fix those up too while here.

>> Also, what about GRO?  Won't we get GRO processing if we do this via
>> netif_rx() but not via the RX_HANDLER_ANOTHER route?  Just curious...
> 
> Wouldn't GRO already happen at the lower level?  For macvlan-to-macvlan,
> you'd typically have large packets so no need for GRO.
> 

Yes, afaict gro is happening a layer below __netif_receive_skb_core().

Here are some results of this optimization on 3.17 using macvlan with
lxc. Test case is (average of 3 runs):

for i in {35,50,65,80,95,110,125,140,155};
do super_netperf $i netperf -H $ip -t TCP_RR;
done

trans./sec (3.17)

494016
612806
673100
696982
710494
716830
714729
713478
711056

trans./sec (3.17 + macvlan patch)

517159  +(4.684733558%)
628382  +(2.541860742%)
669688  -(0.5069080835%)
706181  +(1.319833855%)
716660  +(0.8677995555%)
719581  +(0.3838661811%)
718738  +(0.5609585358%)
718904  +(0.7605470482%)
718344  +(1.02509555%)

On the host I can see that the idle time goes to 0, so this would
appear to be an improvement. I also observed that enqueue_to_backlog()
and process_backlog() are no longer in the 'perf' profiles as
expected.

So if there are no objections, I will post as a formal patch.

Thanks,

-Jason

^ permalink raw reply

* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Luis R. Rodriguez @ 2014-10-07 17:50 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, tiwai, arjan, teg,
	rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
	linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
	One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
	Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
	Abhijit Mahajan, Casey Leedom
In-Reply-To: <20141007173404.GB31328@mtj.dyndns.org>

On Tue, Oct 07, 2014 at 01:34:04PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Tue, Oct 07, 2014 at 01:10:46AM +0200, Luis R. Rodriguez wrote:
> > On Mon, Oct 06, 2014 at 05:01:18PM -0400, Tejun Heo wrote:
> > > For in-kernel stuff, we already have a clear
> > > synchronization point where we already synchronize all async calls.
> > > Shouldn't we be flushing these async probes there too?
> > 
> > This seems to be addressing if what I meant by prepared, "ready", so let
> > me address this as I do think its important.
> > 
> > By async calls do you mean users of async_schedule()? I see it
> 
> Yes.
> 
> > also uses system_unbound_wq as well but I do not see anyone calling
> > flush_workqueue(system_unbound_wq) on the kernel. We do use
> > async_synchronize_full() on kernel_init() but that just waits.
> 
> But you can create a new workqueue and queue all the async probing
> work items there and flush the workqueue right after
> async_synchronize_full().

On second thought I would prefer to avoid this, I see this being good
to help with old userspace but other than that I don't see a requirement
for new userspace. Do you?

> ...
> > bus.enable_kern_async=1 would still also serve as a helper for the driver core
> > to figure out if it should use async probe then on modules if prefer_async_probe
> > was enabled. Let me know if you figure out a way to avoid it.
> 
> Why do we need the choice at all?  It always should, no?

I'm OK to live with that, in that case I see no point to bus.enable_kern_async=1
at all.

  Luis

^ permalink raw reply

* Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
From: Tejun Heo @ 2014-10-07 17:55 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, tiwai, arjan, teg,
	rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
	linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
	One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
	Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
	Abhijit Mahajan, Casey Leedom
In-Reply-To: <20141007175010.GH14081@wotan.suse.de>

Hello,

On Tue, Oct 07, 2014 at 07:50:10PM +0200, Luis R. Rodriguez wrote:
> On Tue, Oct 07, 2014 at 01:34:04PM -0400, Tejun Heo wrote:
> > But you can create a new workqueue and queue all the async probing
> > work items there and flush the workqueue right after
> > async_synchronize_full().
> 
> On second thought I would prefer to avoid this, I see this being good
> to help with old userspace but other than that I don't see a requirement
> for new userspace. Do you?

Hmmm... we batch up and do everything parallel, so I'm not sure how
much gain we'd be looking at by not waiting for at the end before
jumping into the userland.  Also, it's a bit of an orthogonal issue.
If we wanna skip such synchornization point before passing control to
userland, why are we applying that to this but not
async_synchronize_full() which has a far larger impact?  It's weird to
synchronize one while not the other, so yeah, if there are actual
benefits we can consider it but let's do it separately.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: macvlan: optimizing the receive path?
From: Eric Dumazet @ 2014-10-07 18:00 UTC (permalink / raw)
  To: Jason Baron
  Cc: Vlad Yasevich, David Miller, netdev@vger.kernel.org,
	kaber@trash.net
In-Reply-To: <5434246E.1000403@akamai.com>

On Tue, 2014-10-07 at 13:35 -0400, Jason Baron wrote:

> So if there are no objections, I will post as a formal patch.

I think the code used netif_rx() as a precaution because of
multicast/broadcasts.

Now these are taken care (if multicast filter + work queue for
broadcasts), it seems ok to have a fast path.

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Tom Herbert @ 2014-10-07 18:13 UTC (permalink / raw)
  To: David Miller, Linux Netdev List, Or Gerlitz
In-Reply-To: <1411962607-27878-1-git-send-email-therbert@google.com>

David,

I don't think there are any outstanding objections to this patch.
Will you be able to apply it or do you need something more to be done?

Thanks,
Tom


On Sun, Sep 28, 2014 at 8:50 PM, Tom Herbert <therbert@google.com> wrote:
> Add ndo_gso_check which a device can define to indicate whether is
> is capable of doing GSO on a packet. This funciton would be called from
> the stack to determine whether software GSO is needed to be done. A
> driver should populate this function if it advertises GSO types for
> which there are combinations that it wouldn't be able to handle. For
> instance a device that performs UDP tunneling might only implement
> support for transparent Ethernet bridging type of inner packets
> or might have limitations on lengths of inner headers.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
>  include/linux/netdevice.h | 12 +++++++++++-
>  net/core/dev.c            |  2 +-
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 9f5d293..f8c2027 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -997,6 +997,12 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>   *     Callback to use for xmit over the accelerated station. This
>   *     is used in place of ndo_start_xmit on accelerated net
>   *     devices.
> + * bool        (*ndo_gso_check) (struct sk_buff *skb,
> + *                       struct net_device *dev);
> + *     Called by core transmit path to determine if device is capable of
> + *     performing GSO on a packet. The device returns true if it is
> + *     able to GSO the packet, false otherwise. If the return value is
> + *     false the stack will do software GSO.
>   */
>  struct net_device_ops {
>         int                     (*ndo_init)(struct net_device *dev);
> @@ -1146,6 +1152,8 @@ struct net_device_ops {
>                                                         struct net_device *dev,
>                                                         void *priv);
>         int                     (*ndo_get_lock_subclass)(struct net_device *dev);
> +       bool                    (*ndo_gso_check) (struct sk_buff *skb,
> +                                                 struct net_device *dev);
>  };
>
>  /**
> @@ -3536,10 +3544,12 @@ static inline bool skb_gso_ok(struct sk_buff *skb, netdev_features_t features)
>                (!skb_has_frag_list(skb) || (features & NETIF_F_FRAGLIST));
>  }
>
> -static inline bool netif_needs_gso(struct sk_buff *skb,
> +static inline bool netif_needs_gso(struct net_device *dev, struct sk_buff *skb,
>                                    netdev_features_t features)
>  {
>         return skb_is_gso(skb) && (!skb_gso_ok(skb, features) ||
> +               (dev->netdev_ops->ndo_gso_check &&
> +                !dev->netdev_ops->ndo_gso_check(skb, dev)) ||
>                 unlikely((skb->ip_summed != CHECKSUM_PARTIAL) &&
>                          (skb->ip_summed != CHECKSUM_UNNECESSARY)));
>  }
> diff --git a/net/core/dev.c b/net/core/dev.c
> index e2ced01..8c2b9bb 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2680,7 +2680,7 @@ struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
>         if (skb->encapsulation)
>                 features &= dev->hw_enc_features;
>
> -       if (netif_needs_gso(skb, features)) {
> +       if (netif_needs_gso(dev, skb, features)) {
>                 struct sk_buff *segs;
>
>                 segs = skb_gso_segment(skb, features);
> --
> 2.1.0.rc2.206.gedb03e5
>

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Alexei Starovoitov @ 2014-10-07 18:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Tom Herbert, Jesse Gross, Or Gerlitz, Alexander Duyck,
	John Fastabend, Jeff Kirsher, David Miller, Linux Netdev List,
	Thomas Graf, Pravin Shelar, Andy Zhou
In-Reply-To: <1412702616.11091.143.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Oct 7, 2014 at 10:23 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-10-07 at 09:50 -0700, Alexei Starovoitov wrote:
>
>> it's definitely more difficult to properly implement
>> CHECKSUM_UNNECESSARY in HW, but it's worth it.
>> CHECKSUM_COMPLETE is a burden on software. Old NICs
>> used to do that, but overhead of recomputing csum for every
>> step of packet parsing and header modifications is too high.
>> sw routers, bridges and < L4 networking devices are
>> simpler and faster with CHECKSUM_UNNECESSARY.
>
> Really this is wrong. Once you validated/pulled a header,
> adjusting the complete checksum is in the order of 10 cycles or so,
> ie less than 1% of the other costs.

correct, but there is also postpull() cost for every pull.
and there are many of them for encapsulated traffic.
It's small, but it's not zero.

> UNNECESSARY usually requests complex NIC firmware, and usually the NIC
> has fewer cores than the host.
>
> It mostly worked for basic IP+TCP kind of traffic, but once you want
> complex cloud models, it is a major pain.
>
> If we need to optimize csum_partial() for short lengths, lets do it,
> instead of pushing hardware vendors adding more and more schemes.

csum_partial() is in asm already. probably not much more can be squeezed.
I'm not suggesting that NICs must always do UNNECESSARY.
COMPLETE is a low bar which is magnitude better than NONE,
but if HW has programmable parser it should be able to take advantage
of it and UNNECESSARY is an established model.

^ permalink raw reply

* [PATCH net-next v2 1/1] tipc: fix bug in multicast congestion handling
From: Jon Maloy @ 2014-10-07 18:12 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy

One aim of commit 50100a5e39461b2a61d6040e73c384766c29975d ("tipc:
use pseudo message to wake up sockets after link congestion") was
to handle link congestion abatement in a uniform way for both unicast
and multicast transmit. However, the latter doesn't work correctly,
and has been broken since the referenced commit was applied.

If a user now sends a burst of multicast messages that is big
enough to cause broadcast link congestion, it will be put to sleep,
and not be waked up when the congestion abates as it should be.

This has two reasons. First, the flag that is used, TIPC_WAKEUP_USERS,
is set correctly, but in the wrong field. Instead of setting it in the
'action_flags' field of the arrival node struct, it is by mistake set
in the dummy node struct that is owned by the broadcast link, where it
will never tested for. Second, we cannot use the same flag for waking
up unicast and multicast users, since the function tipc_node_unlock()
needs to pick the wakeup pseudo messages to deliver from different
queues. It must hence be able to distinguish between the two cases.

This commit solves this problem by adding a new flag
TIPC_WAKEUP_BCAST_USERS, and a new function tipc_bclink_wakeup_user().
The latter is to be called by tipc_node_unlock() when the named flag,
now set in the correct field, is encountered.

v2: using explicit 'unsigned int' declaration instead of 'uint', as
per comment from David Miller.

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bcast.c | 14 +++++++++++++-
 net/tipc/bcast.h |  2 +-
 net/tipc/node.c  |  5 +++++
 net/tipc/node.h  |  3 ++-
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index b2bbe69..b8670bf 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -226,6 +226,17 @@ static void bclink_retransmit_pkt(u32 after, u32 to)
 }
 
 /**
+ * tipc_bclink_wakeup_users - wake up pending users
+ *
+ * Called with no locks taken
+ */
+void tipc_bclink_wakeup_users(void)
+{
+	while (skb_queue_len(&bclink->link.waiting_sks))
+		tipc_sk_rcv(skb_dequeue(&bclink->link.waiting_sks));
+}
+
+/**
  * tipc_bclink_acknowledge - handle acknowledgement of broadcast packets
  * @n_ptr: node that sent acknowledgement info
  * @acked: broadcast sequence # that has been acknowledged
@@ -300,7 +311,8 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
 		bclink_set_last_sent();
 	}
 	if (unlikely(released && !skb_queue_empty(&bcl->waiting_sks)))
-		bclink->node.action_flags |= TIPC_WAKEUP_USERS;
+		n_ptr->action_flags |= TIPC_WAKEUP_BCAST_USERS;
+
 exit:
 	tipc_bclink_unlock();
 }
diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h
index 4875d95..e7b0f85 100644
--- a/net/tipc/bcast.h
+++ b/net/tipc/bcast.h
@@ -99,5 +99,5 @@ int  tipc_bclink_set_queue_limits(u32 limit);
 void tipc_bcbearer_sort(struct tipc_node_map *nm_ptr, u32 node, bool action);
 uint  tipc_bclink_get_mtu(void);
 int tipc_bclink_xmit(struct sk_buff *buf);
-
+void tipc_bclink_wakeup_users(void);
 #endif
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 17e6378..90cee4a 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -552,6 +552,7 @@ void tipc_node_unlock(struct tipc_node *node)
 	LIST_HEAD(conn_sks);
 	struct sk_buff_head waiting_sks;
 	u32 addr = 0;
+	unsigned int flags = node->action_flags;
 
 	if (likely(!node->action_flags)) {
 		spin_unlock_bh(&node->lock);
@@ -572,6 +573,7 @@ void tipc_node_unlock(struct tipc_node *node)
 		node->action_flags &= ~TIPC_NOTIFY_NODE_UP;
 		addr = node->addr;
 	}
+	node->action_flags &= ~TIPC_WAKEUP_BCAST_USERS;
 	spin_unlock_bh(&node->lock);
 
 	while (!skb_queue_empty(&waiting_sks))
@@ -583,6 +585,9 @@ void tipc_node_unlock(struct tipc_node *node)
 	if (!list_empty(&nsub_list))
 		tipc_nodesub_notify(&nsub_list);
 
+	if (flags & TIPC_WAKEUP_BCAST_USERS)
+		tipc_bclink_wakeup_users();
+
 	if (addr)
 		tipc_named_node_up(addr);
 }
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 522d6f3..67513c3 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -59,7 +59,8 @@ enum {
 	TIPC_WAIT_OWN_LINKS_DOWN	= (1 << 2),
 	TIPC_NOTIFY_NODE_DOWN		= (1 << 3),
 	TIPC_NOTIFY_NODE_UP		= (1 << 4),
-	TIPC_WAKEUP_USERS		= (1 << 5)
+	TIPC_WAKEUP_USERS		= (1 << 5),
+	TIPC_WAKEUP_BCAST_USERS		= (1 << 6)
 };
 
 /**
-- 
1.9.1

^ permalink raw reply related

* Re: Quota in __qdisc_run()
From: Jesper Dangaard Brouer @ 2014-10-07 18:03 UTC (permalink / raw)
  To: David Miller
  Cc: eric.dumazet, hannes, netdev, therbert, fw, dborkman, jhs,
	alexander.duyck, john.r.fastabend, dave.taht, toke, brouer
In-Reply-To: <20141007.131938.1410434352331637585.davem@davemloft.net>

On Tue, 07 Oct 2014 13:19:38 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 07 Oct 2014 08:01:20 -0700
> 
> > On Tue, 2014-10-07 at 16:43 +0200, Hannes Frederic Sowa wrote:
> > 
> >> This needs to be:
> >> 
> >> do
> >>    ...
> >> while ((iskb = iskb->next))
> > 
> > I do not feel needed to break the bulk dequeue at precise quota
> > boundary. These quotas are advisory, and bql prefers to get its full
> > budget for appropriate feedback from TX completion.
> > 
> > Quota was a packet quota, which was quite irrelevant if segmentation had
> > to be done, so I would just let the dequeue be done so that we benefit
> > from optimal xmit_more.
> 
> Yes, this makes sense, do a full qdisc_restart() cycle without boundaries,
> then check how much quota was used afterwards to guard the outermost loop.

According to my measurements, at 10Gbit/s TCP_STREAM test the BQL limit
is 381528 bytes / 1514 = 252 packets, that will (potentially) be bulk
dequeued at once (with your version of the patch).

It seems to have the potential to exceed the weight_p(64) quite a lot.
And with e.g. TX ring size 512, we also also challenge the drivers at
this early adoption phase of tailptr writes.  Just saying...

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: Quota in __qdisc_run()
From: Jesper Dangaard Brouer @ 2014-10-07 18:37 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, hannes, netdev, therbert, fw, dborkman, jhs,
	alexander.duyck, john.r.fastabend, dave.taht, toke, brouer
In-Reply-To: <1412703132.11091.144.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, 07 Oct 2014 10:32:12 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Tue, 2014-10-07 at 13:19 -0400, David Miller wrote:
> 
> > Yes, this makes sense, do a full qdisc_restart() cycle without boundaries,
> > then check how much quota was used afterwards to guard the outermost loop.
> 
> I am testing this, and also am testing the xmit_more patch for I40E.

Check, I'm also testing both yours and Hannes patch.

Results at:
 http://people.netfilter.org/hawk/qdisc/measure18_restore_quota_fairness/
 http://people.netfilter.org/hawk/qdisc/measure19_restore_quota_erics/
 http://people.netfilter.org/hawk/qdisc/measure20_no_quota_baseline_at_git_02c0fc1/

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ 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