* RE: [PATCH] net:fec: fix WARNING caused by lack of calls to dma_mapping_error()
From: Fugang Duan @ 2013-11-13 2:10 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Frank Li, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <1384306989.3802.71.camel@bwh-desktop.uk.level5networks.com>
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Data: Wednesday, November 13, 2013 9:43 AM
>To: Duan Fugang-B38611
>Cc: Li Frank-B20596; davem@davemloft.net; netdev@vger.kernel.org
>Subject: Re: [PATCH] net:fec: fix WARNING caused by lack of calls to
>dma_mapping_error()
>
>On Wed, 2013-11-13 at 01:00 +0000, Ben Hutchings wrote:
>> On Tue, 2013-11-12 at 10:54 +0800, Fugang Duan wrote:
>> > Enable CONFIG_HAVE_DMA_API_DEBUG, the kernel dump warning:
>[...]
>> > @@ -1001,6 +1005,9 @@ fec_enet_rx(struct net_device *ndev, int
>> > budget)
>> >
>> > bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, data,
>> > FEC_ENET_TX_FRSIZE, DMA_FROM_DEVICE);
>> > + /* here dma mapping shouldn't be error, just avoid kernel dump */
>> > + if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr))
>> > + netdev_err(ndev, "Rx DMA memory map failed\n");
>>
>> This is not handling the error.
>[...]
>
>It looks like the same RX DMA buffers are used repeatedly and the driver copies
>into an skb. So fec_enet_rx() should not map or unmap at all; it should use
>dma_sync_single_for_cpu(). Then, indeed, no error handling is required.
>
Yes, there don't need to use dma_unmap_single()/dma_map_single() since the fec_enet_alloc_buffers() have mapped the virtual address.
I will change it. Thanks.
>But there's still a big problem with this function. Instead of remembering the
>virtual address of each buffer, it passes the DMA address to __va(). Physical
>and DMA addresses are *not* the same thing!
>They may differ if the device is behind an IOMMU or bounce buffering was
>required, or just because there is a fixed offset between the device and host
>physical addresses.
>
Yes, I agree that physical and DMA addresses are not the same thing.
But for imx serial platforms, there have no IOMMU, no offset between device and host physical address(this happen on FPGA platforms in mostly), so there have no problem.
I agree it is not common for other platforms like IOMMU...
Since it is another problem, I will submit another patch to fix the problem.
Thanks, Ben.
>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's
>the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply
* Re: gso: Handle new frag_list of frags GRO packets
From: Eric Dumazet @ 2013-11-13 2:14 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131113012920.GA27897@gondor.apana.org.au>
On Wed, 2013-11-13 at 09:29 +0800, Herbert Xu wrote:
> On Tue, Nov 12, 2013 at 05:13:43PM -0800, Eric Dumazet wrote:
> >
> > Note this pskb_trim() will reallocate/copy nskb head completely, since
> > nskb is a clone. (And increment page counts of frags, then eventually
> > decrement them)
> >
> > I tested this patch one 'router', and it seems fine, although we consume
> > ~90% more cpu doing the skb_segment() than not doing it.
>
> I presume this is on a NIC that produces completely linear packets?
Yes, I used one host with a Mellanox (mlx4 driver), as the bnx2x 'GRO'
is partially done by the hardware...
> I wonder what happens if we don't convert head_frag to frags and just
> use frag_lists like we used to?
Yep, but this needs quite invasive changes in the GSO stack in general ?
skb_segment() callers would need to know how many MSS are stuffed per
skb, instead of assuming its 1 MSS.
^ permalink raw reply
* Re: gso: Handle new frag_list of frags GRO packets
From: Eric Dumazet @ 2013-11-13 2:17 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131113012920.GA27897@gondor.apana.org.au>
On Wed, 2013-11-13 at 09:29 +0800, Herbert Xu wrote:
> I presume this is on a NIC that produces completely linear packets?
Sorry : with mlx4 driver, GRO builds nice skbs with one page frag per
MSS, so each skb found on frag_list is fully loaded with 16 MSS
^ permalink raw reply
* [PATCH net RESEND] bonding: add ip checks when store ip target
From: Ding Tianhong @ 2013-11-13 2:19 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
I met a Bug when I add ip target with the wrong ip address:
echo +500.500.500.500 > /sys/class/net/bond0/bonding/arp_ip_target
the wrong ip address will transfor to 245.245.245.244 and add
to the ip target success, it is uncorrect, so I add checks to avoid
adding wrong address.
The in4_pton() will set wrong ip address to 0.0.0.0, it will return by
the next check and will not add to ip target.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index ec9b646..e0c97fb 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -653,7 +653,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
int ind, i, j, ret = -EINVAL;
targets = bond->params.arp_targets;
- newtarget = in_aton(buf + 1);
+ in4_pton(buf + 1, strlen(buf) - 1, (u8 *)&newtarget, -1, NULL);
/* look for adds */
if (buf[0] == '+') {
if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
--
1.8.0
^ permalink raw reply related
* Re: gso: Handle new frag_list of frags GRO packets
From: Herbert Xu @ 2013-11-13 2:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1384309034.28458.69.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 12, 2013 at 06:17:14PM -0800, Eric Dumazet wrote:
> On Wed, 2013-11-13 at 09:29 +0800, Herbert Xu wrote:
>
> > I presume this is on a NIC that produces completely linear packets?
>
> Sorry : with mlx4 driver, GRO builds nice skbs with one page frag per
> MSS, so each skb found on frag_list is fully loaded with 16 MSS
OK, so what are the numbers when GRO is off completely?
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: gso: Handle new frag_list of frags GRO packets
From: Herbert Xu @ 2013-11-13 2:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131113022243.GA28204@gondor.apana.org.au>
On Wed, Nov 13, 2013 at 10:22:43AM +0800, Herbert Xu wrote:
> On Tue, Nov 12, 2013 at 06:17:14PM -0800, Eric Dumazet wrote:
> > On Wed, 2013-11-13 at 09:29 +0800, Herbert Xu wrote:
> >
> > > I presume this is on a NIC that produces completely linear packets?
> >
> > Sorry : with mlx4 driver, GRO builds nice skbs with one page frag per
> > MSS, so each skb found on frag_list is fully loaded with 16 MSS
>
> OK, so what are the numbers when GRO is off completely?
Actually don't bother, it's not a fair comparison at all. In the
first case we're doing TSO and with my patch we're only doing GSO.
So a better test for the time being would be to test with TSO
disabled in both cases.
In the mean time I'm cooking up a patch to generate TSO packets.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: gso: Handle new frag_list of frags GRO packets
From: Eric Dumazet @ 2013-11-13 2:31 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131113022243.GA28204@gondor.apana.org.au>
On Wed, 2013-11-13 at 10:22 +0800, Herbert Xu wrote:
> On Tue, Nov 12, 2013 at 06:17:14PM -0800, Eric Dumazet wrote:
> > On Wed, 2013-11-13 at 09:29 +0800, Herbert Xu wrote:
> >
> > > I presume this is on a NIC that produces completely linear packets?
> >
> > Sorry : with mlx4 driver, GRO builds nice skbs with one page frag per
> > MSS, so each skb found on frag_list is fully loaded with 16 MSS
>
> OK, so what are the numbers when GRO is off completely?
Pretty bad, as we drop many packets.
My (single) tcp flow is no longer full speed. (link speed is 10Gb)
lpaa6:~# mpstat -P 3 1 10
06:28:11 PM CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle intr/s
06:28:12 PM 3 2.04 0.00 7.14 0.00 5.10 73.47 0.00 0.00 12.24 28594.90
06:28:13 PM 3 3.00 0.00 3.00 0.00 5.00 74.00 0.00 0.00 15.00 28492.00
06:28:14 PM 3 0.00 0.00 0.99 0.00 5.94 93.07 0.00 0.00 0.00 35948.51
06:28:15 PM 3 5.00 0.00 4.00 0.00 5.00 74.00 0.00 0.00 12.00 28699.00
06:28:16 PM 3 3.06 0.00 5.10 0.00 5.10 75.51 0.00 0.00 11.22 30037.76
06:28:17 PM 3 0.00 0.00 0.00 0.00 6.00 92.00 0.00 0.00 2.00 36338.00
06:28:18 PM 3 1.00 0.00 2.00 0.00 5.00 73.00 0.00 0.00 19.00 28230.00
06:28:19 PM 3 0.00 0.00 2.02 0.00 6.06 86.87 0.00 0.00 5.05 33987.88
06:28:20 PM 3 0.00 0.00 0.00 0.00 6.00 81.00 0.00 0.00 13.00 32086.00
06:28:21 PM 3 1.98 0.00 1.98 0.00 4.95 73.27 0.00 0.00 17.82 28383.17
Average: 3 1.60 0.00 2.61 0.00 5.42 79.64 0.00 0.00 10.73 31086.06
perf profile (GRO off)
6.88% [kernel] [k] fib_table_lookup
5.97% [kernel] [k] _raw_spin_lock
5.40% [kernel] [k] ipt_do_table
4.79% [mlx4_en] [k] mlx4_en_process_rx_cq
4.29% [kernel] [k] htb_dequeue
3.65% [mlx4_en] [k] mlx4_en_xmit
2.48% [kernel] [k] kmem_cache_free
2.27% [kernel] [k] __netif_receive_skb_core
2.23% [kernel] [k] local_bh_enable
2.22% [mlx4_en] [k] mlx4_en_tx_irq
2.17% [kernel] [k] ip_route_input_noref
2.16% [kernel] [k] kmem_cache_alloc
2.03% [kernel] [k] check_leaf.isra.7
1.89% [kernel] [k] inet_getpeer
1.62% [kernel] [k] nf_iterate
1.44% [kernel] [k] fib_validate_source
1.42% [kernel] [k] dev_kfree_skb_irq
1.39% [kernel] [k] build_skb
1.38% [kernel] [k] ip_rcv
1.26% [kernel] [k] htb_lookup_leaf
1.23% [kernel] [k] dev_queue_xmit
1.17% [kernel] [k] _raw_spin_lock_bh
1.14% [mlx4_en] [k] mlx4_en_complete_rx_desc
1.08% [unknown] [.] 0x00000000019cdd6d
1.03% [kernel] [k] htb_deactivate_prios
0.94% [kernel] [k] htb_activate_prios
0.91% [kernel] [k] htb_enqueue
0.89% [kernel] [k] ip_forward
0.87% [mlx4_en] [k] mlx4_en_free_tx_desc.isra.25
0.84% [mlx4_en] [k] mlx4_en_alloc_frags
0.83% [kernel] [k] __netdev_alloc_frag
0.83% [kernel] [k] read_tsc
0.83% [kernel] [k] netif_receive_skb
0.81% [kernel] [k] nf_hook_slow
0.80% [kernel] [k] dst_alloc
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Vlad Yasevich @ 2013-11-13 2:37 UTC (permalink / raw)
To: Chang Xiangzhong, nhorman, davem; +Cc: linux-sctp, netdev, linux-kernel, dreibh
In-Reply-To: <1384306486-31345-1-git-send-email-changxiangzhong@gmail.com>
On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
> Look for the __two__ most recently used path/transport and set to active_path
> and retran_path respectively
>
> Signed-off-by: changxiangzhong@gmail.com
> ---
> net/sctp/associola.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index ab67efc..070011a 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
> if (!first || t->last_time_heard > first->last_time_heard) {
> second = first;
> first = t;
> + continue;
> }
> if (!second || t->last_time_heard > second->last_time_heard)
> second = t;
You might as well remove this bit and then you don't need a continue.
> }
>
> + if (!second)
> + second = first;
> +
This needs to move down 1 more block. Set the second transport after we
check to see if the primary is back up and we need to go back to using it.
-vlad
> /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
> *
> * By default, an endpoint should always transmit to the
>
^ permalink raw reply
* Re: gso: Handle new frag_list of frags GRO packets
From: Eric Dumazet @ 2013-11-13 2:45 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131113022525.GA28236@gondor.apana.org.au>
On Wed, 2013-11-13 at 10:25 +0800, Herbert Xu wrote:
> So a better test for the time being would be to test with TSO
> disabled in both cases.
>
GRO on, TSO off, little difference between two cases :
(Note some small things run in background, so these numbers are not
ultra precise)
1) Full size GRO packets (frag_list enabled)
lpaa6:~# mpstat -P 3 1 10
06:39:32 PM CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle intr/s
06:39:33 PM 3 3.23 0.00 43.55 0.00 12.90 0.00 0.00 0.00 40.32 123209.68
06:39:34 PM 3 3.39 0.00 28.81 0.00 16.95 0.00 0.00 0.00 50.85 84559.32
06:39:35 PM 3 1.79 0.00 3.57 0.00 10.71 0.00 0.00 0.00 83.93 108898.21
06:39:36 PM 3 3.39 0.00 30.51 0.00 16.95 0.00 0.00 0.00 49.15 84661.02
06:39:37 PM 3 0.00 0.00 7.46 0.00 10.45 0.00 0.00 0.00 82.09 72135.82
06:39:38 PM 3 1.56 0.00 26.56 0.00 9.38 0.00 0.00 0.00 62.50 73562.50
06:39:39 PM 3 0.00 0.00 8.47 0.00 15.25 0.00 0.00 0.00 76.27 75715.25
06:39:40 PM 3 8.33 0.00 8.33 0.00 2.08 0.00 0.00 0.00 81.25 129195.83
06:39:41 PM 3 1.56 0.00 23.44 0.00 23.44 0.00 0.00 0.00 51.56 73389.06
06:39:42 PM 3 0.00 0.00 1.96 0.00 17.65 0.00 0.00 0.00 80.39 102474.51
Average: 3 2.21 0.00 18.85 0.00 13.75 0.00 0.00 0.00 65.20 91433.11
2) No frag_list skbs
lpaa6:~# mpstat -P 3 1 10
06:39:56 PM CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle intr/s
06:39:57 PM 3 0.00 0.00 2.63 0.00 26.32 0.00 0.00 0.00 71.05 173105.26
06:39:58 PM 3 3.85 0.00 13.46 0.00 15.38 0.00 0.00 0.00 67.31 121346.15
06:39:59 PM 3 9.09 0.00 18.18 0.00 14.55 0.00 0.00 0.00 58.18 84216.36
06:40:00 PM 3 0.00 0.00 10.53 0.00 10.53 2.63 0.00 0.00 76.32 170863.16
06:40:01 PM 3 3.45 0.00 20.69 0.00 18.97 0.00 0.00 0.00 56.90 83941.38
06:40:02 PM 3 0.00 0.00 5.88 0.00 19.61 0.00 0.00 0.00 74.51 125625.49
06:40:03 PM 3 1.85 0.00 24.07 0.00 16.67 0.00 0.00 0.00 57.41 88242.59
06:40:04 PM 3 3.33 0.00 13.33 0.00 13.33 0.00 0.00 0.00 70.00 74181.67
06:40:05 PM 3 0.00 0.00 10.00 0.00 16.00 2.00 0.00 0.00 72.00 150906.00
06:40:06 PM 3 3.28 0.00 44.26 0.00 14.75 0.00 0.00 0.00 37.70 79821.31
Average: 3 2.71 0.00 17.41 0.00 16.44 0.39 0.00 0.00 63.06 110094.00
> In the mean time I'm cooking up a patch to generate TSO packets.
That would be very nice !
Thanks
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Chang @ 2013-11-13 2:54 UTC (permalink / raw)
To: Vlad Yasevich, nhorman, davem; +Cc: linux-sctp, netdev, linux-kernel, dreibh
In-Reply-To: <5282E5F9.5000506@gmail.com>
On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>> Look for the __two__ most recently used path/transport and set to
>> active_path
>> and retran_path respectively
>>
>> Signed-off-by: changxiangzhong@gmail.com
>> ---
>> net/sctp/associola.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>> index ab67efc..070011a 100644
>> --- a/net/sctp/associola.c
>> +++ b/net/sctp/associola.c
>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct
>> sctp_association *asoc,
>> if (!first || t->last_time_heard > first->last_time_heard) {
>> second = first;
>> first = t;
>> + continue;
>> }
>> if (!second || t->last_time_heard > second->last_time_heard)
>> second = t;
>
> You might as well remove this bit and then you don't need a continue.
I don't think we could remove this bit. My understanding of these
algorithms are to find the 1st recently used path and the 2nd, assigning
to active_path and retran_path respectively. If we remove the
looking-for-second block, how are we suppose to find the 2nd?
I think we can remove the continue and use else-if in the
2nd-assignment-block.
>
>> }
>>
>> + if (!second)
>> + second = first;
>> +
>
> This needs to move down 1 more block. Set the second transport after we
> check to see if the primary is back up and we need to go back to using
> it.
>
> -vlad
>
I agree with this change
>> /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>> *
>> * By default, an endpoint should always transmit to the
>>
>
^ permalink raw reply
* Re: [PATCH] net:fec: fix WARNING caused by lack of calls to dma_mapping_error()
From: Ben Hutchings @ 2013-11-13 3:02 UTC (permalink / raw)
To: Fugang Duan; +Cc: Frank Li, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <9848F2DB572E5649BA045B288BE08FBE0180475F@039-SN2MPN1-021.039d.mgd.msft.net>
On Wed, 2013-11-13 at 02:10 +0000, Fugang Duan wrote:
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Data: Wednesday, November 13, 2013 9:43 AM
>
> >To: Duan Fugang-B38611
> >Cc: Li Frank-B20596; davem@davemloft.net; netdev@vger.kernel.org
> >Subject: Re: [PATCH] net:fec: fix WARNING caused by lack of calls to
> >dma_mapping_error()
> >
> >On Wed, 2013-11-13 at 01:00 +0000, Ben Hutchings wrote:
> >> On Tue, 2013-11-12 at 10:54 +0800, Fugang Duan wrote:
> >> > Enable CONFIG_HAVE_DMA_API_DEBUG, the kernel dump warning:
> >[...]
> >> > @@ -1001,6 +1005,9 @@ fec_enet_rx(struct net_device *ndev, int
> >> > budget)
> >> >
> >> > bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, data,
> >> > FEC_ENET_TX_FRSIZE, DMA_FROM_DEVICE);
> >> > + /* here dma mapping shouldn't be error, just avoid kernel dump */
> >> > + if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr))
> >> > + netdev_err(ndev, "Rx DMA memory map failed\n");
> >>
> >> This is not handling the error.
> >[...]
> >
> >It looks like the same RX DMA buffers are used repeatedly and the driver copies
> >into an skb. So fec_enet_rx() should not map or unmap at all; it should use
> >dma_sync_single_for_cpu(). Then, indeed, no error handling is required.
> >
> Yes, there don't need to use dma_unmap_single()/dma_map_single() since
> the fec_enet_alloc_buffers() have mapped the virtual address.
> I will change it. Thanks.
>
> >But there's still a big problem with this function. Instead of remembering the
> >virtual address of each buffer, it passes the DMA address to __va(). Physical
> >and DMA addresses are *not* the same thing!
> >They may differ if the device is behind an IOMMU or bounce buffering was
> >required, or just because there is a fixed offset between the device and host
> >physical addresses.
> >
> Yes, I agree that physical and DMA addresses are not the same thing.
> But for imx serial platforms, there have no IOMMU, no offset between
> device and host physical address(this happen on FPGA platforms in
> mostly), so there have no problem.
Then DMA mapping also can't fail and your error checks are unnecessary.
But no doubt there will be a later chip that uses some version of FEC
and *does* require non-trivial DMA mapping. The Cortex A15 core can run
Xen which will break this assumption...
> I agree it is not common for other platforms like IOMMU...
> Since it is another problem, I will submit another patch to fix the
> problem.
That seems reasonable.
Aside from the DMA debug option, the kernel parameter 'swiotlb=force' is
a good way to test that a driver is using the DMA mapping API correctly.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: oom-kill && frozen()
From: Tejun Heo @ 2013-11-13 3:20 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Peter Zijlstra, David Rientjes, David Laight, Geert Uytterhoeven,
Ingo Molnar, netdev, linux-kernel
In-Reply-To: <20131112165643.GA31278@redhat.com>
Hello,
On Tue, Nov 12, 2013 at 05:56:43PM +0100, Oleg Nesterov wrote:
> On 11/12, Oleg Nesterov wrote:
> > I am also wondering if it makes any sense to turn PF_FROZEN into
> > TASK_FROZEN, something like (incomplete, probably racy) patch below.
> > Note that it actually adds the new state, not the the qualifier.
>
> As for the current usage of PF_FROZEN... David, it seems that
> oom_scan_process_thread()->__thaw_task() is dead? Probably this
> was fine before, when __thaw_task() cleared the "need to freeze"
> condition, iirc it was PF_FROZEN.
>
> But today __thaw_task() can't help, no? the task will simply
> schedule() in D state again.
Yeah, it'll have to be actively excluded using e.g. PF_FREEZER_SKIP,
which, BTW, can usually only be manipulated by the task itself. I've
been saying this multiple times but this is yet another cost of having
"frozen" as a separate completely alien task state, which is
essentially close to being undefined when viewed from userland. We're
spreading broken behaviors and complexity throughout the kernel by
making this half broken state visible. :(
Thanks.
--
tejun
^ permalink raw reply
* [PATCH v2 1/1] net:fec: fix WARNING caused by lack of calls to dma_mapping_error()
From: Fugang Duan @ 2013-11-13 3:04 UTC (permalink / raw)
To: bhutchings, davem; +Cc: netdev, b20596
The driver fails to check the results of DMA mapping and results in
the following warning: (with kernel config "CONFIG_DMA_API_DEBUG" enable)
------------[ cut here ]------------
WARNING: at lib/dma-debug.c:937 check_unmap+0x43c/0x7d8()
fec 2188000.ethernet: DMA-API: device driver failed to check map
error[device address=0x00000000383a8040] [size=2048 bytes] [mapped as single]
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.17-16827-g9cdb0ba-dirty #188
[<80013c4c>] (unwind_backtrace+0x0/0xf8) from [<80011704>] (show_stack+0x10/0x14)
[<80011704>] (show_stack+0x10/0x14) from [<80025614>] (warn_slowpath_common+0x4c/0x6c)
[<80025614>] (warn_slowpath_common+0x4c/0x6c) from [<800256c8>] (warn_slowpath_fmt+0x30/0x40)
[<800256c8>] (warn_slowpath_fmt+0x30/0x40) from [<8026bfdc>] (check_unmap+0x43c/0x7d8)
[<8026bfdc>] (check_unmap+0x43c/0x7d8) from [<8026c584>] (debug_dma_unmap_page+0x6c/0x78)
[<8026c584>] (debug_dma_unmap_page+0x6c/0x78) from [<8038049c>] (fec_enet_rx_napi+0x254/0x8a8)
[<8038049c>] (fec_enet_rx_napi+0x254/0x8a8) from [<804dc8c0>] (net_rx_action+0x94/0x160)
[<804dc8c0>] (net_rx_action+0x94/0x160) from [<8002c758>] (__do_softirq+0xe8/0x1d0)
[<8002c758>] (__do_softirq+0xe8/0x1d0) from [<8002c8e8>] (do_softirq+0x4c/0x58)
[<8002c8e8>] (do_softirq+0x4c/0x58) from [<8002cb50>] (irq_exit+0x90/0xc8)
[<8002cb50>] (irq_exit+0x90/0xc8) from [<8000ea88>] (handle_IRQ+0x3c/0x94)
[<8000ea88>] (handle_IRQ+0x3c/0x94) from [<8000855c>] (gic_handle_irq+0x28/0x5c)
[<8000855c>] (gic_handle_irq+0x28/0x5c) from [<8000de00>] (__irq_svc+0x40/0x50)
Exception stack(0x815a5f38 to 0x815a5f80)
5f20: 815a5f80 3b9aca00
5f40: 0fe52383 00000002 0dd8950e 00000002 81e7b080 00000000 00000000 815ac4d8
5f60: 806032ec 00000000 00000017 815a5f80 80059028 8041fc4c 60000013 ffffffff
[<8000de00>] (__irq_svc+0x40/0x50) from [<8041fc4c>] (cpuidle_enter_state+0x50/0xf0)
[<8041fc4c>] (cpuidle_enter_state+0x50/0xf0) from [<8041fd94>] (cpuidle_idle_call+0xa8/0x14c)
[<8041fd94>] (cpuidle_idle_call+0xa8/0x14c) from [<8000edac>] (arch_cpu_idle+0x10/0x4c)
[<8000edac>] (arch_cpu_idle+0x10/0x4c) from [<800582f8>] (cpu_startup_entry+0x60/0x130)
[<800582f8>] (cpu_startup_entry+0x60/0x130) from [<80bc7a48>] (start_kernel+0x2d0/0x328)
[<80bc7a48>] (start_kernel+0x2d0/0x328) from [<10008074>] (0x10008074)
---[ end trace c6edec32436e0042 ]---
Because dma-debug add new interfaces to debug dma mapping errors, pls refer
to: http://lwn.net/Articles/516640/
After dma mapping, it must call dma_mapping_error() to check mapping error,
otherwise the map_err_type alway is MAP_ERR_NOT_CHECKED, check_unmap() define
the mapping is not checked and dump the error msg. So,add dma_mapping_error()
checking to fix the WARNING
And RX DMA buffers are used repeatedly and the driver copies it into an skb,
fec_enet_rx() should not map or unmap, use dma_sync_single_for_cpu()/dma_sync_single_for_device()
instead of dma_map_single()/dma_unmap_single().
There have another potential issue: fec_enet_rx() passes the DMA address to __va().
Physical and DMA addresses are *not* the same thing. They may differ if the device
is behind an IOMMU or bounce buffering was required, or just because there is a fixed
offset between the device and host physical addresses. Also fix it in this patch.
=============================================
V2: add net_ratelimit() to limit map err message.
use dma_sync_single_for_cpu() instead of dma_map_single().
fix the issue that pass DMA addresses to __va() to get virture address.
V1: initial send
=============================================
Signed-off-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
drivers/net/ethernet/freescale/fec_main.c | 31 +++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index b2793b9..4cbebf3 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -386,7 +386,14 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
*/
bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
-
+ if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+ bdp->cbd_bufaddr = 0;
+ fep->tx_skbuff[index] = NULL;
+ dev_kfree_skb_any(skb);
+ if (net_ratelimit())
+ netdev_err(ndev, "Tx DMA memory map failed\n");
+ return NETDEV_TX_OK;
+ }
/* Send it on its way. Tell FEC it's ready, interrupt when done,
* it's the last BD of the frame, and to put the CRC on the end.
*/
@@ -861,6 +868,7 @@ fec_enet_rx(struct net_device *ndev, int budget)
struct bufdesc_ex *ebdp = NULL;
bool vlan_packet_rcvd = false;
u16 vlan_tag;
+ int index = 0;
#ifdef CONFIG_M532x
flush_cache_all();
@@ -916,10 +924,15 @@ fec_enet_rx(struct net_device *ndev, int budget)
ndev->stats.rx_packets++;
pkt_len = bdp->cbd_datlen;
ndev->stats.rx_bytes += pkt_len;
- data = (__u8*)__va(bdp->cbd_bufaddr);
- dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
- FEC_ENET_TX_FRSIZE, DMA_FROM_DEVICE);
+ if (fep->bufdesc_ex)
+ index = (struct bufdesc_ex *)bdp -
+ (struct bufdesc_ex *)fep->rx_bd_base;
+ else
+ index = bdp - fep->rx_bd_base;
+ data = fep->rx_skbuff[index]->data;
+ dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
+ FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
swap_buffer(data, pkt_len);
@@ -999,8 +1012,8 @@ fec_enet_rx(struct net_device *ndev, int budget)
napi_gro_receive(&fep->napi, skb);
}
- bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, data,
- FEC_ENET_TX_FRSIZE, DMA_FROM_DEVICE);
+ dma_sync_single_for_device(&fep->pdev->dev, bdp->cbd_bufaddr,
+ FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
rx_processing_done:
/* Clear the status flags for this buffer */
status &= ~BD_ENET_RX_STATS;
@@ -1719,6 +1732,12 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, skb->data,
FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
+ if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+ fec_enet_free_buffers(ndev);
+ if (net_ratelimit())
+ netdev_err(ndev, "Rx DMA memory map failed\n");
+ return -ENOMEM;
+ }
bdp->cbd_sc = BD_ENET_RX_EMPTY;
if (fep->bufdesc_ex) {
--
1.7.2.rc3
^ permalink raw reply related
* Euromillion:
From: Adrian and Gillian Bayford Donations @ 2013-11-13 0:26 UTC (permalink / raw)
--
My wife and i won 148million pounds in Euromillion jackpot lottery
august last year and we decide to give out 1.5million pounds globally to
5recipient, as the Google Management sent us your email yesterday as our
second recipient.Kindly Reply Name, Country,Occupation and Phone to
adrianbayford.gillian2012@gmail.com for further instructions, see news
headline http://www.bbc.co.uk/news/uk-england-19254228
Regards
Mr & Mrs Adrian Bayford.
^ permalink raw reply
* Re: [PATCH] rds: Error on offset mismatch if not loopback
From: Josh Hunt @ 2013-11-13 4:24 UTC (permalink / raw)
To: David Miller; +Cc: jjolly, LKML, venkat.x.venkatsubra, netdev
In-Reply-To: <CAKA=qzac9=UhLF_Z4FnnH+sR7xvkDux4oayC6dPYe=hMLsDxRg@mail.gmail.com>
On Tue, Nov 12, 2013 at 10:22 PM, Josh Hunt <joshhunt00@gmail.com> wrote:
> On Sat, Sep 22, 2012 at 2:25 PM, David Miller <davem@davemloft.net> wrote:
>>
>> From: John Jolly <jjolly@suse.com>
>> Date: Fri, 21 Sep 2012 15:32:40 -0600
>>
>> > Attempting an rds connection from the IP address of an IPoIB interface
>> > to itself causes a kernel panic due to a BUG_ON() being triggered.
>> > Making the test less strict allows rds-ping to work without crashing
>> > the machine.
>> >
>> > A local unprivileged user could use this flaw to crash the system.
>> >
>> > Signed-off-by: John Jolly <jjolly@suse.com>
>>
>> Besides the questions being asked of you by Venkat Venkatsubra, this
>> patch has another issue.
>>
>> It has been completely corrupted by your email client, it has
>> turned all TAB characters into spaces, making the patch useless.
>>
>> Please learn how to send a patch unmolested in the body of your
>> email. Test it by emailing the patch to yourself, and verifying
>> that you can in fact apply the patch you receive in that email.
>> Then, and only then, should you consider making a new submission
>> of this patch.
>>
>> Use Documentation/email-clients.txt for guidance.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
>
> I think this issue was lost in the shuffle. It appears that redhat, ubuntu,
> and oracle are maintaining local patches to resolve this:
>
> https://oss.oracle.com/git/?p=redpatch.git;a=commit;h=c7b6a0a1d8d636852be130fa15fa8be10d4704e8
> https://bugzilla.redhat.com/show_bug.cgi?id=822754
> http://ubuntu.5.x6.nabble.com/CVE-2012-2372-RDS-local-ping-DOS-td4985388.html
>
> Given that Oracle has applied it I'll make the assumption that Venkat's
> question was answered at some point.
>
> David - I can resubmit the patch with the proper signed-off-by and
> formatting if you are willing to apply it unless John wants to try again. I
> think it's time this got upstream.
>
> --
> Josh
Ugh.. hopefully resending with all the html crap removed...
--
Josh
^ permalink raw reply
* Re: [PATCH net 2/2] macvtap: limit head length of skb allocated
From: Jason Wang @ 2013-11-13 5:58 UTC (permalink / raw)
To: Greg Rose; +Cc: davem, netdev, linux-kernel, mst, stefanha
In-Reply-To: <20131112094501.0000358d@unknown>
On 11/13/2013 01:45 AM, Greg Rose wrote:
> On Tue, 12 Nov 2013 18:02:57 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> We currently use hdr_len as a hint of head length which is advertised
>> by guest. But when guest advertise a very big value, it can lead to
>> an 64K+ allocating of kmalloc() which has a very high possibility of
>> failure when host memory is fragmented or under heavy stress. The
>> huge hdr_len also reduce the effect of zerocopy or even disable if a
>> gso skb is linearized in guest.
>>
>> To solves those issues, this patch introduces an upper limit
>> (PAGE_SIZE) of the head, which guarantees an order 0 allocation each
>> time.
>>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> The patch was needed for stable.
>> ---
>> drivers/net/macvtap.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>> index 9dccb1e..7ee6f9d 100644
>> --- a/drivers/net/macvtap.c
>> +++ b/drivers/net/macvtap.c
>> @@ -523,6 +523,11 @@ static inline struct sk_buff
>> *macvtap_alloc_skb(struct sock *sk, size_t prepad, int noblock, int
>> *err) {
>> struct sk_buff *skb;
>> + int good_linear = SKB_MAX_HEAD(prepad);
>> +
>> + /* Don't use huge linear part */
>> + if (linear > good_linear)
>> + linear = good_linear;
>>
>> /* Under a page? Don't bother with paged skb. */
>> if (prepad + len < PAGE_SIZE || !linear)
> I see no problem with this or the tuntap patch except that in both
> cases kernel coding style would prefer that you align the local
> variable declarations in a reverse pyramid, longest at the beginning,
> shortest at the end.
>
> - Greg
Sure, will do it in V2.
Thanks
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* [PATCH net V2 2/2] macvtap: limit head length of skb allocated
From: Jason Wang @ 2013-11-13 6:00 UTC (permalink / raw)
To: davem, netdev, linux-kernel, mst, stefanha; +Cc: gregory.v.rose, Jason Wang
In-Reply-To: <1384322440-29038-1-git-send-email-jasowang@redhat.com>
We currently use hdr_len as a hint of head length which is advertised by
guest. But when guest advertise a very big value, it can lead to an 64K+
allocating of kmalloc() which has a very high possibility of failure when host
memory is fragmented or under heavy stress. The huge hdr_len also reduce the
effect of zerocopy or even disable if a gso skb is linearized in guest.
To solves those issues, this patch introduces an upper limit (PAGE_SIZE) of the
head, which guarantees an order 0 allocation each time.
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
The patch was needed for stable.
Changes from V1:
- Check the linear size in macvtap_get_user() to avoid iov_pages() under
estimation.
---
drivers/net/macvtap.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 9dccb1e..dc76670 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -628,6 +628,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
const struct iovec *iv, unsigned long total_len,
size_t count, int noblock)
{
+ int good_linear = SKB_MAX_HEAD(NET_IP_ALIGN);
struct sk_buff *skb;
struct macvlan_dev *vlan;
unsigned long len = total_len;
@@ -670,6 +671,8 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
+ if (copylen > good_linear)
+ copylen = good_linear;
linear = copylen;
if (iov_pages(iv, vnet_hdr_len + copylen, count)
<= MAX_SKB_FRAGS)
@@ -678,7 +681,10 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (!zerocopy) {
copylen = len;
- linear = vnet_hdr.hdr_len;
+ if (vnet_hdr.hdr_len > good_linear)
+ linear = good_linear;
+ else
+ linear = vnet_hdr.hdr_len;
}
skb = macvtap_alloc_skb(&q->sk, NET_IP_ALIGN, copylen,
--
1.8.3.2
^ permalink raw reply related
* [PATCH net V2 1/2] tuntap: limit head length of skb allocated
From: Jason Wang @ 2013-11-13 6:00 UTC (permalink / raw)
To: davem, netdev, linux-kernel, mst, stefanha; +Cc: gregory.v.rose, Jason Wang
We currently use hdr_len as a hint of head length which is advertised by
guest. But when guest advertise a very big value, it can lead to an 64K+
allocating of kmalloc() which has a very high possibility of failure when host
memory is fragmented or under heavy stress. The huge hdr_len also reduce the
effect of zerocopy or even disable if a gso skb is linearized in guest.
To solves those issues, this patch introduces an upper limit (PAGE_SIZE) of the
head, which guarantees an order 0 allocation each time.
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
The patch was needed for stable.
Changes from V1:
- check the linear size in tun_get_user() to avoid iov_pages() under estimation
---
drivers/net/tun.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 7cb105c..782e38b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -981,6 +981,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
struct sk_buff *skb;
size_t len = total_len, align = NET_SKB_PAD, linear;
struct virtio_net_hdr gso = { 0 };
+ int good_linear;
int offset = 0;
int copylen;
bool zerocopy = false;
@@ -1021,12 +1022,16 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
return -EINVAL;
}
+ good_linear = SKB_MAX_HEAD(align);
+
if (msg_control) {
/* There are 256 bytes to be copied in skb, so there is
* enough room for skb expand head in case it is used.
* The rest of the buffer is mapped from userspace.
*/
copylen = gso.hdr_len ? gso.hdr_len : GOODCOPY_LEN;
+ if (copylen > good_linear)
+ copylen = good_linear;
linear = copylen;
if (iov_pages(iv, offset + copylen, count) <= MAX_SKB_FRAGS)
zerocopy = true;
@@ -1034,7 +1039,10 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
if (!zerocopy) {
copylen = len;
- linear = gso.hdr_len;
+ if (gso.hdr_len > good_linear)
+ linear = good_linear;
+ else
+ linear = gso.hdr_len;
}
skb = tun_alloc_skb(tfile, align, copylen, linear, noblock);
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] rds: Error on offset mismatch if not loopback
From: David Miller @ 2013-11-13 6:09 UTC (permalink / raw)
To: joshhunt00; +Cc: jjolly, linux-kernel, venkat.x.venkatsubra, netdev
In-Reply-To: <CAKA=qzac9=UhLF_Z4FnnH+sR7xvkDux4oayC6dPYe=hMLsDxRg@mail.gmail.com>
From: Josh Hunt <joshhunt00@gmail.com>
Date: Tue, 12 Nov 2013 22:22:11 -0600
> David - I can resubmit the patch with the proper signed-off-by and
> formatting if you are willing to apply it unless John wants to try again. I
> think it's time this got upstream.
Nothing is going to happen until the patch is submitted properly, so
just do, don't ask.
^ permalink raw reply
* Re: [PATCH] linux: Signal datapath that unaligned Netlink message can be received
From: Jesse Gross @ 2013-11-13 6:11 UTC (permalink / raw)
To: Thomas Graf
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
fleitner-H+wXaHxf7aLQT0dZR+AlfA, dborkmann-H+wXaHxf7aLQT0dZR+AlfA,
netdev, Ben Hutchings
In-Reply-To: <5280FD6D.9000609-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Nov 11, 2013 at 11:53 PM, Thomas Graf <tgraf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 11/11/2013 04:50 PM, Ben Pfaff wrote:
>>
>> On Mon, Nov 11, 2013 at 04:36:24PM +0100, Thomas Graf wrote:
>>>
>>> Following commit (''netlink: Do not enforce alignment of last Netlink
>>> attribute''), signal the ability to receive unaligned Netlink messages
>>> to the datapath to enable utilization of zerocopy optimizations.
>>>
>>> Signed-off-by: Thomas Graf <tgraf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>
>>
>> Seems OK from a userspace point of view. I am a little concerned that
>> downgrading userspace without deleting and re-creating the datapath
>> (e.g. via "force-reload-kmod") will result in a totally broken setup
>> since userspace will then drop every packet from the kernel.
>
>
> Is that something that occurs occasionally in installations? Utilizing
> the version field in the genl header could be used to track this and
> clear user_features.
It's probably a good idea. I could see us having more of these
features flags in the future (although obviously we should try to
avoid them if possible) and, as Ben said, it would potentially lead to
a bad state otherwise.
I'm not sure exactly what you have in mind though, can you elaborate a little?
(By the way, it might be a good idea to keep the same CC list on all
of the patches. Otherwise, some people might miss parts of the
discussion.)
^ permalink raw reply
* Re: [PATCH net-next 1/4] virtio-net: mergeable buffer size should include virtio-net header
From: Jason Wang @ 2013-11-13 6:53 UTC (permalink / raw)
To: Michael Dalton, David S. Miller
Cc: Michael S. Tsirkin, netdev, Daniel Borkmann, virtualization,
Eric Dumazet
In-Reply-To: <1384294885-6444-1-git-send-email-mwdalton@google.com>
On 11/13/2013 06:21 AM, Michael Dalton wrote:
> Commit 2613af0ed18a ("virtio_net: migrate mergeable rx buffers to page
> frag allocators") changed the mergeable receive buffer size from PAGE_SIZE
> to MTU-size. However, the merge buffer size does not take into account the
> size of the virtio-net header. Consequently, packets that are MTU-size
> will take two buffers intead of one (to store the virtio-net header),
> substantially decreasing the throughput of MTU-size traffic due to TCP
> window / SKB truesize effects.
>
> This commit changes the mergeable buffer size to include the virtio-net
> header. The buffer size is cacheline-aligned because skb_page_frag_refill
> will not automatically align the requested size.
>
> Benchmarks taken from an average of 5 netperf 30-second TCP_STREAM runs
> between two QEMU VMs on a single physical machine. Each VM has two VCPUs and
> vhost enabled. All VMs and vhost threads run in a single 4 CPU cgroup
> cpuset, using cgroups to ensure that other processes in the system will not
> be scheduled on the benchmark CPUs. Transmit offloads and mergeable receive
> buffers are enabled, but guest_tso4 / guest_csum are explicitly disabled to
> force MTU-sized packets on the receiver.
>
> next-net trunk before 2613af0ed18a (PAGE_SIZE buf): 3861.08Gb/s
> net-next trunk (MTU 1500- packet uses two buf due to size bug): 4076.62Gb/s
> net-next trunk (MTU 1480- packet fits in one buf): 6301.34Gb/s
> net-next trunk w/ size fix (MTU 1500 - packet fits in one buf): 6445.44Gb/s
>
> Suggested-by: Eric Northup <digitaleric@google.com>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
> ---
> drivers/net/virtio_net.c | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 01f4eb5..69fb225 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -36,7 +36,10 @@ module_param(csum, bool, 0444);
> module_param(gso, bool, 0444);
>
> /* FIXME: MTU in config. */
> -#define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> +#define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> +#define MERGE_BUFFER_LEN (ALIGN(GOOD_PACKET_LEN + \
> + sizeof(struct virtio_net_hdr_mrg_rxbuf), \
> + L1_CACHE_BYTES))
> #define GOOD_COPY_LEN 128
>
> #define VIRTNET_DRIVER_VERSION "1.0.0"
> @@ -314,10 +317,10 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> head_skb->dev->stats.rx_length_errors++;
> return -EINVAL;
> }
> - if (unlikely(len > MAX_PACKET_LEN)) {
> + if (unlikely(len > MERGE_BUFFER_LEN)) {
> pr_debug("%s: rx error: merge buffer too long\n",
> head_skb->dev->name);
> - len = MAX_PACKET_LEN;
> + len = MERGE_BUFFER_LEN;
> }
> if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
> struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> @@ -336,18 +339,17 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> if (curr_skb != head_skb) {
> head_skb->data_len += len;
> head_skb->len += len;
> - head_skb->truesize += MAX_PACKET_LEN;
> + head_skb->truesize += MERGE_BUFFER_LEN;
> }
> page = virt_to_head_page(buf);
> offset = buf - (char *)page_address(page);
> if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
> put_page(page);
> skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
> - len, MAX_PACKET_LEN);
> + len, MERGE_BUFFER_LEN);
> } else {
> skb_add_rx_frag(curr_skb, num_skb_frags, page,
> - offset, len,
> - MAX_PACKET_LEN);
> + offset, len, MERGE_BUFFER_LEN);
> }
> --rq->num;
> }
> @@ -383,7 +385,7 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> struct page *page = virt_to_head_page(buf);
> skb = page_to_skb(rq, page,
> (char *)buf - (char *)page_address(page),
> - len, MAX_PACKET_LEN);
> + len, MERGE_BUFFER_LEN);
> if (unlikely(!skb)) {
> dev->stats.rx_dropped++;
> put_page(page);
> @@ -471,11 +473,11 @@ static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp)
> struct skb_vnet_hdr *hdr;
> int err;
>
> - skb = __netdev_alloc_skb_ip_align(vi->dev, MAX_PACKET_LEN, gfp);
> + skb = __netdev_alloc_skb_ip_align(vi->dev, GOOD_PACKET_LEN, gfp);
> if (unlikely(!skb))
> return -ENOMEM;
>
> - skb_put(skb, MAX_PACKET_LEN);
> + skb_put(skb, GOOD_PACKET_LEN);
>
> hdr = skb_vnet_hdr(skb);
> sg_set_buf(rq->sg, &hdr->hdr, sizeof hdr->hdr);
> @@ -542,20 +544,20 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> int err;
>
> if (gfp & __GFP_WAIT) {
> - if (skb_page_frag_refill(MAX_PACKET_LEN, &vi->alloc_frag,
> + if (skb_page_frag_refill(MERGE_BUFFER_LEN, &vi->alloc_frag,
> gfp)) {
> buf = (char *)page_address(vi->alloc_frag.page) +
> vi->alloc_frag.offset;
> get_page(vi->alloc_frag.page);
> - vi->alloc_frag.offset += MAX_PACKET_LEN;
> + vi->alloc_frag.offset += MERGE_BUFFER_LEN;
> }
> } else {
> - buf = netdev_alloc_frag(MAX_PACKET_LEN);
> + buf = netdev_alloc_frag(MERGE_BUFFER_LEN);
> }
> if (!buf)
> return -ENOMEM;
>
> - sg_init_one(rq->sg, buf, MAX_PACKET_LEN);
> + sg_init_one(rq->sg, buf, MERGE_BUFFER_LEN);
> err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
> if (err < 0)
> put_page(virt_to_head_page(buf));
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Re: [PATCH net-next 4/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Jason Wang @ 2013-11-13 7:10 UTC (permalink / raw)
To: Michael Dalton, David S. Miller
Cc: Michael S. Tsirkin, netdev, Daniel Borkmann, virtualization,
Eric Dumazet
In-Reply-To: <1384294885-6444-4-git-send-email-mwdalton@google.com>
On 11/13/2013 06:21 AM, Michael Dalton wrote:
> Commit 2613af0ed18a ("virtio_net: migrate mergeable rx buffers to page frag
> allocators") changed the mergeable receive buffer size from PAGE_SIZE to
> MTU-size, introducing a single-stream regression for benchmarks with large
> average packet size. There is no single optimal buffer size for all workloads.
> For workloads with packet size <= MTU bytes, MTU + virtio-net header-sized
> buffers are preferred as larger buffers reduce the TCP window due to SKB
> truesize. However, single-stream workloads with large average packet sizes
> have higher throughput if larger (e.g., PAGE_SIZE) buffers are used.
>
> This commit auto-tunes the mergeable receiver buffer packet size by choosing
> the packet buffer size based on an EWMA of the recent packet sizes for the
> receive queue. Packet buffer sizes range from MTU_SIZE + virtio-net header
> len to PAGE_SIZE. This improves throughput for large packet workloads, as
> any workload with average packet size >= PAGE_SIZE will use PAGE_SIZE
> buffers.
Hi Michael:
There's one concern with EWMA. How well does it handle multiple streams
each with different packet size? E.g there may be two flows, one with
256 bytes each packet another is 64K. Looks like it can result we
allocate PAGE_SIZE buffer for 256 (which is bad since the
payload/truesize is low) bytes or 1500+ for 64K buffer (which is ok
since we can do coalescing).
>
> These optimizations interact positively with recent commit
> ba275241030c ("virtio-net: coalesce rx frags when possible during rx"),
> which coalesces adjacent RX SKB fragments in virtio_net. The coalescing
> optimizations benefit buffers of any size.
>
> Benchmarks taken from an average of 5 netperf 30-second TCP_STREAM runs
> between two QEMU VMs on a single physical machine. Each VM has two VCPUs
> with all offloads & vhost enabled. All VMs and vhost threads run in a
> single 4 CPU cgroup cpuset, using cgroups to ensure that other processes
> in the system will not be scheduled on the benchmark CPUs. Trunk includes
> SKB rx frag coalescing.
>
> net-next trunk w/ virtio_net before 2613af0ed18a (PAGE_SIZE bufs): 14642.85Gb/s
> net-next trunk (MTU-size bufs): 13170.01Gb/s
> net-next trunk + auto-tune: 14555.94Gb/s
Do you have perf numbers that just without this patch? We need to know
how much EWMA help exactly.
>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
> ---
> drivers/net/virtio_net.c | 73 +++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 53 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0c93054..b1086e0 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -27,6 +27,7 @@
> #include <linux/if_vlan.h>
> #include <linux/slab.h>
> #include <linux/cpu.h>
> +#include <linux/average.h>
>
> static int napi_weight = NAPI_POLL_WEIGHT;
> module_param(napi_weight, int, 0444);
> @@ -37,10 +38,8 @@ module_param(gso, bool, 0444);
>
> /* FIXME: MTU in config. */
> #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> -#define MERGE_BUFFER_LEN (ALIGN(GOOD_PACKET_LEN + \
> - sizeof(struct virtio_net_hdr_mrg_rxbuf), \
> - L1_CACHE_BYTES))
> #define GOOD_COPY_LEN 128
> +#define RECEIVE_AVG_WEIGHT 64
Maybe we can make this as a module parameter.
>
> #define VIRTNET_DRIVER_VERSION "1.0.0"
>
> @@ -79,6 +78,9 @@ struct receive_queue {
> /* Chain pages by the private ptr. */
> struct page *pages;
>
> + /* Average packet length for mergeable receive buffers. */
> + struct ewma mrg_avg_pkt_len;
> +
> /* Page frag for GFP_ATOMIC packet buffer allocation. */
> struct page_frag atomic_frag;
>
> @@ -302,14 +304,17 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
> return skb;
> }
>
> -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> +static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb,
> + struct page *head_page)
> {
> struct skb_vnet_hdr *hdr = skb_vnet_hdr(head_skb);
> struct sk_buff *curr_skb = head_skb;
> + struct page *page = head_page;
> char *buf;
> - struct page *page;
> - int num_buf, len, offset, truesize;
> + int num_buf, len, offset;
> + u32 est_buffer_len;
>
> + len = head_skb->len;
> num_buf = hdr->mhdr.num_buffers;
> while (--num_buf) {
> int num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
> @@ -320,7 +325,6 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> head_skb->dev->stats.rx_length_errors++;
> return -EINVAL;
> }
> - truesize = max_t(int, len, MERGE_BUFFER_LEN);
> if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
> struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC);
> if (unlikely(!nskb)) {
> @@ -338,20 +342,38 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb)
> if (curr_skb != head_skb) {
> head_skb->data_len += len;
> head_skb->len += len;
> - head_skb->truesize += truesize;
> + head_skb->truesize += len;
> }
> page = virt_to_head_page(buf);
> offset = buf - (char *)page_address(page);
> if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
> put_page(page);
> skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
> - len, truesize);
> + len, len);
> } else {
> skb_add_rx_frag(curr_skb, num_skb_frags, page,
> - offset, len, truesize);
> + offset, len, len);
> }
> --rq->num;
> }
> + /* All frags before the last frag are fully used -- for those frags,
> + * truesize = len. Use the size of the most recent buffer allocation
> + * from the last frag's page to estimate the truesize of the last frag.
> + * EWMA with a weight of 64 makes the size adjustments quite small in
> + * the frags allocated on one page (even a order-3 one), and truesize
> + * doesn't need to be 100% accurate.
> + */
> + if (page) {
> + est_buffer_len = page_private(page);
> + if (est_buffer_len > len) {
> + u32 truesize_delta = est_buffer_len - len;
> +
> + curr_skb->truesize += truesize_delta;
> + if (curr_skb != head_skb)
> + head_skb->truesize += truesize_delta;
> + }
Is there a chance that est_buffer_len was smaller than or equal with len?
> + }
> + ewma_add(&rq->mrg_avg_pkt_len, head_skb->len);
> return 0;
> }
>
> @@ -382,16 +404,21 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> skb_trim(skb, len);
> } else if (vi->mergeable_rx_bufs) {
> struct page *page = virt_to_head_page(buf);
> - int truesize = max_t(int, len, MERGE_BUFFER_LEN);
> + /* Use an initial truesize of 'len' bytes for page_to_skb --
> + * receive_mergeable will fixup the truesize of the last page
> + * frag if the packet is non-linear (> GOOD_COPY_LEN bytes).
> + */
> skb = page_to_skb(rq, page,
> (char *)buf - (char *)page_address(page),
> - len, truesize);
> + len, len);
> if (unlikely(!skb)) {
> dev->stats.rx_dropped++;
> put_page(page);
> return;
> }
> - if (receive_mergeable(rq, skb)) {
> + if (!skb_is_nonlinear(skb))
> + page = NULL;
> + if (receive_mergeable(rq, skb, page)) {
> dev_kfree_skb(skb);
> return;
> }
> @@ -540,24 +567,29 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
> static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> {
> struct virtnet_info *vi = rq->vq->vdev->priv;
> + const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> struct page_frag *alloc_frag;
> char *buf;
> - int err, len, hole;
> + int err, hole;
> + u32 buflen;
>
> + buflen = hdr_len + clamp_t(u32, ewma_read(&rq->mrg_avg_pkt_len),
> + GOOD_PACKET_LEN, PAGE_SIZE - hdr_len);
> + buflen = ALIGN(buflen, L1_CACHE_BYTES);
> alloc_frag = (gfp & __GFP_WAIT) ? &vi->sleep_frag : &rq->atomic_frag;
> - if (unlikely(!skb_page_frag_refill(MERGE_BUFFER_LEN, alloc_frag, gfp)))
> + if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, gfp)))
> return -ENOMEM;
> buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> get_page(alloc_frag->page);
> - len = MERGE_BUFFER_LEN;
> - alloc_frag->offset += len;
> + alloc_frag->offset += buflen;
> + set_page_private(alloc_frag->page, buflen);
Not sure this is accurate, since buflen may change and several frags may
share a single page. So the est_buffer_len we get in receive_mergeable()
may not be the correct value.
> hole = alloc_frag->size - alloc_frag->offset;
> - if (hole < MERGE_BUFFER_LEN) {
> - len += hole;
> + if (hole < buflen) {
> + buflen += hole;
> alloc_frag->offset += hole;
> }
>
> - sg_init_one(rq->sg, buf, len);
> + sg_init_one(rq->sg, buf, buflen);
> err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
> if (err < 0)
> put_page(virt_to_head_page(buf));
> @@ -1475,6 +1507,7 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
> napi_weight);
>
> sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
> + ewma_init(&vi->rq[i].mrg_avg_pkt_len, 1, RECEIVE_AVG_WEIGHT);
> sg_init_table(vi->sq[i].sg, ARRAY_SIZE(vi->sq[i].sg));
> }
>
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Willy Tarreau @ 2013-11-13 7:22 UTC (permalink / raw)
To: Arnaud Ebalard; +Cc: Cong Wang, edumazet, stable, linux-arm-kernel, netdev
In-Reply-To: <87vbzxd473.fsf@natisbad.org>
On Tue, Nov 12, 2013 at 04:34:24PM +0100, Arnaud Ebalard wrote:
> Hi,
>
> Willy Tarreau <w@1wt.eu> writes:
>
> > On Tue, Nov 12, 2013 at 10:14:34AM +0100, Arnaud Ebalard wrote:
> >> Tests for the rgression were done w/ scp, and were hence limited by the
> >> crypto (16MB/s using arcfour128). But I also did some tests w/ a simple
> >> wget for a file served by Apache *before* the regression and I never got
> >> more than 60MB/s from what I recall. Can you beat that?
> >
> > Yes, I finally picked my mirabox out of my bag for a quick test. It boots
> > off 3.10.0-rc7 and I totally saturate one port (stable 988 Mbps) with even
> > a single TCP stream.
>
> Thanks for the feedback. That's interesting. What are you using for your tests
> (wget, ...)?
No, inject (for the client) + httpterm (for the server), but it also works with
a simple netcat < /dev/zero, except that netcat uses 8kB buffers and is quickly
CPU-bound. The tools I'm talking about are available here :
http://1wt.eu/tools/inject/?C=M;O=D
http://1wt.eu/tools/httpterm/httpterm-1.7.2.tar.gz
Httpterm is a dummy web server. You can send requests like
"GET /?s=1m HTTP/1.0" and it returns 1 MB of data in the response,
which is quite convenient! I'm sorry for the limited documentation
(don't even try to write a config file, it's a fork of an old haproxy
version). Simply start it as :
httpterm -D -L ip:port (where 'ip' is optional)
Inject is an HTTP client initially designed to test applications but still
doing well enough for component testing (though it does not scale well with
large numbers of connections). I remember that Pablo Neira rewrote a simpler
equivalent here : http://1984.lsi.us.es/git/http-client-benchmark, but I'm
used to use my old version.
There's an old doc in PDF in the download directory. Unfortunately it
speaks french which is not always very convenient. But what I like there
is that you get one line of stats per second so you can easily follow how
the test goes, as opposite to some tools like "ab" which only give you a
summary at the end. That's one of the key points that Pablo has reimplemented
in his tool BTW.
> > With two systems, one directly connected (dockstar) and the other one via
> > a switch, I get 2*650 Mbps (a single TCP stream is enough on each).
> >
> > I'll have to re-run some tests using a more up to date kernel, but that
> > will probably not be today though.
>
> Can you give a pre-3.11.7 kernel a try if you find the time? I started
> working on RN102 during 3.10-rc cycle but do not remember if I did the
> first preformance tests on 3.10 or 3.11. And if you find more time,
> 3.11.7 would be nice too ;-)
Still have not found time for this but I observed something intriguing
which might possibly match your experience : if I use large enough send
buffers on the mirabox and receive buffers on the client, then the
traffic drops for objects larger than 1 MB. I have quickly checked what's
happening and it's just that there are pauses of up to 8 ms between some
packets when the TCP send window grows larger than about 200 kB. And
since there are no drops, there is no reason for the window to shrink.
I suspect it's exactly related to the issue explained by Eric about the
timer used to recycle the Tx descriptors. However last time I checked,
these ones were also processed in the Rx path, which means that the
ACKs that flow back should have had the same effect as a Tx IRQ (unless
I'd use asymmetric routing, which was not the case). So there might be
another issue. Ah, and it only happens with GSO.
I really need some time to perform more tests, I'm sorry Arnaud, but I
can't do them right now. What you can do is to try to reduce your send
window to 1 MB or less to see if the issue persists :
$ cat /proc/sys/ipv4/tcp_wmem
$ echo 4096 16384 1048576 > /proc/sys/ipv4/tcp_wmem
You also need to monitor your CPU usage to ensure that you're not limited
by some processing inside apache. At 1 Gbps, you should use only something
like 40-50% of the CPU.
Cheers,
Willy
^ permalink raw reply
* Re: VLAN filtering/VLAN aware bridge problems
From: Stefan Priebe - Profihost AG @ 2013-11-13 7:28 UTC (permalink / raw)
To: vyasevic; +Cc: David Miller, Linux Netdev List
In-Reply-To: <5282B8E6.5090109@redhat.com>
Am 13.11.2013 00:25, schrieb Vlad Yasevich:
> On 11/12/2013 04:31 PM, Stefan Priebe wrote:
>> sorry for the very late response.
>>
>> Am 10.09.2013 16:11, schrieb Vlad Yasevich:
>>> On 08/30/2013 11:01 AM, Stefan Priebe - Profihost AG wrote:
>>>> Yes
>>>>
>>>
>>> Can you apply this patch and see if this fixes your problem.
>>> http://patchwork.ozlabs.org/patch/273841/
>>>
>>> In my attempts to reproduce your problem I didn't configuring filtering
>>> on the upper bridge, but that is what could have been causing
>>> your problem. I'll attempt it and let you know.
>>
>> Even with the complete patchset which was merged upstream it doesn't
>> work ;-(
>>
>> What's wrong there and / or how can i debug?
>
> Can you provide the filtering settings for both bridges you use?
I don't filter at all right now it's compiled in but not enabled - but i
have these problems since these patches were included.
It only start to work if i set the eth0 and eth1 (slaves of the bond) to
promisc mode. So the problem seems to be that the ethernet devices do
not accept the VLAN tagged packages.
Stefan
> Thanks
> -vlad
>
>>
>> Stefan
>>
>>> -vlad
>>>
>>>
>>>> Stefan
>>>>
>>>> This mail was sent with my iPhone.
>>>>
>>>> Am 30.08.2013 um 16:13 schrieb Vlad Yasevich <vyasevic@redhat.com>:
>>>>
>>>>> On 08/30/2013 03:24 AM, Stefan Priebe - Profihost AG wrote:
>>>>>> Am 29.08.2013 22:45, schrieb Vlad Yasevich:
>>>>>>> On 08/29/2013 08:50 AM, Stefan Priebe - Profihost AG wrote:
>>>>>>
>>>>>>>> The packets never reach the TAP device.
>>>>>>>>
>>>>>>>> Here is an output of ip a l (vlan 3021):
>>>>>>>
>>>>>>> Can you provide output of brctl show?
>>>>>>
>>>>>> Sure:
>>>>>> # brctl show
>>>>>> bridge name bridge id STP enabled interfaces
>>>>>> vmbr0 8000.00259084dea8 no bond0
>>>>>> tap320i0
>>>>>> vmbr1 8000.00259084deaa no bond1
>>>>>> vmbr1v3021 8000.00259084deaa no
>>>>>> tap320i1
>>>>>> vmbr1.3021
>>>>>
>>>>> so let me see if I can understand this configuration.
>>>>>
>>>>> vmbr1v3021 (bridge)
>>>>> / \
>>>>> tap320i1 vmbr1.3021 (vlan)
>>>>> \
>>>>> vmbr1 (bridge)
>>>>> \
>>>>> bond1
>>>>> \
>>>>> eth X
>>>>>
>>>>>
>>>>> Is that right? Is this the setup that has the problem you are
>>>>> describing?
>>>>>
>>>>> Thanks
>>>>> -vlad
>>>>>
>>>>>>> On the off chance that you are actually trying to configure vlan
>>>>>>> filtering, can you give this patch a try (net-2.6 tree):
>>>>>>>
>>>>>>> Author: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>>>>>> Date: Tue Aug 20 17:10:18 2013 +0900
>>>>>>>
>>>>>>> bridge: Use the correct bit length for bitmap functions in the
>>>>>>> VLAN
>>>>>>> code
>>>>>>>
>>>>>>> I don't think it made it to stable yet.
>>>>>>
>>>>>> I addd that patch and now the vlan stuff works at least on the host
>>>>>> node. But my tap devices still don't work.
>>>>>>
>>>>>> I also tried to attach the tap device on top of a vlan attached to
>>>>>> bond1
>>>>>> but then gvrp does not work anymore. The kernel announces gvrp once
>>>>>> and
>>>>>> then does not answer the query packets from the switch.
>>>>>>
>>>>>> Stefan
>>>>>
>>>
>
^ permalink raw reply
* Re: [PATCH net-next 4/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Eric Dumazet @ 2013-11-13 7:40 UTC (permalink / raw)
To: Jason Wang
Cc: Michael Dalton, Michael S. Tsirkin, netdev, Daniel Borkmann,
virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <528325DC.3050801@redhat.com>
On Wed, 2013-11-13 at 15:10 +0800, Jason Wang wrote:
> There's one concern with EWMA. How well does it handle multiple streams
> each with different packet size? E.g there may be two flows, one with
> 256 bytes each packet another is 64K. Looks like it can result we
> allocate PAGE_SIZE buffer for 256 (which is bad since the
> payload/truesize is low) bytes or 1500+ for 64K buffer (which is ok
> since we can do coalescing).
It's hard to predict the future ;)
256 bytes frames consume 2.5 KB anyway on a traditional NIC.
If it was a concern, we would have it already.
If you receive a mix of big and small frames, there is no win.
> > + if (page) {
> > + est_buffer_len = page_private(page);
> > + if (est_buffer_len > len) {
> > + u32 truesize_delta = est_buffer_len - len;
> > +
> > + curr_skb->truesize += truesize_delta;
> > + if (curr_skb != head_skb)
> > + head_skb->truesize += truesize_delta;
> > + }
>
> Is there a chance that est_buffer_len was smaller than or equal with len?
Yes, and in this case we do not really care, see below.
> > + }
> > + ewma_add(&rq->mrg_avg_pkt_len, head_skb->len);
> > return 0;
> > }
> >
> > @@ -382,16 +404,21 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> > skb_trim(skb, len);
> > } else if (vi->mergeable_rx_bufs) {
> > struct page *page = virt_to_head_page(buf);
> > - int truesize = max_t(int, len, MERGE_BUFFER_LEN);
> > + /* Use an initial truesize of 'len' bytes for page_to_skb --
> > + * receive_mergeable will fixup the truesize of the last page
> > + * frag if the packet is non-linear (> GOOD_COPY_LEN bytes).
> > + */
> > skb = page_to_skb(rq, page,
> > (char *)buf - (char *)page_address(page),
> > - len, truesize);
> > + len, len);
> > if (unlikely(!skb)) {
> > dev->stats.rx_dropped++;
> > put_page(page);
> > return;
> > }
> > - if (receive_mergeable(rq, skb)) {
> > + if (!skb_is_nonlinear(skb))
> > + page = NULL;
> > + if (receive_mergeable(rq, skb, page)) {
> > dev_kfree_skb(skb);
> > return;
> > }
> > @@ -540,24 +567,29 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
> > static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> > {
> > struct virtnet_info *vi = rq->vq->vdev->priv;
> > + const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> > struct page_frag *alloc_frag;
> > char *buf;
> > - int err, len, hole;
> > + int err, hole;
> > + u32 buflen;
> >
> > + buflen = hdr_len + clamp_t(u32, ewma_read(&rq->mrg_avg_pkt_len),
> > + GOOD_PACKET_LEN, PAGE_SIZE - hdr_len);
> > + buflen = ALIGN(buflen, L1_CACHE_BYTES);
> > alloc_frag = (gfp & __GFP_WAIT) ? &vi->sleep_frag : &rq->atomic_frag;
> > - if (unlikely(!skb_page_frag_refill(MERGE_BUFFER_LEN, alloc_frag, gfp)))
> > + if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, gfp)))
> > return -ENOMEM;
> > buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> > get_page(alloc_frag->page);
> > - len = MERGE_BUFFER_LEN;
> > - alloc_frag->offset += len;
> > + alloc_frag->offset += buflen;
> > + set_page_private(alloc_frag->page, buflen);
>
> Not sure this is accurate, since buflen may change and several frags may
> share a single page. So the est_buffer_len we get in receive_mergeable()
> may not be the correct value.
skb->truesize has to be reasonably accurate.
For example, fast clone storage is not accounted for in TCP skbs stored
in socket write queues. Thats ~256 bytes per skb of 'missing'
accounting.
This is about 10% error when TSO/GSO is off.
With this EWMA using a factor of 64, the potential error will be much
less than 10%.
Small frames tend to be consumed quite fast (ACK messages, small UDP
frames) in most cases.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox