* Job Offer In USA
From: Valero Energy Recruitment @ 2017-09-15 8:12 UTC (permalink / raw)
To: Recipients
Valero Energy Oil & Gas Company USA
Employment Opportunity in Texas, USA.
Oil & Gas Sector ( NEW PLANT RIG )
Salary Range $7,000 USD - $11,700 Per Month
Kindly send your CV/RESUME for more details regarding our Job Program, Employment offers for all Offices.
CC: Forward All Documents/Certificates & Resume to: jobs.valerousa@gmail.com
^ permalink raw reply
* [PATCH net] ip6_tunnel: do not allow loading ip6_tunnel if ipv6 is disabled in cmdline
From: Xin Long @ 2017-09-15 7:58 UTC (permalink / raw)
To: network dev; +Cc: davem, Dmitry Kozlov, jbenc
If ipv6 has been disabled from cmdline since kernel started, it makes
no sense to allow users to create any ip6 tunnel. Otherwise, it could
some potential problem.
Jianlin found a kernel crash caused by this in ip6_gre when he set
ipv6.disable=1 in grub:
[ 209.588865] Unable to handle kernel paging request for data at address 0x00000080
[ 209.588872] Faulting instruction address: 0xc000000000a3aa6c
[ 209.588879] Oops: Kernel access of bad area, sig: 11 [#1]
[ 209.589062] NIP [c000000000a3aa6c] fib_rules_lookup+0x4c/0x260
[ 209.589071] LR [c000000000b9ad90] fib6_rule_lookup+0x50/0xb0
[ 209.589076] Call Trace:
[ 209.589097] fib6_rule_lookup+0x50/0xb0
[ 209.589106] rt6_lookup+0xc4/0x110
[ 209.589116] ip6gre_tnl_link_config+0x214/0x2f0 [ip6_gre]
[ 209.589125] ip6gre_newlink+0x138/0x3a0 [ip6_gre]
[ 209.589134] rtnl_newlink+0x798/0xb80
[ 209.589142] rtnetlink_rcv_msg+0xec/0x390
[ 209.589151] netlink_rcv_skb+0x138/0x150
[ 209.589159] rtnetlink_rcv+0x48/0x70
[ 209.589169] netlink_unicast+0x538/0x640
[ 209.589175] netlink_sendmsg+0x40c/0x480
[ 209.589184] ___sys_sendmsg+0x384/0x4e0
[ 209.589194] SyS_sendmsg+0xd4/0x140
[ 209.589201] SyS_socketcall+0x3e0/0x4f0
[ 209.589209] system_call+0x38/0xe0
This patch is to return -EOPNOTSUPP in ip6_tunnel_init if ipv6 has been
disabled from cmdline.
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/ipv6/ip6_tunnel.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index ae73164..f2f21c2 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -2259,6 +2259,9 @@ static int __init ip6_tunnel_init(void)
{
int err;
+ if (!ipv6_mod_enabled())
+ return -EOPNOTSUPP;
+
err = register_pernet_device(&ip6_tnl_net_ops);
if (err < 0)
goto out_pernet;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] net: phy: Fix mask value write on gmii2rgmii converter speed register.
From: Michal Simek @ 2017-09-15 7:49 UTC (permalink / raw)
To: Andrew Lunn, Fahad Kunnathadi
Cc: f.fainelli, netdev, michal.simek, linux-kernel, soren.brinkmann,
linux-arm-kernel
In-Reply-To: <20170914143454.GB27601@lunn.ch>
On 14.9.2017 16:34, Andrew Lunn wrote:
> On Thu, Sep 14, 2017 at 12:46:31PM +0530, Fahad Kunnathadi wrote:
>> To clear Speed Selection in MDIO control register(0x10),
>> ie, clear bits 6 and 13 to zero while keeping other bits same.
>> Before AND operation,The Mask value has to be perform with bitwise NOT
>> operation (ie, ~ operator)
>>
>> This patch clears current speed selection before writing the
>> new speed settings to gmii2rgmii converter
>
> Hi Fahad
>
> I expect you will find other issues with this driver. I pointed some
> out at the time it is submitted, but the developers went quiet as soon
> as it was accepted.
Can you please point me to that email?
I will create ticket about it in our system to get them resolved.
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface
From: Egil Hjelmeland @ 2017-09-15 7:35 UTC (permalink / raw)
To: Jiri Pirko, Andrew Lunn
Cc: Alexander Duyck, Maxim Uvarov, Vivien Didelot, Greg KH, netdev,
linux-kernel@vger.kernel.org, kernel, David S. Miller,
Florian Fainelli, John Crispin, Woojung Huh, Sean Wang,
Nikita Yushchenko, Chris Healy
In-Reply-To: <20170915055107.GA1927@nanopsycho.orion>
On 15. sep. 2017 07:51, Jiri Pirko wrote:
> Thu, Sep 14, 2017 at 11:01:32PM CEST, andrew@lunn.ch wrote:
>>> Can you clarify what type of registers it is you are wanting to read?
>>> We already have ethtool which is meant to allow reading the device
>>> registers for a given netdev. As long as the port has a netdev
>>> associated it then there is no need to be getting into debugfs since
>>> we should probably just be using ethtool.
>>
>> Not all ports of a DSA switch have a netdev. This is by design. The
>> presentation we gave to Netdev 2.1 gives some of the background.
>>
>> Plus a switch has a lot of registers not associated to port. Often a
>> switch has more global registers than port registers.
>>
>>> Also as Jiri pointed out there is already devlink which would probably
>>> be a better way to get the associated information for those pieces
>>> that don't have a netdev associated with them.
>>
>> We have looked at the devlink a few times. The current dpipe code is
>> not generic enough. It makes assumptions about the architecture of the
>> switch, that it is all match/action based. The niche of top of rack
>> switches might be like that, but average switches are not.
>>
>> If dpipe was to support simple generic two dimensional tables, we
>> probably would use it.
>>
>> David suggested making a class device for DSA. It is not ideal, but we
>> are probably going to go that way.
>
> I believe that is also big mistake.
>
> Could you put together your requirements so we can work it out to extend
> devlink to support them?
>
> Thanks.
>
$ ack -i devlink Documentation/
$ ack -i dpipe Documentation/
$
How you expect new mechanisms to be taken into use with zero documentation?
To all: Why does reviewers nitpick about undocumented formatting rules,
but not ask about documentation?
Egil
^ permalink raw reply
* Re: Page allocator bottleneck
From: Jesper Dangaard Brouer @ 2017-09-15 7:28 UTC (permalink / raw)
To: Tariq Toukan
Cc: David Miller, Mel Gorman, Eric Dumazet, Alexei Starovoitov,
Saeed Mahameed, Eran Ben Elisha, Linux Kernel Network Developers,
Andrew Morton, Michal Hocko, linux-mm, brouer
In-Reply-To: <cef85936-10b2-5d76-9f97-cb03b418fd94@mellanox.com>
On Thu, 14 Sep 2017 19:49:31 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> Hi all,
>
> As part of the efforts to support increasing next-generation NIC speeds,
> I am investigating SW bottlenecks in network stack receive flow.
>
> Here I share some numbers I got for a simple experiment, in which I
> simulate the page allocation rate needed in 200Gpbs NICs.
Thanks for bringing this up again.
> I ran the test below over 3 different (modified) mlx5 driver versions,
> loaded on server side (RX):
> 1) RX page cache disabled, 2 packets per page.
2 packets per page basically reduce the overhead you see from the page
allocator to half.
> 2) RX page cache disabled, one packet per page.
This, should stress the page allocator.
> 3) Huge RX page cache, one packet per page.
A driver level page-cache will look nice, as long as it "works".
Drivers usually have no other option than basing their recycle facility
to be based on the page-refcnt (as there is no destructor callback).
Which implies packets/pages need to be returned quickly enough for it
to work.
> All page allocations are of order 0.
>
> NIC: Connectx-5 100 Gbps.
> CPU: Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz
>
> Test:
> 128 TCP streams (using super_netperf).
> Changing num of RX queues.
> HW LRO OFF, GRO ON, MTU 1500.
With TCP streams and GRO, is actually a good stress test for the page
allocator (or drivers page-recycle cache). As Eric Dumazet have made
some nice optimizations, that (in most situations) cause us to quickly
free/recycle the SKB (coming from driver) and store the pages in 1-SKB.
This cause us to hit the SLUB fastpath for the SKBs, but once the pages
need to be free'ed this stress the page allocator more.
Also be aware that with TCP flows, the packets are likely delivered
into a socket, that is consumed on another CPU. Thus, the pages are
allocated on one CPU and free'ed on another. AFAIK this stress the
order-0 cache PCP (Per-Cpu-Pages).
> Observe: BW as a function of num RX queues.
>
> Results:
>
> Driver #1:
> #rings BW (Mbps)
> 1 23,813
> 2 44,086
> 3 62,128
> 4 78,058
> 6 94,210 (linerate)
> 8 94,205 (linerate)
> 12 94,202 (linerate)
> 16 94,191 (linerate)
>
> Driver #2:
> #rings BW (Mbps)
> 1 18,835
> 2 36,716
> 3 50,521
> 4 61,746
> 6 63,637
> 8 60,299
> 12 51,048
> 16 43,337
>
> Driver #3:
> #rings BW (Mbps)
> 1 19,316
> 2 44,850
> 3 69,549
> 4 87,434
> 6 94,342 (linerate)
> 8 94,350 (linerate)
> 12 94,327 (linerate)
> 16 94,327 (linerate)
>
>
> Insights:
> Major degradation between #1 and #2, not getting any close to linerate!
> Degradation is fixed between #2 and #3.
> This is because page allocator cannot stand the higher allocation rate.
> In #2, we also see that the addition of rings (cores) reduces BW (!!),
> as result of increasing congestion over shared resources.
>
> Congestion in this case is very clear.
> When monitored in perf top:
> 85.58% [kernel] [k] queued_spin_lock_slowpath
Well, we obviously need to know the caller of the spin_lock. In this
case it is likely the page allocator lock. It could also be the TCP
socket locks, but given GRO is enabled, they should be hit much less.
> I think that page allocator issues should be discussed separately:
> 1) Rate: Increase the allocation rate on a single core.
> 2) Scalability: Reduce congestion and sync overhead between cores.
Yes, but this no small task. I is on my TODO-list (emacs org-mode),
but I have other tasks that have higher priority atm. I'll be working
on XDP_REDIRECT for the next many months. Currently trying to convince
people that we do an explicit packet-page return/free callback (which
would avoid many of these issues).
> This is clearly the current bottleneck in the network stack receive
> flow.
>
> I know about some efforts that were made in the past two years.
> For example the ones from Jesper et al.:
>
> - Page-pool (not accepted AFAIK).
The page-pool have many purposes.
1. generic page-cache for drivers,
2. keep pages DMA-mapped
3. facilitate drivers to change RX-ring memory model
From a MM-point-of-view the page pool is just a destructor callback,
that can "steal" the page.
If I can convince XDP_REDIRECT to use an explicit destructor callback,
then I almost get what I need. Except for the generic part, and the
normal network path will not see the benefit. Thus, not helping your
use-case, I guess.
> - Page-allocation bulking.
Notice, that page-allocator bulking, would still be needed by the
page-pool and other page-cache facilities. We should implement it
regardless of the page_pool.
Without a page pool facility to hide the use of page bulking. You
could use page-bulk-alloc in driver RX-ring refill, and find where TCP
free the GRO packets, and do page-bulk-free there.
> - Optimize order-0 allocations in Per-Cpu-Pages.
There is a need to optimize PCP some more for the single-core XDP
performance target (~14Mpps). I guess, the easiest way around this is
implement/integrate a page bulk API into PCP.
The TCP-GRO use-case you are hitting is a different bottleneck.
It is a multi-CPU parallel workload, that exceed the PCP cache size,
and cause you to hit the page buddy allocator.
I wonder if you could "solve"/mitigate the issue if you tune the size
of the PCP cache?
AFAIK it only keeps 128 pages cached per CPU... I know you can see this
via a proc file, but I cannot remember which(?). And I'm not sure how
you tune this(?)
> I am not an mm expert, but wanted to raise the issue again, to combine
> the efforts and hear from you guys about status and possible directions.
Regarding recent changes... if you have you kernel compiled with
CONFIG_NUMA then the page-allocator is slower (due to keeping
numa-stats), except that this was recently optimized and merged(?)
What (exact) kernel git tree did you run these tests on?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] net: phy: Fix mask value write on gmii2rgmii converter speed register.
From: Fahad Kunnathadi @ 2017-09-15 7:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: f fainelli, netdev, Michal Simek, linux-kernel, soren brinkmann,
linux-arm-kernel
In-Reply-To: <20170914143454.GB27601@lunn.ch>
Hi Andrew,
Thank you for review and comments.
I will review this driver code with any review comments before, and update you if I could find any..
Best Regards
Fahad K
----- Original Message -----
From: "Andrew Lunn" <andrew@lunn.ch>
To: "Fahad Kunnathadi" <fahad.kunnathadi@dexceldesigns.com>
Cc: "f fainelli" <f.fainelli@gmail.com>, netdev@vger.kernel.org, "Michal Simek" <michal.simek@xilinx.com>, linux-kernel@vger.kernel.org, "soren brinkmann" <soren.brinkmann@xilinx.com>, linux-arm-kernel@lists.infradead.org
Sent: Thursday, September 14, 2017 8:04:54 PM
Subject: Re: [PATCH] net: phy: Fix mask value write on gmii2rgmii converter speed register.
On Thu, Sep 14, 2017 at 12:46:31PM +0530, Fahad Kunnathadi wrote:
> To clear Speed Selection in MDIO control register(0x10),
> ie, clear bits 6 and 13 to zero while keeping other bits same.
> Before AND operation,The Mask value has to be perform with bitwise NOT
> operation (ie, ~ operator)
>
> This patch clears current speed selection before writing the
> new speed settings to gmii2rgmii converter
Hi Fahad
I expect you will find other issues with this driver. I pointed some
out at the time it is submitted, but the developers went quiet as soon
as it was accepted.
Anyway, please ensure David Miller <davem@davemloft.net> gets a copy.
The subject line should be:
[PATCH net] net: phy: Fix mask value write on gmii2rgmii converter speed register.
and include a fixes tag:
Fixes: f411a6160bd4 ("net: phy: Add gmiitorgmii converter support")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH net] net: phy: Fix mask value write on gmii2rgmii converter speed register
From: Fahad Kunnathadi @ 2017-09-15 6:31 UTC (permalink / raw)
To: f.fainelli
Cc: michal.simek, davem, andrew, appanad, soren.brinkmann, netdev,
linux-kernel, Fahad Kunnathadi
To clear Speed Selection in MDIO control register(0x10),
ie, clear bits 6 and 13 to zero while keeping other bits same.
Before AND operation,The Mask value has to be perform with bitwise NOT
operation (ie, ~ operator)
This patch clears current speed selection before writing the
new speed settings to gmii2rgmii converter
Fixes: f411a6160bd4 ("net: phy: Add gmiitorgmii converter support")
Signed-off-by: Fahad Kunnathadi <fahad.kunnathadi@dexceldesigns.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/xilinx_gmii2rgmii.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
index d15dd39..2e5150b 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -44,7 +44,7 @@ static int xgmiitorgmii_read_status(struct phy_device *phydev)
priv->phy_drv->read_status(phydev);
val = mdiobus_read(phydev->mdio.bus, priv->addr, XILINX_GMII2RGMII_REG);
- val &= XILINX_GMII2RGMII_SPEED_MASK;
+ val &= ~XILINX_GMII2RGMII_SPEED_MASK;
if (phydev->speed == SPEED_1000)
val |= BMCR_SPEED1000;
--
1.9.1
^ permalink raw reply related
* [PATCH v2] net/ethernet/freescale: fix warning for ucc_geth
From: Valentin Longchamp @ 2017-09-15 5:58 UTC (permalink / raw)
To: leoli, netdev, christophe.leroy; +Cc: linuxppc-dev, Valentin Longchamp
uf_info.regs is resource_size_t i.e. phys_addr_t that can be either u32
or u64 according to CONFIG_PHYS_ADDR_T_64BIT.
The printk format is thus adaptet to u64 and the regs value cast to u64
to take both u32 and u64 into account.
Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index f77ba9fa257b..a96b838cffce 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3857,8 +3857,9 @@ static int ucc_geth_probe(struct platform_device* ofdev)
}
if (netif_msg_probe(&debug))
- pr_info("UCC%1d at 0x%8x (irq = %d)\n",
- ug_info->uf_info.ucc_num + 1, ug_info->uf_info.regs,
+ pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
+ ug_info->uf_info.ucc_num + 1,
+ (u64)ug_info->uf_info.regs,
ug_info->uf_info.irq);
/* Create an ethernet device instance */
--
2.13.5
^ permalink raw reply related
* Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface
From: Jiri Pirko @ 2017-09-15 5:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexander Duyck, Maxim Uvarov, Vivien Didelot, Greg KH, netdev,
linux-kernel@vger.kernel.org, kernel, David S. Miller,
Florian Fainelli, Egil Hjelmeland, John Crispin, Woojung Huh,
Sean Wang, Nikita Yushchenko, Chris Healy
In-Reply-To: <20170914210132.GC3084@lunn.ch>
Thu, Sep 14, 2017 at 11:01:32PM CEST, andrew@lunn.ch wrote:
>> Can you clarify what type of registers it is you are wanting to read?
>> We already have ethtool which is meant to allow reading the device
>> registers for a given netdev. As long as the port has a netdev
>> associated it then there is no need to be getting into debugfs since
>> we should probably just be using ethtool.
>
>Not all ports of a DSA switch have a netdev. This is by design. The
>presentation we gave to Netdev 2.1 gives some of the background.
>
>Plus a switch has a lot of registers not associated to port. Often a
>switch has more global registers than port registers.
>
>> Also as Jiri pointed out there is already devlink which would probably
>> be a better way to get the associated information for those pieces
>> that don't have a netdev associated with them.
>
>We have looked at the devlink a few times. The current dpipe code is
>not generic enough. It makes assumptions about the architecture of the
>switch, that it is all match/action based. The niche of top of rack
>switches might be like that, but average switches are not.
>
>If dpipe was to support simple generic two dimensional tables, we
>probably would use it.
>
>David suggested making a class device for DSA. It is not ideal, but we
>are probably going to go that way.
I believe that is also big mistake.
Could you put together your requirements so we can work it out to extend
devlink to support them?
Thanks.
^ permalink raw reply
* [PATCH 1/1] ipv6_skip_exthdr: use ipv6_authlen for AH header length computation
From: Xiang Gao @ 2017-09-15 5:04 UTC (permalink / raw)
To: trivial, netdev, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI
>From 09cf2e3cf09cf591283785aaa8159baf39ac2e08 Mon Sep 17 00:00:00 2001
From: Xiang Gao <qasdfgtyuiop@gmail.com>
Date: Fri, 15 Sep 2017 00:44:12 -0400
Subject: [PATCH] ipv6_skip_exthdr: use ipv6_authlen for AH hdrlen
In ipv6_skip_exthdr, the lengh of AH header is computed manually
as (hp->hdrlen+2)<<2. However, in include/linux/ipv6.h, a macro
named ipv6_authlen is already defined for exactly the same job. This
commit replaces the manual computation code with the macro.
---
net/ipv6/exthdrs_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
index 305e2ed730bf..115d60919f72 100644
--- a/net/ipv6/exthdrs_core.c
+++ b/net/ipv6/exthdrs_core.c
@@ -99,7 +99,7 @@ int ipv6_skip_exthdr(const struct sk_buff *skb, int
start, u8 *nexthdrp,
break;
hdrlen = 8;
} else if (nexthdr == NEXTHDR_AUTH)
- hdrlen = (hp->hdrlen+2)<<2;
+ hdrlen = ipv6_authlen(hp);
else
hdrlen = ipv6_optlen(hp);
--
2.14.1
Signed-off-by: Xiang Gao <qasdfgtyuiop@gmail.com>
^ permalink raw reply related
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-15 5:03 UTC (permalink / raw)
To: Neal Cardwell
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Netdev,
Yuchung Cheng
In-Reply-To: <CADVnQy=dzjROPgrOw3vAZvtb9ETRJgTPVGtau8Q3ChteDKnYow@mail.gmail.com>
Hi.
I've applied your test patch but it doesn't fix the issue for me since the
warning is still there.
Were you able to reproduce it?
On pondělí 11. září 2017 1:59:02 CEST Neal Cardwell wrote:
> Thanks for the detailed report!
>
> I suspect this is due to the following commit, which happened between
> 4.10 and 4.11:
>
> 89fe18e44f7e tcp: extend F-RTO to catch more spurious timeouts
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?
> id=89fe18e44f7e
>
> This commit expanded the set of scenarios where we would undo a
> CA_Loss cwnd reduction and return to TCP_CA_Open, but did not include
> a check to see if there were any in-flight retransmissions. I think we
> need a fix like the following:
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 659d1baefb2b..730a2de9d2b0 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -2439,7 +2439,7 @@ static bool tcp_try_undo_loss(struct sock *sk,
> bool frto_undo)
> {
> struct tcp_sock *tp = tcp_sk(sk);
>
> - if (frto_undo || tcp_may_undo(tp)) {
> + if ((frto_undo || tcp_may_undo(tp)) && !tp->retrans_out) {
> tcp_undo_cwnd_reduction(sk, true);
>
> DBGUNDO(sk, "partial loss");
>
> I will try a packetdrill test to see if I can reproduce this issue and
> verify the fix.
>
> thanks,
> neal
^ permalink raw reply
* [PATCH net] ip6_gre: skb_push ipv6hdr before packing the header in ip6gre_header
From: Xin Long @ 2017-09-15 4:00 UTC (permalink / raw)
To: network dev; +Cc: davem, Dmitry Kozlov
Now in ip6gre_header before packing the ipv6 header, it skb_push t->hlen
which only includes encap_hlen + tun_hlen. It means greh and inner header
would be over written by ipv6 stuff and ipv6h might have no chance to set
up.
Jianlin found this issue when using remote any on ip6_gre, the packets he
captured on gre dev are truncated:
22:50:26.210866 Out ethertype IPv6 (0x86dd), length 120: truncated-ip6 -\
8128 bytes missing!(flowlabel 0x92f40, hlim 0, next-header Options (0) \
payload length: 8192) ::1:2000:0 > ::1:0:86dd: HBH [trunc] ip-proto-128 \
8184
It should also skb_push ipv6hdr so that ipv6h points to the right position
to set ipv6 stuff up.
This patch is to skb_push hlen + sizeof(*ipv6h) and also fix some indents
in ip6gre_header.
Fixes: c12b395a4664 ("gre: Support GRE over IPv6")
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/ipv6/ip6_gre.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index b7a72d4..20f66f4 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -940,24 +940,25 @@ static int ip6gre_tunnel_ioctl(struct net_device *dev,
}
static int ip6gre_header(struct sk_buff *skb, struct net_device *dev,
- unsigned short type,
- const void *daddr, const void *saddr, unsigned int len)
+ unsigned short type, const void *daddr,
+ const void *saddr, unsigned int len)
{
struct ip6_tnl *t = netdev_priv(dev);
- struct ipv6hdr *ipv6h = skb_push(skb, t->hlen);
- __be16 *p = (__be16 *)(ipv6h+1);
+ struct ipv6hdr *ipv6h;
+ __be16 *p;
- ip6_flow_hdr(ipv6h, 0,
- ip6_make_flowlabel(dev_net(dev), skb,
- t->fl.u.ip6.flowlabel, true,
- &t->fl.u.ip6));
+ ipv6h = skb_push(skb, t->hlen + sizeof(*ipv6h));
+ ip6_flow_hdr(ipv6h, 0, ip6_make_flowlabel(dev_net(dev), skb,
+ t->fl.u.ip6.flowlabel,
+ true, &t->fl.u.ip6));
ipv6h->hop_limit = t->parms.hop_limit;
ipv6h->nexthdr = NEXTHDR_GRE;
ipv6h->saddr = t->parms.laddr;
ipv6h->daddr = t->parms.raddr;
- p[0] = t->parms.o_flags;
- p[1] = htons(type);
+ p = (__be16 *)(ipv6h + 1);
+ p[0] = t->parms.o_flags;
+ p[1] = htons(type);
/*
* Set the source hardware address.
--
2.1.0
^ permalink raw reply related
* Re: Regression in throughput between kvm guests over virtual bridge
From: Matthew Rosato @ 2017-09-15 3:36 UTC (permalink / raw)
To: Jason Wang, netdev; +Cc: davem, mst
In-Reply-To: <50891c14-3fc6-f519-8c03-07bdef3090f4@redhat.com>
> Is the issue gone if you reduce VHOST_RX_BATCH to 1? And it would be
> also helpful to collect perf diff to see if anything interesting.
> (Consider 4.4 shows more obvious regression, please use 4.4).
>
Issue still exists when I force VHOST_RX_BATCH = 1
Collected perf data, with 4.12 as the baseline, 4.13 as delta1 and
4.13+VHOST_RX_BATCH=1 as delta2. All guests running 4.4. Same scenario,
2 uperf client guests, 2 uperf slave guests - I collected perf data
against 1 uperf client process and 1 uperf slave process. Here are the
significant diffs:
uperf client:
75.09% +9.32% +8.52% [kernel.kallsyms] [k] enabled_wait
9.04% -4.11% -3.79% [kernel.kallsyms] [k] __copy_from_user
2.30% -0.79% -0.71% [kernel.kallsyms] [k] arch_free_page
2.17% -0.65% -0.58% [kernel.kallsyms] [k] arch_alloc_page
0.69% -0.25% -0.24% [kernel.kallsyms] [k] get_page_from_freelist
0.56% +0.08% +0.14% [kernel.kallsyms] [k] virtio_ccw_kvm_notify
0.42% -0.11% -0.09% [kernel.kallsyms] [k] tcp_sendmsg
0.31% -0.15% -0.14% [kernel.kallsyms] [k] tcp_write_xmit
uperf slave:
72.44% +8.99% +8.85% [kernel.kallsyms] [k] enabled_wait
8.99% -3.67% -3.51% [kernel.kallsyms] [k] __copy_to_user
2.31% -0.71% -0.67% [kernel.kallsyms] [k] arch_free_page
2.16% -0.67% -0.63% [kernel.kallsyms] [k] arch_alloc_page
0.89% -0.14% -0.11% [kernel.kallsyms] [k] virtio_ccw_kvm_notify
0.71% -0.30% -0.30% [kernel.kallsyms] [k] get_page_from_freelist
0.70% -0.25% -0.29% [kernel.kallsyms] [k] __wake_up_sync_key
0.61% -0.22% -0.22% [kernel.kallsyms] [k] virtqueue_add_inbuf
>
> May worth to try disable zerocopy or do the test form host to guest
> instead of guest to guest to exclude the possible issue of sender.
>
With zerocopy disabled, still seeing the regression. The provided perf
#s have zerocopy enabled.
I replaced 1 uperf guest and instead ran that uperf client as a host
process, pointing at a guest. All traffic still over the virtual
bridge. In this setup, it's still easy to see the regression for the
remaining guest1<->guest2 uperf run, but the host<->guest3 run does NOT
exhibit a reliable regression pattern. The significant perf diffs from
the host uperf process (baseline=4.12, delta=4.13):
59.96% +5.03% [kernel.kallsyms] [k] enabled_wait
6.47% -2.27% [kernel.kallsyms] [k] raw_copy_to_user
5.52% -1.63% [kernel.kallsyms] [k] raw_copy_from_user
0.87% -0.30% [kernel.kallsyms] [k] get_page_from_freelist
0.69% +0.30% [kernel.kallsyms] [k] finish_task_switch
0.66% -0.15% [kernel.kallsyms] [k] swake_up
0.58% -0.00% [vhost] [k] vhost_get_vq_desc
...
0.42% +0.50% [kernel.kallsyms] [k] ckc_irq_pending
I also tried flipping the uperf stream around (a guest uperf client is
communicating to a slave uperf process on the host) and also cannot see
the regression pattern. So it seems to require a guest on both ends of
the connection.
^ permalink raw reply
* Re: [iproute PATCH v2] ipaddress: Fix segfault in 'addr showdump'
From: Julien Fortin @ 2017-09-15 3:05 UTC (permalink / raw)
To: Phil Sutter; +Cc: Stephen Hemminger, Hangbin Liu, netdev
In-Reply-To: <20170913092034.7002-1-phil@nwl.cc>
v2 looks good to me, thanks for catching this segfault.
On Wed, Sep 13, 2017 at 2:20 AM, Phil Sutter <phil@nwl.cc> wrote:
> Obviously, 'addr showdump' feature wasn't adjusted to json output
> support. As a consequence, calls to print_string() in print_addrinfo()
> tried to dereference a NULL FILE pointer.
>
> Cc: Julien Fortin <julien@cumulusnetworks.com>
> Fixes: d0e720111aad2 ("ip: ipaddress.c: add support for json output")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Julien Fortin <julien@cumulusnetworks.com>
> --
> Changes since v1:
> Align json output with that of 'ip -j addr show':
> - Interface index label is 'ifindex', not 'index' and it doesn't belong
> to 'addr_info' array.
> - Create one 'addr_info' array per dumped address, not one for all.
> ---
> ip/ipaddress.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 9797145023966..4c47809570410 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -1801,17 +1801,33 @@ static int show_handler(const struct sockaddr_nl *nl,
> {
> struct ifaddrmsg *ifa = NLMSG_DATA(n);
>
> - printf("if%d:\n", ifa->ifa_index);
> + open_json_object(NULL);
> + print_int(PRINT_ANY, "ifindex", "if%d:\n", ifa->ifa_index);
> +
> + open_json_array(PRINT_JSON, "addr_info");
> + open_json_object(NULL);
> +
> print_addrinfo(NULL, n, stdout);
> +
> + close_json_object();
> + close_json_array(PRINT_JSON, NULL);
> +
> + close_json_object();
> return 0;
> }
>
> static int ipaddr_showdump(void)
> {
> + int err;
> +
> if (ipadd_dump_check_magic())
> exit(-1);
>
> - exit(rtnl_from_file(stdin, &show_handler, NULL));
> + new_json_obj(json, stdout);
> + err = rtnl_from_file(stdin, &show_handler, NULL);
> + delete_json_obj();
> +
> + exit(err);
> }
>
> static int restore_handler(const struct sockaddr_nl *nl,
> --
> 2.13.1
>
^ permalink raw reply
* [PATCH net] sctp: do not mark sk dumped when inet_sctp_diag_fill returns err
From: Xin Long @ 2017-09-15 3:02 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman
sctp_diag would not actually dump out sk/asoc if inet_sctp_diag_fill
returns err, in which case it shouldn't mark sk dumped by setting
cb->args[3] as 1 in sctp_sock_dump().
Otherwise, it could cause some asocs to have no parent's sk dumped
in 'ss --sctp'.
So this patch is to not set cb->args[3] when inet_sctp_diag_fill()
returns err in sctp_sock_dump().
Fixes: 8f840e47f190 ("sctp: add the sctp_diag.c file")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/sctp_diag.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
index 7008a99..22ed01a 100644
--- a/net/sctp/sctp_diag.c
+++ b/net/sctp/sctp_diag.c
@@ -309,7 +309,6 @@ static int sctp_sock_dump(struct sctp_transport *tsp, void *p)
cb->nlh->nlmsg_seq,
NLM_F_MULTI, cb->nlh,
commp->net_admin) < 0) {
- cb->args[3] = 1;
err = 1;
goto release;
}
--
2.1.0
^ permalink raw reply related
* [PATCH net] sctp: fix an use-after-free issue in sctp_sock_dump
From: Xin Long @ 2017-09-15 3:02 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Neil Horman, pabeni
Commit 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the
dump") tried to fix an use-after-free issue by checking !sctp_sk(sk)->ep
with holding sock and sock lock.
But Paolo noticed that endpoint could be destroyed in sctp_rcv without
sock lock protection. It means the use-after-free issue still could be
triggered when sctp_rcv put and destroy ep after sctp_sock_dump checks
!ep, although it's pretty hard to reproduce.
I could reproduce it by mdelay in sctp_rcv while msleep in sctp_close
and sctp_sock_dump long time.
This patch is to add another param cb_done to sctp_for_each_transport
and dump ep->assocs with holding tsp after jumping out of transport's
traversal in it to avoid this issue.
It can also improve sctp diag dump to make it run faster, as no need
to save sk into cb->args[5] and keep calling sctp_for_each_transport
any more.
This patch is also to use int * instead of int for the pos argument
in sctp_for_each_transport, which could make postion increment only
in sctp_for_each_transport and no need to keep changing cb->args[2]
in sctp_sock_filter and sctp_sock_dump any more.
Fixes: 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the dump")
Reported-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/sctp.h | 3 ++-
net/sctp/sctp_diag.c | 32 +++++++++-----------------------
net/sctp/socket.c | 40 +++++++++++++++++++++++++---------------
3 files changed, 36 insertions(+), 39 deletions(-)
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 06b4f51..d7d8cba 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -127,7 +127,8 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
const union sctp_addr *laddr,
const union sctp_addr *paddr, void *p);
int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
- struct net *net, int pos, void *p);
+ int (*cb_done)(struct sctp_transport *, void *),
+ struct net *net, int *pos, void *p);
int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), void *p);
int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
struct sctp_info *info);
diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
index e99518e..7008a99 100644
--- a/net/sctp/sctp_diag.c
+++ b/net/sctp/sctp_diag.c
@@ -279,9 +279,11 @@ static int sctp_tsp_dump_one(struct sctp_transport *tsp, void *p)
return err;
}
-static int sctp_sock_dump(struct sock *sk, void *p)
+static int sctp_sock_dump(struct sctp_transport *tsp, void *p)
{
+ struct sctp_endpoint *ep = tsp->asoc->ep;
struct sctp_comm_param *commp = p;
+ struct sock *sk = ep->base.sk;
struct sk_buff *skb = commp->skb;
struct netlink_callback *cb = commp->cb;
const struct inet_diag_req_v2 *r = commp->r;
@@ -289,9 +291,7 @@ static int sctp_sock_dump(struct sock *sk, void *p)
int err = 0;
lock_sock(sk);
- if (!sctp_sk(sk)->ep)
- goto release;
- list_for_each_entry(assoc, &sctp_sk(sk)->ep->asocs, asocs) {
+ list_for_each_entry(assoc, &ep->asocs, asocs) {
if (cb->args[4] < cb->args[1])
goto next;
@@ -327,40 +327,30 @@ static int sctp_sock_dump(struct sock *sk, void *p)
cb->args[4]++;
}
cb->args[1] = 0;
- cb->args[2]++;
cb->args[3] = 0;
cb->args[4] = 0;
release:
release_sock(sk);
- sock_put(sk);
return err;
}
-static int sctp_get_sock(struct sctp_transport *tsp, void *p)
+static int sctp_sock_filter(struct sctp_transport *tsp, void *p)
{
struct sctp_endpoint *ep = tsp->asoc->ep;
struct sctp_comm_param *commp = p;
struct sock *sk = ep->base.sk;
- struct netlink_callback *cb = commp->cb;
const struct inet_diag_req_v2 *r = commp->r;
struct sctp_association *assoc =
list_entry(ep->asocs.next, struct sctp_association, asocs);
/* find the ep only once through the transports by this condition */
if (tsp->asoc != assoc)
- goto out;
+ return 0;
if (r->sdiag_family != AF_UNSPEC && sk->sk_family != r->sdiag_family)
- goto out;
-
- sock_hold(sk);
- cb->args[5] = (long)sk;
+ return 0;
return 1;
-
-out:
- cb->args[2]++;
- return 0;
}
static int sctp_ep_dump(struct sctp_endpoint *ep, void *p)
@@ -503,12 +493,8 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
if (!(idiag_states & ~(TCPF_LISTEN | TCPF_CLOSE)))
goto done;
-next:
- cb->args[5] = 0;
- sctp_for_each_transport(sctp_get_sock, net, cb->args[2], &commp);
-
- if (cb->args[5] && !sctp_sock_dump((struct sock *)cb->args[5], &commp))
- goto next;
+ sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
+ net, (int *)&cb->args[2], &commp);
done:
cb->args[1] = cb->args[4];
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 1b00a1e..d4730ad 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4658,29 +4658,39 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);
int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
- struct net *net, int pos, void *p) {
+ int (*cb_done)(struct sctp_transport *, void *),
+ struct net *net, int *pos, void *p) {
struct rhashtable_iter hti;
- void *obj;
- int err;
-
- err = sctp_transport_walk_start(&hti);
- if (err)
- return err;
+ struct sctp_transport *tsp;
+ int ret;
- obj = sctp_transport_get_idx(net, &hti, pos + 1);
- for (; !IS_ERR_OR_NULL(obj); obj = sctp_transport_get_next(net, &hti)) {
- struct sctp_transport *transport = obj;
+again:
+ ret = sctp_transport_walk_start(&hti);
+ if (ret)
+ return ret;
- if (!sctp_transport_hold(transport))
+ tsp = sctp_transport_get_idx(net, &hti, *pos + 1);
+ for (; !IS_ERR_OR_NULL(tsp); tsp = sctp_transport_get_next(net, &hti)) {
+ if (!sctp_transport_hold(tsp))
continue;
- err = cb(transport, p);
- sctp_transport_put(transport);
- if (err)
+ ret = cb(tsp, p);
+ if (ret)
break;
+ (*pos)++;
+ sctp_transport_put(tsp);
}
sctp_transport_walk_stop(&hti);
- return err;
+ if (ret) {
+ if (cb_done && !cb_done(tsp, p)) {
+ (*pos)++;
+ sctp_transport_put(tsp);
+ goto again;
+ }
+ sctp_transport_put(tsp);
+ }
+
+ return ret;
}
EXPORT_SYMBOL_GPL(sctp_for_each_transport);
--
2.1.0
^ permalink raw reply related
* [PATCH 1/1] forcedeth: replace pci_map_single with dma_map_single functions
From: Zhu Yanjun @ 2017-09-15 3:01 UTC (permalink / raw)
To: jarod, netdev
pci_map_single functions are obsolete. So replace them with
dma_map_single functions.
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 70 +++++++++++++++++----------------
1 file changed, 36 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index e6e0de4..0a7ba3a 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -1812,12 +1812,12 @@ static int nv_alloc_rx(struct net_device *dev)
struct sk_buff *skb = netdev_alloc_skb(dev, np->rx_buf_sz + NV_RX_ALLOC_PAD);
if (skb) {
np->put_rx_ctx->skb = skb;
- np->put_rx_ctx->dma = pci_map_single(np->pci_dev,
+ np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
skb->data,
skb_tailroom(skb),
- PCI_DMA_FROMDEVICE);
- if (pci_dma_mapping_error(np->pci_dev,
- np->put_rx_ctx->dma)) {
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(&np->pci_dev->dev,
+ np->put_rx_ctx->dma)) {
kfree_skb(skb);
goto packet_dropped;
}
@@ -1853,12 +1853,12 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
struct sk_buff *skb = netdev_alloc_skb(dev, np->rx_buf_sz + NV_RX_ALLOC_PAD);
if (skb) {
np->put_rx_ctx->skb = skb;
- np->put_rx_ctx->dma = pci_map_single(np->pci_dev,
+ np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
skb->data,
skb_tailroom(skb),
- PCI_DMA_FROMDEVICE);
- if (pci_dma_mapping_error(np->pci_dev,
- np->put_rx_ctx->dma)) {
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(&np->pci_dev->dev,
+ np->put_rx_ctx->dma)) {
kfree_skb(skb);
goto packet_dropped;
}
@@ -1975,9 +1975,9 @@ static void nv_unmap_txskb(struct fe_priv *np, struct nv_skb_map *tx_skb)
{
if (tx_skb->dma) {
if (tx_skb->dma_single)
- pci_unmap_single(np->pci_dev, tx_skb->dma,
+ dma_unmap_single(&np->pci_dev->dev, tx_skb->dma,
tx_skb->dma_len,
- PCI_DMA_TODEVICE);
+ DMA_TO_DEVICE);
else
pci_unmap_page(np->pci_dev, tx_skb->dma,
tx_skb->dma_len,
@@ -2045,10 +2045,10 @@ static void nv_drain_rx(struct net_device *dev)
}
wmb();
if (np->rx_skb[i].skb) {
- pci_unmap_single(np->pci_dev, np->rx_skb[i].dma,
+ dma_unmap_single(&np->pci_dev->dev, np->rx_skb[i].dma,
(skb_end_pointer(np->rx_skb[i].skb) -
- np->rx_skb[i].skb->data),
- PCI_DMA_FROMDEVICE);
+ np->rx_skb[i].skb->data),
+ DMA_FROM_DEVICE);
dev_kfree_skb(np->rx_skb[i].skb);
np->rx_skb[i].skb = NULL;
}
@@ -2221,10 +2221,11 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
prev_tx = put_tx;
prev_tx_ctx = np->put_tx_ctx;
bcnt = (size > NV_TX2_TSO_MAX_SIZE) ? NV_TX2_TSO_MAX_SIZE : size;
- np->put_tx_ctx->dma = pci_map_single(np->pci_dev, skb->data + offset, bcnt,
- PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(np->pci_dev,
- np->put_tx_ctx->dma)) {
+ np->put_tx_ctx->dma = dma_map_single(&np->pci_dev->dev,
+ skb->data + offset, bcnt,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(&np->pci_dev->dev,
+ np->put_tx_ctx->dma)) {
/* on DMA mapping error - drop the packet */
dev_kfree_skb_any(skb);
u64_stats_update_begin(&np->swstats_tx_syncp);
@@ -2369,10 +2370,11 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
prev_tx = put_tx;
prev_tx_ctx = np->put_tx_ctx;
bcnt = (size > NV_TX2_TSO_MAX_SIZE) ? NV_TX2_TSO_MAX_SIZE : size;
- np->put_tx_ctx->dma = pci_map_single(np->pci_dev, skb->data + offset, bcnt,
- PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(np->pci_dev,
- np->put_tx_ctx->dma)) {
+ np->put_tx_ctx->dma = dma_map_single(&np->pci_dev->dev,
+ skb->data + offset, bcnt,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(&np->pci_dev->dev,
+ np->put_tx_ctx->dma)) {
/* on DMA mapping error - drop the packet */
dev_kfree_skb_any(skb);
u64_stats_update_begin(&np->swstats_tx_syncp);
@@ -2805,9 +2807,9 @@ static int nv_rx_process(struct net_device *dev, int limit)
* TODO: check if a prefetch of the first cacheline improves
* the performance.
*/
- pci_unmap_single(np->pci_dev, np->get_rx_ctx->dma,
- np->get_rx_ctx->dma_len,
- PCI_DMA_FROMDEVICE);
+ dma_unmap_single(&np->pci_dev->dev, np->get_rx_ctx->dma,
+ np->get_rx_ctx->dma_len,
+ DMA_FROM_DEVICE);
skb = np->get_rx_ctx->skb;
np->get_rx_ctx->skb = NULL;
@@ -2911,9 +2913,9 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
* TODO: check if a prefetch of the first cacheline improves
* the performance.
*/
- pci_unmap_single(np->pci_dev, np->get_rx_ctx->dma,
- np->get_rx_ctx->dma_len,
- PCI_DMA_FROMDEVICE);
+ dma_unmap_single(&np->pci_dev->dev, np->get_rx_ctx->dma,
+ np->get_rx_ctx->dma_len,
+ DMA_FROM_DEVICE);
skb = np->get_rx_ctx->skb;
np->get_rx_ctx->skb = NULL;
@@ -5065,11 +5067,11 @@ static int nv_loopback_test(struct net_device *dev)
ret = 0;
goto out;
}
- test_dma_addr = pci_map_single(np->pci_dev, tx_skb->data,
+ test_dma_addr = dma_map_single(&np->pci_dev->dev, tx_skb->data,
skb_tailroom(tx_skb),
- PCI_DMA_FROMDEVICE);
- if (pci_dma_mapping_error(np->pci_dev,
- test_dma_addr)) {
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(&np->pci_dev->dev,
+ test_dma_addr)) {
dev_kfree_skb_any(tx_skb);
goto out;
}
@@ -5124,9 +5126,9 @@ static int nv_loopback_test(struct net_device *dev)
}
}
- pci_unmap_single(np->pci_dev, test_dma_addr,
- (skb_end_pointer(tx_skb) - tx_skb->data),
- PCI_DMA_TODEVICE);
+ dma_unmap_single(&np->pci_dev->dev, test_dma_addr,
+ (skb_end_pointer(tx_skb) - tx_skb->data),
+ DMA_TO_DEVICE);
dev_kfree_skb_any(tx_skb);
out:
/* stop engines */
--
2.7.4
^ permalink raw reply related
* RE: [Intel-wired-lan] [PATCH 5/5] e1000e: Avoid receiver overrun interrupt bursts
From: Brown, Aaron F @ 2017-09-15 0:38 UTC (permalink / raw)
To: Benjamin Poirier, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org, Lennart Sorensen
In-Reply-To: <20170721183627.13373-5-bpoirier@suse.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux-
> kernel@vger.kernel.org; Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
> Subject: [Intel-wired-lan] [PATCH 5/5] e1000e: Avoid receiver overrun
> interrupt bursts
>
> When e1000e_poll() is not fast enough to keep up with incoming traffic, the
> adapter (when operating in msix mode) raises the Other interrupt to signal
> Receiver Overrun.
>
> This is a double problem because 1) at the moment e1000_msix_other()
> assumes that it is only called in case of Link Status Change and 2) if the
> condition persists, the interrupt is repeatedly raised again in quick
> succession.
>
> Ideally we would configure the Other interrupt to not be raised in case of
> receiver overrun but this doesn't seem possible on this adapter. Instead,
> we handle the first part of the problem by reverting to the practice of
> reading ICR in the other interrupt handler, like before commit 16ecba59bc33
> ("e1000e: Do not read ICR in Other interrupt"). Thanks to commit
> 0a8047ac68e5 ("e1000e: Fix msi-x interrupt automask") which cleared IAME
> from CTRL_EXT, reading ICR doesn't interfere with RxQ0, TxQ0 interrupts
> anymore. We handle the second part of the problem by not re-enabling the
> Other interrupt right away when there is overrun. Instead, we wait until
> traffic subsides, napi polling mode is exited and interrupts are
> re-enabled.
>
> Reported-by: Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
> Fixes: 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt")
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/intel/e1000e/defines.h | 1 +
> drivers/net/ethernet/intel/e1000e/netdev.c | 33
> +++++++++++++++++++++++------
> 2 files changed, 27 insertions(+), 7 deletions(-)
>
I get an error and a few warnings out of checkpatch from this, but I think the error is false (thinking the reference to a commit in the description is this commit, a fixes commit or something like that) and I'm more concerned with the fix than the warnings, so...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Here is the checkpatch output in case anyone has a different opinion on the severity:
-------------
u1484:[0]/usr/src/kernels/next-queue> git format-patch d81d1e6 -1 --stdout|./scripts/checkpatch.pl -
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 0a8047ac68e5 ("e1000e: Fix msi-x interrupt automask")'
#20:
0a8047ac68e5 ("e1000e: Fix msi-x interrupt automask") which cleared IAME
WARNING: braces {} are not necessary for single statement blocks
#73: FILE: drivers/net/ethernet/intel/e1000e/netdev.c:1931:
+ if (!test_bit(__E1000_DOWN, &adapter->state)) {
+ mod_timer(&adapter->watchdog_timer, jiffies + 1);
+ }
WARNING: braces {} are not necessary for single statement blocks
#83: FILE: drivers/net/ethernet/intel/e1000e/netdev.c:1936:
+ if (enable && !test_bit(__E1000_DOWN, &adapter->state)) {
ew32(IMS, E1000_IMS_OTHER);
}
total: 1 errors, 2 warnings, 0 checks, 59 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Your patch has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
u1484:[0]/usr/src/kernels/next-queue>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH 4/5] e1000e: Separate signaling for link check/link up
From: Brown, Aaron F @ 2017-09-15 0:27 UTC (permalink / raw)
To: Benjamin Poirier
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org, Lennart Sorensen
In-Reply-To: <20170802144922.txmee23d35o4r7mh@f1.synalogic.ca>
On 7/21/2017 21:36, Benjamin Poirier wrote:
> Lennart reported the following race condition:
>
> \ e1000_watchdog_task
> \ e1000e_has_link
> \ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link
> /* link is up */
> mac->get_link_status = false;
>
> /* interrupt */
> \ e1000_msix_other
> hw->mac.get_link_status = true;
>
> link_active = !hw->mac.get_link_status
> /* link_active is false, wrongly */
>
> This problem arises because the single flag get_link_status is used to
> signal two different states: link status needs checking and link status is
> down.
>
> Avoid the problem by using the return value of .check_for_link to signal
> the link status to e1000e_has_link().
>
> Reported-by: Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/intel/e1000e/mac.c | 11 ++++++++---
> drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
> 2 files changed, 9 insertions(+), 4 deletions(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH 3/5] e1000e: Fix return value test
From: Brown, Aaron F @ 2017-09-15 0:20 UTC (permalink / raw)
To: Benjamin Poirier, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org, Lennart Sorensen
In-Reply-To: <20170721183627.13373-3-bpoirier@suse.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux-
> kernel@vger.kernel.org; Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
> Subject: [Intel-wired-lan] [PATCH 3/5] e1000e: Fix return value test
>
> All the helpers return -E1000_ERR_PHY.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH 1/5] e1000e: Fix error path in link detection
From: Brown, Aaron F @ 2017-09-15 0:18 UTC (permalink / raw)
To: Benjamin Poirier, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org, Lennart Sorensen
In-Reply-To: <20170721183627.13373-1-bpoirier@suse.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux-
> kernel@vger.kernel.org; Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
> Subject: [Intel-wired-lan] [PATCH 1/5] e1000e: Fix error path in link detection
>
> In case of error from e1e_rphy(), the loop will exit early and "success"
> will be set to true erroneously.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/intel/e1000e/phy.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH net-next v3] e1000e: Be drop monitor friendly
From: Brown, Aaron F @ 2017-09-14 23:54 UTC (permalink / raw)
To: Florian Fainelli, netdev@vger.kernel.org
Cc: edumazet@gmail.com, open list,
moderated list:INTEL ETHERNET DRIVERS, davem@davemloft.net
In-Reply-To: <20170826011424.27251-1-f.fainelli@gmail.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Florian Fainelli
> Sent: Friday, August 25, 2017 6:14 PM
> To: netdev@vger.kernel.org
> Cc: edumazet@gmail.com; Florian Fainelli <f.fainelli@gmail.com>; open list
> <linux-kernel@vger.kernel.org>; moderated list:INTEL ETHERNET DRIVERS
> <intel-wired-lan@lists.osuosl.org>; davem@davemloft.net
> Subject: [Intel-wired-lan] [PATCH net-next v3] e1000e: Be drop monitor
> friendly
>
> e1000e_put_txbuf() can be called from normal reclamation path as well as
> when a DMA mapping failure, so we need to differentiate these two cases
> when freeing SKBs to be drop monitor friendly. e1000e_tx_hwtstamp_work()
> and e1000_remove() are processing TX timestamped SKBs and those should
> not be accounted as drops either.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Changes in v3:
>
> - differentiate normal reclamation from TX DMA fragment mapping errors
> - removed a few invalid dev_kfree_skb() replacements (those are already
> drop monitor friendly)
>
> Changes in v2:
>
> - make it compile
>
> drivers/net/ethernet/intel/e1000e/netdev.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* RE: [PATCH] e1000e: apply burst mode settings only on default
From: Brown, Aaron F @ 2017-09-14 23:48 UTC (permalink / raw)
To: Willem de Bruijn, Kirsher, Jeffrey T
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
Brandeburg, Jesse, Willem de Bruijn
In-Reply-To: <20170825150626.2843-1-willemdebruijn.kernel@gmail.com>
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Willem de Bruijn
> Sent: Friday, August 25, 2017 8:06 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; Willem de Bruijn
> <willemb@google.com>
> Subject: [PATCH] e1000e: apply burst mode settings only on default
>
> From: Willem de Bruijn <willemb@google.com>
>
> Devices that support FLAG2_DMA_BURST have different default values
> for RDTR and RADV. Apply burst mode default settings only when no
> explicit value was passed at module load.
>
> The RDTR default is zero. If the module is loaded for low latency
> operation with RxIntDelay=0, do not override this value with a burst
> default of 32.
>
> Move the decision to apply burst values earlier, where explicitly
> initialized module variables can be distinguished from defaults.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
> drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
> drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
> 3 files changed, 15 insertions(+), 13 deletions(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* Re: Bug with BPF_ALU64 | BPF_END?
From: Daniel Borkmann @ 2017-09-14 23:04 UTC (permalink / raw)
To: Edward Cree, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <52c7df55-84d9-f6d2-ed84-51ac90eb6bcc@solarflare.com>
On 09/14/2017 07:53 PM, Edward Cree wrote:
> Is BPF_END supposed to only be used with BPF_ALU, never with BPF_ALU64?
> In kernel/bpf/core.c:___bpf_prog_run(), there are only jump table targets
> for the BPF_ALU case, not for the BPF_ALU64 case (opcodes 0xd7 and 0xdf).
> But the verifier doesn't enforce this; by crafting a program that uses
> these opcodes I can get a WARN when they're run (without JIT; it looks
> like the x86 JIT, at least, won't like it either).
> Proposed patch below the cut; build-tested only.
>
> -Ed
> ---
>
> [PATCH net] bpf/verifier: reject BPF_ALU64|BPF_END
>
> Neither ___bpf_prog_run nor the JITs accept it.
>
> Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
> Signed-off-by: Edward Cree <ecree@solarflare.com>
Good catch! Can you submit this as an official patch for -net together
with a test case for tools/testing/selftests/bpf/test_verifier.c?
Thanks!
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> kernel/bpf/verifier.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 477b693..799b245 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2292,7 +2292,8 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
> }
> } else {
> if (insn->src_reg != BPF_REG_0 || insn->off != 0 ||
> - (insn->imm != 16 && insn->imm != 32 && insn->imm != 64)) {
> + (insn->imm != 16 && insn->imm != 32 && insn->imm != 64) ||
> + BPF_CLASS(insn->code) == BPF_ALU64) {
> verbose("BPF_END uses reserved fields\n");
> return -EINVAL;
> }
>
^ permalink raw reply
* Re: [PATCH net] packet: hold bind lock when rebinding to fanout hook
From: Eric Dumazet @ 2017-09-14 22:53 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, nixiaoming
In-Reply-To: <20170914211441.67326-1-willemb@google.com>
On Thu, 2017-09-14 at 17:14 -0400, Willem de Bruijn wrote:
> Packet socket bind operations must hold the po->bind_lock. This keeps
> po->running consistent with whether the socket is actually on a ptype
> list to receive packets.
>
> fanout_add unbinds a socket and its packet_rcv/tpacket_rcv call, then
> binds the fanout object to receive through packet_rcv_fanout.
>
> Make it hold the po->bind_lock when testing po->running and rebinding.
> Else, it can race with other rebind operations, such as that in
> packet_set_ring from packet_rcv to tpacket_rcv. Concurrent updates
> can result in a socket being added to a fanout group twice, causing
> use-after-free KASAN bug reports, among others.
>
> Reported independently by both trinity and syzkaller.
> Verified that the syzkaller reproducer passes after this patch.
>
> Reported-by: nixioaming <nixiaoming@huawei.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> net/packet/af_packet.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index c26172995511..d288f52c53f7 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1684,10 +1684,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>
> mutex_lock(&fanout_mutex);
>
> - err = -EINVAL;
> - if (!po->running)
> - goto out;
> -
> err = -EALREADY;
> if (po->fanout)
> goto out;
> @@ -1749,7 +1745,10 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> list_add(&match->list, &fanout_list);
> }
> err = -EINVAL;
> - if (match->type == type &&
> +
> + spin_lock(&po->bind_lock);
> + if (po->running &&
> + match->type == type &&
> match->prot_hook.type == po->prot_hook.type &&
> match->prot_hook.dev == po->prot_hook.dev) {
> err = -ENOSPC;
> @@ -1761,6 +1760,13 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> err = 0;
> }
> }
> + spin_unlock(&po->bind_lock);
> +
> + if (err && !refcount_read(&match->sk_ref)) {
It seems sk_ref is always read/changed under
mutex_lock(&fanout_mutex) protection.
Not sure why we use a refcount_t (or an atomic_t in older kernels)
All these atomic/spinlock/mutexes are a maze.
> + list_del(&match->list);
> + kfree(match);
> + }
> +
> out:
> if (err && rollover) {
> kfree(rollover);
^ 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