* [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
* [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
* 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
* 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
* 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
* [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
* 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
* 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: [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: 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: 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: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: 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: 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
* [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: 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
* 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: [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: shutdown(3) and bluetooth.
From: Marcel Holtmann @ 2013-11-13 1:58 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev, linux-bluetooth@vger.kernel.org development
In-Reply-To: <20131113002819.GB12615@redhat.com>
Hi Dave,
>>> So it seems it affects both SCO and RFCOMM.
>>>
>>>> What kernel did you run this against? It is a shot in the dark, but can you try linux-next quickly.
>>>> There was a socket related fix for the socket options where we confused RFCOMM vs L2CAP struct sock.
>>>
>>> first noticed it on Linus' latest HEAD, and then reproduced it on 3.11.6
>>> I'll look at linux-next tomorrow.
>>
>> I looked through the code and only call bt_sock_wait_state when SOCK_LINGER and sk_lingertime is set. In that case we actually block until the socket state changes to BT_CLOSED.
>>
>> The only way I see this could happen is if you have a huge linger timeout and confused the socket state before. What is actually the list of system calls that you are throwing at this socket.
>
> Ah. I recently changed some code that's now doing this on every socket at shutdown..
> (simplified cut-n-paste)
>
> struct linger ling = { .l_onoff = FALSE, };
>
> for (i = 0; i < nr_sockets; i++) {
> fd = shm->sockets[i].fd;
> shm->sockets[i].fd = 0;
>
> setsockopt(fd, SOL_SOCKET, SO_LINGER, &ling, sizeof(struct linger));
> shutdown(fd, SHUT_RDWR);
> close(fd);
> }
>
> I could just rip out that linger code completely and just hope that sockets staying in
> TIME_WAIT is good enough. iirc, I added it when after multiple runs, some of the
> weirder protocols would fail to open a socket once a certain number of existing
> sockets had opened, even if they were in SOCK_WAIT
>
> two remaining questions though. That code is setting linger to false. Why would
> that cause the sk_lingertime to be taken into consideration ? And why is this
> only a problem for bluetooth (apparently) ?
we are not touching that part of setsockopt. That is handled by net/core/sock.c and we just check if SOCK_LINGER flag is set and if we have a positive sk_lingertime. So this is a bit suspicious on why this is happening, but I don’t think it is our mistake.
Regards
Marcel
^ permalink raw reply
* Re: Fw: [Bug 63321] tcp_fastretrans_alert
From: Ben Hutchings @ 2013-11-13 1:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20131112172615.498b3ac9@samsung-9>
On Tue, 2013-11-12 at 17:26 -0800, Stephen Hemminger wrote:
>
> Begin forwarded message:
>
> Date: Tue, 12 Nov 2013 08:43:45 -0800
> From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
> To: "stephen@networkplumber.org" <stephen@networkplumber.org>
> Subject: [Bug 63321] tcp_fastretrans_alert
>
>
> https://bugzilla.kernel.org/show_bug.cgi?id=63321
>
> --- Comment #3 from root@securitydot.net ---
> Once again someone is delaying a fix for a major issue causing a lot of
> trouble. Please fix it...
>
> WARNING: CPU: 2 PID: 0 at net/ipv4/tcp_input.c:2711
> tcp_fastretrans_alert+0x56c/0x580()
[...]
> CPU: 2 PID: 0 Comm: swapper/2 Tainted: G W 3.11.6-1.el6.elrepo.x86_64
I think this is fixed by:
commit 031afe4990a7c9dbff41a3a742c44d3e740ea0a1
Author: Yuchung Cheng <ycheng@google.com>
Date: Sat Oct 12 10:16:27 2013 -0700
tcp: fix incorrect ca_state in tail loss probe
which went into 3.12-rc7 and then 3.11.7.
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: [PATCH] net:fec: fix WARNING caused by lack of calls to dma_mapping_error()
From: Ben Hutchings @ 2013-11-13 1:43 UTC (permalink / raw)
To: Fugang Duan; +Cc: b20596, davem, netdev
In-Reply-To: <1384304453.3802.62.camel@bwh-desktop.uk.level5networks.com>
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.
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.
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
* [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Chang Xiangzhong @ 2013-11-13 1:34 UTC (permalink / raw)
To: vyasevich, nhorman, davem
Cc: linux-sctp, netdev, linux-kernel, dreibh, Chang Xiangzhong
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;
}
+ if (!second)
+ second = first;
+
/* RFC 2960 6.4 Multi-Homed SCTP Endpoints
*
* By default, an endpoint should always transmit to the
--
1.7.9.5
^ permalink raw reply related
* Re: gso: Handle new frag_list of frags GRO packets
From: Herbert Xu @ 2013-11-13 1:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1384305223.28458.58.camel@edumazet-glaptop2.roam.corp.google.com>
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?
I wonder what happens if we don't convert head_frag to frags and just
use frag_lists like we used to?
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
* Fw: [Bug 63321] tcp_fastretrans_alert
From: Stephen Hemminger @ 2013-11-13 1:26 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Tue, 12 Nov 2013 08:43:45 -0800
From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: [Bug 63321] tcp_fastretrans_alert
https://bugzilla.kernel.org/show_bug.cgi?id=63321
--- Comment #3 from root@securitydot.net ---
Once again someone is delaying a fix for a major issue causing a lot of
trouble. Please fix it...
WARNING: CPU: 2 PID: 0 at net/ipv4/tcp_input.c:2711
tcp_fastretrans_alert+0x56c/0x580()
Modules linked in: iptable_filter ip_tables autofs4 bonding ipv6 xt_LOG
ipt_REJECT xt_multiport xfs libcrc32c gpio_ich iTCO_wdt iTCO_vendor_support
joydev raid0 microcode pcspkr serio_raw sg lpc_ich hpilo hpwdt e1000e ptp
pps_core freq_table mperf ext4 jbd2 mbcache xts gf128mul dm_crypt raid1 sd_mod
crc_t10dif ahci libahci mgag200 ttm drm_kms_helper sysimgblt sysfillrect
syscopyarea dm_mirror dm_region_hash dm_log dm_mod
CPU: 2 PID: 0 Comm: swapper/2 Tainted: G W 3.11.6-1.el6.elrepo.x86_64
#1
Hardware name: HP ProLiant DL120 G7, BIOS J01 02/01/2012
0000000000000a97 ffff88020b4438e0 ffffffff815f7f89 0000000000000a97
0000000000000000 ffff88020b443920 ffffffff8106705c ffff880200138000
ffff8800beb7c880 0000000000000009 0000000000004120 0000000000000001
Call Trace:
<IRQ> [<ffffffff815f7f89>] dump_stack+0x49/0x60
[<ffffffff8106705c>] warn_slowpath_common+0x8c/0xc0
[<ffffffff810670aa>] warn_slowpath_null+0x1a/0x20
[<ffffffff8158425c>] tcp_fastretrans_alert+0x56c/0x580
[<ffffffff81585d3f>] tcp_ack+0x34f/0x7d0
[<ffffffff81586e5a>] tcp_rcv_established+0x2ca/0x700
[<ffffffff81590309>] tcp_v4_do_rcv+0x169/0x290
[<ffffffff81591c81>] tcp_v4_rcv+0x601/0x760
[<ffffffff8156ce90>] ? ip_rcv+0x350/0x350
[<ffffffff8156325d>] ? nf_hook_slow+0x7d/0x150
[<ffffffff8156ce90>] ? ip_rcv+0x350/0x350
[<ffffffff8156cf5e>] ip_local_deliver_finish+0xce/0x250
[<ffffffff8156d168>] ip_local_deliver+0x88/0x90
[<ffffffff8150a01f>] ? __alloc_and_insert_iova_range+0x17f/0x1f0
[<ffffffff8156c7b9>] ip_rcv_finish+0x119/0x360
[<ffffffff8156cdad>] ip_rcv+0x26d/0x350
[<ffffffffa0383880>] ? bond_hw_addr_swap+0x120/0x120 [bonding]
[<ffffffff81536f4e>] __netif_receive_skb_core+0x53e/0x6f0
[<ffffffff81537127>] __netif_receive_skb+0x27/0x70
[<ffffffff8153739d>] netif_receive_skb+0x2d/0x90
[<ffffffff81537fa8>] napi_gro_receive+0xb8/0x140
[<ffffffffa019745a>] e1000_receive_skb+0x7a/0xf0 [e1000e]
[<ffffffffa019a900>] e1000_clean_rx_irq+0x2a0/0x4a0 [e1000e]
[<ffffffffa019d1ed>] e1000e_poll+0xbd/0x2d0 [e1000e]
[<ffffffff81537ab2>] net_rx_action+0x112/0x290
[<ffffffff814df346>] ? cpuidle_enter_state+0x56/0xd0
[<ffffffff8106c147>] __do_softirq+0xf7/0x270
[<ffffffff81606adc>] call_softirq+0x1c/0x30
[<ffffffff81015885>] do_softirq+0x65/0xa0
[<ffffffff8106be75>] irq_exit+0xc5/0xd0
[<ffffffff816073e6>] do_IRQ+0x66/0xe0
[<ffffffff815fc6ed>] common_interrupt+0x6d/0x6d
<EOI> [<ffffffff814df346>] ? cpuidle_enter_state+0x56/0xd0
[<ffffffff814df33f>] ? cpuidle_enter_state+0x4f/0xd0
[<ffffffff814df79d>] cpuidle_idle_call+0xcd/0x160
[<ffffffff8101c70e>] arch_cpu_idle+0xe/0x30
[<ffffffff810bb109>] cpu_idle_loop+0x69/0x210
[<ffffffff810bb320>] cpu_startup_entry+0x70/0x80
[<ffffffff810437bd>] start_secondary+0xcd/0xd0
---[ end trace 858cf267ca4ce6b0 ]---
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: gso: Handle new frag_list of frags GRO packets
From: Eric Dumazet @ 2013-11-13 1:13 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131111185207.GA12277@gondor.apana.org.au>
On Tue, 2013-11-12 at 02:52 +0800, Herbert Xu wrote:
> Recently GRO started generating packets with frag_lists of frags.
> This was not handled by GSO, thus leading to a crash.
>
> Thankfully these packets are of a regular form and are easy to
> handle. This patch handles them in two ways. For completely
> non-linear frag_list entries, we simply continue to iterate over
> the frag_list frags once we exhaust the normal frags. For frag_list
> entries with linear parts, we call pskb_trim on the first part
> of the frag_list skb, and then process the rest of the frags in
> the usual way.
>
> This patch also kills a chunk of dead frag_list code that has
> obviously never ever been run since it ends up generating a bogus
> GSO-segmented packet with a frag_list entry.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 3735fad..557e1a5 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2776,6 +2776,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> struct sk_buff *segs = NULL;
> struct sk_buff *tail = NULL;
> struct sk_buff *fskb = skb_shinfo(skb)->frag_list;
> + skb_frag_t *skb_frag = skb_shinfo(skb)->frags;
> unsigned int mss = skb_shinfo(skb)->gso_size;
> unsigned int doffset = skb->data - skb_mac_header(skb);
> unsigned int offset = doffset;
> @@ -2815,16 +2816,38 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> if (hsize > len || !sg)
> hsize = len;
>
> - if (!hsize && i >= nfrags) {
> - BUG_ON(fskb->len != len);
> + if (!hsize && i >= nfrags && skb_headlen(fskb) &&
> + (skb_headlen(fskb) == len || sg)) {
> + BUG_ON(skb_headlen(fskb) > len);
> +
Hmm, yet another BUG_ON() case...
> + i = 0;
> + nfrags = skb_shinfo(fskb)->nr_frags;
> + skb_frag = skb_shinfo(fskb)->frags;
> + pos += skb_headlen(fskb);
> +
> + while (pos < offset + len) {
> + BUG_ON(i >= nfrags);
> +
> + size = skb_frag_size(skb_frag);
> + if (pos + size > offset + len)
> + break;
> +
> + i++;
> + pos += size;
> + skb_frag++;
> + }
>
> - pos += len;
> nskb = skb_clone(fskb, GFP_ATOMIC);
> fskb = fskb->next;
>
> if (unlikely(!nskb))
> goto err;
>
> + if (unlikely(pskb_trim(nskb, len))) {
> + kfree_skb(nskb);
> + goto err;
> + }
> +
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.
GRO not building frag_list skbs :
lpaa6:~# mpstat -P 8 1 10
05:09:47 PM CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle intr/s
05:09:48 PM 8 0.00 0.00 1.01 0.00 3.03 23.23 0.00 0.00 72.73 43462.63
05:09:49 PM 8 0.00 0.00 0.00 0.00 5.10 28.57 0.00 0.00 66.33 88079.59
05:09:50 PM 8 0.00 0.00 0.00 0.00 2.13 17.02 0.00 0.00 80.85 41297.87
05:09:51 PM 8 0.00 0.00 0.95 0.00 3.81 29.52 0.00 0.00 65.71 45741.90
05:09:52 PM 8 0.00 0.00 0.00 0.00 2.11 17.89 0.00 0.00 80.00 25413.68
05:09:53 PM 8 1.03 0.00 1.03 0.00 2.06 20.62 0.00 0.00 75.26 36131.96
05:09:54 PM 8 0.00 0.00 0.94 0.00 3.77 30.19 0.00 0.00 65.09 47100.00
05:09:55 PM 8 0.00 0.00 0.00 0.00 3.26 21.74 0.00 0.00 75.00 71805.43
05:09:56 PM 8 0.00 0.00 0.00 0.00 3.19 22.34 0.00 0.00 74.47 70672.34
05:09:57 PM 8 0.00 0.00 0.00 0.00 4.50 32.43 0.00 0.00 63.06 45919.82
Average: 8 0.10 0.00 0.40 0.00 3.33 24.62 0.00 0.00 71.54 51339.66
Current GRO (large skbs -> need to split them with skb_segment(), no TSO is used)
lpaa6:~# mpstat -P 8 1 10
05:10:05 PM CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle intr/s
05:10:06 PM 8 0.00 0.00 0.00 0.00 8.16 44.90 0.00 0.00 46.94 88982.65
05:10:07 PM 8 0.00 0.00 1.05 0.00 8.42 50.53 0.00 0.00 40.00 54796.84
05:10:08 PM 8 0.00 0.00 1.94 0.00 8.74 52.43 0.00 0.00 36.89 50163.11
05:10:09 PM 8 0.00 0.00 0.00 0.00 7.14 39.80 0.00 0.00 53.06 85137.76
05:10:10 PM 8 0.00 0.00 0.00 0.00 8.08 44.44 0.00 0.00 47.47 42262.63
05:10:11 PM 8 0.00 0.00 0.00 0.00 8.00 53.00 0.00 0.00 39.00 53444.00
05:10:12 PM 8 0.00 0.00 0.00 0.00 5.00 27.50 0.00 0.00 67.50 91098.75
05:10:13 PM 8 0.00 0.00 0.00 0.00 8.55 47.86 0.00 0.00 43.59 34316.24
05:10:14 PM 8 0.00 0.00 0.00 0.00 8.70 48.91 0.00 0.00 42.39 56921.74
05:10:15 PM 8 0.00 0.00 0.93 0.00 7.41 46.30 0.00 0.00 45.37 77129.63
Average: 8 0.00 0.00 0.40 0.00 7.88 45.96 0.00 0.00 45.76 62458.99
^ 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