* [PATCH net-next 0/2] net: speedup geneve/vxlan tunnels dismantle
From: Haishuang Yan @ 2017-12-16 9:54 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet; +Cc: netdev, linux-kernel, Haishuang Yan
This patch series add batching to vxlan/geneve tunnels so that netns
dismantles are less costly.
Haishuang Yan (2):
vxlan: speedup vxlan tunnels dismantle
geneve: speedup geneve tunnels dismantle
drivers/net/geneve.c | 24 ++++++++++++++++--------
drivers/net/vxlan.c | 26 +++++++++++++++++---------
2 files changed, 33 insertions(+), 17 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 1/2] vxlan: speedup vxlan tunnels dismantle
From: Haishuang Yan @ 2017-12-16 9:54 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet; +Cc: netdev, linux-kernel, Haishuang Yan
In-Reply-To: <1513418090-8595-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
Since we now hold RTNL lock in vxlan_exit_net, it's better to batch them
to speedup vxlan tunnels dismantle.
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
drivers/net/vxlan.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 19b9cc5..48a0dc2 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3692,18 +3692,16 @@ static __net_init int vxlan_init_net(struct net *net)
return 0;
}
-static void __net_exit vxlan_exit_net(struct net *net)
+static void vxlan_destroy_tunnels(struct net *net, struct list_head *head)
{
struct vxlan_net *vn = net_generic(net, vxlan_net_id);
struct vxlan_dev *vxlan, *next;
struct net_device *dev, *aux;
unsigned int h;
- LIST_HEAD(list);
- rtnl_lock();
for_each_netdev_safe(net, dev, aux)
if (dev->rtnl_link_ops == &vxlan_link_ops)
- unregister_netdevice_queue(dev, &list);
+ unregister_netdevice_queue(dev, head);
list_for_each_entry_safe(vxlan, next, &vn->vxlan_list, next) {
/* If vxlan->dev is in the same netns, it has already been added
@@ -3711,20 +3709,30 @@ static void __net_exit vxlan_exit_net(struct net *net)
*/
if (!net_eq(dev_net(vxlan->dev), net)) {
gro_cells_destroy(&vxlan->gro_cells);
- unregister_netdevice_queue(vxlan->dev, &list);
+ unregister_netdevice_queue(vxlan->dev, head);
}
}
- unregister_netdevice_many(&list);
- rtnl_unlock();
-
for (h = 0; h < PORT_HASH_SIZE; ++h)
WARN_ON_ONCE(!hlist_empty(&vn->sock_list[h]));
}
+static void __net_exit vxlan_exit_batch_net(struct list_head *net_list)
+{
+ struct net *net;
+ LIST_HEAD(list);
+
+ rtnl_lock();
+ list_for_each_entry(net, net_list, exit_list)
+ vxlan_destroy_tunnels(net, &list);
+
+ unregister_netdevice_many(&list);
+ rtnl_unlock();
+}
+
static struct pernet_operations vxlan_net_ops = {
.init = vxlan_init_net,
- .exit = vxlan_exit_net,
+ .exit_batch = vxlan_exit_batch_net,
.id = &vxlan_net_id,
.size = sizeof(struct vxlan_net),
};
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 2/2] geneve: speedup geneve tunnels dismantle
From: Haishuang Yan @ 2017-12-16 9:54 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet; +Cc: netdev, linux-kernel, Haishuang Yan
In-Reply-To: <1513418090-8595-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
Since we now hold RTNL lock in geneve_exit_net, it's better batch them
to speedup geneve tunnel dismantle.
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
drivers/net/geneve.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index b718a02..667c44f 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1638,19 +1638,16 @@ static __net_init int geneve_init_net(struct net *net)
return 0;
}
-static void __net_exit geneve_exit_net(struct net *net)
+static void geneve_destroy_tunnels(struct net *net, struct list_head *head)
{
struct geneve_net *gn = net_generic(net, geneve_net_id);
struct geneve_dev *geneve, *next;
struct net_device *dev, *aux;
- LIST_HEAD(list);
-
- rtnl_lock();
/* gather any geneve devices that were moved into this ns */
for_each_netdev_safe(net, dev, aux)
if (dev->rtnl_link_ops == &geneve_link_ops)
- unregister_netdevice_queue(dev, &list);
+ unregister_netdevice_queue(dev, head);
/* now gather any other geneve devices that were created in this ns */
list_for_each_entry_safe(geneve, next, &gn->geneve_list, next) {
@@ -1658,18 +1655,29 @@ static void __net_exit geneve_exit_net(struct net *net)
* to the list by the previous loop.
*/
if (!net_eq(dev_net(geneve->dev), net))
- unregister_netdevice_queue(geneve->dev, &list);
+ unregister_netdevice_queue(geneve->dev, head);
}
+ WARN_ON_ONCE(!list_empty(&gn->sock_list));
+}
+
+static void __net_exit geneve_exit_batch_net(struct list_head *net_list)
+{
+ struct net *net;
+ LIST_HEAD(list);
+
+ rtnl_lock();
+ list_for_each_entry(net, net_list, exit_list)
+ geneve_destroy_tunnels(net, &list);
+
/* unregister the devices gathered above */
unregister_netdevice_many(&list);
rtnl_unlock();
- WARN_ON_ONCE(!list_empty(&gn->sock_list));
}
static struct pernet_operations geneve_net_ops = {
.init = geneve_init_net,
- .exit = geneve_exit_net,
+ .exit_batch = geneve_exit_batch_net,
.id = &geneve_net_id,
.size = sizeof(struct geneve_net),
};
--
1.8.3.1
^ permalink raw reply related
* Re: BUG: unable to handle kernel NULL pointer dereference in fdb_find_rcu
From: Nikolay Aleksandrov @ 2017-12-16 10:40 UTC (permalink / raw)
To: Andrei Vagin, Linux Kernel Network Developers, LKML
Cc: David S. Miller, Toshiaki Makita, Stephen Hemminger, roopa
In-Reply-To: <8791c6f9-e69d-2269-f840-d8e05b0b44da@cumulusnetworks.com>
On 16/12/17 11:29, Nikolay Aleksandrov wrote:
> On 16/12/17 11:17, Nikolay Aleksandrov wrote:
>> On 16/12/17 02:37, Andrei Vagin wrote:
>>> Hi,
>>>
>>> We run criu tests for linux-next and today we get this bug:
>>>
>>> The kernel version is 4.15.0-rc3-next-20171215
>>>
>>> [ 235.397328] BUG: unable to handle kernel NULL pointer dereference
>>> at 000000000000000c
>>> [ 235.398624] IP: fdb_find_rcu+0x3c/0x130
>> [snip]
>>
>> Hi,
>> Thanks for the report, I've missed the changelink before dev creation case when I did
>
> err, s/changelink/br_stp_change_bridge_id/
> the other options are set after register_netdevice, this is the only one changed before
>
>> the rhashtable conversion, some of the options do fdb lookups as part of their routine
>> but we don't have the table initialized yet at that point.
>> I'll send a fix after some testing.
>>
>> Thanks,
>> Nik
>>
>>
>
We need to fix this in -net, it has a memory leak that has existed since the
introduction of br_stp_change_bridge_id() before register_netdevice because
it adds an fdb entry which never gets deleted if an error happens, also the
notifications for that fdb entry come with ifindex = 0 because the bridge netdev
doesn't exist yet. All of that looks wrong, I'll send a fix for -net to move
the bridge id change after the netdev register and cleanup any bridge fdbs
on error.
The commit with that change is:
30313a3d5794 ("bridge: Handle IFLA_ADDRESS correctly when creating bridge device")
Before the changelink in while doing newlink in bridge was possible, this would happen
only on netdev register fail, but now it is much easier to trigger (as below) since
changelink can fail if called with wrong arguments.
Here's the trace of rmmod bridge after a failed bridge newlink with mac address set
(this kernel is before my rhashtable change):
$ ip l add br0 address 00:11:22:33:44:55 type bridge group_fwd_mask 1
RTNETLINK answers: Invalid argument
$ rmmod bridge
[ 1822.142525] =============================================================================
[ 1822.143640] BUG bridge_fdb_cache (Tainted: G O ): Objects remaining in bridge_fdb_cache on __kmem_cache_shutdown()
[ 1822.144821] -----------------------------------------------------------------------------
[ 1822.145990] Disabling lock debugging due to kernel taint
[ 1822.146732] INFO: Slab 0x0000000092a844b2 objects=32 used=2 fp=0x00000000fef011b0 flags=0x1ffff8000000100
[ 1822.147700] CPU: 2 PID: 13584 Comm: rmmod Tainted: G B O 4.15.0-rc2+ #87
[ 1822.148578] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[ 1822.150008] Call Trace:
[ 1822.150510] dump_stack+0x78/0xa9
[ 1822.151156] slab_err+0xb1/0xd3
[ 1822.151834] ? __kmalloc+0x1bb/0x1ce
[ 1822.152546] __kmem_cache_shutdown+0x151/0x28b
[ 1822.153395] shutdown_cache+0x13/0x144
[ 1822.154126] kmem_cache_destroy+0x1c0/0x1fb
[ 1822.154669] SyS_delete_module+0x194/0x244
[ 1822.155199] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 1822.155773] entry_SYSCALL_64_fastpath+0x23/0x9a
[ 1822.156343] RIP: 0033:0x7f929bd38b17
[ 1822.156859] RSP: 002b:00007ffd160e9a98 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
[ 1822.157728] RAX: ffffffffffffffda RBX: 00005578316ba090 RCX: 00007f929bd38b17
[ 1822.158422] RDX: 00007f929bd9ec60 RSI: 0000000000000800 RDI: 00005578316ba0f0
[ 1822.159114] RBP: 0000000000000003 R08: 00007f929bff5f20 R09: 00007ffd160e8a11
[ 1822.159808] R10: 00007ffd160e9860 R11: 0000000000000202 R12: 00007ffd160e8a80
[ 1822.160513] R13: 0000000000000000 R14: 0000000000000000 R15: 00005578316ba090
[ 1822.161278] INFO: Object 0x000000007645de29 @offset=0
[ 1822.161666] INFO: Object 0x00000000d5df2ab5 @offset=128
^ permalink raw reply
* Re: BUG: unable to handle kernel NULL pointer dereference in fdb_find_rcu
From: Nikolay Aleksandrov @ 2017-12-16 11:22 UTC (permalink / raw)
To: Andrei Vagin, Linux Kernel Network Developers, LKML
Cc: David S. Miller, Toshiaki Makita, Stephen Hemminger, roopa
In-Reply-To: <70905249-4854-726f-a3fb-258d25d2c1de@cumulusnetworks.com>
On 16/12/17 12:40, Nikolay Aleksandrov wrote:
> On 16/12/17 11:29, Nikolay Aleksandrov wrote:
>> On 16/12/17 11:17, Nikolay Aleksandrov wrote:
>>> On 16/12/17 02:37, Andrei Vagin wrote:
>>>> Hi,
>>>>
>>>> We run criu tests for linux-next and today we get this bug:
>>>>
>>>> The kernel version is 4.15.0-rc3-next-20171215
>>>>
>>>> [ 235.397328] BUG: unable to handle kernel NULL pointer dereference
>>>> at 000000000000000c
>>>> [ 235.398624] IP: fdb_find_rcu+0x3c/0x130
>>> [snip]
>>>
>>> Hi,
>>> Thanks for the report, I've missed the changelink before dev creation case when I did
>>
>> err, s/changelink/br_stp_change_bridge_id/
>> the other options are set after register_netdevice, this is the only one changed before
>>
>>> the rhashtable conversion, some of the options do fdb lookups as part of their routine
>>> but we don't have the table initialized yet at that point.
>>> I'll send a fix after some testing.
>>>
>>> Thanks,
>>> Nik
>>>
>>>
>>
>
> We need to fix this in -net, it has a memory leak that has existed since the
> introduction of br_stp_change_bridge_id() before register_netdevice because
> it adds an fdb entry which never gets deleted if an error happens, also the
> notifications for that fdb entry come with ifindex = 0 because the bridge netdev
> doesn't exist yet. All of that looks wrong, I'll send a fix for -net to move
> the bridge id change after the netdev register and cleanup any bridge fdbs
> on error.
>
> The commit with that change is:
> 30313a3d5794 ("bridge: Handle IFLA_ADDRESS correctly when creating bridge device")
> Before the changelink in while doing newlink in bridge was possible, this would happen
> only on netdev register fail, but now it is much easier to trigger (as below) since
> changelink can fail if called with wrong arguments.
>
One more thing before sending the patch, the actual commit that introduced the fdb
insert in br_stp_change_bridge_id() is:
commit a4b816d8ba1c
Author: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Fri Feb 7 16:48:21 2014 +0900
bridge: Change local fdb entries whenever mac address of bridge device changes
So that is what we need to fix.
^ permalink raw reply
* [PATCH net] net: bridge: fix early call to br_stp_change_bridge_id
From: Nikolay Aleksandrov @ 2017-12-16 11:31 UTC (permalink / raw)
To: netdev
Cc: roopa, davem, makita.toshiaki, stephen, avagin, bridge,
Nikolay Aleksandrov
In-Reply-To: <73a472e3-e521-1930-0643-a6f2714db6ce@cumulusnetworks.com>
The early call to br_stp_change_bridge_id in bridge's newlink can cause
a memory leak if an error occurs during the newlink because the fdb
entries are not cleaned up if a different lladdr was specified, also
another minor issue is that it generates fdb notifications with
ifindex = 0. To remove this special case the call is done after netdev
register and we cleanup any bridge fdb entries on changelink error.
That also doesn't slow down normal bridge removal, alternative is to call
it in its ndo_uninit.
To reproduce the issue:
$ ip l add br0 address 00:11:22:33:44:55 type bridge group_fwd_mask 1
RTNETLINK answers: Invalid argument
$ rmmod bridge
[ 1822.142525] =============================================================================
[ 1822.143640] BUG bridge_fdb_cache (Tainted: G O ): Objects remaining in bridge_fdb_cache on __kmem_cache_shutdown()
[ 1822.144821] -----------------------------------------------------------------------------
[ 1822.145990] Disabling lock debugging due to kernel taint
[ 1822.146732] INFO: Slab 0x0000000092a844b2 objects=32 used=2 fp=0x00000000fef011b0 flags=0x1ffff8000000100
[ 1822.147700] CPU: 2 PID: 13584 Comm: rmmod Tainted: G B O 4.15.0-rc2+ #87
[ 1822.148578] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[ 1822.150008] Call Trace:
[ 1822.150510] dump_stack+0x78/0xa9
[ 1822.151156] slab_err+0xb1/0xd3
[ 1822.151834] ? __kmalloc+0x1bb/0x1ce
[ 1822.152546] __kmem_cache_shutdown+0x151/0x28b
[ 1822.153395] shutdown_cache+0x13/0x144
[ 1822.154126] kmem_cache_destroy+0x1c0/0x1fb
[ 1822.154669] SyS_delete_module+0x194/0x244
[ 1822.155199] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 1822.155773] entry_SYSCALL_64_fastpath+0x23/0x9a
[ 1822.156343] RIP: 0033:0x7f929bd38b17
[ 1822.156859] RSP: 002b:00007ffd160e9a98 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
[ 1822.157728] RAX: ffffffffffffffda RBX: 00005578316ba090 RCX: 00007f929bd38b17
[ 1822.158422] RDX: 00007f929bd9ec60 RSI: 0000000000000800 RDI: 00005578316ba0f0
[ 1822.159114] RBP: 0000000000000003 R08: 00007f929bff5f20 R09: 00007ffd160e8a11
[ 1822.159808] R10: 00007ffd160e9860 R11: 0000000000000202 R12: 00007ffd160e8a80
[ 1822.160513] R13: 0000000000000000 R14: 0000000000000000 R15: 00005578316ba090
[ 1822.161278] INFO: Object 0x000000007645de29 @offset=0
[ 1822.161666] INFO: Object 0x00000000d5df2ab5 @offset=128
Fixes: a4b816d8ba1c ("bridge: Change local fdb entries whenever mac address of bridge device changes")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
Consequently this also would fix the null ptr deref due to the rhashtable
not being initialized in net-next when br_stp_change_bridge_id is called.
Toshiaki, any reason you called br_stp_change_bridge_id before
register_netdevice when you introduced it in 30313a3d5794 ?
net/bridge/br_netlink.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index d0ef0a8e8831..b0362cadb7c8 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1262,19 +1262,23 @@ static int br_dev_newlink(struct net *src_net, struct net_device *dev,
struct net_bridge *br = netdev_priv(dev);
int err;
+ err = register_netdevice(dev);
+ if (err)
+ return err;
+
if (tb[IFLA_ADDRESS]) {
spin_lock_bh(&br->lock);
br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS]));
spin_unlock_bh(&br->lock);
}
- err = register_netdevice(dev);
- if (err)
- return err;
-
err = br_changelink(dev, tb, data, extack);
- if (err)
+ if (err) {
+ /* clean possible fdbs from br_stp_change_bridge_id above */
+ br_fdb_delete_by_port(br, NULL, 0, 1);
unregister_netdevice(dev);
+ }
+
return err;
}
--
2.1.4
^ permalink raw reply related
* [PATCH] ip: add vxcan/veth to ip-link man page
From: Oliver Hartkopp @ 2017-12-16 11:38 UTC (permalink / raw)
To: linux-can, netdev, Stephen Hemminger; +Cc: Oliver Hartkopp
veth and vxcan both create a vitual tunnel between a pair of virtual network
devices. This patch adds the content for the now supported vxcan netdevices
and the documentation to create peer devices for vxcan and veth.
Additional remove 'can' that accidently was on the list of link types which
can be created by 'ip link add' as 'can' devices are real network devices.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
man/man8/ip-link.8.in | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index a6a10e57..94ecbece 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -194,6 +194,7 @@ ip-link \- network device configuration
.BR macvlan " | "
.BR macvtap " | "
.BR vcan " | "
+.BR vxcan " | "
.BR veth " | "
.BR vlan " | "
.BR vxlan " |"
@@ -252,9 +253,6 @@ Link types:
.B bond
- Bonding device
.sp
-.B can
-- Controller Area Network interface
-.sp
.B dummy
- Dummy network interface
.sp
@@ -276,6 +274,9 @@ Link types:
.B vcan
- Virtual Controller Area Network interface
.sp
+.B vxcan
+- Virtual Controller Area Network tunnel interface
+.sp
.B veth
- Virtual ethernet interface
.sp
@@ -651,6 +652,29 @@ keyword.
.in -8
+.TP
+VETH, VXCAN Type Support
+For a link of types
+.I VETH/VXCAN
+the following additional arguments are supported:
+
+.BI "ip link add " DEVICE
+.BR type " { " veth " | " vxcan " }"
+[
+.BR peer
+.BI "name " NAME
+]
+
+.in +8
+.sp
+.BR peer
+.BI "name " NAME
+- specifies the virtual pair device name of the
+.I VETH/VXCAN
+tunnel.
+
+.in -8
+
.TP
GRE, IPIP, SIT, ERSPAN Type Support
For a link of types
--
2.15.1
^ permalink raw reply related
* Re: Grant
From: The Mayrhofer's @ 2017-12-16 8:33 UTC (permalink / raw)
To: Recipients
My wife and I have awarded you with a donation of $ 1,000,000.00, respond with your details for claims.
Best Regards,
Friedrich And Annand Mayrhofer.
^ permalink raw reply
* Re: [PATCHv2] ipv6: ip6mr: Recalc UDP checksum before forwarding
From: Brendan McGrath @ 2017-12-16 12:19 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, marcelo.leitner, kuznet, yoshfuji, netdev,
linux-kernel
In-Reply-To: <20171215.132728.1988249158186497781.davem@davemloft.net>
The network diagram is very simple. It is just:
VM <------* MR --------> PH
Where:
MR = Multicast Router
VM = Virtual Machine (connected by a Virtual Interface using the
'virtio_net' driver)
PH = Physical Host (connected by a physical Ethernet connection)
* = The interface where the packet originates (the 'virtio net' interface)
Due to a MFC entry - the packet is forwarded from the virtio interface
to the physical interface. There is an assumption in this forwarding
process that the checksum would already be calculated.
But I have found that with 'tx checksum offloading' on - the
'virtio_net' driver does not appear to generate a checksum at all. The
assumption here is that the packet will only ever be seen internal to
the virtio network.
But this scenario sits outside both those assumptions - hence the issue.
This patch looked to address the assumption made in the forwarding
process - but I now think the issue is with the virtio assumption. Some
research on the Internet highlighted other issues with the virtio
assumption. They are:
1. Applications that look at the entire packet and validate checksum
themselves (such as some DHCP clients); and
2. Tunnels - where the packet is placed inside a tunnel as is and
delivered elsewhere
And of course this scenario.
This archived libvirt-users post actually gave me a couple of ideas to try:
https://www.redhat.com/archives/libvirt-users/2016-March/msg00034.html
When I disable tx checksum offloading on the virtio interface (via
'ethtool -K virbr0 tx off') - the checksum is calculated correctly and
everything works.
I can also get it to work by adding the following ip6filter entry:
ip6tables -t mangle -A POSTROUTING -o virbr0 -d ff00::/8 -j CHECKSUM
--checksum-fill
So I think this patch can be withdrawn in favour of one of these two
workarounds.
On 16/12/17 05:27, David Miller wrote:
> From: Brendan McGrath <redmcg@redmandi.dyndns.org>
> Date: Thu, 14 Dec 2017 22:37:03 +1100
>
>> Currently, when forwarding a multicast packet originating from a
>> Virtual Interface on a Multicast Router to one of its Physical
>> Interfaces, ip_summed is set to a value of CHECKSUM_UNNECESSARY and
>> the UDP checksum is not calculated.
>>
>> The checksum value of the forwarded packet is left as is and
>> therefore rejected by the receiving machine(s).
>>
>> This patch ensures the checksum is recalculated before forwarding.
>>
>> Signed-off-by: Brendan McGrath <redmcg@redmandi.dyndns.org>
> I still don't like this, UDP can't be the only protocol that goes
> over multicast and might therefore have this problem.
>
> Actually, I'm still also having trouble figuring out how this happens
> in the first place.
>
> Please draw a specific detailed network diagram, show the exact
> configuration of each interface and exactly what driver runs that
> interface, and show where the packet comes from, who creates it, and
> where these checksum settings are done that lead to this problem.
>
> Like Eric, I'm very suspicious and I think that there is some problem
> with whoever builds or modifies this packet, and the code you are
> touching has no business "fixing it up"
>
> Thank you.
^ permalink raw reply
* Re: [PATCH net] net: bridge: fix early call to br_stp_change_bridge_id
From: Nikolay Aleksandrov @ 2017-12-16 12:38 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, makita.toshiaki, stephen, avagin, bridge
In-Reply-To: <1513423896-30294-1-git-send-email-nikolay@cumulusnetworks.com>
On 16/12/17 13:31, Nikolay Aleksandrov wrote:
> The early call to br_stp_change_bridge_id in bridge's newlink can cause
> a memory leak if an error occurs during the newlink because the fdb
> entries are not cleaned up if a different lladdr was specified, also
> another minor issue is that it generates fdb notifications with
> ifindex = 0. To remove this special case the call is done after netdev
> register and we cleanup any bridge fdb entries on changelink error.
> That also doesn't slow down normal bridge removal, alternative is to call
> it in its ndo_uninit.
>
> To reproduce the issue:
> $ ip l add br0 address 00:11:22:33:44:55 type bridge group_fwd_mask 1
> RTNETLINK answers: Invalid argument
>
> $ rmmod bridge
> [ 1822.142525] =============================================================================
> [ 1822.143640] BUG bridge_fdb_cache (Tainted: G O ): Objects remaining in bridge_fdb_cache on __kmem_cache_shutdown()
> [ 1822.144821] -----------------------------------------------------------------------------
>
> [ 1822.145990] Disabling lock debugging due to kernel taint
> [ 1822.146732] INFO: Slab 0x0000000092a844b2 objects=32 used=2 fp=0x00000000fef011b0 flags=0x1ffff8000000100
> [ 1822.147700] CPU: 2 PID: 13584 Comm: rmmod Tainted: G B O 4.15.0-rc2+ #87
> [ 1822.148578] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
> [ 1822.150008] Call Trace:
> [ 1822.150510] dump_stack+0x78/0xa9
> [ 1822.151156] slab_err+0xb1/0xd3
> [ 1822.151834] ? __kmalloc+0x1bb/0x1ce
> [ 1822.152546] __kmem_cache_shutdown+0x151/0x28b
> [ 1822.153395] shutdown_cache+0x13/0x144
> [ 1822.154126] kmem_cache_destroy+0x1c0/0x1fb
> [ 1822.154669] SyS_delete_module+0x194/0x244
> [ 1822.155199] ? trace_hardirqs_on_thunk+0x1a/0x1c
> [ 1822.155773] entry_SYSCALL_64_fastpath+0x23/0x9a
> [ 1822.156343] RIP: 0033:0x7f929bd38b17
> [ 1822.156859] RSP: 002b:00007ffd160e9a98 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
> [ 1822.157728] RAX: ffffffffffffffda RBX: 00005578316ba090 RCX: 00007f929bd38b17
> [ 1822.158422] RDX: 00007f929bd9ec60 RSI: 0000000000000800 RDI: 00005578316ba0f0
> [ 1822.159114] RBP: 0000000000000003 R08: 00007f929bff5f20 R09: 00007ffd160e8a11
> [ 1822.159808] R10: 00007ffd160e9860 R11: 0000000000000202 R12: 00007ffd160e8a80
> [ 1822.160513] R13: 0000000000000000 R14: 0000000000000000 R15: 00005578316ba090
> [ 1822.161278] INFO: Object 0x000000007645de29 @offset=0
> [ 1822.161666] INFO: Object 0x00000000d5df2ab5 @offset=128
>
> Fixes: a4b816d8ba1c ("bridge: Change local fdb entries whenever mac address of bridge device changes")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> Consequently this also would fix the null ptr deref due to the rhashtable
> not being initialized in net-next when br_stp_change_bridge_id is called.
>
> Toshiaki, any reason you called br_stp_change_bridge_id before
> register_netdevice when you introduced it in 30313a3d5794 ?
>
> net/bridge/br_netlink.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
Oops, wrong Fixes commit id, this one was introduced in Feb/2014, should be:
Fixes: 30313a3d5794 ("bridge: Handle IFLA_ADDRESS correctly when creating bridge device")
as I said before since that was introduced in Apr/2014.
Dave, would you like me to send v2 with the proper Fixes tag ?
^ permalink raw reply
* [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program
From: Knut Omang @ 2017-12-16 14:42 UTC (permalink / raw)
To: linux-kernel
Cc: Knut Omang, Mauro Carvalho Chehab, Nicolas Palix, Jonathan Corbet,
Santosh Shilimkar, Matthew Wilcox, cocci, rds-devel, linux-rdma,
linux-doc, Doug Ledford, Mickaël Salaün, Shuah Khan,
linux-kbuild, Michal Marek, Julia Lawall, John Haxby,
Åsmund Østvold, Jason Gunthorpe, Masahiro Yamada
This patch series implements features to make it easier to run checkers on the
entire kernel as part of automatic and developer testing.
This is done by replacing the sparse specific setup for the C={1,2} variable
in the makefiles with setup for running scripts/runchecks, a new program that
can run any number of different "checkers". The behaviour of runchecks is
defined by simple "global" configuration in scripts/runchecks.cfg which can be
extended by local configuration applying to individual files, directories or
subtrees in the source.
It also fixes a minor issue with "checkpatch --fix-inplace" found during testing
(patch #3).
The runchecks.cfg files are parsed according to this minimal language:
# comments
# "Global configuration in scripts/runchecks.cfg:
checker <name>
typedef NAME regex
run <list of checkers or "all"
# "local" configuration:
line_len <n>
except checkpatch_type [files ...]
pervasive checkpatch_type1 [checkpatch_type2 ...]
With "make C=2" runchecks first parse the file scripts/runchecks.cfg, then
look for a file named 'runchecks.cfg' in the same directory as the source file.
If that file exists, it will be parsed and it's local configuration applied to
allow suppression on a per checker, per check and per file basis.
If a "local" configuration does not exist, either in the source directory or
above, make will simply silently ignore the file.
The idea is that the community can work to add runchecks.cfg files to
directories, serving both as documentation and as a way for subsystem
maintainers to enforce policies and individual tastes as well as TODOs and/or
priorities, to make it easier for newcomers to contribute in this area. By
ignoring directories/subtrees without such files, automation can start right
away as it is trivially possible to run errorless with C=2 for the entire
kernel.
For the checker maintainers this should be a benefit as well: new
or improved checks would generate new errors/warnings. With automatic testing
for the checkers, these new checks would generate error reports and cause
builds to fail. Adding the new check a 'pervasive' option at the top level or
even for specific files, marked with a "new check - please consider fixing" comment
or similar would make those builds pass while documenting and making the new check
more visible.
The patches includes a documentation file with some more details.
This patch set has evolved from an earlier shell script implementation I made
as only a wrapper script around checkpatch. That version have been used for a
number of years on a driver project I worked on where we had automatic checkin
regression testing. I extended that to also run checkpatch to avoid having to
clean up frequent unintended whitespace changes and style violations from others...
I have also tested this version on some directories I am familiar with. The
result of that work is available in two patch sets of 10 and 11 patches, but we
agreed that it would be better to post them as separate patch sets later.
Those patch sets illustrates how I picture the "flow" from just "reining in" the
checkpatch detections to actually fixing classes of checkpatch issues one by
one, while updating the checkpatch.cfg file(s) to have 0 errors or warnings at
any commit boundary.
The combined set is available here:
git://github.com/knuto/linux.git branch runchecks
I only include version 0 of runchecks.cfg in the two directories I
worked on here as the final two patches. These files both documents where
the issues are in those two directories, and can be used by the maintainer
to indicate to potential helpers what to focus on as I have tried to
illustrate by means of comments.
Changes from v1:
-----------------
Based on feedback, the implementation is completely rewritten and extended.
Instead of patches to checkpatch, and a sole checkpatch focus, it is now a
generic solution implemented in python, for any type of checkers, extendable
through some basic generic functionality, and for special needs by subclassing
the Checker class in the implementation.
This implementation fully supports checkpatch, sparse and
checkdoc == kernel-doc -none, and also has been tested with coccicheck.
To facilitate the same mechanism of using check types to filter what
checks to be suppressed, I introduced the concept of "typedefs" which allows
runchecks to effectively augment the check type space of the checker in cases
where types either are not available at all (checkdoc) or where only a few
can be filtered out (sparse)
With this in place it also became trivial to make the look and feel similar
for sparse and checkdoc as for checkpatch, with some optional color support
too, to make fixing issues in the code, the goal of this whole exercise,
much more pleasant IMHO :-)
Thanks,
Knut
Knut Omang (5):
runchecks: Generalize make C={1,2} to support multiple checkers
Documentation: Add doc for runchecks, a checker runner
checkpatch: Improve --fix-inplace for TABSTOP
rds: Add runchecks.cfg for net/rds
RDMA/core: Add runchecks.cfg for drivers/infiniband/core
Documentation/dev-tools/coccinelle.rst | 12 +-
Documentation/dev-tools/index.rst | 1 +-
Documentation/dev-tools/runchecks.rst | 215 ++++++++-
Documentation/dev-tools/sparse.rst | 30 +-
Documentation/kbuild/kbuild.txt | 9 +-
Makefile | 23 +-
drivers/infiniband/core/runchecks.cfg | 83 +++-
net/rds/runchecks.cfg | 76 +++-
scripts/Makefile.build | 4 +-
scripts/checkpatch.pl | 2 +-
scripts/runchecks | 734 ++++++++++++++++++++++++++-
scripts/runchecks.cfg | 63 ++-
scripts/runchecks_help.txt | 43 ++-
13 files changed, 1274 insertions(+), 21 deletions(-)
create mode 100644 Documentation/dev-tools/runchecks.rst
create mode 100644 drivers/infiniband/core/runchecks.cfg
create mode 100644 net/rds/runchecks.cfg
create mode 100755 scripts/runchecks
create mode 100644 scripts/runchecks.cfg
create mode 100644 scripts/runchecks_help.txt
base-commit: ae64f9bd1d3621b5e60d7363bc20afb46aede215
--
git-series 0.9.1
^ permalink raw reply
* [PATCH v2 4/5] rds: Add runchecks.cfg for net/rds
From: Knut Omang @ 2017-12-16 14:42 UTC (permalink / raw)
To: linux-kernel
Cc: Knut Omang, linux-rdma, netdev, rds-devel, Santosh Shilimkar,
David S. Miller
In-Reply-To: <cover.630ac8faeeda67bf7a778c158174422042942d08.1513430008.git-series.knut.omang@oracle.com>
Add a runchecks.cfg to net/rds to start "reining in"
future checker errors, and making it easier to
selectively clean up existing issues.
This runchecks.cfg lets make C=2 M=net/rds
pass with all errors/warnings suppressed
See Documentation/dev-tools/runchecks.rst for
motivation and details.
Signed-off-by: Knut Omang <knut.omang@oracle.com>
Reviewed-by: Håkon Bugge <haakon.bugge@oracle.com>
Reviewed-by: Åsmund Østvold <asmund.ostvold@oracle.com>
---
net/rds/runchecks.cfg | 76 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 76 insertions(+)
create mode 100644 net/rds/runchecks.cfg
diff --git a/net/rds/runchecks.cfg b/net/rds/runchecks.cfg
new file mode 100644
index 0000000..2a02701
--- /dev/null
+++ b/net/rds/runchecks.cfg
@@ -0,0 +1,76 @@
+#
+# checker suppression lists for net/rds
+#
+# see Documentation/dev-tools/runchecks.rst
+#
+
+checker checkpatch
+##################
+
+# Accept somewhat longer lines:
+line_len 110
+
+# Important to fix from a quality perspective:
+#
+except AVOID_BUG connection.c ib.c ib_cm.c ib_rdma.c ib_recv.c ib_ring.c ib_send.c info.c loop.c message.c
+except AVOID_BUG rdma.c recv.c send.c stats.c tcp_recv.c transport.c
+except MEMORY_BARRIER ib_recv.c send.c tcp_send.c
+except WAITQUEUE_ACTIVE cong.c ib_rdma.c ib_ring.c ib_send.c
+except UNNECESSARY_ELSE bind.c ib_cm.c
+except MACRO_ARG_PRECEDENCE connection.c ib.h rds.h
+except MACRO_ARG_REUSE rds.h
+except ALLOC_SIZEOF_STRUCT cong.c ib.c ib_cm.c loop.c message.c rdma.c
+except UNCOMMENTED_DEFINITION ib_cm.c
+
+# Code simplification:
+#
+except ALLOC_WITH_MULTIPLY ib.c
+except PREFER_PR_LEVEL ib_cm.c ib_recv.c ib_send.c rdma_transport.c threads.c transport.c
+except UNNECESSARY_ELSE ib_fmr.c
+except UNNECESSARY_PARENTHESES ib_rdma.c rdma.c recv.c send.c
+except PRINTK_RATELIMITED ib_frmr.c
+except EMBEDDED_FUNCTION_NAME ib_rdma.c
+
+# Style and readability:
+#
+except BRACES ib_cm.c ib_rdma.c ib_recv.c send.c transport.c
+except OOM_MESSAGE ib.c tcp.c
+except LONG_LINE_STRING ib.c ib_recv.c ib_send.c
+except FUNCTION_ARGUMENTS ib.h ib_mr.h rds.h tcp.h
+except OPEN_ENDED_LINE recv.c ib_recv.c
+
+# Candidates to leave as exceptions (don't fix):
+except MULTIPLE_ASSIGNMENTS ib_send.c
+except LONG_LINE_STRING connection.c
+except OPEN_BRACE connection.c
+
+# These are in most of the source files, ignore for all files:
+#
+pervasive NETWORKING_BLOCK_COMMENT_STYLE BLOCK_COMMENT_STYLE
+
+# These are easily autocorrected by checkpatch with --fix-inplace:
+# Just ignore here - fixed in a separate commit:
+#
+pervasive PREFER_KERNEL_TYPES LINE_SPACING SPACING SPACE_BEFORE_TAB SPLIT_STRING
+pervasive BIT_MACRO SIZEOF_PARENTHESIS LOGICAL_CONTINUATIONS GLOBAL_INITIALISERS
+pervasive ALLOC_WITH_MULTIPLY TABSTOP
+
+# These were easy to fix manually while getting make C=2 to pass.
+# Fixed in a separate commit:
+#
+pervasive SUSPECT_CODE_INDENT PARENTHESIS_ALIGNMENT BRACES PRINTF_L COMPARISON_TO_NULL
+pervasive LOGICAL_CONTINUATIONS
+
+
+checker sparse
+##############
+
+except VLA connection.c
+except COND_ADDRESS_ARRAY connection.c
+except PTR_SUBTRACTION_BLOWS ib_recv.c
+except TYPESIGN ib_frmr.c
+
+# This is coming from linux/dma-mapping.h:
+except RETURN_VOID ib_cm.c
+
+pervasive BITWISE SHADOW
--
git-series 0.9.1
^ permalink raw reply related
* Re: Grant
From: The Mayrhofer's @ 2017-12-16 7:32 UTC (permalink / raw)
To: Recipients
My wife and I have awarded you with a donation of $ 1,000,000.00, respond with your details for claims.
Best Regards,
Friedrich And Annand Mayrhofer.
^ permalink raw reply
* Re: [PATCH net-next v5 1/5] net: Introduce NETIF_F_GRO_HW.
From: Alexander Duyck @ 2017-12-16 16:38 UTC (permalink / raw)
To: Michael Chan
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <1513411784-17653-2-git-send-email-michael.chan@broadcom.com>
On Sat, Dec 16, 2017 at 12:09 AM, Michael Chan
<michael.chan@broadcom.com> wrote:
> Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
> GRO. With this flag, we can now independently turn on or off hardware
> GRO when GRO is on. Previously, drivers were using NETIF_F_GRO to
> control hardware GRO and so it cannot be independently turned on or
> off without affecting GRO.
>
> Hardware GRO (just like GRO) guarantees that packets can be re-segmented
> by TSO/GSO to reconstruct the original packet stream. Logically,
> GRO_HW should depend on GRO since it a subset, but we will let
> individual drivers enforce this dependency as they see fit.
>
> Since NETIF_F_GRO is not propagated between upper and lower devices,
> NETIF_F_GRO_HW should follow suit since it is a subset of GRO. In other
> words, a lower device can independent have GRO/GRO_HW enabled or disabled
> and no feature propagation is required. This will preserve the current
> GRO behavior. This can be changed later if we decide to propagate GRO/
> GRO_HW/RXCSUM from upper to lower devices.
>
> Cc: Ariel Elior <Ariel.Elior@cavium.com>
> Cc: everest-linux-l2@cavium.com
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
The changes look good to me. Thanks for doing all this work.
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> Documentation/networking/netdev-features.txt | 9 +++++++++
> include/linux/netdev_features.h | 3 +++
> net/core/dev.c | 12 ++++++++++++
> net/core/ethtool.c | 1 +
> 4 files changed, 25 insertions(+)
>
> diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt
> index 7413eb0..c77f9d5 100644
> --- a/Documentation/networking/netdev-features.txt
> +++ b/Documentation/networking/netdev-features.txt
> @@ -163,3 +163,12 @@ This requests that the NIC receive all possible frames, including errored
> frames (such as bad FCS, etc). This can be helpful when sniffing a link with
> bad packets on it. Some NICs may receive more packets if also put into normal
> PROMISC mode.
> +
> +* rx-gro-hw
> +
> +This requests that the NIC enables Hardware GRO (generic receive offload).
> +Hardware GRO is basically the exact reverse of TSO, and is generally
> +stricter than Hardware LRO. A packet stream merged by Hardware GRO must
> +be re-segmentable by GSO or TSO back to the exact original packet stream.
> +Hardware GRO is dependent on RXCSUM since every packet successfully merged
> +by hardware must also have the checksum verified by hardware.
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index b1b0ca7..db84c51 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -78,6 +78,8 @@ enum {
> NETIF_F_HW_ESP_TX_CSUM_BIT, /* ESP with TX checksum offload */
> NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP tunnels */
>
> + NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> +
> /*
> * Add your fresh new feature above and remember to update
> * netdev_features_strings[] in net/core/ethtool.c and maybe
> @@ -97,6 +99,7 @@ enum {
> #define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST)
> #define NETIF_F_FSO __NETIF_F(FSO)
> #define NETIF_F_GRO __NETIF_F(GRO)
> +#define NETIF_F_GRO_HW __NETIF_F(GRO_HW)
> #define NETIF_F_GSO __NETIF_F(GSO)
> #define NETIF_F_GSO_ROBUST __NETIF_F(GSO_ROBUST)
> #define NETIF_F_HIGHDMA __NETIF_F(HIGHDMA)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b0eee49..4b43f5d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7424,6 +7424,18 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
> features &= ~dev->gso_partial_features;
> }
>
> + if (!(features & NETIF_F_RXCSUM)) {
> + /* NETIF_F_GRO_HW implies doing RXCSUM since every packet
> + * successfully merged by hardware must also have the
> + * checksum verified by hardware. If the user does not
> + * want to enable RXCSUM, logically, we should disable GRO_HW.
> + */
> + if (features & NETIF_F_GRO_HW) {
> + netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since no RXCSUM feature.\n");
> + features &= ~NETIF_F_GRO_HW;
> + }
> + }
> +
> return features;
> }
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index f8fcf45..50a7920 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -73,6 +73,7 @@ int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
> [NETIF_F_LLTX_BIT] = "tx-lockless",
> [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
> [NETIF_F_GRO_BIT] = "rx-gro",
> + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> [NETIF_F_LRO_BIT] = "rx-lro",
>
> [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH v2 1/2] r8169: fix RTL8111EVL EEE and green settings
From: Heiner Kallweit @ 2017-12-16 17:11 UTC (permalink / raw)
To: Andrew Lunn; +Cc: nic_swsd, Chun-Hao Lin, David Miller, netdev@vger.kernel.org
In-Reply-To: <20171121013416.GA15896@lunn.ch>
Am 21.11.2017 um 02:34 schrieb Andrew Lunn:
> Hi Heiner
>
> Do you have access to the data sheet?
>
> I had a quick look through the driver. It would be nice to refactor it
> to follow the usual Linux conventions:
>
> Turn the MDIO read/write functions into an MDIO bus driver.
>
> Move the PHY code into drivers/net/phy/realtek.c, and in the process,
> replace all the magic numbers with #defines.
>
> Do you have any interest in doing this?
>
> Andrew
>
Hi Andrew,
I worked a little on this topic and meanwhile have an experimental
patch set for switching the driver to phylib, incl. MDIO bus driver.
It works well on a RTL8168evl (RTL_GIGA_MAC_VER_34).
Will submit this patch set as RfC in the next days.
Still open is factoring out all phy init stuff to drivers/net/phy.
There are open issues where I would appreciate advice from the
Realtek guys.
The PHY in RTL8168evl identifies as RTL8211E. Question would be
whether such an internal PHY with this id is identical to an
external PHY with the same id.
For most NIC's the driver loads firmware. The firmware can refer
to PHY and MAC as well. In case of RTL8168evl the firmware
(rtl8168e-3.fw) seems to be for the PHY only. So next question
is whether this firmware would be applicable for any RTL8211E
external PHY too.
In this case one option would be to move the firmware handling to
drivers/firmware so that it can be used from PHY drivers and from
NIC drivers as well.
Regards, Heiner
^ permalink raw reply
* Re: [PATCH v2 4/5] rds: Add runchecks.cfg for net/rds
From: Stephen Hemminger @ 2017-12-16 17:45 UTC (permalink / raw)
To: Knut Omang
Cc: linux-kernel, linux-rdma, netdev, rds-devel, Santosh Shilimkar,
David S. Miller
In-Reply-To: <4dc9b2fc0ddd1eb91d9b8785ae4886c6b08f3ee5.1513430008.git-series.knut.omang@oracle.com>
On Sat, 16 Dec 2017 15:42:29 +0100
Knut Omang <knut.omang@oracle.com> wrote:
> +
> +# Important to fix from a quality perspective:
> +#
> +except AVOID_BUG connection.c ib.c ib_cm.c ib_rdma.c ib_recv.c ib_ring.c ib_send.c info.c loop.c message.c
> +except AVOID_BUG rdma.c recv.c send.c stats.c tcp_recv.c transport.c
> +except MEMORY_BARRIER ib_recv.c send.c tcp_send.c
> +except WAITQUEUE_ACTIVE cong.c ib_rdma.c ib_ring.c ib_send.c
> +except UNNECESSARY_ELSE bind.c ib_cm.c
> +except MACRO_ARG_PRECEDENCE connection.c ib.h rds.h
> +except MACRO_ARG_REUSE rds.h
> +except ALLOC_SIZEOF_STRUCT cong.c ib.c ib_cm.c loop.c message.c rdma.c
> +except UNCOMMENTED_DEFINITION ib_cm.c
> +
> +# Code simplification:
> +#
> +except ALLOC_WITH_MULTIPLY ib.c
> +except PREFER_PR_LEVEL ib_cm.c ib_recv.c ib_send.c rdma_transport.c threads.c transport.c
> +except UNNECESSARY_ELSE ib_fmr.c
> +except UNNECESSARY_PARENTHESES ib_rdma.c rdma.c recv.c send.c
> +except PRINTK_RATELIMITED ib_frmr.c
> +except EMBEDDED_FUNCTION_NAME ib_rdma.c
> +
> +# Style and readability:
> +#
> +except BRACES ib_cm.c ib_rdma.c ib_recv.c send.c transport.c
> +except OOM_MESSAGE ib.c tcp.c
> +except LONG_LINE_STRING ib.c ib_recv.c ib_send.c
> +except FUNCTION_ARGUMENTS ib.h ib_mr.h rds.h tcp.h
> +except OPEN_ENDED_LINE recv.c ib_recv.c
> +
> +# Candidates to leave as exceptions (don't fix):
> +except MULTIPLE_ASSIGNMENTS ib_send.c
> +except LONG_LINE_STRING connection.c
> +except OPEN_BRACE connection.c
> +
Why start letting subsystems have a free-pass?
Also this would mean that new patches to IB would continue the bad habits.
^ permalink raw reply
* Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program
From: Stephen Hemminger @ 2017-12-16 17:47 UTC (permalink / raw)
To: Knut Omang
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mauro Carvalho Chehab,
Nicolas Palix, Jonathan Corbet, Santosh Shilimkar, Matthew Wilcox,
cocci-/FJkirnvOdkvYVN+rsErww, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, Doug Ledford,
Mickaël Salaün, Shuah Khan,
linux-kbuild-u79uwXL29TY76Z2rM5mHXA, Michal Marek, Julia Lawall,
John Haxby, Åsmund Østvold, Jason Gunthorpe,
Masahiro Yamada
In-Reply-To: <cover.630ac8faeeda67bf7a778c158174422042942d08.1513430008.git-series.knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Sat, 16 Dec 2017 15:42:25 +0100
Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> This patch series implements features to make it easier to run checkers on the
> entire kernel as part of automatic and developer testing.
>
> This is done by replacing the sparse specific setup for the C={1,2} variable
> in the makefiles with setup for running scripts/runchecks, a new program that
> can run any number of different "checkers". The behaviour of runchecks is
> defined by simple "global" configuration in scripts/runchecks.cfg which can be
> extended by local configuration applying to individual files, directories or
> subtrees in the source.
>
> It also fixes a minor issue with "checkpatch --fix-inplace" found during testing
> (patch #3).
>
> The runchecks.cfg files are parsed according to this minimal language:
>
> # comments
> # "Global configuration in scripts/runchecks.cfg:
> checker <name>
> typedef NAME regex
> run <list of checkers or "all"
>
> # "local" configuration:
> line_len <n>
> except checkpatch_type [files ...]
> pervasive checkpatch_type1 [checkpatch_type2 ...]
>
> With "make C=2" runchecks first parse the file scripts/runchecks.cfg, then
> look for a file named 'runchecks.cfg' in the same directory as the source file.
> If that file exists, it will be parsed and it's local configuration applied to
> allow suppression on a per checker, per check and per file basis.
> If a "local" configuration does not exist, either in the source directory or
> above, make will simply silently ignore the file.
>
> The idea is that the community can work to add runchecks.cfg files to
> directories, serving both as documentation and as a way for subsystem
> maintainers to enforce policies and individual tastes as well as TODOs and/or
> priorities, to make it easier for newcomers to contribute in this area. By
> ignoring directories/subtrees without such files, automation can start right
> away as it is trivially possible to run errorless with C=2 for the entire
> kernel.
>
> For the checker maintainers this should be a benefit as well: new
> or improved checks would generate new errors/warnings. With automatic testing
> for the checkers, these new checks would generate error reports and cause
> builds to fail. Adding the new check a 'pervasive' option at the top level or
> even for specific files, marked with a "new check - please consider fixing" comment
> or similar would make those builds pass while documenting and making the new check
> more visible.
>
> The patches includes a documentation file with some more details.
>
> This patch set has evolved from an earlier shell script implementation I made
> as only a wrapper script around checkpatch. That version have been used for a
> number of years on a driver project I worked on where we had automatic checkin
> regression testing. I extended that to also run checkpatch to avoid having to
> clean up frequent unintended whitespace changes and style violations from others...
>
> I have also tested this version on some directories I am familiar with. The
> result of that work is available in two patch sets of 10 and 11 patches, but we
> agreed that it would be better to post them as separate patch sets later.
>
> Those patch sets illustrates how I picture the "flow" from just "reining in" the
> checkpatch detections to actually fixing classes of checkpatch issues one by
> one, while updating the checkpatch.cfg file(s) to have 0 errors or warnings at
> any commit boundary.
>
> The combined set is available here:
>
> git://github.com/knuto/linux.git branch runchecks
>
> I only include version 0 of runchecks.cfg in the two directories I
> worked on here as the final two patches. These files both documents where
> the issues are in those two directories, and can be used by the maintainer
> to indicate to potential helpers what to focus on as I have tried to
> illustrate by means of comments.
>
> Changes from v1:
> -----------------
> Based on feedback, the implementation is completely rewritten and extended.
> Instead of patches to checkpatch, and a sole checkpatch focus, it is now a
> generic solution implemented in python, for any type of checkers, extendable
> through some basic generic functionality, and for special needs by subclassing
> the Checker class in the implementation.
>
> This implementation fully supports checkpatch, sparse and
> checkdoc == kernel-doc -none, and also has been tested with coccicheck.
> To facilitate the same mechanism of using check types to filter what
> checks to be suppressed, I introduced the concept of "typedefs" which allows
> runchecks to effectively augment the check type space of the checker in cases
> where types either are not available at all (checkdoc) or where only a few
> can be filtered out (sparse)
>
> With this in place it also became trivial to make the look and feel similar
> for sparse and checkdoc as for checkpatch, with some optional color support
> too, to make fixing issues in the code, the goal of this whole exercise,
> much more pleasant IMHO :-)
>
> Thanks,
> Knut
>
> Knut Omang (5):
> runchecks: Generalize make C={1,2} to support multiple checkers
> Documentation: Add doc for runchecks, a checker runner
> checkpatch: Improve --fix-inplace for TABSTOP
> rds: Add runchecks.cfg for net/rds
> RDMA/core: Add runchecks.cfg for drivers/infiniband/core
>
> Documentation/dev-tools/coccinelle.rst | 12 +-
> Documentation/dev-tools/index.rst | 1 +-
> Documentation/dev-tools/runchecks.rst | 215 ++++++++-
> Documentation/dev-tools/sparse.rst | 30 +-
> Documentation/kbuild/kbuild.txt | 9 +-
> Makefile | 23 +-
> drivers/infiniband/core/runchecks.cfg | 83 +++-
> net/rds/runchecks.cfg | 76 +++-
> scripts/Makefile.build | 4 +-
> scripts/checkpatch.pl | 2 +-
> scripts/runchecks | 734 ++++++++++++++++++++++++++-
> scripts/runchecks.cfg | 63 ++-
> scripts/runchecks_help.txt | 43 ++-
> 13 files changed, 1274 insertions(+), 21 deletions(-)
> create mode 100644 Documentation/dev-tools/runchecks.rst
> create mode 100644 drivers/infiniband/core/runchecks.cfg
> create mode 100644 net/rds/runchecks.cfg
> create mode 100755 scripts/runchecks
> create mode 100644 scripts/runchecks.cfg
> create mode 100644 scripts/runchecks_help.txt
>
> base-commit: ae64f9bd1d3621b5e60d7363bc20afb46aede215
I like the ability to add more checkers and keep then in the main
upstream tree. But adding overrides for specific subsystems goes against
the policy that all subsystems should be treated equally.
There was discussion at Kernel Summit about how the different
subsystems already have different rules. This appears to be a
way to make that worse.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program
From: Joe Perches @ 2017-12-16 18:02 UTC (permalink / raw)
To: Stephen Hemminger, Knut Omang
Cc: linux-kernel, Mauro Carvalho Chehab, Nicolas Palix,
Jonathan Corbet, Santosh Shilimkar, Matthew Wilcox, cocci,
rds-devel, linux-rdma, linux-doc, Doug Ledford,
Mickaël Salaün, Shuah Khan, linux-kbuild, Michal Marek,
Julia Lawall, John Haxby, Åsmund Østvold,
Jason Gunthorpe, Masahiro Yamada
In-Reply-To: <20171216094745.5e41ac51@xeon-e3>
On Sat, 2017-12-16 at 09:47 -0800, Stephen Hemminger wrote:
> On Sat, 16 Dec 2017 15:42:25 +0100
> Knut Omang <knut.omang@oracle.com> wrote:
>
> > This patch series implements features to make it easier to run checkers on the
> > entire kernel as part of automatic and developer testing.
> >
> > This is done by replacing the sparse specific setup for the C={1,2} variable
> > in the makefiles with setup for running scripts/runchecks, a new program that
> > can run any number of different "checkers". The behaviour of runchecks is
> > defined by simple "global" configuration in scripts/runchecks.cfg which can be
> > extended by local configuration applying to individual files, directories or
> > subtrees in the source.
[]
> I like the ability to add more checkers and keep then in the main
> upstream tree. But adding overrides for specific subsystems goes against
> the policy that all subsystems should be treated equally.
>
> There was discussion at Kernel Summit about how the different
> subsystems already have different rules. This appears to be a
> way to make that worse.
I think that's OK and somewhat reasonable.
What is perhaps unreasonable is requiring subsystems with
a local specific style to change to some universal style.
see comments like:
https://lkml.org/lkml/2017/12/11/689
^ permalink raw reply
* Re: [PATCH v3 iproute2 1/1] ss: add missing path MTU parameter
From: Stephen Hemminger @ 2017-12-16 18:03 UTC (permalink / raw)
To: Roman Mashak; +Cc: netdev, jhs
In-Reply-To: <1513348062-15968-1-git-send-email-mrv@mojatatu.com>
On Fri, 15 Dec 2017 09:27:42 -0500
Roman Mashak <mrv@mojatatu.com> wrote:
> v3:
> Rebase and use out() instead of printf().
> v2:
> Print the path MTU immediately after the MSS, as it is easier to parse
> for humans (suggested by Neal Cardwell).
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
Applied, thanks for following up
^ permalink raw reply
* Re: [PATCH v2 1/2] r8169: fix RTL8111EVL EEE and green settings
From: Andrew Lunn @ 2017-12-16 18:03 UTC (permalink / raw)
To: Heiner Kallweit
Cc: nic_swsd, Chun-Hao Lin, David Miller, netdev@vger.kernel.org
In-Reply-To: <4be4ff8c-ab74-419c-43b3-7992e8070f70@gmail.com>
On Sat, Dec 16, 2017 at 06:11:01PM +0100, Heiner Kallweit wrote:
> Am 21.11.2017 um 02:34 schrieb Andrew Lunn:
> > Hi Heiner
> >
> > Do you have access to the data sheet?
> >
> > I had a quick look through the driver. It would be nice to refactor it
> > to follow the usual Linux conventions:
> >
> > Turn the MDIO read/write functions into an MDIO bus driver.
> >
> > Move the PHY code into drivers/net/phy/realtek.c, and in the process,
> > replace all the magic numbers with #defines.
> >
> > Do you have any interest in doing this?
> >
> > Andrew
> >
> Hi Andrew,
>
> I worked a little on this topic and meanwhile have an experimental
> patch set for switching the driver to phylib, incl. MDIO bus driver.
> It works well on a RTL8168evl (RTL_GIGA_MAC_VER_34).
> Will submit this patch set as RfC in the next days.
>
> Still open is factoring out all phy init stuff to drivers/net/phy.
> There are open issues where I would appreciate advice from the
> Realtek guys.
Hi Heiner
> The PHY in RTL8168evl identifies as RTL8211E. Question would be
> whether such an internal PHY with this id is identical to an
> external PHY with the same id.
I cannot make a 100% reliable recommendation, but i would say it is
likely the internal and the external PHY are compatible. I've seen
similar situations with Marvell Ethernet switches with Internal PHYs,
which use the same ID as discreet PHYs, and the same driver has
worked.
> In this case one option would be to move the firmware handling to
> drivers/firmware so that it can be used from PHY drivers and from
> NIC drivers as well.
If the core is the same for MAC and PHY, then moving it somewhere it
can be shared would be good.
Andrew
^ permalink raw reply
* Re: [PATCH] ip: add vxcan/veth to ip-link man page
From: Stephen Hemminger @ 2017-12-16 18:08 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, netdev
In-Reply-To: <20171216113857.2397-1-socketcan@hartkopp.net>
On Sat, 16 Dec 2017 12:38:57 +0100
Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> veth and vxcan both create a vitual tunnel between a pair of virtual network
> devices. This patch adds the content for the now supported vxcan netdevices
> and the documentation to create peer devices for vxcan and veth.
>
> Additional remove 'can' that accidently was on the list of link types which
> can be created by 'ip link add' as 'can' devices are real network devices.
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Applied. Thanks for the doing this.
^ permalink raw reply
* Re: [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling
From: Stephen Hemminger @ 2017-12-16 18:10 UTC (permalink / raw)
To: Serhey Popovych; +Cc: netdev
In-Reply-To: <1513193762-1580-1-git-send-email-serhe.popovych@gmail.com>
On Wed, 13 Dec 2017 21:35:59 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:
> In this series following problems addressed:
>
> 1) Size of IFLA_GRE_LINK attribute is 32 bits long in , not 8 bit.
>
> 2) Use get_addr() instead of get_prefix() to parse local/remote
> tunnel endpoints as IPADDR, not PREFIX as per ip-link(8).
>
> 3) No need to check if local/remote endpoints are zero (e.g. INADDR_ANY):
> it is fully legal value, accepted by the kernel.
>
> See individual patch description message for details.
>
> Thanks,
> Serhii
>
> Serhey Popovych (3):
> ip/tunnel: Unify setup and accept zero address for local/remote
> endpoints
> ip/tunnel: Use get_addr() instead of get_prefix() for local/remote
> endpoints
> ip: gre: fix IFLA_GRE_LINK attribute sizing
>
> ip/ip6tunnel.c | 8 ++------
> ip/iptunnel.c | 10 ++--------
> ip/link_gre.c | 8 +++-----
> ip/link_gre6.c | 8 ++------
> ip/link_ip6tnl.c | 12 ++++--------
> ip/link_iptnl.c | 10 ++--------
> ip/link_vti.c | 14 ++------------
> ip/link_vti6.c | 26 ++++++++------------------
> 8 files changed, 25 insertions(+), 71 deletions(-)
>
Looks good, applied
^ permalink raw reply
* Re: [patch iproute2] tc: implement filter block sharing to ingress and clsact qdiscs
From: Stephen Hemminger @ 2017-12-16 18:12 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel
In-Reply-To: <20171213151357.29822-1-jiri@resnulli.us>
On Wed, 13 Dec 2017 16:13:57 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
This needs to wait until block sharing makes it into net-next upstream.
^ permalink raw reply
* Re: [PATCH v2 4/5] rds: Add runchecks.cfg for net/rds
From: Joe Perches @ 2017-12-16 18:24 UTC (permalink / raw)
To: Stephen Hemminger, Knut Omang
Cc: linux-kernel, linux-rdma, netdev, rds-devel, Santosh Shilimkar
In-Reply-To: <20171216094525.5e9c985c@xeon-e3>
On Sat, 2017-12-16 at 09:45 -0800, Stephen Hemminger wrote:
> On Sat, 16 Dec 2017 15:42:29 +0100 Knut Omang <knut.omang@oracle.com> wrote:
> > +# Code simplification:
> > +#
> > +except ALLOC_WITH_MULTIPLY ib.c
> > +except PREFER_PR_LEVEL ib_cm.c ib_recv.c ib_send.c rdma_transport.c threads.c transport.c
> > +except UNNECESSARY_ELSE ib_fmr.c
> > +except UNNECESSARY_PARENTHESES ib_rdma.c rdma.c recv.c send.c
> > +except PRINTK_RATELIMITED ib_frmr.c
> > +except EMBEDDED_FUNCTION_NAME ib_rdma.c
> > +
> > +# Style and readability:
> > +#
> > +except BRACES ib_cm.c ib_rdma.c ib_recv.c send.c transport.c
> > +except OOM_MESSAGE ib.c tcp.c
> > +except LONG_LINE_STRING ib.c ib_recv.c ib_send.c
> > +except FUNCTION_ARGUMENTS ib.h ib_mr.h rds.h tcp.h
> > +except OPEN_ENDED_LINE recv.c ib_recv.c
> > +
> > +# Candidates to leave as exceptions (don't fix):
> > +except MULTIPLE_ASSIGNMENTS ib_send.c
> > +except LONG_LINE_STRING connection.c
> > +except OPEN_BRACE connection.c
> > +
>
> Why start letting subsystems have a free-pass?
> Also this would mean that new patches to IB would continue the bad habits.
I agree with this comment at least for net/rds.
Most of these existing messages from checkpatch should
probably be inspected and corrected where possible to
minimize the style differences between this subsystem
and the rest of the kernel.
For instance, here's a trivial patch to substitute
pr_<level> for printks and a couple braces next to
these substitutions.
btw:
in ib_cm, why is one call to ib_modify_qp emitted
with a -ret and the other with a positive err?
---
net/rds/ib_cm.c | 21 ++++++++++-----------
net/rds/ib_recv.c | 5 ++---
net/rds/ib_send.c | 23 ++++++++++++-----------
net/rds/rdma_transport.c | 14 +++++++-------
net/rds/send.c | 8 ++++----
net/rds/tcp_send.c | 4 +---
net/rds/threads.c | 6 ++----
net/rds/transport.c | 12 ++++++------
8 files changed, 44 insertions(+), 49 deletions(-)
diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 80fb6f63e768..92694c9cb7c9 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -86,7 +86,7 @@ rds_ib_tune_rnr(struct rds_ib_connection *ic, struct ib_qp_attr *attr)
attr->min_rnr_timer = IB_RNR_TIMER_000_32;
ret = ib_modify_qp(ic->i_cm_id->qp, attr, IB_QP_MIN_RNR_TIMER);
if (ret)
- printk(KERN_NOTICE "ib_modify_qp(IB_QP_MIN_RNR_TIMER): err=%d\n", -ret);
+ pr_notice("ib_modify_qp(IB_QP_MIN_RNR_TIMER): err=%d\n", -ret);
}
/*
@@ -146,13 +146,12 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
qp_attr.qp_state = IB_QPS_RTS;
err = ib_modify_qp(ic->i_cm_id->qp, &qp_attr, IB_QP_STATE);
if (err)
- printk(KERN_NOTICE "ib_modify_qp(IB_QP_STATE, RTS): err=%d\n", err);
+ pr_notice("ib_modify_qp(IB_QP_STATE, RTS): err=%d\n", err);
/* update ib_device with this local ipaddr */
err = rds_ib_update_ipaddr(ic->rds_ibdev, conn->c_laddr);
if (err)
- printk(KERN_ERR "rds_ib_update_ipaddr failed (%d)\n",
- err);
+ pr_err("rds_ib_update_ipaddr failed (%d)\n", err);
/* If the peer gave us the last packet it saw, process this as if
* we had received a regular ACK. */
@@ -594,8 +593,7 @@ static u32 rds_ib_protocol_compatible(struct rdma_cm_event *event)
/* Be paranoid. RDS always has privdata */
if (!event->param.conn.private_data_len) {
- printk(KERN_NOTICE "RDS incoming connection has no private data, "
- "rejecting\n");
+ pr_notice("RDS incoming connection has no private data, rejecting\n");
return 0;
}
@@ -609,11 +607,12 @@ static u32 rds_ib_protocol_compatible(struct rdma_cm_event *event)
version = RDS_PROTOCOL_3_0;
while ((common >>= 1) != 0)
version++;
- } else
- printk_ratelimited(KERN_NOTICE "RDS: Connection from %pI4 using incompatible protocol version %u.%u\n",
- &dp->dp_saddr,
- dp->dp_protocol_major,
- dp->dp_protocol_minor);
+ } else {
+ pr_notice_ratelimited("RDS: Connection from %pI4 using incompatible protocol version %u.%u\n",
+ &dp->dp_saddr,
+ dp->dp_protocol_major,
+ dp->dp_protocol_minor);
+ }
return version;
}
diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index b4e421aa9727..9dfc8233c488 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -105,7 +105,7 @@ static int rds_ib_recv_alloc_cache(struct rds_ib_refill_cache *cache)
cache->percpu = alloc_percpu(struct rds_ib_cache_head);
if (!cache->percpu)
- return -ENOMEM;
+ return -ENOMEM;
for_each_possible_cpu(cpu) {
head = per_cpu_ptr(cache->percpu, cpu);
@@ -399,8 +399,7 @@ void rds_ib_recv_refill(struct rds_connection *conn, int prefill, gfp_t gfp)
while ((prefill || rds_conn_up(conn)) &&
rds_ib_ring_alloc(&ic->i_recv_ring, 1, &pos)) {
if (pos >= ic->i_recv_ring.w_nr) {
- printk(KERN_NOTICE "Argh - ring alloc returned pos=%u\n",
- pos);
+ pr_notice("Argh - ring alloc returned pos=%u\n", pos);
break;
}
diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
index 8557a1cae041..cb1ce8d06582 100644
--- a/net/rds/ib_send.c
+++ b/net/rds/ib_send.c
@@ -180,9 +180,8 @@ static struct rds_message *rds_ib_send_unmap_op(struct rds_ib_connection *ic,
}
break;
default:
- printk_ratelimited(KERN_NOTICE
- "RDS/IB: %s: unexpected opcode 0x%x in WR!\n",
- __func__, send->s_wr.opcode);
+ pr_notice_ratelimited("RDS/IB: %s: unexpected opcode 0x%x in WR!\n",
+ __func__, send->s_wr.opcode);
break;
}
@@ -730,8 +729,8 @@ int rds_ib_xmit(struct rds_connection *conn, struct rds_message *rm,
first, &first->s_wr, ret, failed_wr);
BUG_ON(failed_wr != &first->s_wr);
if (ret) {
- printk(KERN_WARNING "RDS/IB: ib_post_send to %pI4 "
- "returned %d\n", &conn->c_faddr, ret);
+ pr_warn("RDS/IB: ib_post_send to %pI4 returned %d\n",
+ &conn->c_faddr, ret);
rds_ib_ring_unalloc(&ic->i_send_ring, work_alloc);
rds_ib_sub_signaled(ic, nr_sig);
if (prev->s_op) {
@@ -827,15 +826,16 @@ int rds_ib_xmit_atomic(struct rds_connection *conn, struct rm_atomic_op *op)
send, &send->s_atomic_wr, ret, failed_wr);
BUG_ON(failed_wr != &send->s_atomic_wr.wr);
if (ret) {
- printk(KERN_WARNING "RDS/IB: atomic ib_post_send to %pI4 "
- "returned %d\n", &conn->c_faddr, ret);
+ pr_warn("RDS/IB: atomic ib_post_send to %pI4 returned %d\n",
+ &conn->c_faddr, ret);
rds_ib_ring_unalloc(&ic->i_send_ring, work_alloc);
rds_ib_sub_signaled(ic, nr_sig);
goto out;
}
if (unlikely(failed_wr != &send->s_atomic_wr.wr)) {
- printk(KERN_WARNING "RDS/IB: atomic ib_post_send() rc=%d, but failed_wqe updated!\n", ret);
+ pr_warn("RDS/IB: atomic ib_post_send() rc=%d, but failed_wqe updated!\n",
+ ret);
BUG_ON(failed_wr != &send->s_atomic_wr.wr);
}
@@ -967,15 +967,16 @@ int rds_ib_xmit_rdma(struct rds_connection *conn, struct rm_rdma_op *op)
first, &first->s_rdma_wr.wr, ret, failed_wr);
BUG_ON(failed_wr != &first->s_rdma_wr.wr);
if (ret) {
- printk(KERN_WARNING "RDS/IB: rdma ib_post_send to %pI4 "
- "returned %d\n", &conn->c_faddr, ret);
+ pr_warn("RDS/IB: rdma ib_post_send to %pI4 returned %d\n",
+ &conn->c_faddr, ret);
rds_ib_ring_unalloc(&ic->i_send_ring, work_alloc);
rds_ib_sub_signaled(ic, nr_sig);
goto out;
}
if (unlikely(failed_wr != &first->s_rdma_wr.wr)) {
- printk(KERN_WARNING "RDS/IB: ib_post_send() rc=%d, but failed_wqe updated!\n", ret);
+ pr_warn("RDS/IB: ib_post_send() rc=%d, but failed_wqe updated!\n",
+ ret);
BUG_ON(failed_wr != &first->s_rdma_wr.wr);
}
diff --git a/net/rds/rdma_transport.c b/net/rds/rdma_transport.c
index fc59821f0a27..0ccb1cde4c52 100644
--- a/net/rds/rdma_transport.c
+++ b/net/rds/rdma_transport.c
@@ -131,7 +131,7 @@ int rds_rdma_cm_event_handler(struct rdma_cm_id *cm_id,
default:
/* things like device disconnect? */
- printk(KERN_ERR "RDS: unknown event %u (%s)!\n",
+ pr_err("RDS: unknown event %u (%s)!\n",
event->event, rdma_event_msg(event->event));
break;
}
@@ -156,8 +156,8 @@ static int rds_rdma_listen_init(void)
RDMA_PS_TCP, IB_QPT_RC);
if (IS_ERR(cm_id)) {
ret = PTR_ERR(cm_id);
- printk(KERN_ERR "RDS/RDMA: failed to setup listener, "
- "rdma_create_id() returned %d\n", ret);
+ pr_err("RDS/RDMA: failed to setup listener, rdma_create_id() returned %d\n",
+ ret);
return ret;
}
@@ -171,15 +171,15 @@ static int rds_rdma_listen_init(void)
*/
ret = rdma_bind_addr(cm_id, (struct sockaddr *)&sin);
if (ret) {
- printk(KERN_ERR "RDS/RDMA: failed to setup listener, "
- "rdma_bind_addr() returned %d\n", ret);
+ pr_err("RDS/RDMA: failed to setup listener, rdma_bind_addr() returned %d\n",
+ ret);
goto out;
}
ret = rdma_listen(cm_id, 128);
if (ret) {
- printk(KERN_ERR "RDS/RDMA: failed to setup listener, "
- "rdma_listen() returned %d\n", ret);
+ pr_err("RDS/RDMA: failed to setup listener, rdma_listen() returned %d\n",
+ ret);
goto out;
}
diff --git a/net/rds/send.c b/net/rds/send.c
index b52cdc8ae428..f9bc3d499576 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1130,15 +1130,15 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
}
if (rm->rdma.op_active && !conn->c_trans->xmit_rdma) {
- printk_ratelimited(KERN_NOTICE "rdma_op %p conn xmit_rdma %p\n",
- &rm->rdma, conn->c_trans->xmit_rdma);
+ pr_notice_ratelimited("rdma_op %p conn xmit_rdma %p\n",
+ &rm->rdma, conn->c_trans->xmit_rdma);
ret = -EOPNOTSUPP;
goto out;
}
if (rm->atomic.op_active && !conn->c_trans->xmit_atomic) {
- printk_ratelimited(KERN_NOTICE "atomic_op %p conn xmit_atomic %p\n",
- &rm->atomic, conn->c_trans->xmit_atomic);
+ pr_notice_ratelimited("atomic_op %p conn xmit_atomic %p\n",
+ &rm->atomic, conn->c_trans->xmit_atomic);
ret = -EOPNOTSUPP;
goto out;
}
diff --git a/net/rds/tcp_send.c b/net/rds/tcp_send.c
index dc860d1bb608..0e23e9d06c7e 100644
--- a/net/rds/tcp_send.c
+++ b/net/rds/tcp_send.c
@@ -153,9 +153,7 @@ int rds_tcp_xmit(struct rds_connection *conn, struct rds_message *rm,
* an incoming RST.
*/
if (rds_conn_path_up(cp)) {
- pr_warn("RDS/tcp: send to %pI4 on cp [%d]"
- "returned %d, "
- "disconnecting and reconnecting\n",
+ pr_warn("RDS/tcp: send to %pI4 on cp [%d]returned %d, disconnecting and reconnecting\n",
&conn->c_faddr, cp->cp_index, ret);
rds_conn_path_drop(cp, false);
}
diff --git a/net/rds/threads.c b/net/rds/threads.c
index f121daa402c8..499a0a8287cc 100644
--- a/net/rds/threads.c
+++ b/net/rds/threads.c
@@ -74,10 +74,8 @@ EXPORT_SYMBOL_GPL(rds_wq);
void rds_connect_path_complete(struct rds_conn_path *cp, int curr)
{
if (!rds_conn_path_transition(cp, curr, RDS_CONN_UP)) {
- printk(KERN_WARNING "%s: Cannot transition to state UP, "
- "current state is %d\n",
- __func__,
- atomic_read(&cp->cp_state));
+ pr_warn("%s: Cannot transition to state UP, current state is %d\n",
+ __func__, atomic_read(&cp->cp_state));
rds_conn_path_drop(cp, false);
return;
}
diff --git a/net/rds/transport.c b/net/rds/transport.c
index 0b188dd0a344..a0d7ccecdec3 100644
--- a/net/rds/transport.c
+++ b/net/rds/transport.c
@@ -46,12 +46,12 @@ void rds_trans_register(struct rds_transport *trans)
down_write(&rds_trans_sem);
- if (transports[trans->t_type])
- printk(KERN_ERR "RDS Transport type %d already registered\n",
- trans->t_type);
- else {
+ if (transports[trans->t_type]) {
+ pr_err("RDS Transport type %d already registered\n",
+ trans->t_type);
+ } else {
transports[trans->t_type] = trans;
- printk(KERN_INFO "Registered RDS/%s transport\n", trans->t_name);
+ pr_info("Registered RDS/%s transport\n", trans->t_name);
}
up_write(&rds_trans_sem);
@@ -63,7 +63,7 @@ void rds_trans_unregister(struct rds_transport *trans)
down_write(&rds_trans_sem);
transports[trans->t_type] = NULL;
- printk(KERN_INFO "Unregistered RDS/%s transport\n", trans->t_name);
+ pr_info("Unregistered RDS/%s transport\n", trans->t_name);
up_write(&rds_trans_sem);
}
^ permalink raw reply related
* Re: [PATCHv2 net-next 10/15] net: sch: api: add extack support in tcf_block_get
From: kbuild test robot @ 2017-12-16 18:27 UTC (permalink / raw)
To: Alexander Aring
Cc: kbuild-all, jhs, xiyou.wangcong, jiri, davem, netdev, kernel,
Alexander Aring, David Ahern
In-Reply-To: <20171214183905.23066-11-aring@mojatatu.com>
[-- Attachment #1: Type: text/plain, Size: 11773 bytes --]
Hi Alexander,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Alexander-Aring/net-sched-sch-introduce-extack-support/20171217-015839
config: x86_64-randconfig-x003-201751 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
net/sched/sch_atm.c: In function 'atm_tc_change':
>> net/sched/sch_atm.c:285:10: error: too few arguments to function 'tcf_block_get'
error = tcf_block_get(&flow->block, &flow->filter_list, sch);
^~~~~~~~~~~~~
In file included from net/sched/sch_atm.c:18:0:
include/net/pkt_cls.h:41:5: note: declared here
int tcf_block_get(struct tcf_block **p_block,
^~~~~~~~~~~~~
net/sched/sch_atm.c: In function 'atm_tc_init':
net/sched/sch_atm.c:550:8: error: too few arguments to function 'tcf_block_get'
err = tcf_block_get(&p->link.block, &p->link.filter_list, sch);
^~~~~~~~~~~~~
In file included from net/sched/sch_atm.c:18:0:
include/net/pkt_cls.h:41:5: note: declared here
int tcf_block_get(struct tcf_block **p_block,
^~~~~~~~~~~~~
net/sched/sch_atm.c: At top level:
net/sched/sch_atm.c:660:12: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.graft = atm_tc_graft,
^~~~~~~~~~~~
net/sched/sch_atm.c:660:12: note: (near initialization for 'atm_class_ops.graft')
net/sched/sch_atm.c:666:15: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.tcf_block = atm_tc_tcf_block,
^~~~~~~~~~~~~~~~
net/sched/sch_atm.c:666:15: note: (near initialization for 'atm_class_ops.tcf_block')
net/sched/sch_atm.c:680:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.init = atm_tc_init,
^~~~~~~~~~~
net/sched/sch_atm.c:680:11: note: (near initialization for 'atm_qdisc_ops.init')
cc1: some warnings being treated as errors
vim +/tcf_block_get +285 net/sched/sch_atm.c
27a3421e4 Patrick McHardy 2008-01-23 192
^1da177e4 Linus Torvalds 2005-04-16 193 static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent,
7eb3baddc Alexander Aring 2017-12-14 194 struct nlattr **tca, unsigned long *arg,
7eb3baddc Alexander Aring 2017-12-14 195 struct netlink_ext_ack *extack)
^1da177e4 Linus Torvalds 2005-04-16 196 {
786a90366 Stephen Hemminger 2008-01-21 197 struct atm_qdisc_data *p = qdisc_priv(sch);
^1da177e4 Linus Torvalds 2005-04-16 198 struct atm_flow_data *flow = (struct atm_flow_data *)*arg;
^1da177e4 Linus Torvalds 2005-04-16 199 struct atm_flow_data *excess = NULL;
1e90474c3 Patrick McHardy 2008-01-22 200 struct nlattr *opt = tca[TCA_OPTIONS];
1e90474c3 Patrick McHardy 2008-01-22 201 struct nlattr *tb[TCA_ATM_MAX + 1];
^1da177e4 Linus Torvalds 2005-04-16 202 struct socket *sock;
^1da177e4 Linus Torvalds 2005-04-16 203 int fd, error, hdr_len;
^1da177e4 Linus Torvalds 2005-04-16 204 void *hdr;
^1da177e4 Linus Torvalds 2005-04-16 205
786a90366 Stephen Hemminger 2008-01-21 206 pr_debug("atm_tc_change(sch %p,[qdisc %p],classid %x,parent %x,"
^1da177e4 Linus Torvalds 2005-04-16 207 "flow %p,opt %p)\n", sch, p, classid, parent, flow, opt);
^1da177e4 Linus Torvalds 2005-04-16 208 /*
^1da177e4 Linus Torvalds 2005-04-16 209 * The concept of parents doesn't apply for this qdisc.
^1da177e4 Linus Torvalds 2005-04-16 210 */
^1da177e4 Linus Torvalds 2005-04-16 211 if (parent && parent != TC_H_ROOT && parent != sch->handle)
^1da177e4 Linus Torvalds 2005-04-16 212 return -EINVAL;
^1da177e4 Linus Torvalds 2005-04-16 213 /*
^1da177e4 Linus Torvalds 2005-04-16 214 * ATM classes cannot be changed. In order to change properties of the
^1da177e4 Linus Torvalds 2005-04-16 215 * ATM connection, that socket needs to be modified directly (via the
^1da177e4 Linus Torvalds 2005-04-16 216 * native ATM API. In order to send a flow to a different VC, the old
^1da177e4 Linus Torvalds 2005-04-16 217 * class needs to be removed and a new one added. (This may be changed
^1da177e4 Linus Torvalds 2005-04-16 218 * later.)
^1da177e4 Linus Torvalds 2005-04-16 219 */
b0188d4db Patrick McHardy 2007-07-15 220 if (flow)
b0188d4db Patrick McHardy 2007-07-15 221 return -EBUSY;
cee63723b Patrick McHardy 2008-01-23 222 if (opt == NULL)
^1da177e4 Linus Torvalds 2005-04-16 223 return -EINVAL;
27a3421e4 Patrick McHardy 2008-01-23 224
fceb6435e Johannes Berg 2017-04-12 225 error = nla_parse_nested(tb, TCA_ATM_MAX, opt, atm_policy, NULL);
cee63723b Patrick McHardy 2008-01-23 226 if (error < 0)
cee63723b Patrick McHardy 2008-01-23 227 return error;
cee63723b Patrick McHardy 2008-01-23 228
27a3421e4 Patrick McHardy 2008-01-23 229 if (!tb[TCA_ATM_FD])
^1da177e4 Linus Torvalds 2005-04-16 230 return -EINVAL;
1587bac49 Patrick McHardy 2008-01-23 231 fd = nla_get_u32(tb[TCA_ATM_FD]);
786a90366 Stephen Hemminger 2008-01-21 232 pr_debug("atm_tc_change: fd %d\n", fd);
1e90474c3 Patrick McHardy 2008-01-22 233 if (tb[TCA_ATM_HDR]) {
1e90474c3 Patrick McHardy 2008-01-22 234 hdr_len = nla_len(tb[TCA_ATM_HDR]);
1e90474c3 Patrick McHardy 2008-01-22 235 hdr = nla_data(tb[TCA_ATM_HDR]);
b0188d4db Patrick McHardy 2007-07-15 236 } else {
^1da177e4 Linus Torvalds 2005-04-16 237 hdr_len = RFC1483LLC_LEN;
^1da177e4 Linus Torvalds 2005-04-16 238 hdr = NULL; /* default LLC/SNAP for IP */
^1da177e4 Linus Torvalds 2005-04-16 239 }
1e90474c3 Patrick McHardy 2008-01-22 240 if (!tb[TCA_ATM_EXCESS])
b0188d4db Patrick McHardy 2007-07-15 241 excess = NULL;
^1da177e4 Linus Torvalds 2005-04-16 242 else {
b0188d4db Patrick McHardy 2007-07-15 243 excess = (struct atm_flow_data *)
143976ce9 WANG Cong 2017-08-24 244 atm_tc_find(sch, nla_get_u32(tb[TCA_ATM_EXCESS]));
b0188d4db Patrick McHardy 2007-07-15 245 if (!excess)
b0188d4db Patrick McHardy 2007-07-15 246 return -ENOENT;
^1da177e4 Linus Torvalds 2005-04-16 247 }
f5e5cb755 Patrick McHardy 2008-01-23 248 pr_debug("atm_tc_change: type %d, payload %d, hdr_len %d\n",
1e90474c3 Patrick McHardy 2008-01-22 249 opt->nla_type, nla_len(opt), hdr_len);
786a90366 Stephen Hemminger 2008-01-21 250 sock = sockfd_lookup(fd, &error);
786a90366 Stephen Hemminger 2008-01-21 251 if (!sock)
b0188d4db Patrick McHardy 2007-07-15 252 return error; /* f_count++ */
516e0cc56 Al Viro 2008-07-26 253 pr_debug("atm_tc_change: f_count %ld\n", file_count(sock->file));
^1da177e4 Linus Torvalds 2005-04-16 254 if (sock->ops->family != PF_ATMSVC && sock->ops->family != PF_ATMPVC) {
^1da177e4 Linus Torvalds 2005-04-16 255 error = -EPROTOTYPE;
^1da177e4 Linus Torvalds 2005-04-16 256 goto err_out;
^1da177e4 Linus Torvalds 2005-04-16 257 }
^1da177e4 Linus Torvalds 2005-04-16 258 /* @@@ should check if the socket is really operational or we'll crash
^1da177e4 Linus Torvalds 2005-04-16 259 on vcc->send */
^1da177e4 Linus Torvalds 2005-04-16 260 if (classid) {
^1da177e4 Linus Torvalds 2005-04-16 261 if (TC_H_MAJ(classid ^ sch->handle)) {
786a90366 Stephen Hemminger 2008-01-21 262 pr_debug("atm_tc_change: classid mismatch\n");
^1da177e4 Linus Torvalds 2005-04-16 263 error = -EINVAL;
^1da177e4 Linus Torvalds 2005-04-16 264 goto err_out;
^1da177e4 Linus Torvalds 2005-04-16 265 }
b0188d4db Patrick McHardy 2007-07-15 266 } else {
^1da177e4 Linus Torvalds 2005-04-16 267 int i;
^1da177e4 Linus Torvalds 2005-04-16 268 unsigned long cl;
^1da177e4 Linus Torvalds 2005-04-16 269
^1da177e4 Linus Torvalds 2005-04-16 270 for (i = 1; i < 0x8000; i++) {
^1da177e4 Linus Torvalds 2005-04-16 271 classid = TC_H_MAKE(sch->handle, 0x8000 | i);
143976ce9 WANG Cong 2017-08-24 272 cl = atm_tc_find(sch, classid);
786a90366 Stephen Hemminger 2008-01-21 273 if (!cl)
b0188d4db Patrick McHardy 2007-07-15 274 break;
^1da177e4 Linus Torvalds 2005-04-16 275 }
^1da177e4 Linus Torvalds 2005-04-16 276 }
786a90366 Stephen Hemminger 2008-01-21 277 pr_debug("atm_tc_change: new id %x\n", classid);
782f79568 vignesh babu 2007-07-16 278 flow = kzalloc(sizeof(struct atm_flow_data) + hdr_len, GFP_KERNEL);
786a90366 Stephen Hemminger 2008-01-21 279 pr_debug("atm_tc_change: flow %p\n", flow);
^1da177e4 Linus Torvalds 2005-04-16 280 if (!flow) {
^1da177e4 Linus Torvalds 2005-04-16 281 error = -ENOBUFS;
^1da177e4 Linus Torvalds 2005-04-16 282 goto err_out;
^1da177e4 Linus Torvalds 2005-04-16 283 }
6529eaba3 Jiri Pirko 2017-05-17 284
69d78ef25 Jiri Pirko 2017-10-13 @285 error = tcf_block_get(&flow->block, &flow->filter_list, sch);
6529eaba3 Jiri Pirko 2017-05-17 286 if (error) {
6529eaba3 Jiri Pirko 2017-05-17 287 kfree(flow);
6529eaba3 Jiri Pirko 2017-05-17 288 goto err_out;
6529eaba3 Jiri Pirko 2017-05-17 289 }
6529eaba3 Jiri Pirko 2017-05-17 290
3511c9132 Changli Gao 2010-10-16 291 flow->q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, classid);
786a90366 Stephen Hemminger 2008-01-21 292 if (!flow->q)
^1da177e4 Linus Torvalds 2005-04-16 293 flow->q = &noop_qdisc;
786a90366 Stephen Hemminger 2008-01-21 294 pr_debug("atm_tc_change: qdisc %p\n", flow->q);
^1da177e4 Linus Torvalds 2005-04-16 295 flow->sock = sock;
^1da177e4 Linus Torvalds 2005-04-16 296 flow->vcc = ATM_SD(sock); /* speedup */
^1da177e4 Linus Torvalds 2005-04-16 297 flow->vcc->user_back = flow;
786a90366 Stephen Hemminger 2008-01-21 298 pr_debug("atm_tc_change: vcc %p\n", flow->vcc);
^1da177e4 Linus Torvalds 2005-04-16 299 flow->old_pop = flow->vcc->pop;
^1da177e4 Linus Torvalds 2005-04-16 300 flow->parent = p;
^1da177e4 Linus Torvalds 2005-04-16 301 flow->vcc->pop = sch_atm_pop;
f7ebdff75 Jiri Pirko 2017-08-04 302 flow->common.classid = classid;
^1da177e4 Linus Torvalds 2005-04-16 303 flow->ref = 1;
^1da177e4 Linus Torvalds 2005-04-16 304 flow->excess = excess;
6accec76f David S. Miller 2010-07-18 305 list_add(&flow->list, &p->link.list);
^1da177e4 Linus Torvalds 2005-04-16 306 flow->hdr_len = hdr_len;
^1da177e4 Linus Torvalds 2005-04-16 307 if (hdr)
^1da177e4 Linus Torvalds 2005-04-16 308 memcpy(flow->hdr, hdr, hdr_len);
^1da177e4 Linus Torvalds 2005-04-16 309 else
^1da177e4 Linus Torvalds 2005-04-16 310 memcpy(flow->hdr, llc_oui_ip, sizeof(llc_oui_ip));
^1da177e4 Linus Torvalds 2005-04-16 311 *arg = (unsigned long)flow;
^1da177e4 Linus Torvalds 2005-04-16 312 return 0;
^1da177e4 Linus Torvalds 2005-04-16 313 err_out:
^1da177e4 Linus Torvalds 2005-04-16 314 sockfd_put(sock);
^1da177e4 Linus Torvalds 2005-04-16 315 return error;
^1da177e4 Linus Torvalds 2005-04-16 316 }
^1da177e4 Linus Torvalds 2005-04-16 317
:::::: The code at line 285 was first introduced by commit
:::::: 69d78ef25c7b0058674145500efb12255738ba8a net: sched: store Qdisc pointer in struct block
:::::: TO: Jiri Pirko <jiri@mellanox.com>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27258 bytes --]
^ 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