* scheduling while atomic from vmci_transport_recv_stream_cb in 3.16 kernels
From: Michal Hocko @ 2017-09-12 9:08 UTC (permalink / raw)
To: Jorgen Hansen
Cc: Aditya Asarwade, Thomas Hellstrom, LKML, netdev, Masik Petr,
Ben Hutchings, Sasha Levin, Stable tree
Hi,
we are seeing the following splat with Debian 3.16 stable kernel
BUG: scheduling while atomic: MATLAB/26771/0x00000100
Modules linked in: veeamsnap(O) hmac cbc cts nfsv4 dns_resolver rpcsec_gss_krb5 nfsd auth_rpcgss oid_registry nfs_acl nfs lockd fscache sunrpc vmw_vso$
CPU: 0 PID: 26771 Comm: MATLAB Tainted: G O 3.16.0-4-amd64 #1 Debian 3.16.7-ckt20-1+deb8u3
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 09/21/2015
ffff88315c1e4c20 ffffffff8150db3f ffff88193f803dc8 ffffffff8150acdf
ffffffff815103a2 0000000000012f00 ffff8819423dbfd8 0000000000012f00
ffff88315c1e4c20 ffff88193f803dc8 ffff88193f803d50 ffff88193f803dc0
Call Trace:
<IRQ> [<ffffffff8150db3f>] ? dump_stack+0x41/0x51
[<ffffffff8150acdf>] ? __schedule_bug+0x48/0x55
[<ffffffff815103a2>] ? __schedule+0x5d2/0x700
[<ffffffff8150f9b9>] ? schedule_timeout+0x229/0x2a0
[<ffffffff8109ba70>] ? select_task_rq_fair+0x390/0x700
[<ffffffff8109f780>] ? check_preempt_wakeup+0x120/0x1d0
[<ffffffff81510eb8>] ? wait_for_completion+0xa8/0x120
[<ffffffff81096de0>] ? wake_up_state+0x10/0x10
[<ffffffff810c3da0>] ? call_rcu_bh+0x20/0x20
[<ffffffff810c180b>] ? wait_rcu_gp+0x4b/0x60
[<ffffffff810c17b0>] ? ftrace_raw_output_rcu_utilization+0x40/0x40
[<ffffffffa02ca6f5>] ? vmci_event_unsubscribe+0x75/0xb0 [vmw_vmci]
[<ffffffffa031f5cd>] ? vmci_transport_destruct+0x1d/0xe0 [vmw_vsock_vmci_transport]
[<ffffffffa03167e3>] ? vsock_sk_destruct+0x13/0x60 [vsock]
[<ffffffff81409f7a>] ? __sk_free+0x1a/0x130
[<ffffffffa0320218>] ? vmci_transport_recv_stream_cb+0x1e8/0x2d0 [vmw_vsock_vmci_transport]
[<ffffffffa02c9cba>] ? vmci_datagram_invoke_guest_handler+0xaa/0xd0 [vmw_vmci]
[<ffffffffa02cab51>] ? vmci_dispatch_dgs+0xc1/0x200 [vmw_vmci]
[<ffffffff8106c294>] ? tasklet_action+0xf4/0x100
[<ffffffff8106c681>] ? __do_softirq+0xf1/0x290
[<ffffffff8106ca55>] ? irq_exit+0x95/0xa0
[<ffffffff81516b22>] ? do_IRQ+0x52/0xe0
[<ffffffff8151496d>] ? common_interrupt+0x6d/0x6d
AFAICS this has been fixed by 4ef7ea9195ea ("VSOCK: sock_put wasn't safe
to call in interrupt context") but this patch hasn't been backported to
stable trees. It applies cleanly on top of 3.16 stable tree but I am not
familiar with the code to send the backport to the stable maintainer
directly.
Could you double check that the patch below (just a blind cherry-pick)
is correct and it doesn't need additional patches on top?
Ben could you take this to your stable 3.16 branch if the patch is correct?
I am CCing Sasha for 4.1 stable tree as well. I haven't checked whether
pre 3.16 kernels are affected as well.
---
commit fac774c40b5c512113b6373cad498f35bee7a409
Author: Jorgen Hansen <jhansen@vmware.com>
Date: Wed Oct 21 04:53:56 2015 -0700
VSOCK: sock_put wasn't safe to call in interrupt context
commit 4ef7ea9195ea73262cd9730fb54e1eb726da157b upstream.
In the vsock vmci_transport driver, sock_put wasn't safe to call
in interrupt context, since that may call the vsock destructor
which in turn calls several functions that should only be called
from process context. This change defers the callling of these
functions to a worker thread. All these functions were
deallocation of resources related to the transport itself.
Furthermore, an unused callback was removed to simplify the
cleanup.
Multiple customers have been hitting this issue when using
VMware tools on vSphere 2015.
Also added a version to the vmci transport module (starting from
1.0.2.0-k since up until now it appears that this module was
sharing version with vsock that is currently at 1.0.1.0-k).
Reviewed-by: Aditya Asarwade <asarwade@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Jorgen Hansen <jhansen@vmware.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 9bb63ffec4f2..aed136d27b01 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -40,13 +40,11 @@
static int vmci_transport_recv_dgram_cb(void *data, struct vmci_datagram *dg);
static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg);
-static void vmci_transport_peer_attach_cb(u32 sub_id,
- const struct vmci_event_data *ed,
- void *client_data);
static void vmci_transport_peer_detach_cb(u32 sub_id,
const struct vmci_event_data *ed,
void *client_data);
static void vmci_transport_recv_pkt_work(struct work_struct *work);
+static void vmci_transport_cleanup(struct work_struct *work);
static int vmci_transport_recv_listen(struct sock *sk,
struct vmci_transport_packet *pkt);
static int vmci_transport_recv_connecting_server(
@@ -75,6 +73,10 @@ struct vmci_transport_recv_pkt_info {
struct vmci_transport_packet pkt;
};
+static LIST_HEAD(vmci_transport_cleanup_list);
+static DEFINE_SPINLOCK(vmci_transport_cleanup_lock);
+static DECLARE_WORK(vmci_transport_cleanup_work, vmci_transport_cleanup);
+
static struct vmci_handle vmci_transport_stream_handle = { VMCI_INVALID_ID,
VMCI_INVALID_ID };
static u32 vmci_transport_qp_resumed_sub_id = VMCI_INVALID_ID;
@@ -791,44 +793,6 @@ static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg)
return err;
}
-static void vmci_transport_peer_attach_cb(u32 sub_id,
- const struct vmci_event_data *e_data,
- void *client_data)
-{
- struct sock *sk = client_data;
- const struct vmci_event_payload_qp *e_payload;
- struct vsock_sock *vsk;
-
- e_payload = vmci_event_data_const_payload(e_data);
-
- vsk = vsock_sk(sk);
-
- /* We don't ask for delayed CBs when we subscribe to this event (we
- * pass 0 as flags to vmci_event_subscribe()). VMCI makes no
- * guarantees in that case about what context we might be running in,
- * so it could be BH or process, blockable or non-blockable. So we
- * need to account for all possible contexts here.
- */
- local_bh_disable();
- bh_lock_sock(sk);
-
- /* XXX This is lame, we should provide a way to lookup sockets by
- * qp_handle.
- */
- if (vmci_handle_is_equal(vmci_trans(vsk)->qp_handle,
- e_payload->handle)) {
- /* XXX This doesn't do anything, but in the future we may want
- * to set a flag here to verify the attach really did occur and
- * we weren't just sent a datagram claiming it was.
- */
- goto out;
- }
-
-out:
- bh_unlock_sock(sk);
- local_bh_enable();
-}
-
static void vmci_transport_handle_detach(struct sock *sk)
{
struct vsock_sock *vsk;
@@ -871,28 +835,38 @@ static void vmci_transport_peer_detach_cb(u32 sub_id,
const struct vmci_event_data *e_data,
void *client_data)
{
- struct sock *sk = client_data;
+ struct vmci_transport *trans = client_data;
const struct vmci_event_payload_qp *e_payload;
- struct vsock_sock *vsk;
e_payload = vmci_event_data_const_payload(e_data);
- vsk = vsock_sk(sk);
- if (vmci_handle_is_invalid(e_payload->handle))
- return;
-
- /* Same rules for locking as for peer_attach_cb(). */
- local_bh_disable();
- bh_lock_sock(sk);
/* XXX This is lame, we should provide a way to lookup sockets by
* qp_handle.
*/
- if (vmci_handle_is_equal(vmci_trans(vsk)->qp_handle,
- e_payload->handle))
- vmci_transport_handle_detach(sk);
+ if (vmci_handle_is_invalid(e_payload->handle) ||
+ vmci_handle_is_equal(trans->qp_handle, e_payload->handle))
+ return;
- bh_unlock_sock(sk);
- local_bh_enable();
+ /* We don't ask for delayed CBs when we subscribe to this event (we
+ * pass 0 as flags to vmci_event_subscribe()). VMCI makes no
+ * guarantees in that case about what context we might be running in,
+ * so it could be BH or process, blockable or non-blockable. So we
+ * need to account for all possible contexts here.
+ */
+ spin_lock_bh(&trans->lock);
+ if (!trans->sk)
+ goto out;
+
+ /* Apart from here, trans->lock is only grabbed as part of sk destruct,
+ * where trans->sk isn't locked.
+ */
+ bh_lock_sock(trans->sk);
+
+ vmci_transport_handle_detach(trans->sk);
+
+ bh_unlock_sock(trans->sk);
+ out:
+ spin_unlock_bh(&trans->lock);
}
static void vmci_transport_qp_resumed_cb(u32 sub_id,
@@ -1181,7 +1155,7 @@ vmci_transport_recv_connecting_server(struct sock *listener,
*/
err = vmci_event_subscribe(VMCI_EVENT_QP_PEER_DETACH,
vmci_transport_peer_detach_cb,
- pending, &detach_sub_id);
+ vmci_trans(vpending), &detach_sub_id);
if (err < VMCI_SUCCESS) {
vmci_transport_send_reset(pending, pkt);
err = vmci_transport_error_to_vsock_error(err);
@@ -1321,7 +1295,6 @@ vmci_transport_recv_connecting_client(struct sock *sk,
|| vmci_trans(vsk)->qpair
|| vmci_trans(vsk)->produce_size != 0
|| vmci_trans(vsk)->consume_size != 0
- || vmci_trans(vsk)->attach_sub_id != VMCI_INVALID_ID
|| vmci_trans(vsk)->detach_sub_id != VMCI_INVALID_ID) {
skerr = EPROTO;
err = -EINVAL;
@@ -1389,7 +1362,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
struct vsock_sock *vsk;
struct vmci_handle handle;
struct vmci_qp *qpair;
- u32 attach_sub_id;
u32 detach_sub_id;
bool is_local;
u32 flags;
@@ -1399,7 +1371,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
vsk = vsock_sk(sk);
handle = VMCI_INVALID_HANDLE;
- attach_sub_id = VMCI_INVALID_ID;
detach_sub_id = VMCI_INVALID_ID;
/* If we have gotten here then we should be past the point where old
@@ -1444,23 +1415,15 @@ static int vmci_transport_recv_connecting_client_negotiate(
goto destroy;
}
- /* Subscribe to attach and detach events first.
+ /* Subscribe to detach events first.
*
* XXX We attach once for each queue pair created for now so it is easy
* to find the socket (it's provided), but later we should only
* subscribe once and add a way to lookup sockets by queue pair handle.
*/
- err = vmci_event_subscribe(VMCI_EVENT_QP_PEER_ATTACH,
- vmci_transport_peer_attach_cb,
- sk, &attach_sub_id);
- if (err < VMCI_SUCCESS) {
- err = vmci_transport_error_to_vsock_error(err);
- goto destroy;
- }
-
err = vmci_event_subscribe(VMCI_EVENT_QP_PEER_DETACH,
vmci_transport_peer_detach_cb,
- sk, &detach_sub_id);
+ vmci_trans(vsk), &detach_sub_id);
if (err < VMCI_SUCCESS) {
err = vmci_transport_error_to_vsock_error(err);
goto destroy;
@@ -1496,7 +1459,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
vmci_trans(vsk)->produce_size = vmci_trans(vsk)->consume_size =
pkt->u.size;
- vmci_trans(vsk)->attach_sub_id = attach_sub_id;
vmci_trans(vsk)->detach_sub_id = detach_sub_id;
vmci_trans(vsk)->notify_ops->process_negotiate(sk);
@@ -1504,9 +1466,6 @@ static int vmci_transport_recv_connecting_client_negotiate(
return 0;
destroy:
- if (attach_sub_id != VMCI_INVALID_ID)
- vmci_event_unsubscribe(attach_sub_id);
-
if (detach_sub_id != VMCI_INVALID_ID)
vmci_event_unsubscribe(detach_sub_id);
@@ -1607,9 +1566,11 @@ static int vmci_transport_socket_init(struct vsock_sock *vsk,
vmci_trans(vsk)->qp_handle = VMCI_INVALID_HANDLE;
vmci_trans(vsk)->qpair = NULL;
vmci_trans(vsk)->produce_size = vmci_trans(vsk)->consume_size = 0;
- vmci_trans(vsk)->attach_sub_id = vmci_trans(vsk)->detach_sub_id =
- VMCI_INVALID_ID;
+ vmci_trans(vsk)->detach_sub_id = VMCI_INVALID_ID;
vmci_trans(vsk)->notify_ops = NULL;
+ INIT_LIST_HEAD(&vmci_trans(vsk)->elem);
+ vmci_trans(vsk)->sk = &vsk->sk;
+ vmci_trans(vsk)->lock = __SPIN_LOCK_UNLOCKED(vmci_trans(vsk)->lock);
if (psk) {
vmci_trans(vsk)->queue_pair_size =
vmci_trans(psk)->queue_pair_size;
@@ -1629,29 +1590,57 @@ static int vmci_transport_socket_init(struct vsock_sock *vsk,
return 0;
}
-static void vmci_transport_destruct(struct vsock_sock *vsk)
+static void vmci_transport_free_resources(struct list_head *transport_list)
{
- if (vmci_trans(vsk)->attach_sub_id != VMCI_INVALID_ID) {
- vmci_event_unsubscribe(vmci_trans(vsk)->attach_sub_id);
- vmci_trans(vsk)->attach_sub_id = VMCI_INVALID_ID;
- }
+ while (!list_empty(transport_list)) {
+ struct vmci_transport *transport =
+ list_first_entry(transport_list, struct vmci_transport,
+ elem);
+ list_del(&transport->elem);
- if (vmci_trans(vsk)->detach_sub_id != VMCI_INVALID_ID) {
- vmci_event_unsubscribe(vmci_trans(vsk)->detach_sub_id);
- vmci_trans(vsk)->detach_sub_id = VMCI_INVALID_ID;
- }
+ if (transport->detach_sub_id != VMCI_INVALID_ID) {
+ vmci_event_unsubscribe(transport->detach_sub_id);
+ transport->detach_sub_id = VMCI_INVALID_ID;
+ }
- if (!vmci_handle_is_invalid(vmci_trans(vsk)->qp_handle)) {
- vmci_qpair_detach(&vmci_trans(vsk)->qpair);
- vmci_trans(vsk)->qp_handle = VMCI_INVALID_HANDLE;
- vmci_trans(vsk)->produce_size = 0;
- vmci_trans(vsk)->consume_size = 0;
+ if (!vmci_handle_is_invalid(transport->qp_handle)) {
+ vmci_qpair_detach(&transport->qpair);
+ transport->qp_handle = VMCI_INVALID_HANDLE;
+ transport->produce_size = 0;
+ transport->consume_size = 0;
+ }
+
+ kfree(transport);
}
+}
+
+static void vmci_transport_cleanup(struct work_struct *work)
+{
+ LIST_HEAD(pending);
+
+ spin_lock_bh(&vmci_transport_cleanup_lock);
+ list_replace_init(&vmci_transport_cleanup_list, &pending);
+ spin_unlock_bh(&vmci_transport_cleanup_lock);
+ vmci_transport_free_resources(&pending);
+}
+
+static void vmci_transport_destruct(struct vsock_sock *vsk)
+{
+ /* Ensure that the detach callback doesn't use the sk/vsk
+ * we are about to destruct.
+ */
+ spin_lock_bh(&vmci_trans(vsk)->lock);
+ vmci_trans(vsk)->sk = NULL;
+ spin_unlock_bh(&vmci_trans(vsk)->lock);
if (vmci_trans(vsk)->notify_ops)
vmci_trans(vsk)->notify_ops->socket_destruct(vsk);
- kfree(vsk->trans);
+ spin_lock_bh(&vmci_transport_cleanup_lock);
+ list_add(&vmci_trans(vsk)->elem, &vmci_transport_cleanup_list);
+ spin_unlock_bh(&vmci_transport_cleanup_lock);
+ schedule_work(&vmci_transport_cleanup_work);
+
vsk->trans = NULL;
}
@@ -2148,6 +2137,9 @@ module_init(vmci_transport_init);
static void __exit vmci_transport_exit(void)
{
+ cancel_work_sync(&vmci_transport_cleanup_work);
+ vmci_transport_free_resources(&vmci_transport_cleanup_list);
+
if (!vmci_handle_is_invalid(vmci_transport_stream_handle)) {
if (vmci_datagram_destroy_handle(
vmci_transport_stream_handle) != VMCI_SUCCESS)
@@ -2166,6 +2158,7 @@ module_exit(vmci_transport_exit);
MODULE_AUTHOR("VMware, Inc.");
MODULE_DESCRIPTION("VMCI transport for Virtual Sockets");
+MODULE_VERSION("1.0.2.0-k");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("vmware_vsock");
MODULE_ALIAS_NETPROTO(PF_VSOCK);
diff --git a/net/vmw_vsock/vmci_transport.h b/net/vmw_vsock/vmci_transport.h
index ce6c9623d5f0..2ad46f39649f 100644
--- a/net/vmw_vsock/vmci_transport.h
+++ b/net/vmw_vsock/vmci_transport.h
@@ -119,10 +119,12 @@ struct vmci_transport {
u64 queue_pair_size;
u64 queue_pair_min_size;
u64 queue_pair_max_size;
- u32 attach_sub_id;
u32 detach_sub_id;
union vmci_transport_notify notify;
struct vmci_transport_notify_ops *notify_ops;
+ struct list_head elem;
+ struct sock *sk;
+ spinlock_t lock; /* protects sk. */
};
int vmci_transport_register(void);
--
Michal Hocko
SUSE Labs
^ permalink raw reply related
* Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Michal Kubecek @ 2017-09-12 8:47 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Phil Sutter, netdev, Stephen Hemminger
In-Reply-To: <20170911071955.GZ5465@leo.usersys.redhat.com>
On Mon, Sep 11, 2017 at 03:19:55PM +0800, Hangbin Liu wrote:
> On Fri, Sep 08, 2017 at 04:51:13PM +0200, Phil Sutter wrote:
> > Regarding Michal's concern about reentrancy, maybe we should go into a
> > different direction and make rtnl_recvmsg() return a newly allocated
> > buffer which the caller has to free.
>
> Hmm... But we could not free the buf in __rtnl_talk(). Because in
> __rtnl_talk() we assign the answer with the buf address and return to caller.
>
> for (h = (struct nlmsghdr *)buf; status >= sizeof(*h); ) {
> [...]
> if (answer) {
> *answer= h;
> return 0;
> }
> }
>
> And the caller will keep use it in later code. Since there are plenty of
> functions called rtnl_talk. I think it would be much more complex to free
> the buffer every time.
>
>
> Hi Michal,
>
> Would you like to tell me more about your concern with reentrancy? It's looks
> arpd doesn't call rtnl_talk() or rtnl_dump_filter_l().
I checked again and arpd indeed isn't a problem. It doesn't seem to call
any of the two functions (directly or indirectly) and while it's linked
with "-lpthread", it's not really multithreaded.
But my concern was rather about other potential users of libnetlink
(i.e. those which are not part of iproute2). I must admit, though, that
I'm not sure if libnetlink code is reentrant as of now. (And people are
discouraged from using it in its own manual page.)
That being said, I still like Phil's idea for a different reason. While
investigating the issue with "ip link show dev eth ..." which led me to
commit 6599162b958e ("iplink: check for message truncation in
iplink_get()"), I quickly peeked at some other callers of rtnl_talk()
and I'm afraid there may be others which wouldn't handle truncated
message correctly. I assume the maxlen argument was always chosen to be
sufficient for any expected messages but as the example of iplink_get()
shows, messages returned by kernel my grow over time.
That's why I like the idea of __rtnl_talk() returning a pointer to newly
allocated buffer (of sufficient size) rather than copying the response
into a buffer provided by caller and potentially truncating it.
Michal Kubecek
^ permalink raw reply
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Corentin Labbe @ 2017-09-12 7:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170911201920.GA5983@lunn.ch>
On Mon, Sep 11, 2017 at 10:19:20PM +0200, Andrew Lunn wrote:
> > Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
> > So no the CLK/RST are really for the PHY.
>
> Thanks for trying that.
>
> You said it was probably during scanning of the bus it times out. What
> address is causing the timeout? 0 or 1? If the internal bus can only
> have one PHY on it, maybe we need to set bus->phy_mask to 0x1?
>
I have added a trace in begin and end of stmmac_mdio_read()
[ 18.145451] libphy: stmmac: probed
[ 18.148398] libphy: mdio_mux: probed
[ 18.148650] dwmac-sun8i 1c30000.ethernet: Switch mux to internal PHY
[ 18.248751] dwmac-sun8i 1c30000.ethernet: EMAC reset timeout
[ 18.249297] libphy: mdio_mux: probed
[ 18.249362] dwmac-sun8i 1c30000.ethernet: Switch mux to external PHY
[ 18.249391] stmmac_mdio_read 0 2
[ 18.249598] stmmac_mdio_read 0 2 1c
[ 18.249623] stmmac_mdio_read 0 3
[ 18.249811] stmmac_mdio_read 0 3 c915
[ 20.737271] EXT4-fs (mmcblk0p1): re-mounted. Opts: (null)
[ 31.294868] stmmac_mdio_read 0 0
[ 31.295311] stmmac_mdio_read 0 0 1140
It seems that the timeout is unrelated to MDIO bus.
Regards
^ permalink raw reply
* [PATCH v2] geneve: Fix setting ttl value in collect metadata mode
From: Haishuang Yan @ 2017-09-12 7:05 UTC (permalink / raw)
To: David S. Miller, pravin shelar, Girish Moodalbail
Cc: netdev, linux-kernel, Haishuang Yan
Similar to vxlan/ipip tunnel, if key->tos is zero in collect metadata
mode, tos should also fallback to ip{4,6}_dst_hoplimit.
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
Changes since v2:
* Make the commit message more clearer.
---
drivers/net/geneve.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index f640407..d52a65f 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -834,11 +834,10 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
if (geneve->collect_md) {
tos = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
- ttl = key->ttl;
} else {
tos = ip_tunnel_ecn_encap(fl4.flowi4_tos, ip_hdr(skb), skb);
- ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
}
+ ttl = key->ttl ? : ip4_dst_hoplimit(&rt->dst);
df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
err = geneve_build_skb(&rt->dst, skb, info, xnet, sizeof(struct iphdr));
@@ -873,12 +872,11 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
if (geneve->collect_md) {
prio = ip_tunnel_ecn_encap(key->tos, ip_hdr(skb), skb);
- ttl = key->ttl;
} else {
prio = ip_tunnel_ecn_encap(ip6_tclass(fl6.flowlabel),
ip_hdr(skb), skb);
- ttl = key->ttl ? : ip6_dst_hoplimit(dst);
}
+ ttl = key->ttl ? : ip6_dst_hoplimit(dst);
err = geneve_build_skb(dst, skb, info, xnet, sizeof(struct ipv6hdr));
if (unlikely(err))
return err;
--
1.8.3.1
^ permalink raw reply related
* [patch net] mlxsw: spectrum: Prevent mirred-related crash on removal
From: Jiri Pirko @ 2017-09-12 6:50 UTC (permalink / raw)
To: netdev; +Cc: davem, yuvalm, idosch, flokli, mlxsw
From: Yuval Mintz <yuvalm@mellanox.com>
When removing the offloading of mirred actions under
matchall classifiers, mlxsw would find the destination port
associated with the offloaded action and utilize it for undoing
the configuration.
Depending on the order by which ports are removed, it's possible that
the destination port would get removed before the source port.
In such a scenario, when actions would be flushed for the source port
mlxsw would perform an illegal dereference as the destination port is
no longer listed.
Since the only item necessary for undoing the configuration on the
destination side is the port-id and that in turn is already maintained
by mlxsw on the source-port, simply stop trying to access the
destination port and use the port-id directly instead.
Fixes: 763b4b70af ("mlxsw: spectrum: Add support in matchall mirror TC offloading")
Signed-off-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index e080459..696b99e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -575,15 +575,14 @@ static void mlxsw_sp_span_entry_destroy(struct mlxsw_sp *mlxsw_sp,
}
static struct mlxsw_sp_span_entry *
-mlxsw_sp_span_entry_find(struct mlxsw_sp_port *port)
+mlxsw_sp_span_entry_find(struct mlxsw_sp *mlxsw_sp, u8 local_port)
{
- struct mlxsw_sp *mlxsw_sp = port->mlxsw_sp;
int i;
for (i = 0; i < mlxsw_sp->span.entries_count; i++) {
struct mlxsw_sp_span_entry *curr = &mlxsw_sp->span.entries[i];
- if (curr->used && curr->local_port == port->local_port)
+ if (curr->used && curr->local_port == local_port)
return curr;
}
return NULL;
@@ -594,7 +593,8 @@ static struct mlxsw_sp_span_entry
{
struct mlxsw_sp_span_entry *span_entry;
- span_entry = mlxsw_sp_span_entry_find(port);
+ span_entry = mlxsw_sp_span_entry_find(port->mlxsw_sp,
+ port->local_port);
if (span_entry) {
/* Already exists, just take a reference */
span_entry->ref_count++;
@@ -783,12 +783,13 @@ static int mlxsw_sp_span_mirror_add(struct mlxsw_sp_port *from,
}
static void mlxsw_sp_span_mirror_remove(struct mlxsw_sp_port *from,
- struct mlxsw_sp_port *to,
+ u8 destination_port,
enum mlxsw_sp_span_type type)
{
struct mlxsw_sp_span_entry *span_entry;
- span_entry = mlxsw_sp_span_entry_find(to);
+ span_entry = mlxsw_sp_span_entry_find(from->mlxsw_sp,
+ destination_port);
if (!span_entry) {
netdev_err(from->dev, "no span entry found\n");
return;
@@ -1563,14 +1564,12 @@ static void
mlxsw_sp_port_del_cls_matchall_mirror(struct mlxsw_sp_port *mlxsw_sp_port,
struct mlxsw_sp_port_mall_mirror_tc_entry *mirror)
{
- struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
enum mlxsw_sp_span_type span_type;
- struct mlxsw_sp_port *to_port;
- to_port = mlxsw_sp->ports[mirror->to_local_port];
span_type = mirror->ingress ?
MLXSW_SP_SPAN_INGRESS : MLXSW_SP_SPAN_EGRESS;
- mlxsw_sp_span_mirror_remove(mlxsw_sp_port, to_port, span_type);
+ mlxsw_sp_span_mirror_remove(mlxsw_sp_port, mirror->to_local_port,
+ span_type);
}
static int
--
2.9.3
^ permalink raw reply related
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Samudrala, Sridhar @ 2017-09-12 6:27 UTC (permalink / raw)
To: Eric Dumazet, Tom Herbert
Cc: Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <1505188437.15310.137.camel@edumazet-glaptop3.roam.corp.google.com>
On 9/11/2017 8:53 PM, Eric Dumazet wrote:
> On Mon, 2017-09-11 at 20:12 -0700, Tom Herbert wrote:
>
>> Two ints in sock_common for this purpose is quite expensive and the
>> use case for this is limited-- even if a RX->TX queue mapping were
>> introduced to eliminate the queue pair assumption this still won't
>> help if the receive and transmit interfaces are different for the
>> connection. I think we really need to see some very compelling results
>> to be able to justify this.
Will try to collect and post some perf data with symmetric queue
configuration.
> Yes, this is unreasonable cost.
>
> XPS should really cover the case already.
>
Eric,
Can you clarify how XPS covers the RX-> TX queue mapping case?
Is it possible to configure XPS to select TX queue based on the RX queue
of a flow?
IIUC, it is based on the CPU of the thread doing the transmit OR based
on skb->priority to TC mapping?
It may be possible to get this effect if the the threads are pinned to a
core, but if the app threads are
freely moving, i am not sure how XPS can be configured to select the TX
queue based on the RX queue of a flow.
Thanks
Sridhar
^ permalink raw reply
* Re: [PATCH] tcp: TCP_USER_TIMEOUT can not work in tcp_probe_timer()
From: liujian @ 2017-09-12 6:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuznet, yoshfuji, edumazet, ycheng, hkchu, netdev,
weiyongjun1, wangkefeng 00227729
In-Reply-To: <1505143348.15310.118.camel@edumazet-glaptop3.roam.corp.google.com>
Hi,
In the scenario, tcp server side IP changed, and at that memont,
userspace application still send data continuously;
tcp_send_head(sk)'s timestamp always be refreshed.
Here is the packetdrill script:
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 7>
+.1 < . 1:1(0) ack 1 win 65530
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 24) = 24
+0 > P. 1:25(24) ack 1 win 229
+.1 < . 1:1(0) ack 25 win 65530
//change the ipaddress
+1 `ifconfig tun0 192.168.0.10/16`
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+0 `ifconfig tun0 192.168.0.1/16`
+0 < . 1:1(0) ack 1 win 1000
+0 write(4, ..., 24) = -1
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:50: runtime error in write call: Expected result -1 but got 24 with errno 2 (No such file or directory)
real 1m11.364s
user 0m0.028s
sys 0m0.106s
[root@localhost ~]# netstat -toen
Active Internet connections (w/o servers)
Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
tcp 0 504 192.168.0.1:8080 192.0.2.1:33993 ESTABLISHED 0 45453 probe (22.38/0/7)
since the script didn't wait for enough time, here only got 7 probes.
在 2017/9/11 23:22, Eric Dumazet 写道:
> On Mon, 2017-09-11 at 08:13 -0700, Eric Dumazet wrote:
>
>> You can see we got only 3 probes, not 4.
>
> Here is complete packetdrill test showing that code behaves as expected.
>
> 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0 bind(3, ..., ...) = 0
> +0 listen(3, 1) = 0
>
> +0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
> +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
>
> // Client advertises a zero receive window, so we can't send.
> +.1 < . 1:1(0) ack 1 win 0
> +0 accept(3, ..., ...) = 4
>
> +0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
> +0 write(4, ..., 2920) = 2920
>
> // Window probes are scheduled just like RTOs.
> +.3~+.31 > . 0:0(0) ack 1
> +.6~+.62 > . 0:0(0) ack 1
> +1.2~+1.24 > . 0:0(0) ack 1
>
> // Peer opens its window too late !
> +3 < . 1:1(0) ack 1 win 1000
> +0 > R 1:1(0)
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v2] openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Pravin Shelar @ 2017-09-12 4:57 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Pravin Shelar, David S. Miller, Tonghao Zhang,
Linux Kernel Network Developers, ovs dev, linux-kernel,
kernel-janitors
In-Reply-To: <20170911195620.17931-1-christophe.jaillet@wanadoo.fr>
On Mon, Sep 11, 2017 at 12:56 PM, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
> All other error handling paths in this function go through the 'error'
> label. This one should do the same.
>
> Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> I think that the comment above the function could be improved. It looks
> like the commit log which has introduced this function.
>
> I'm also not sure that commit 9cc9a5cb176c is of any help. It is
> supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
> is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
> used by each function is reduced, the overall stack should be the same, if
> not larger.
>
It depends on which function stack depth are are looking at. for some
function it remains same. For nested function it goes down.
> So this commit sounds like adding a bug where the code was fine and states
> to fix an issue but, at the best, only hides it.
>
> Instead of fixing the code with the proposed patch, reverting the initial
> commit could also be considered.
>
> V2: update Subject line
> ---
> net/openvswitch/datapath.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 76cf273a56c7..c3aec6227c91 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
> if (!a[OVS_FLOW_ATTR_KEY]) {
> OVS_NLERR(log,
> "Flow key attribute not present in set flow.");
> - return -EINVAL;
> + error = -EINVAL;
> + goto error;
> }
>
Patch looks good to me.
Acked-by: Pravin B Shelar <pshelar@ovn.org>
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-12 4:56 UTC (permalink / raw)
To: Vinicius Costa Gomes
Cc: Henrik Austad, netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan,
andre.guedes, ivan.briano, jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <87k21am10j.fsf@intel.com>
On Thu, Sep 07, 2017 at 06:29:00PM -0700, Vinicius Costa Gomes wrote:
> >> * Time-aware shaper (802.1Qbv):
> >>
> >> The idea we are currently exploring is to add a "time-aware", priority based
> >> qdisc, that also exposes the Tx queues available and provides a mechanism for
> >> mapping priority <-> traffic class <-> Tx queues in a similar fashion as
> >> mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
> >
> > As far as I know, this is not supported by i210, and if time-aware shaping
> > is enabled in the network - you'll be queued on a bridge until the window
> > opens as time-aware shaping is enforced on the tx-port and not on rx. Is
> > this required in this driver?
>
> Yeah, i210 doesn't support the time-aware shaper. I think the second
> part of your question doesn't really apply, then.
Actually, you can implement 802.1Qbv (as an end station) quite easily
using the i210. I'll show how by posting a series after net-next
opens up again.
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Eric Dumazet @ 2017-09-12 3:53 UTC (permalink / raw)
To: Tom Herbert
Cc: Sridhar Samudrala, Alexander Duyck,
Linux Kernel Network Developers
In-Reply-To: <CALx6S35sM1CDrFuNE+L59Op_wKTRpATLAdRJafihr0mB9+vQ8g@mail.gmail.com>
On Mon, 2017-09-11 at 20:12 -0700, Tom Herbert wrote:
> Two ints in sock_common for this purpose is quite expensive and the
> use case for this is limited-- even if a RX->TX queue mapping were
> introduced to eliminate the queue pair assumption this still won't
> help if the receive and transmit interfaces are different for the
> connection. I think we really need to see some very compelling results
> to be able to justify this.
>
Yes, this is unreasonable cost.
XPS should really cover the case already.
^ permalink raw reply
* Subject: [PATCH] vxlan: only reduce known arp boardcast request to support, virtual IP
From: oc @ 2017-09-12 3:26 UTC (permalink / raw)
To: davem, jbenc, Linux Kernel Network Developers, oc
The purpose of vxlan arp reduce feature is to reply the boardcast
arp request in vtep instead of sending it out to save traffic.
The current implemention drops arp packet, if the ip cannot be
found in neigh table. In the case of virtual IP address, user
defines IP address without management from SDN controller. The IP
address does not exist in neigh table, so the arp boardcast request
from a client can not be sent to the server who owns the virtual IP
address.
This patch allow the arp request to be sent out if:
1. not arp boardcast request
2. cannot be found in neigh table
3. arp record status is not NUD_CONNECTED
The user defined of virtual IP address works while arp reduce still
suppress the arp boardcast for IP address managed by SDN controller
with this patch.
Signed-off-by: Chen Haiquan <oc@yunify.com>
---
drivers/net/vxlan.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d7c49cf1d5e9..913b838b260b 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1473,7 +1473,7 @@ static int arp_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
parp->ar_op != htons(ARPOP_REQUEST) ||
parp->ar_hln != dev->addr_len ||
parp->ar_pln != 4)
- goto out;
+ return 1;
arpptr = (u8 *)parp + sizeof(struct arphdr);
sha = arpptr;
arpptr += dev->addr_len; /* sha */
@@ -1494,7 +1494,7 @@ static int arp_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ return 1;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1526,6 +1526,10 @@ static int arp_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ return 1;
+ } else {
+ /* broadcast unknown arp */
+ return 1;
}
out:
consume_skb(skb);
@@ -1642,7 +1646,7 @@ static int neigh_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
msg = (struct nd_msg *)(iphdr + 1);
if (msg->icmph.icmp6_code != 0 ||
msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
- goto out;
+ return 1;
if (ipv6_addr_loopback(daddr) ||
ipv6_addr_is_multicast(&msg->target))
@@ -1656,7 +1660,7 @@ static int neigh_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ return 1;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1684,6 +1688,10 @@ static int neigh_reduce(struct net_device *dev,
struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ return 1;
+ } else {
+ /* broadcast unknown neigh */
+ return 1;
}
out:
@@ -2266,8 +2274,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff
*skb, struct net_device *dev)
if (vxlan->cfg.flags & VXLAN_F_PROXY) {
eth = eth_hdr(skb);
- if (ntohs(eth->h_proto) == ETH_P_ARP)
- return arp_reduce(dev, skb, vni);
+ if (ntohs(eth->h_proto) == ETH_P_ARP){
+ if (NETDEV_TX_OK == arp_reduce(dev, skb, vni))
+ return NETDEV_TX_OK;
+ }
#if IS_ENABLED(CONFIG_IPV6)
else if (ntohs(eth->h_proto) == ETH_P_IPV6) {
struct ipv6hdr *hdr, _hdr;
@@ -2275,7 +2285,9 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb,
struct net_device *dev)
skb_network_offset(skb),
sizeof(_hdr), &_hdr)) &&
hdr->nexthdr == IPPROTO_ICMPV6)
- return neigh_reduce(dev, skb, vni);
+ if (NETDEV_TX_OK == neigh_reduce(dev,
+ skb, vni))
+ return NETDEV_TX_OK;
}
#endif
}
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Tom Herbert @ 2017-09-12 3:12 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <1504222032-6337-1-git-send-email-sridhar.samudrala@intel.com>
On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
<sridhar.samudrala@intel.com> wrote:
> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be used
> to enable symmetric tx and rx queues on a socket.
>
> This option is specifically useful for epoll based multi threaded workloads
> where each thread handles packets received on a single RX queue . In this model,
> we have noticed that it helps to send the packets on the same TX queue
> corresponding to the queue-pair associated with the RX queue specifically when
> busy poll is enabled with epoll().
>
> Two new fields are added to struct sock_common to cache the last rx ifindex and
> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns the cached
> rx queue when this option is enabled and the TX is happening on the same device.
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
> include/net/request_sock.h | 1 +
> include/net/sock.h | 17 +++++++++++++++++
> include/uapi/asm-generic/socket.h | 2 ++
> net/core/dev.c | 8 +++++++-
> net/core/sock.c | 10 ++++++++++
> net/ipv4/tcp_input.c | 1 +
> net/ipv4/tcp_ipv4.c | 1 +
> net/ipv4/tcp_minisocks.c | 1 +
> 8 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index 23e2205..c3bc12e 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct request_sock *req)
> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
> sk_node_init(&req_to_sk(req)->sk_node);
> sk_tx_queue_clear(req_to_sk(req));
> + req_to_sk(req)->sk_symmetric_queues = sk_listener->sk_symmetric_queues;
> req->saved_syn = NULL;
> refcount_set(&req->rsk_refcnt, 0);
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 03a3625..3421809 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char *msg, ...)
> * @skc_node: main hash linkage for various protocol lookup tables
> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
> * @skc_tx_queue_mapping: tx queue number for this connection
> + * @skc_rx_queue_mapping: rx queue number for this connection
> + * @skc_rx_ifindex: rx ifindex for this connection
> * @skc_flags: place holder for sk_flags
> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
> * @skc_incoming_cpu: record/match cpu processing incoming packets
> * @skc_refcnt: reference count
> + * @skc_symmetric_queues: symmetric tx/rx queues
> *
> * This is the minimal network layer representation of sockets, the header
> * for struct sock and struct inet_timewait_sock.
> @@ -177,6 +180,7 @@ struct sock_common {
> unsigned char skc_reuseport:1;
> unsigned char skc_ipv6only:1;
> unsigned char skc_net_refcnt:1;
> + unsigned char skc_symmetric_queues:1;
> int skc_bound_dev_if;
> union {
> struct hlist_node skc_bind_node;
> @@ -214,6 +218,8 @@ struct sock_common {
> struct hlist_nulls_node skc_nulls_node;
> };
> int skc_tx_queue_mapping;
> + int skc_rx_queue_mapping;
> + int skc_rx_ifindex;
Two ints in sock_common for this purpose is quite expensive and the
use case for this is limited-- even if a RX->TX queue mapping were
introduced to eliminate the queue pair assumption this still won't
help if the receive and transmit interfaces are different for the
connection. I think we really need to see some very compelling results
to be able to justify this.
Thanks,
Tom
^ permalink raw reply
* RE: Extended IDR API
From: Chris Mi @ 2017-09-12 2:27 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jiri Pirko, David S. Miller, Tejun Heo,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
Rehas Sachdeva
In-Reply-To: <20170911211408.GA26747@bombadil.infradead.org>
This improvement is good. But I have a concern that
the parameters of idr_alloc and idr_alloc_ul are different.
I mean in idr_alloc, we have start and end.
In our new API, we keep them. So our design goal is to
make them consistent. Your new API has its advantage surely.
If you want to change it, I don't object personally.
> -----Original Message-----
> From: Matthew Wilcox [mailto:willy@infradead.org]
> Sent: Tuesday, September 12, 2017 5:14 AM
> To: Chris Mi <chrism@mellanox.com>
> Cc: Jiri Pirko <jiri@mellanox.com>; David S. Miller <davem@davemloft.net>;
> Tejun Heo <tj@kernel.org>; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; Rehas Sachdeva <aquannie@gmail.com>
> Subject: Extended IDR API
>
>
> I really don't like your new API. I wish you'd discussed it before merging it.
> Here's my redesign. Does anybody have a suggestion for improvement?
>
> We have a lovely new test-suite for the IDR (in tools/testing/radix-tree) ...
> when adding a new API, it's polite to update the test-suite too.
> Do you have any plans to add test cases?
OK, we will add it once these APIs are stabilized.
Thanks,
Chris
>
> (Compile tested only; I'm at a conference. Also, I didn't check the kerneldoc
> because I don't have Sphinx installed on my laptop.)
>
> From ff45b2a6806cd0e4177c5a10f26c97999164c10c Mon Sep 17 00:00:00 2001
> From: Matthew Wilcox <mawilcox@microsoft.com>
> Date: Mon, 11 Sep 2017 16:16:29 -0400
> Subject: [PATCH] idr: Rewrite extended IDR API
>
> - Rename the API to be 'ul' for unsigned long instead of 'ext'. This
> fits better with other usage in the Linux kernel.
> - idr_alloc() moves back to being a function instead of inline
> - idr_alloc_ul() takes 'nextid' as an in-out parameter like idr_get_next(),
> instead of having 'index' as an out-only parameter.
> - idr_alloc_ul() needs a __must_check to ensure that users don't look at
> the result without checking whether the function succeeded.
> - idr_alloc_ul() takes 'max' rather than 'end', or it is impossible to
> allocate the ULONG_MAX id.
> - idr_replace() can simply take an unsigned long parameter instead of
> an int.
> - idr_remove() and idr_find() are the same as idr_replace().
> - We don't need separate idr_get_free() and idr_get_free_ext().
> - Add kerneldoc for idr_alloc_ul().
>
> Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> ---
> include/linux/idr.h | 75 +++++----------------------------------
> include/linux/radix-tree.h | 17 +--------
> lib/idr.c | 88 +++++++++++++++++++++++++++++++++-------------
> lib/radix-tree.c | 2 +-
> net/sched/act_api.c | 22 ++++++------
> net/sched/cls_flower.c | 16 +++++----
> 6 files changed, 95 insertions(+), 125 deletions(-)
>
> diff --git a/include/linux/idr.h b/include/linux/idr.h index
> 7c3a365f7e12..90faf8279559 100644
> --- a/include/linux/idr.h
> +++ b/include/linux/idr.h
> @@ -81,74 +81,22 @@ static inline void idr_set_cursor(struct idr *idr,
> unsigned int val)
>
> void idr_preload(gfp_t gfp_mask);
>
> -int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
> - unsigned long start, unsigned long end, gfp_t gfp,
> - bool ext);
> -
> -/**
> - * idr_alloc - allocate an id
> - * @idr: idr handle
> - * @ptr: pointer to be associated with the new id
> - * @start: the minimum id (inclusive)
> - * @end: the maximum id (exclusive)
> - * @gfp: memory allocation flags
> - *
> - * Allocates an unused ID in the range [start, end). Returns -ENOSPC
> - * if there are no unused IDs in that range.
> - *
> - * Note that @end is treated as max when <= 0. This is to always allow
> - * using @start + N as @end as long as N is inside integer range.
> - *
> - * Simultaneous modifications to the @idr are not allowed and should be
> - * prevented by the user, usually with a lock. idr_alloc() may be called
> - * concurrently with read-only accesses to the @idr, such as idr_find() and
> - * idr_for_each_entry().
> - */
> -static inline int idr_alloc(struct idr *idr, void *ptr,
> - int start, int end, gfp_t gfp)
> -{
> - unsigned long id;
> - int ret;
> -
> - if (WARN_ON_ONCE(start < 0))
> - return -EINVAL;
> -
> - ret = idr_alloc_cmn(idr, ptr, &id, start, end, gfp, false);
> -
> - if (ret)
> - return ret;
> -
> - return id;
> -}
> -
> -static inline int idr_alloc_ext(struct idr *idr, void *ptr,
> - unsigned long *index,
> - unsigned long start,
> - unsigned long end,
> - gfp_t gfp)
> -{
> - return idr_alloc_cmn(idr, ptr, index, start, end, gfp, true);
> -}
> -
> +int idr_alloc(struct idr *, void *, int start, int end, gfp_t); int
> +__must_check idr_alloc_ul(struct idr *, void *, unsigned long *nextid,
> + unsigned long max, gfp_t);
> int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t); int
> idr_for_each(const struct idr *,
> int (*fn)(int id, void *p, void *data), void *data); void
> *idr_get_next(struct idr *, int *nextid); -void *idr_get_next_ext(struct idr
> *idr, unsigned long *nextid); -void *idr_replace(struct idr *, void *, int id); -
> void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
> +void *idr_get_next_ul(struct idr *, unsigned long *nextid); void
> +*idr_replace(struct idr *, void *, unsigned long id);
> void idr_destroy(struct idr *);
>
> -static inline void *idr_remove_ext(struct idr *idr, unsigned long id)
> +static inline void *idr_remove(struct idr *idr, unsigned long id)
> {
> return radix_tree_delete_item(&idr->idr_rt, id, NULL); }
>
> -static inline void *idr_remove(struct idr *idr, int id) -{
> - return idr_remove_ext(idr, id);
> -}
> -
> static inline void idr_init(struct idr *idr) {
> INIT_RADIX_TREE(&idr->idr_rt, IDR_RT_MARKER); @@ -184,16
> +132,11 @@ static inline void idr_preload_end(void)
> * This function can be called under rcu_read_lock(), given that the leaf
> * pointers lifetimes are correctly managed.
> */
> -static inline void *idr_find_ext(const struct idr *idr, unsigned long id)
> +static inline void *idr_find(const struct idr *idr, unsigned long id)
> {
> return radix_tree_lookup(&idr->idr_rt, id); }
>
> -static inline void *idr_find(const struct idr *idr, int id) -{
> - return idr_find_ext(idr, id);
> -}
> -
> /**
> * idr_for_each_entry - iterate over an idr's elements of a given type
> * @idr: idr handle
> @@ -206,8 +149,8 @@ static inline void *idr_find(const struct idr *idr, int id)
> */
> #define idr_for_each_entry(idr, entry, id) \
> for (id = 0; ((entry) = idr_get_next(idr, &(id))) != NULL; ++id)
> -#define idr_for_each_entry_ext(idr, entry, id) \
> - for (id = 0; ((entry) = idr_get_next_ext(idr, &(id))) != NULL; ++id)
> +#define idr_for_each_entry_ul(idr, entry, id) \
> + for (id = 0; ((entry) = idr_get_next_ul(idr, &(id))) != NULL; ++id)
>
> /**
> * idr_for_each_entry_continue - continue iteration over an idr's elements
> of a given type diff --git a/include/linux/radix-tree.h b/include/linux/radix-
> tree.h index 567ebb5eaab0..8275fc2ed0f4 100644
> --- a/include/linux/radix-tree.h
> +++ b/include/linux/radix-tree.h
> @@ -358,24 +358,9 @@ int radix_tree_split(struct radix_tree_root *,
> unsigned long index, int radix_tree_join(struct radix_tree_root *, unsigned
> long index,
> unsigned new_order, void *);
>
> -void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
> +void __rcu **idr_get_free(struct radix_tree_root *root,
> struct radix_tree_iter *iter, gfp_t gfp,
> unsigned long max);
> -static inline void __rcu **idr_get_free(struct radix_tree_root *root,
> - struct radix_tree_iter *iter,
> - gfp_t gfp,
> - int end)
> -{
> - return idr_get_free_cmn(root, iter, gfp, end > 0 ? end - 1 : INT_MAX);
> -}
> -
> -static inline void __rcu **idr_get_free_ext(struct radix_tree_root *root,
> - struct radix_tree_iter *iter,
> - gfp_t gfp,
> - unsigned long end)
> -{
> - return idr_get_free_cmn(root, iter, gfp, end - 1);
> -}
>
> enum {
> RADIX_TREE_ITER_TAG_MASK = 0x0f, /* tag index in lower nybble
> */
> diff --git a/lib/idr.c b/lib/idr.c
> index 082778cf883e..230879a65d99 100644
> --- a/lib/idr.c
> +++ b/lib/idr.c
> @@ -7,9 +7,26 @@
> DEFINE_PER_CPU(struct ida_bitmap *, ida_bitmap); static
> DEFINE_SPINLOCK(simple_ida_lock);
>
> -int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
> - unsigned long start, unsigned long end, gfp_t gfp,
> - bool ext)
> +/**
> + * idr_alloc_ul - allocate a large ID
> + * @idr: idr handle
> + * @ptr: pointer to be associated with the new ID
> + * @nextid: Pointer to minimum ID to allocate
> + * @max: the maximum ID (inclusive)
> + * @gfp: memory allocation flags
> + *
> + * Allocates an unused ID in the range [*nextid, end] and stores it in
> + * @nextid. Note that @max differs from the @end parameter to
> idr_alloc().
> + *
> + * Simultaneous modifications to the @idr are not allowed and should be
> + * prevented by the user, usually with a lock. idr_alloc_ul() may be
> +called
> + * concurrently with read-only accesses to the @idr, such as idr_find()
> +and
> + * idr_for_each_entry().
> + *
> + * Return: 0 on success or a negative errno on failure (ENOMEM or
> +ENOSPC) */ int idr_alloc_ul(struct idr *idr, void *ptr, unsigned long
> +*nextid,
> + unsigned long max, gfp_t gfp)
> {
> struct radix_tree_iter iter;
> void __rcu **slot;
> @@ -17,22 +34,54 @@ int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned
> long *index,
> if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
> return -EINVAL;
>
> - radix_tree_iter_init(&iter, start);
> - if (ext)
> - slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
> - else
> - slot = idr_get_free(&idr->idr_rt, &iter, gfp, end);
> + radix_tree_iter_init(&iter, *nextid);
> + slot = idr_get_free(&idr->idr_rt, &iter, gfp, max);
> if (IS_ERR(slot))
> return PTR_ERR(slot);
>
> radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
> radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
>
> - if (index)
> - *index = iter.index;
> + *nextid = iter.index;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(idr_alloc_cmn);
> +EXPORT_SYMBOL_GPL(idr_alloc_ul);
> +
> +/**
> + * idr_alloc - allocate an id
> + * @idr: idr handle
> + * @ptr: pointer to be associated with the new id
> + * @start: the minimum id (inclusive)
> + * @end: the maximum id (exclusive)
> + * @gfp: memory allocation flags
> + *
> + * Allocates an unused ID in the range [start, end). Returns -ENOSPC
> + * if there are no unused IDs in that range.
> + *
> + * Note that @end is treated as max when <= 0. This is to always allow
> + * using @start + N as @end as long as N is inside integer range.
> + *
> + * Simultaneous modifications to the @idr are not allowed and should be
> + * prevented by the user, usually with a lock. idr_alloc() may be
> +called
> + * concurrently with read-only accesses to the @idr, such as idr_find()
> +and
> + * idr_for_each_entry().
> + */
> +int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t
> +gfp) {
> + unsigned long id = start;
> + int ret;
> +
> + if (WARN_ON_ONCE(start < 0))
> + return -EINVAL;
> +
> + ret = idr_alloc_ul(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
> +
> + if (ret)
> + return ret;
> +
> + return id;
> +}
> +EXPORT_SYMBOL_GPL(idr_alloc);
>
> /**
> * idr_alloc_cyclic - allocate new idr entry in a cyclical fashion @@ -121,7
> +170,7 @@ void *idr_get_next(struct idr *idr, int *nextid) }
> EXPORT_SYMBOL(idr_get_next);
>
> -void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
> +void *idr_get_next_ul(struct idr *idr, unsigned long *nextid)
> {
> struct radix_tree_iter iter;
> void __rcu **slot;
> @@ -133,7 +182,7 @@ void *idr_get_next_ext(struct idr *idr, unsigned long
> *nextid)
> *nextid = iter.index;
> return rcu_dereference_raw(*slot);
> }
> -EXPORT_SYMBOL(idr_get_next_ext);
> +EXPORT_SYMBOL(idr_get_next_ul);
>
> /**
> * idr_replace - replace pointer for given id @@ -149,16 +198,7 @@
> EXPORT_SYMBOL(idr_get_next_ext);
> * Returns: 0 on success. %-ENOENT indicates that @id was not found.
> * %-EINVAL indicates that @id or @ptr were not valid.
> */
> -void *idr_replace(struct idr *idr, void *ptr, int id) -{
> - if (WARN_ON_ONCE(id < 0))
> - return ERR_PTR(-EINVAL);
> -
> - return idr_replace_ext(idr, ptr, id);
> -}
> -EXPORT_SYMBOL(idr_replace);
> -
> -void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
> +void *idr_replace(struct idr *idr, void *ptr, unsigned long id)
> {
> struct radix_tree_node *node;
> void __rcu **slot = NULL;
> @@ -175,7 +215,7 @@ void *idr_replace_ext(struct idr *idr, void *ptr,
> unsigned long id)
>
> return entry;
> }
> -EXPORT_SYMBOL(idr_replace_ext);
> +EXPORT_SYMBOL(idr_replace);
>
> /**
> * DOC: IDA description
> diff --git a/lib/radix-tree.c b/lib/radix-tree.c index
> 8b1feca1230a..9fcd4e5c5237 100644
> --- a/lib/radix-tree.c
> +++ b/lib/radix-tree.c
> @@ -2139,7 +2139,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp) }
> EXPORT_SYMBOL(ida_pre_get);
>
> -void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
> +void __rcu **idr_get_free(struct radix_tree_root *root,
> struct radix_tree_iter *iter, gfp_t gfp,
> unsigned long max)
> {
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c index
> a306974e2fb4..131817ab3ad3 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -73,7 +73,7 @@ static void free_tcf(struct rcu_head *head) static void
> tcf_idr_remove(struct tcf_idrinfo *idrinfo, struct tc_action *p) {
> spin_lock_bh(&idrinfo->lock);
> - idr_remove_ext(&idrinfo->action_idr, p->tcfa_index);
> + idr_remove(&idrinfo->action_idr, p->tcfa_index);
> spin_unlock_bh(&idrinfo->lock);
> gen_kill_estimator(&p->tcfa_rate_est);
> /*
> @@ -121,7 +121,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo,
> struct sk_buff *skb,
>
> s_i = cb->args[0];
>
> - idr_for_each_entry_ext(idr, p, id) {
> + idr_for_each_entry_ul(idr, p, id) {
> index++;
> if (index < s_i)
> continue;
> @@ -178,7 +178,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo,
> struct sk_buff *skb,
> if (nla_put_string(skb, TCA_KIND, ops->kind))
> goto nla_put_failure;
>
> - idr_for_each_entry_ext(idr, p, id) {
> + idr_for_each_entry_ul(idr, p, id) {
> ret = __tcf_idr_release(p, false, true);
> if (ret == ACT_P_DELETED) {
> module_put(p->ops->owner);
> @@ -216,10 +216,10 @@ EXPORT_SYMBOL(tcf_generic_walker);
>
> static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
> {
> - struct tc_action *p = NULL;
> + struct tc_action *p;
>
> spin_lock_bh(&idrinfo->lock);
> - p = idr_find_ext(&idrinfo->action_idr, index);
> + p = idr_find(&idrinfo->action_idr, index);
> spin_unlock_bh(&idrinfo->lock);
>
> return p;
> @@ -296,10 +296,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32
> index, struct nlattr *est,
> spin_lock_init(&p->tcfa_lock);
> /* user doesn't specify an index */
> if (!index) {
> + idr_index = 1;
> idr_preload(GFP_KERNEL);
> spin_lock_bh(&idrinfo->lock);
> - err = idr_alloc_ext(idr, NULL, &idr_index, 1, 0,
> - GFP_ATOMIC);
> + err = idr_alloc_ul(idr, NULL, &idr_index, UINT_MAX,
> GFP_ATOMIC);
> spin_unlock_bh(&idrinfo->lock);
> idr_preload_end();
> if (err) {
> @@ -309,10 +309,10 @@ int tcf_idr_create(struct tc_action_net *tn, u32
> index, struct nlattr *est,
> }
> p->tcfa_index = idr_index;
> } else {
> + idr_index = index;
> idr_preload(GFP_KERNEL);
> spin_lock_bh(&idrinfo->lock);
> - err = idr_alloc_ext(idr, NULL, NULL, index, index + 1,
> - GFP_ATOMIC);
> + err = idr_alloc_ul(idr, NULL, &idr_index, index, GFP_ATOMIC);
> spin_unlock_bh(&idrinfo->lock);
> idr_preload_end();
> if (err)
> @@ -345,7 +345,7 @@ void tcf_idr_insert(struct tc_action_net *tn, struct
> tc_action *a)
> struct tcf_idrinfo *idrinfo = tn->idrinfo;
>
> spin_lock_bh(&idrinfo->lock);
> - idr_replace_ext(&idrinfo->action_idr, a, a->tcfa_index);
> + idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
> spin_unlock_bh(&idrinfo->lock);
> }
> EXPORT_SYMBOL(tcf_idr_insert);
> @@ -358,7 +358,7 @@ void tcf_idrinfo_destroy(const struct tc_action_ops
> *ops,
> int ret;
> unsigned long id = 1;
>
> - idr_for_each_entry_ext(idr, p, id) {
> + idr_for_each_entry_ul(idr, p, id) {
> ret = __tcf_idr_release(p, false, true);
> if (ret == ACT_P_DELETED)
> module_put(ops->owner);
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index
> 1a267e77c6de..ad30968f587d 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -298,7 +298,7 @@ static void __fl_delete(struct tcf_proto *tp, struct
> cls_fl_filter *f) {
> struct cls_fl_head *head = rtnl_dereference(tp->root);
>
> - idr_remove_ext(&head->handle_idr, f->handle);
> + idr_remove(&head->handle_idr, f->handle);
> list_del_rcu(&f->list);
> if (!tc_skip_hw(f->flags))
> fl_hw_destroy_filter(tp, f);
> @@ -341,7 +341,7 @@ static void *fl_get(struct tcf_proto *tp, u32 handle) {
> struct cls_fl_head *head = rtnl_dereference(tp->root);
>
> - return idr_find_ext(&head->handle_idr, handle);
> + return idr_find(&head->handle_idr, handle);
> }
>
> static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = { @@ -901,8
> +901,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> goto errout;
>
> if (!handle) {
> - err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> - 1, 0x80000000, GFP_KERNEL);
> + idr_index = 1;
> + err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
> + INT_MAX, GFP_KERNEL);
> if (err)
> goto errout;
> fnew->handle = idr_index;
> @@ -910,8 +911,9 @@ static int fl_change(struct net *net, struct sk_buff
> *in_skb,
>
> /* user specifies a handle and it doesn't exist */
> if (handle && !fold) {
> - err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> - handle, handle + 1, GFP_KERNEL);
> + idr_index = handle;
> + err = idr_alloc_ul(&head->handle_idr, fnew, &idr_index,
> + handle, GFP_KERNEL);
> if (err)
> goto errout;
> fnew->handle = idr_index;
> @@ -970,7 +972,7 @@ static int fl_change(struct net *net, struct sk_buff
> *in_skb,
>
> if (fold) {
> fnew->handle = handle;
> - idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
> + idr_replace(&head->handle_idr, fnew, fnew->handle);
> list_replace_rcu(&fold->list, &fnew->list);
> tcf_unbind_filter(tp, &fold->res);
> call_rcu(&fold->rcu, fl_destroy_filter);
> --
> 2.14.1
>
^ permalink raw reply
* RE: [PATCH] mwifiex: remove unnecessary call to memset
From: Xinming Hu @ 2017-09-12 1:36 UTC (permalink / raw)
To: Himanshu Jha, amitkarwar@gmail.com
Cc: Nishant Sarmukadam, Ganapathi Bhat, kvalo@codeaurora.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org
Hi Himanshu,
> -----Original Message-----
> From: Himanshu Jha [mailto:himanshujha199640@gmail.com]
> Sent: 2017年9月11日 20:46
> To: amitkarwar@gmail.com
> Cc: Nishant Sarmukadam <nishants@marvell.com>; Ganapathi Bhat
> <gbhat@marvell.com>; Xinming Hu <huxm@marvell.com>;
> kvalo@codeaurora.org; linux-wireless@vger.kernel.org;
> netdev@vger.kernel.org; Himanshu Jha <himanshujha199640@gmail.com>
> Subject: [EXT] [PATCH] mwifiex: remove unnecessary call to memset
>
> External Email
>
> ----------------------------------------------------------------------
> call to memset to assign 0 value immediately after allocating memory with
> kzalloc is unnecesaary as kzalloc allocates the memory filled with 0 value.
>
Looks find to me, Thanks.
Regards,
Simon
> Semantic patch used to resolve this issue:
>
> @@
> expression e,e2; constant c;
> statement S;
> @@
>
> e = kzalloc(e2, c);
> if(e == NULL) S
> - memset(e, 0, e2);
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/net/wireless/marvell/mwifiex/scan.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c
> b/drivers/net/wireless/marvell/mwifiex/scan.c
> index c9d41ed..8838b88 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1936,8 +1936,6 @@ mwifiex_active_scan_req_for_passive_chan(struct
> mwifiex_private *priv)
> if (!user_scan_cfg)
> return -ENOMEM;
>
> - memset(user_scan_cfg, 0, sizeof(*user_scan_cfg));
> -
> for (id = 0; id < MWIFIEX_USER_SCAN_CHAN_MAX; id++) {
> if (!priv->hidden_chan[id].chan_number)
> break;
> --
> 2.7.4
^ permalink raw reply
* RE: [PATCH] e1000e: changed some expensive calls of udelay to usleep_range
From: Brown, Aaron F @ 2017-09-12 1:34 UTC (permalink / raw)
To: Kardonik Michael, leoyang.li@nxp.com, Kirsher, Jeffrey T,
Brandeburg, Jesse, Shannon Nelson, Wyborny, Carolyn,
Skidmore, Donald C, Vick, Matthew, John Ronciak,
Williams, Mitch A, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Matthew Tan
In-Reply-To: <1504643244-9233-1-git-send-email-Michael.Kardonik@nxp.com>
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Kardonik Michael
> Sent: Tuesday, September 5, 2017 1:27 PM
> To: leoyang.li@nxp.com; michael.kardonik@nxp.com; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; Brandeburg, Jesse
> <jesse.brandeburg@intel.com>; Shannon Nelson
> <shannon.nelson@intel.com>; Wyborny, Carolyn
> <carolyn.wyborny@intel.com>; Skidmore, Donald C
> <donald.c.skidmore@intel.com>; Vick, Matthew <matthew.vick@intel.com>;
> John Ronciak <john.ronciak@intel.com>; Williams, Mitch A
> <mitch.a.williams@intel.com>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Matthew Tan <matthew.tan_1@nxp.com>
> Subject: [PATCH] e1000e: changed some expensive calls of udelay to
> usleep_range
>
> Calls to udelay are not preemtable by userspace so userspace
> applications experience a large (~200us) latency when running on core0.
> Instead usleep_range can be used to be more friendly to userspace
> since it is preemtable. This is due to udelay using busy-wait loops
> while usleep_rang uses hrtimers instead. It is recommended to use
> udelay when the delay is <10us since at that precision overhead of
> usleep_range hrtimer setup causes issues. However, the replaced calls
> are for 50us and 100us so this should not be not an issue.
> There is no open bug that this patch is fixing, but we see a good
> boost in zero loss performance of specific user space application
> (dpdk l3fwd) when this patch is applied: we get from 32% of 10Gb line
> to 49%.
>
> Signed-off-by: Matthew Tan <matthew.tan_1@nxp.com>
> Signed-off-by: Michael Kardonik <michael.kardonik@nxp.com>
>
> ---
> drivers/net/ethernet/intel/e1000e/phy.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
This version continues to crash the same systems as the last (an 82577 and an i218.) As before, when link partner is a lower speed (autoneg set for 100 half for this instance) and I simply bring up the interface I start getting stuck CPU messages:
----------------------------------------------------------------------------
Message from syslogd@u1459 at Sep 11 17:16:25 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:4204]
Message from syslogd@u1459 at Sep 11 17:16:53 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:4204]
----------------------------------------------------------------------------
System becomes unresponsive and on a reboot I am able to pull out a trace file:
----------------------------------------------------------------------------
Sep 11 17:16:00 u1459 kernel: INFO: rcu_sched self-detected stall on CPU
Sep 11 17:16:00 u1459 kernel: 2-...: (20999 ticks this GP) idle=a5a/140000000000001/0 softirq=8232/8232 fqs=5250
Sep 11 17:16:00 u1459 kernel: (t=21000 jiffies g=4013 c=4012 q=616)
Sep 11 17:16:00 u1459 kernel: NMI backtrace for cpu 2
Sep 11 17:16:00 u1459 kernel: CPU: 2 PID: 4204 Comm: kworker/2:0 Tainted: G E 4.13.0_next-queue_dev-queue_41b1c97 #39
Sep 11 17:16:00 u1459 kernel: Hardware name: /NUC5i5MYBE, BIOS MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep 11 17:16:00 u1459 kernel: Workqueue: events linkwatch_event
Sep 11 17:16:00 u1459 kernel: Call Trace:
Sep 11 17:16:00 u1459 kernel: <IRQ>
Sep 11 17:16:00 u1459 kernel: dump_stack+0x51/0x78
Sep 11 17:16:00 u1459 kernel: nmi_cpu_backtrace+0xd3/0xe0
Sep 11 17:16:00 u1459 kernel: ? hw_nmi_get_sample_period+0x20/0x20
Sep 11 17:16:00 u1459 kernel: nmi_trigger_cpumask_backtrace+0xf5/0x130
Sep 11 17:16:00 u1459 kernel: arch_trigger_cpumask_backtrace+0x19/0x20
Sep 11 17:16:00 u1459 kernel: rcu_dump_cpu_stacks+0xb7/0xe0
Sep 11 17:16:00 u1459 kernel: print_cpu_stall+0xfa/0x170
Sep 11 17:16:00 u1459 kernel: ? __update_load_avg_cfs_rq+0x183/0x190
Sep 11 17:16:00 u1459 kernel: ? sched_slice+0x55/0xa0
Sep 11 17:16:00 u1459 kernel: check_cpu_stall+0x110/0x120
Sep 11 17:16:00 u1459 kernel: ? check_preempt_curr+0x73/0x90
Sep 11 17:16:00 u1459 kernel: ? ttwu_do_wakeup+0x2b/0x170
Sep 11 17:16:00 u1459 kernel: ? cpu_needs_another_gp+0x7b/0x80
Sep 11 17:16:00 u1459 kernel: ? notifier_call_chain+0x56/0x80
Sep 11 17:16:00 u1459 kernel: rcu_pending+0x5f/0x180
Sep 11 17:16:00 u1459 kernel: ? raw_notifier_call_chain+0x16/0x20
Sep 11 17:16:00 u1459 kernel: ? timekeeping_update+0xd9/0x130
Sep 11 17:16:00 u1459 kernel: rcu_check_callbacks+0x8a/0x1b0
Sep 11 17:16:00 u1459 kernel: ? account_system_index_time+0x6b/0x80
Sep 11 17:16:00 u1459 kernel: update_process_times+0x39/0x70
Sep 11 17:16:00 u1459 kernel: tick_sched_handle+0x37/0x70
Sep 11 17:16:00 u1459 kernel: tick_sched_timer+0x52/0xa0
Sep 11 17:16:00 u1459 kernel: __run_hrtimer+0x77/0x1b0
Sep 11 17:16:00 u1459 kernel: ? tick_nohz_handler+0xc0/0xc0
Sep 11 17:16:00 u1459 kernel: ? ktime_get+0x5a/0xd0
Sep 11 17:16:00 u1459 kernel: __hrtimer_run_queues+0x67/0x90
Sep 11 17:16:00 u1459 kernel: hrtimer_interrupt+0xa4/0x1d0
Sep 11 17:16:00 u1459 kernel: smp_apic_timer_interrupt+0x6b/0x140
Sep 11 17:16:00 u1459 kernel: ? irq_exit+0xa2/0xc0
Sep 11 17:16:00 u1459 kernel: ? do_IRQ+0x60/0xf0
Sep 11 17:16:00 u1459 kernel: apic_timer_interrupt+0x90/0xa0
Sep 11 17:16:00 u1459 kernel: </IRQ>
Sep 11 17:16:00 u1459 kernel: RIP: 0010:queued_spin_lock_slowpath+0x56/0x1d0
Sep 11 17:16:00 u1459 kernel: RSP: 0018:ffffc9000223f8c8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff10
Sep 11 17:16:00 u1459 kernel: RAX: 0000000000000101 RBX: ffff8802399f8000 RCX: 0000000000000101
Sep 11 17:16:00 u1459 kernel: RDX: ffffc9000223f948 RSI: 0000000000000001 RDI: ffff8802399fb1a0
Sep 11 17:16:00 u1459 kernel: RBP: ffffc9000223f968 R08: 0000000000000001 R09: ffff88024a1cf1a4
Sep 11 17:16:00 u1459 kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff8802399f8840
Sep 11 17:16:00 u1459 kernel: R13: ffff88024a1cf0e4 R14: ffff88024a1cf0e4 R15: ffffc9000223fa48
Sep 11 17:16:00 u1459 kernel: ? __schedule+0x2ba/0x6d0
Sep 11 17:16:00 u1459 kernel: ? netlink_broadcast_filtered+0x15b/0x1a0
Sep 11 17:16:00 u1459 kernel: _raw_spin_lock+0x21/0x30
Sep 11 17:16:00 u1459 kernel: e1000e_get_stats64+0x2b/0x140 [e1000e]
Sep 11 17:16:00 u1459 kernel: dev_get_stats+0x3d/0xc0
Sep 11 17:16:00 u1459 kernel: ? __nla_reserve+0x53/0x70
Sep 11 17:16:00 u1459 kernel: rtnl_fill_stats+0x6b/0x150
Sep 11 17:16:00 u1459 kernel: rtnl_fill_ifinfo+0x542/0x920
Sep 11 17:16:00 u1459 kernel: ? if_nlmsg_size+0xae/0x180
Sep 11 17:16:00 u1459 kernel: rtmsg_ifinfo_build_skb+0x90/0xe0
Sep 11 17:16:00 u1459 kernel: rtmsg_ifinfo_event+0x35/0x60
Sep 11 17:16:00 u1459 kernel: rtmsg_ifinfo+0x13/0x20
Sep 11 17:16:00 u1459 kernel: netdev_state_change+0x46/0x50
Sep 11 17:16:00 u1459 kernel: linkwatch_do_dev+0x3c/0x60
Sep 11 17:16:00 u1459 kernel: __linkwatch_run_queue+0x13a/0x210
Sep 11 17:16:00 u1459 kernel: ? put_prev_entity+0x32/0x530
Sep 11 17:16:00 u1459 kernel: linkwatch_event+0x25/0x30
Sep 11 17:16:00 u1459 kernel: process_one_work+0x193/0x410
Sep 11 17:16:00 u1459 kernel: ? __schedule+0x2ba/0x6d0
Sep 11 17:16:00 u1459 kernel: ? activate_task+0x4e/0xc0
Sep 11 17:16:00 u1459 kernel: ? idle_balance+0x9d/0x2f0
Sep 11 17:16:00 u1459 kernel: ? schedule+0x3a/0xa0
Sep 11 17:16:00 u1459 kernel: worker_thread+0x7c/0x420
Sep 11 17:16:00 u1459 kernel: ? do_task_dead+0x41/0x50
Sep 11 17:16:00 u1459 kernel: ? default_wake_function+0x12/0x20
Sep 11 17:16:00 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 11 17:16:00 u1459 kernel: ? schedule+0x3a/0xa0
Sep 11 17:16:00 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 11 17:16:00 u1459 kernel: kthread+0x111/0x150
Sep 11 17:16:00 u1459 kernel: ? __kthread_init_worker+0x40/0x40
Sep 11 17:16:00 u1459 kernel: ret_from_fork+0x22/0x30
Sep 11 17:16:25 u1459 kernel: watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:4204]
Sep 11 17:16:25 u1459 kernel: Modules linked in: e1000e ptp pps_core ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack libcrc32c ipt_REJECT nf_reject_ipv4 xt_CHECKSUM iptable_mangle iptable_filter ip_tables bridge stp llc nfsd lockd grace nfs_acl auth_rpcgss sunrpc autofs4 ipv6 crc_ccitt dm_mirror dm_region_hash dm_log vhost_net vhost tap tun kvm_intel kvm irqbypass uinput joydev ax88179_178a usbnet mii iTCO_wdt iTCO_vendor_support sg i2c_i801 lpc_ich mfd_core xhci_pci snd_hda_codec_realtek xhci_hcd snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore dm_mod(E) dax(E) ext4(E) jbd2(E) mbcache(E) sd_mod(E) ahci(E) libahci(E) i915(E) drm_kms_helper
(E) drm(E) fb_sys_fops(E)
Sep 11 17:16:25 u1459 kernel: sysimgblt(E) sysfillrect(E) syscopyarea(E) i2c_algo_bit(E) i2c_core(E) iosf_mbi(E) video(E)
Sep 11 17:16:25 u1459 kernel: CPU: 2 PID: 4204 Comm: kworker/2:0 Tainted: G E 4.13.0_next-queue_dev-queue_41b1c97 #39
Sep 11 17:16:25 u1459 kernel: Hardware name: /NUC5i5MYBE, BIOS MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep 11 17:16:25 u1459 kernel: Workqueue: events linkwatch_event
Sep 11 17:16:25 u1459 kernel: task: ffff880239810080 task.stack: ffffc9000223c000
Sep 11 17:16:25 u1459 kernel: RIP: 0010:queued_spin_lock_slowpath+0x58/0x1d0
Sep 11 17:16:25 u1459 kernel: RSP: 0018:ffffc9000223f8c8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff10
Sep 11 17:16:25 u1459 kernel: RAX: 0000000000000101 RBX: ffff8802399f8000 RCX: 0000000000000101
Sep 11 17:16:25 u1459 kernel: RDX: ffffc9000223f948 RSI: 0000000000000001 RDI: ffff8802399fb1a0
Sep 11 17:16:25 u1459 kernel: RBP: ffffc9000223f968 R08: 0000000000000001 R09: ffff88024a1cf1a4
Sep 11 17:16:25 u1459 kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff8802399f8840
Sep 11 17:16:25 u1459 kernel: R13: ffff88024a1cf0e4 R14: ffff88024a1cf0e4 R15: ffffc9000223fa48
Sep 11 17:16:25 u1459 kernel: FS: 0000000000000000(0000) GS:ffff880255d00000(0000) knlGS:0000000000000000
Sep 11 17:16:25 u1459 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Sep 11 17:16:25 u1459 kernel: CR2: 00000030d7e03088 CR3: 000000024a932002 CR4: 00000000003606e0
Sep 11 17:16:25 u1459 kernel: DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Sep 11 17:16:25 u1459 kernel: DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Sep 11 17:16:25 u1459 kernel: Call Trace:
Sep 11 17:16:25 u1459 kernel: ? netlink_broadcast_filtered+0x15b/0x1a0
Sep 11 17:16:25 u1459 kernel: _raw_spin_lock+0x21/0x30
Sep 11 17:16:25 u1459 kernel: e1000e_get_stats64+0x2b/0x140 [e1000e]
Sep 11 17:16:25 u1459 kernel: dev_get_stats+0x3d/0xc0
Sep 11 17:16:25 u1459 kernel: ? __nla_reserve+0x53/0x70
Sep 11 17:16:25 u1459 kernel: rtnl_fill_stats+0x6b/0x150
Sep 11 17:16:25 u1459 kernel: rtnl_fill_ifinfo+0x542/0x920
Sep 11 17:16:25 u1459 kernel: ? if_nlmsg_size+0xae/0x180
Sep 11 17:16:25 u1459 kernel: rtmsg_ifinfo_build_skb+0x90/0xe0
Sep 11 17:16:25 u1459 kernel: rtmsg_ifinfo_event+0x35/0x60
Sep 11 17:16:25 u1459 kernel: rtmsg_ifinfo+0x13/0x20
Sep 11 17:16:25 u1459 kernel: netdev_state_change+0x46/0x50
Sep 11 17:16:25 u1459 kernel: linkwatch_do_dev+0x3c/0x60
Sep 11 17:16:25 u1459 kernel: __linkwatch_run_queue+0x13a/0x210
Sep 11 17:16:25 u1459 kernel: ? put_prev_entity+0x32/0x530
Sep 11 17:16:25 u1459 kernel: linkwatch_event+0x25/0x30
Sep 11 17:16:25 u1459 kernel: process_one_work+0x193/0x410
Sep 11 17:16:25 u1459 kernel: ? __schedule+0x2ba/0x6d0
Sep 11 17:16:25 u1459 kernel: ? activate_task+0x4e/0xc0
Sep 11 17:16:25 u1459 kernel: ? idle_balance+0x9d/0x2f0
Sep 11 17:16:25 u1459 kernel: ? schedule+0x3a/0xa0
Sep 11 17:16:25 u1459 kernel: worker_thread+0x7c/0x420
Sep 11 17:16:25 u1459 kernel: ? do_task_dead+0x41/0x50
Sep 11 17:16:25 u1459 kernel: ? default_wake_function+0x12/0x20
Sep 11 17:16:25 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 11 17:16:25 u1459 kernel: ? schedule+0x3a/0xa0
Sep 11 17:16:25 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 11 17:16:25 u1459 kernel: kthread+0x111/0x150
Sep 11 17:16:25 u1459 kernel: ? __kthread_init_worker+0x40/0x40
Sep 11 17:16:25 u1459 kernel: ret_from_fork+0x22/0x30
Sep 11 17:16:25 u1459 kernel: Code: 75 57 83 fe 01 89 ca 89 f0 41 0f 45 d0 f0 0f b1 17 39 f0 75 e3 83 fa 01 74 1c 8b 07 48 8d 55 e0 84 c0 89 45 e0 74 0a f3 90 8b 07 <84> c0 89 02 75 f6 66 c7 07 01 00 c9 c3 8b 37 48 8d 45 a0 81 fe
...
And repeats untill rebooted.
^ permalink raw reply
* [net-next] dummy: declare dummy devices as enumerated devices
From: Zhang Shengju @ 2017-09-12 1:32 UTC (permalink / raw)
To: davem, netdev, ezegomez
Dummy device name is enumerated by the kernel, let user space be aware
of the naming scheme used by dummy devices:
(visible in /sys/class/net/<iface>/name_assign_type).
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
---
drivers/net/dummy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index d0a1f9c..e31ab3b 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -388,7 +388,7 @@ static int __init dummy_init_one(void)
int err;
dev_dummy = alloc_netdev(sizeof(struct dummy_priv),
- "dummy%d", NET_NAME_UNKNOWN, dummy_setup);
+ "dummy%d", NET_NAME_ENUM, dummy_setup);
if (!dev_dummy)
return -ENOMEM;
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net] net: sched: fix memleak for chain zero
From: Cong Wang @ 2017-09-11 23:43 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jakub Kicinski, mlxsw
In-Reply-To: <20170910140338.GA1860@nanopsycho>
On Sun, Sep 10, 2017 at 7:03 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Sat, Sep 09, 2017 at 08:46:42PM CEST, xiyou.wangcong@gmail.com wrote:
>>On Thu, Sep 7, 2017 at 7:18 PM, David Miller <davem@davemloft.net> wrote:
>>>
>>> This doesn't apply cleanly any more, please respin.
>>>
>>
>>Sigh, you applied this patch despite of strong objections from me.
>>
>>I seriously doubt your tastes now, David. Fine, this code does not
>>deserve a good taste at all, let bugs stay there.
>
> Actually, the reported bug is resolved by this patch. I like your
> approach Cong, we can make it work and follow-up on this patch.
> Please stay calm and be patient...
OK. I already rebased my patches and sent v3.
^ permalink raw reply
* [Patch net v3 3/3] net_sched: carefully handle tcf_block_put()
From: Cong Wang @ 2017-09-11 23:33 UTC (permalink / raw)
To: netdev; +Cc: jiri, jakub.kicinski, jhs, Cong Wang
In-Reply-To: <20170911233332.7594-1-xiyou.wangcong@gmail.com>
As pointed out by Jiri, there is still a race condition between
tcf_block_put() and tcf_chain_destroy() in a RCU callback. There
is no way to make it correct without proper locking or synchronization,
because both operate on a shared list.
Locking is hard, because the only lock we can pick here is a spinlock,
however, in tc_dump_tfilter() we iterate this list with a sleeping
function called (tcf_chain_dump()), which makes using a lock to protect
chain_list almost impossible.
Jiri suggested the idea of holding a refcnt before flushing, this works
because it guarantees us there would be no parallel tcf_chain_destroy()
during the loop, therefore the race condition is gone. But we have to
be very careful with proper synchronization with RCU callbacks.
Suggested-by: Jiri Pirko <jiri@mellanox.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d29e79d98a69..0b2219adf520 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -275,15 +275,27 @@ void tcf_block_put(struct tcf_block *block)
/* XXX: Standalone actions are not allowed to jump to any chain, and
* bound actions should be all removed after flushing. However,
- * filters are destroyed in RCU callbacks, we have to flush and wait
- * for them inside the loop, otherwise we race with RCU callbacks on
- * this list.
+ * filters are destroyed in RCU callbacks, we have to hold the chains
+ * first, otherwise we would always race with RCU callbacks on this list
+ * without proper locking.
*/
- list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
+
+ /* Wait for existing RCU callbacks to cool down. */
+ rcu_barrier();
+
+ /* Hold a refcnt for all chains, except 0, in case they are gone. */
+ list_for_each_entry(chain, &block->chain_list, list)
+ if (chain->index)
+ tcf_chain_hold(chain);
+
+ /* No race on the list, because no chain could be destroyed. */
+ list_for_each_entry(chain, &block->chain_list, list)
tcf_chain_flush(chain);
- rcu_barrier();
- }
+ /* Wait for RCU callbacks to release the reference count. */
+ rcu_barrier();
+
+ /* At this point, all the chains should have refcnt == 1. */
list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
tcf_chain_put(chain);
kfree(block);
--
2.13.0
^ permalink raw reply related
* [Patch net v3 2/3] net_sched: fix reference counting of tc filter chain
From: Cong Wang @ 2017-09-11 23:33 UTC (permalink / raw)
To: netdev; +Cc: jiri, jakub.kicinski, jhs, Cong Wang
In-Reply-To: <20170911233332.7594-1-xiyou.wangcong@gmail.com>
This patch fixes the following ugliness of tc filter chain refcnt:
a) tp proto should hold a refcnt to the chain too. This significantly
simplifies the logic.
b) Chain 0 is no longer special, it is created with refcnt=1 like any
other chains. All the ugliness in tcf_chain_put() can be gone!
c) No need to handle the flushing oddly, because block still holds
chain 0, it can not be released, this guarantees block is the last
user.
d) The race condition with RCU callbacks is easier to handle with just
a rcu_barrier(). Much easier to understand, nothing to hide. Thanks
to the previous patch. Please see also the comments in code.
e) Make the code understandable by humans, much less error-prone.
Fixes: 744a4cf63e52 ("net: sched: fix use after free when tcf_chain_destroy is called multiple times")
Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 47 ++++++++++++++++++++++++++---------------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index c743f03cfebd..d29e79d98a69 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -182,7 +182,7 @@ static struct tcf_chain *tcf_chain_create(struct tcf_block *block,
list_add_tail(&chain->list, &block->chain_list);
chain->block = block;
chain->index = chain_index;
- chain->refcnt = 0;
+ chain->refcnt = 1;
return chain;
}
@@ -194,21 +194,20 @@ static void tcf_chain_flush(struct tcf_chain *chain)
RCU_INIT_POINTER(*chain->p_filter_chain, NULL);
while ((tp = rtnl_dereference(chain->filter_chain)) != NULL) {
RCU_INIT_POINTER(chain->filter_chain, tp->next);
+ tcf_chain_put(chain);
tcf_proto_destroy(tp);
}
}
static void tcf_chain_destroy(struct tcf_chain *chain)
{
- /* May be already removed from the list by the previous call. */
- if (!list_empty(&chain->list))
- list_del_init(&chain->list);
+ list_del(&chain->list);
+ kfree(chain);
+}
- /* There might still be a reference held when we got here from
- * tcf_block_put. Wait for the user to drop reference before free.
- */
- if (!chain->refcnt)
- kfree(chain);
+static void tcf_chain_hold(struct tcf_chain *chain)
+{
+ ++chain->refcnt;
}
struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
@@ -217,24 +216,19 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
struct tcf_chain *chain;
list_for_each_entry(chain, &block->chain_list, list) {
- if (chain->index == chain_index)
- goto incref;
+ if (chain->index == chain_index) {
+ tcf_chain_hold(chain);
+ return chain;
+ }
}
- chain = create ? tcf_chain_create(block, chain_index) : NULL;
-incref:
- if (chain)
- chain->refcnt++;
- return chain;
+ return create ? tcf_chain_create(block, chain_index) : NULL;
}
EXPORT_SYMBOL(tcf_chain_get);
void tcf_chain_put(struct tcf_chain *chain)
{
- /* Destroy unused chain, with exception of chain 0, which is the
- * default one and has to be always present.
- */
- if (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0)
+ if (--chain->refcnt == 0)
tcf_chain_destroy(chain);
}
EXPORT_SYMBOL(tcf_chain_put);
@@ -279,10 +273,19 @@ void tcf_block_put(struct tcf_block *block)
if (!block)
return;
+ /* XXX: Standalone actions are not allowed to jump to any chain, and
+ * bound actions should be all removed after flushing. However,
+ * filters are destroyed in RCU callbacks, we have to flush and wait
+ * for them inside the loop, otherwise we race with RCU callbacks on
+ * this list.
+ */
list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
tcf_chain_flush(chain);
- tcf_chain_destroy(chain);
+ rcu_barrier();
}
+
+ list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
+ tcf_chain_put(chain);
kfree(block);
}
EXPORT_SYMBOL(tcf_block_put);
@@ -360,6 +363,7 @@ static void tcf_chain_tp_insert(struct tcf_chain *chain,
rcu_assign_pointer(*chain->p_filter_chain, tp);
RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain_info));
rcu_assign_pointer(*chain_info->pprev, tp);
+ tcf_chain_hold(chain);
}
static void tcf_chain_tp_remove(struct tcf_chain *chain,
@@ -371,6 +375,7 @@ static void tcf_chain_tp_remove(struct tcf_chain *chain,
if (chain->p_filter_chain && tp == chain->filter_chain)
RCU_INIT_POINTER(*chain->p_filter_chain, next);
RCU_INIT_POINTER(*chain_info->pprev, next);
+ tcf_chain_put(chain);
}
static struct tcf_proto *tcf_chain_tp_find(struct tcf_chain *chain,
--
2.13.0
^ permalink raw reply related
* [Patch net v3 1/3] net_sched: get rid of tcfa_rcu
From: Cong Wang @ 2017-09-11 23:33 UTC (permalink / raw)
To: netdev; +Cc: jiri, jakub.kicinski, jhs, Cong Wang, Eric Dumazet
In-Reply-To: <20170911233332.7594-1-xiyou.wangcong@gmail.com>
gen estimator has been rewritten in commit 1c0d32fde5bd
("net_sched: gen_estimator: complete rewrite of rate estimators"),
the caller is no longer needed to wait for a grace period.
So this patch gets rid of it.
This also completely closes a race condition between action free
path and filter chain add/remove path for the following patch.
Because otherwise the nested RCU callback can't be caught by
rcu_barrier().
Please see also the comments in code.
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/act_api.h | 2 --
net/sched/act_api.c | 17 ++++++++---------
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 8f3d5d8b5ae0..b944e0eb93be 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -34,7 +34,6 @@ struct tc_action {
struct gnet_stats_queue tcfa_qstats;
struct net_rate_estimator __rcu *tcfa_rate_est;
spinlock_t tcfa_lock;
- struct rcu_head tcfa_rcu;
struct gnet_stats_basic_cpu __percpu *cpu_bstats;
struct gnet_stats_queue __percpu *cpu_qstats;
struct tc_cookie *act_cookie;
@@ -50,7 +49,6 @@ struct tc_action {
#define tcf_qstats common.tcfa_qstats
#define tcf_rate_est common.tcfa_rate_est
#define tcf_lock common.tcfa_lock
-#define tcf_rcu common.tcfa_rcu
/* Update lastuse only if needed, to avoid dirtying a cache line.
* We use a temp variable to avoid fetching jiffies twice.
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a306974e2fb4..fcd7dc7b807a 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -53,10 +53,13 @@ static void tcf_action_goto_chain_exec(const struct tc_action *a,
res->goto_tp = rcu_dereference_bh(chain->filter_chain);
}
-static void free_tcf(struct rcu_head *head)
+/* XXX: For standalone actions, we don't need a RCU grace period either, because
+ * actions are always connected to filters and filters are already destroyed in
+ * RCU callbacks, so after a RCU grace period actions are already disconnected
+ * from filters. Readers later can not find us.
+ */
+static void free_tcf(struct tc_action *p)
{
- struct tc_action *p = container_of(head, struct tc_action, tcfa_rcu);
-
free_percpu(p->cpu_bstats);
free_percpu(p->cpu_qstats);
@@ -76,11 +79,7 @@ static void tcf_idr_remove(struct tcf_idrinfo *idrinfo, struct tc_action *p)
idr_remove_ext(&idrinfo->action_idr, p->tcfa_index);
spin_unlock_bh(&idrinfo->lock);
gen_kill_estimator(&p->tcfa_rate_est);
- /*
- * gen_estimator est_timer() might access p->tcfa_lock
- * or bstats, wait a RCU grace period before freeing p
- */
- call_rcu(&p->tcfa_rcu, free_tcf);
+ free_tcf(p);
}
int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
@@ -259,7 +258,7 @@ void tcf_idr_cleanup(struct tc_action *a, struct nlattr *est)
{
if (est)
gen_kill_estimator(&a->tcfa_rate_est);
- call_rcu(&a->tcfa_rcu, free_tcf);
+ free_tcf(a);
}
EXPORT_SYMBOL(tcf_idr_cleanup);
--
2.13.0
^ permalink raw reply related
* [Patch net v3 0/3] net_sched: fix filter chain reference counting
From: Cong Wang @ 2017-09-11 23:33 UTC (permalink / raw)
To: netdev; +Cc: jiri, jakub.kicinski, jhs, Cong Wang
This patchset fixes tc filter chain reference counting and nasty race
conditions with RCU callbacks. Please see each patch for details.
---
v3: Rebase on the latest -net
Add code comment in patch 1
Improve comment and changelog for patch 2
Add patch 3
v2: Add patch 1
Get rid of more ugly code in patch 2
Cong Wang (3):
net_sched: get rid of tcfa_rcu
net_sched: fix reference counting of tc filter chain
net_sched: carefully handle tcf_block_put()
include/net/act_api.h | 2 --
net/sched/act_api.c | 17 +++++++-------
net/sched/cls_api.c | 63 ++++++++++++++++++++++++++++++++-------------------
3 files changed, 48 insertions(+), 34 deletions(-)
--
2.13.0
^ permalink raw reply
* [PATCH net] tcp/dccp: remove reqsk_put() from inet_child_forget()
From: Eric Dumazet @ 2017-09-11 22:58 UTC (permalink / raw)
To: Shankara Pailoor, David Miller
Cc: Linux Kernel Network Developers, Cong Wang, Andrew Zhu Aday
In-Reply-To: <CAASgV=txONWMR2_au12ASV40mP0mf6O=4DLoq3+FfGubqG4NQQ@mail.gmail.com>
From: Eric Dumazet <edumazet@google.com>
Back in linux-4.4, I inadvertently put a call to reqsk_put() in
inet_child_forget(), forgetting it could be called from two different
points.
In the case it is called from inet_csk_reqsk_queue_add(), we want to
keep the reference on the request socket, since it is released later by
the caller (tcp_v{4|6}_rcv())
This bug never showed up because atomic_dec_and_test() was not signaling
the underflow, and SLAB_DESTROY_BY RCU semantic for request sockets
prevented the request to be put in quarantine.
Recent conversion of socket refcount from atomic_t to refcount_t finally
exposed the bug.
So move the reqsk_put() to inet_csk_listen_stop() to fix this.
Thanks to Shankara Pailoor for using syzkaller and providing
a nice set of .config and C repro.
WARNING: CPU: 2 PID: 4277 at lib/refcount.c:186
refcount_sub_and_test+0x167/0x1b0 lib/refcount.c:186
Kernel panic - not syncing: panic_on_warn set ...
CPU: 2 PID: 4277 Comm: syz-executor0 Not tainted 4.13.0-rc7 #3
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
Ubuntu-1.8.2-1ubuntu1 04/01/2014
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0xf7/0x1aa lib/dump_stack.c:52
panic+0x1ae/0x3a7 kernel/panic.c:180
__warn+0x1c4/0x1d9 kernel/panic.c:541
report_bug+0x211/0x2d0 lib/bug.c:183
fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:190
do_trap_no_signal arch/x86/kernel/traps.c:224 [inline]
do_trap+0x260/0x390 arch/x86/kernel/traps.c:273
do_error_trap+0x118/0x340 arch/x86/kernel/traps.c:310
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:323
invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:846
RIP: 0010:refcount_sub_and_test+0x167/0x1b0 lib/refcount.c:186
RSP: 0018:ffff88006e006b60 EFLAGS: 00010286
RAX: 0000000000000026 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000026 RSI: 1ffff1000dc00d2c RDI: ffffed000dc00d60
RBP: ffff88006e006bf0 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 1ffff1000dc00d6d
R13: 00000000ffffffff R14: 0000000000000001 R15: ffff88006ce9d340
refcount_dec_and_test+0x1a/0x20 lib/refcount.c:211
reqsk_put+0x71/0x2b0 include/net/request_sock.h:123
tcp_v4_rcv+0x259e/0x2e20 net/ipv4/tcp_ipv4.c:1729
ip_local_deliver_finish+0x2e2/0xba0 net/ipv4/ip_input.c:216
NF_HOOK include/linux/netfilter.h:248 [inline]
ip_local_deliver+0x1ce/0x6d0 net/ipv4/ip_input.c:257
dst_input include/net/dst.h:477 [inline]
ip_rcv_finish+0x8db/0x19c0 net/ipv4/ip_input.c:397
NF_HOOK include/linux/netfilter.h:248 [inline]
ip_rcv+0xc3f/0x17d0 net/ipv4/ip_input.c:488
__netif_receive_skb_core+0x1fb7/0x31f0 net/core/dev.c:4298
__netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4336
process_backlog+0x1c5/0x6d0 net/core/dev.c:5102
napi_poll net/core/dev.c:5499 [inline]
net_rx_action+0x6d3/0x14a0 net/core/dev.c:5565
__do_softirq+0x2cb/0xb2d kernel/softirq.c:284
do_softirq_own_stack+0x1c/0x30 arch/x86/entry/entry_64.S:898
</IRQ>
do_softirq.part.16+0x63/0x80 kernel/softirq.c:328
do_softirq kernel/softirq.c:176 [inline]
__local_bh_enable_ip+0x84/0x90 kernel/softirq.c:181
local_bh_enable include/linux/bottom_half.h:31 [inline]
rcu_read_unlock_bh include/linux/rcupdate.h:705 [inline]
ip_finish_output2+0x8ad/0x1360 net/ipv4/ip_output.c:231
ip_finish_output+0x74e/0xb80 net/ipv4/ip_output.c:317
NF_HOOK_COND include/linux/netfilter.h:237 [inline]
ip_output+0x1cc/0x850 net/ipv4/ip_output.c:405
dst_output include/net/dst.h:471 [inline]
ip_local_out+0x95/0x160 net/ipv4/ip_output.c:124
ip_queue_xmit+0x8c6/0x1810 net/ipv4/ip_output.c:504
tcp_transmit_skb+0x1963/0x3320 net/ipv4/tcp_output.c:1123
tcp_send_ack.part.35+0x38c/0x620 net/ipv4/tcp_output.c:3575
tcp_send_ack+0x49/0x60 net/ipv4/tcp_output.c:3545
tcp_rcv_synsent_state_process net/ipv4/tcp_input.c:5795 [inline]
tcp_rcv_state_process+0x4876/0x4b60 net/ipv4/tcp_input.c:5930
tcp_v4_do_rcv+0x58a/0x820 net/ipv4/tcp_ipv4.c:1483
sk_backlog_rcv include/net/sock.h:907 [inline]
__release_sock+0x124/0x360 net/core/sock.c:2223
release_sock+0xa4/0x2a0 net/core/sock.c:2715
inet_wait_for_connect net/ipv4/af_inet.c:557 [inline]
__inet_stream_connect+0x671/0xf00 net/ipv4/af_inet.c:643
inet_stream_connect+0x58/0xa0 net/ipv4/af_inet.c:682
SYSC_connect+0x204/0x470 net/socket.c:1628
SyS_connect+0x24/0x30 net/socket.c:1609
entry_SYSCALL_64_fastpath+0x18/0xad
RIP: 0033:0x451e59
RSP: 002b:00007f474843fc08 EFLAGS: 00000216 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 0000000000718000 RCX: 0000000000451e59
RDX: 0000000000000010 RSI: 0000000020002000 RDI: 0000000000000007
RBP: 0000000000000046 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000216 R12: 0000000000000000
R13: 00007ffc040a0f8f R14: 00007f47484409c0 R15: 0000000000000000
Fixes: ebb516af60e1 ("tcp/dccp: fix race at listener dismantle phase")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Shankara Pailoor <sp3485@columbia.edu>
Tested-by: Shankara Pailoor <sp3485@columbia.edu>
---
net/ipv4/inet_connection_sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 4089c013cb03b12e31ddffbb7ae903542c012ae0..b9c64b40a83af1e151f553ba0e624fae5060ffd0 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -916,7 +916,6 @@ static void inet_child_forget(struct sock *sk, struct request_sock *req,
tcp_sk(child)->fastopen_rsk = NULL;
}
inet_csk_destroy_sock(child);
- reqsk_put(req);
}
struct sock *inet_csk_reqsk_queue_add(struct sock *sk,
@@ -987,6 +986,7 @@ void inet_csk_listen_stop(struct sock *sk)
sock_hold(child);
inet_child_forget(sk, req, child);
+ reqsk_put(req);
bh_unlock_sock(child);
local_bh_enable();
sock_put(child);
^ permalink raw reply related
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Alexander Duyck @ 2017-09-11 22:50 UTC (permalink / raw)
To: Tom Herbert
Cc: Samudrala, Sridhar, Alexander Duyck,
Linux Kernel Network Developers
In-Reply-To: <CALx6S37XvbjcRon6g+gS19wj14SebL4qKFrVeGyagG0K2EStzA@mail.gmail.com>
On Mon, Sep 11, 2017 at 3:07 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, Sep 11, 2017 at 9:49 AM, Samudrala, Sridhar
> <sridhar.samudrala@intel.com> wrote:
>> On 9/10/2017 8:19 AM, Tom Herbert wrote:
>>>
>>> On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
>>> <sridhar.samudrala@intel.com> wrote:
>>>>
>>>> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be
>>>> used
>>>> to enable symmetric tx and rx queues on a socket.
>>>>
>>>> This option is specifically useful for epoll based multi threaded
>>>> workloads
>>>> where each thread handles packets received on a single RX queue . In this
>>>> model,
>>>> we have noticed that it helps to send the packets on the same TX queue
>>>> corresponding to the queue-pair associated with the RX queue specifically
>>>> when
>>>> busy poll is enabled with epoll().
>>>>
>>>> Two new fields are added to struct sock_common to cache the last rx
>>>> ifindex and
>>>> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns
>>>> the cached
>>>> rx queue when this option is enabled and the TX is happening on the same
>>>> device.
>>>>
>>>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>>> ---
>>>> include/net/request_sock.h | 1 +
>>>> include/net/sock.h | 17 +++++++++++++++++
>>>> include/uapi/asm-generic/socket.h | 2 ++
>>>> net/core/dev.c | 8 +++++++-
>>>> net/core/sock.c | 10 ++++++++++
>>>> net/ipv4/tcp_input.c | 1 +
>>>> net/ipv4/tcp_ipv4.c | 1 +
>>>> net/ipv4/tcp_minisocks.c | 1 +
>>>> 8 files changed, 40 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
>>>> index 23e2205..c3bc12e 100644
>>>> --- a/include/net/request_sock.h
>>>> +++ b/include/net/request_sock.h
>>>> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct
>>>> request_sock *req)
>>>> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
>>>> sk_node_init(&req_to_sk(req)->sk_node);
>>>> sk_tx_queue_clear(req_to_sk(req));
>>>> + req_to_sk(req)->sk_symmetric_queues =
>>>> sk_listener->sk_symmetric_queues;
>>>> req->saved_syn = NULL;
>>>> refcount_set(&req->rsk_refcnt, 0);
>>>>
>>>> diff --git a/include/net/sock.h b/include/net/sock.h
>>>> index 03a3625..3421809 100644
>>>> --- a/include/net/sock.h
>>>> +++ b/include/net/sock.h
>>>> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char
>>>> *msg, ...)
>>>> * @skc_node: main hash linkage for various protocol lookup tables
>>>> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
>>>> * @skc_tx_queue_mapping: tx queue number for this connection
>>>> + * @skc_rx_queue_mapping: rx queue number for this connection
>>>> + * @skc_rx_ifindex: rx ifindex for this connection
>>>> * @skc_flags: place holder for sk_flags
>>>> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
>>>> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
>>>> * @skc_incoming_cpu: record/match cpu processing incoming packets
>>>> * @skc_refcnt: reference count
>>>> + * @skc_symmetric_queues: symmetric tx/rx queues
>>>> *
>>>> * This is the minimal network layer representation of sockets, the
>>>> header
>>>> * for struct sock and struct inet_timewait_sock.
>>>> @@ -177,6 +180,7 @@ struct sock_common {
>>>> unsigned char skc_reuseport:1;
>>>> unsigned char skc_ipv6only:1;
>>>> unsigned char skc_net_refcnt:1;
>>>> + unsigned char skc_symmetric_queues:1;
>>>> int skc_bound_dev_if;
>>>> union {
>>>> struct hlist_node skc_bind_node;
>>>> @@ -214,6 +218,8 @@ struct sock_common {
>>>> struct hlist_nulls_node skc_nulls_node;
>>>> };
>>>> int skc_tx_queue_mapping;
>>>> + int skc_rx_queue_mapping;
>>>> + int skc_rx_ifindex;
>>>> union {
>>>> int skc_incoming_cpu;
>>>> u32 skc_rcv_wnd;
>>>> @@ -324,6 +330,8 @@ struct sock {
>>>> #define sk_nulls_node __sk_common.skc_nulls_node
>>>> #define sk_refcnt __sk_common.skc_refcnt
>>>> #define sk_tx_queue_mapping __sk_common.skc_tx_queue_mapping
>>>> +#define sk_rx_queue_mapping __sk_common.skc_rx_queue_mapping
>>>> +#define sk_rx_ifindex __sk_common.skc_rx_ifindex
>>>>
>>>> #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin
>>>> #define sk_dontcopy_end __sk_common.skc_dontcopy_end
>>>> @@ -340,6 +348,7 @@ struct sock {
>>>> #define sk_reuseport __sk_common.skc_reuseport
>>>> #define sk_ipv6only __sk_common.skc_ipv6only
>>>> #define sk_net_refcnt __sk_common.skc_net_refcnt
>>>> +#define sk_symmetric_queues __sk_common.skc_symmetric_queues
>>>> #define sk_bound_dev_if __sk_common.skc_bound_dev_if
>>>> #define sk_bind_node __sk_common.skc_bind_node
>>>> #define sk_prot __sk_common.skc_prot
>>>> @@ -1676,6 +1685,14 @@ static inline int sk_tx_queue_get(const struct
>>>> sock *sk)
>>>> return sk ? sk->sk_tx_queue_mapping : -1;
>>>> }
>>>>
>>>> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff
>>>> *skb)
>>>> +{
>>>> + if (sk->sk_symmetric_queues) {
>>>> + sk->sk_rx_ifindex = skb->skb_iif;
>>>> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
>>>> + }
>>>> +}
>>>> +
>>>> static inline void sk_set_socket(struct sock *sk, struct socket *sock)
>>>> {
>>>> sk_tx_queue_clear(sk);
>>>> diff --git a/include/uapi/asm-generic/socket.h
>>>> b/include/uapi/asm-generic/socket.h
>>>> index e47c9e4..f6b416e 100644
>>>> --- a/include/uapi/asm-generic/socket.h
>>>> +++ b/include/uapi/asm-generic/socket.h
>>>> @@ -106,4 +106,6 @@
>>>>
>>>> #define SO_ZEROCOPY 60
>>>>
>>>> +#define SO_SYMMETRIC_QUEUES 61
>>>> +
>>>> #endif /* __ASM_GENERIC_SOCKET_H */
>>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>>> index 270b547..d96cda8 100644
>>>> --- a/net/core/dev.c
>>>> +++ b/net/core/dev.c
>>>> @@ -3322,7 +3322,13 @@ static u16 __netdev_pick_tx(struct net_device
>>>> *dev, struct sk_buff *skb)
>>>>
>>>> if (queue_index < 0 || skb->ooo_okay ||
>>>> queue_index >= dev->real_num_tx_queues) {
>>>> - int new_index = get_xps_queue(dev, skb);
>>>> + int new_index = -1;
>>>> +
>>>> + if (sk && sk->sk_symmetric_queues && dev->ifindex ==
>>>> sk->sk_rx_ifindex)
>>>> + new_index = sk->sk_rx_queue_mapping;
>>>> +
>>>> + if (new_index < 0 || new_index >=
>>>> dev->real_num_tx_queues)
>>>> + new_index = get_xps_queue(dev, skb);
>>>
>>> This enforces that notion of queue pairs which is not universal
>>> concept to NICs. There are many devices and instances where we
>>> purposely avoid having a 1-1 relationship between rx and tx queues.
>>
>>
>> Yes. This patch assumes that TX and RX queues come in pairs.
>>
>>> An alternative might be to create a rx queue to tx queue map, add the
>>> rx queue argument to get_xps_queue, and then that function can
>>> consider the mapping. The administrator can configure the mapping as
>>> appropriate and can select which rx queues are subject to the mapping.
>>
>> This alternative looks much cleaner and doesn't require the apps to
>> configure the
>> queues. Do we need to support 1 to many rx to tx queue mappings?
>> For our symmetric queues usecase, where a single application thread is
>> associated with
>> 1 queue-pair, 1-1 mapping is sufficient.
>> Do you see any usecase where it is useful to support 1-many mappings?
>> I guess i can add a sysfs entry per rx-queue to setup a tx-queue OR
>> tx-queue-map.
>
> There is no reason do disallow 1 to many, XPS already does that. In
> fact, the mapping algorithm in XSP is pretty much what is needed where
> instead of mapping a CPU to a queue set, this just maps a rx queue to
> queue set. ooo handling can still be done, although it might be less
> critical in this case.
>
> Tom
Actually I wonder if we couldn't re-purpose the queue_mapping field
that is already there in the sk_buff for this. It might provide a more
elegant way to deal with the logic for already dealing with the
recorded Rx queue at the start of __skb_tx_hash. Then if the socket
wants this it could send down the packet with the queue_mapping field
set and we would be using some the map to figure out the Rx to Tx
mapping for either routing/bridging or this socket option.
Anyway just a thought.
- Alex
^ permalink raw reply
* Re: broken vlan support on Realtek RTL8111/8168/8411 rev 9
From: Francois Romieu @ 2017-09-11 22:23 UTC (permalink / raw)
To: Benoit Panizzon; +Cc: netdev
In-Reply-To: <20170910191645.549d2a3a@krup.woody.ch>
Benoit Panizzon <benoit.panizzon@imp.ch> :
[...]
> It's not marked as fixed but:
>
> when I try to turn offloading off I get:
>
> # /sbin/ethtool -K eth0 rx-vlan-offload off
> ethtool: bad command line argument(s)
> For more information run ethtool -h
>
> Could anyone give me a hint how to further debug the problem or disable
> vlan offloading completely?
ethtool -K eth0 rxvlan off
For my reward, please send a complete dmesg where the messages from
the vanilla r8169 module appear for the rev 09 card (r81..f ?). I won't
dig it right now.
--
Ueimor
^ permalink raw reply
* Re: Kernel 4.13.0-rc4-next-20170811 - IP Routing / Forwarding performance vs Core/RSS number / HT on
From: Paweł Staszewski @ 2017-09-11 22:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paolo Abeni, Jesper Dangaard Brouer,
Linux Kernel Network Developers, Alexander Duyck
In-Reply-To: <0123de5c-a7ad-7e62-b7d0-cd19bd34c573@itcare.pl>
Another test for this patch with linux-next tree
with patch:
bwm-ng v0.6.1 (probing every 1.000s), press 'h' for help
input: /proc/net/dev type: rate
- iface Rx Tx Total
==============================================================================
vlan1004: 1.00 P/s 606842.31 P/s 606843.31 P/s
lo: 0.00 P/s 0.00 P/s
0.00 P/s
vlan1016: 0.00 P/s 607730.56 P/s 607730.56 P/s
vlan1020: 0.00 P/s 606891.25 P/s 606891.25 P/s
vlan1018: 0.00 P/s 607580.88 P/s 607580.88 P/s
vlan1014: 0.00 P/s 607606.81 P/s 607606.81 P/s
vlan1005: 0.00 P/s 606788.44 P/s 606788.44 P/s
enp2s0f0: 2.00 P/s 2.00 P/s
3.99 P/s
vlan1017: 0.00 P/s 607643.75 P/s 607643.75 P/s
enp132s0: 13079658.00 P/s 0.00 P/s 13079658.00 P/s
vlan1000: 0.00 P/s 604409.19 P/s 604409.19 P/s
vlan1010: 0.00 P/s 606984.06 P/s 606984.06 P/s
vlan1019: 0.00 P/s 607452.12 P/s 607452.12 P/s
vlan1008: 0.00 P/s 606803.44 P/s 606803.44 P/s
vlan1011: 0.00 P/s 607048.94 P/s 607048.94 P/s
vlan1001: 0.00 P/s 606773.50 P/s 606773.50 P/s
vlan1006: 0.00 P/s 606811.38 P/s 606811.38 P/s
vlan1012: 0.00 P/s 607051.94 P/s 607051.94 P/s
vlan1013: 0.00 P/s 607067.88 P/s 607067.88 P/s
enp4s0: 2.00 P/s 13020803.00 P/s 13020805.00 P/s
vlan1007: 0.00 P/s 606798.44 P/s 606798.44 P/s
vlan1002: 0.00 P/s 606840.31 P/s 606840.31 P/s
vlan1009: 0.00 P/s 606809.38 P/s 606809.38 P/s
enp2s0f1: 100.80 P/s 0.00 P/s
100.80 P/s
vlan1015: 0.00 P/s 607089.81 P/s 607089.81 P/s
vlan1003: 1.00 P/s 606928.19 P/s 606929.19 P/s
------------------------------------------------------------------------------
total: 13079765.00 P/s 25766758.00 P/s 38846524.00 P/s
13Mpps forwarded (32cores active for two mlx5 nics)
80% cpu load (20%idle all cores)
PerfTop: 126552 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
cycles], (all, 32 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
8.25% [kernel] [k] fib_table_lookup
7.98% [kernel] [k] do_raw_spin_lock
6.20% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
4.21% [kernel] [k] mlx5e_xmit
3.37% [kernel] [k] __dev_queue_xmit
2.95% [kernel] [k] ip_rcv
2.72% [kernel] [k] ipt_do_table
2.24% [kernel] [k] ip_finish_output2
2.22% [kernel] [k] __netif_receive_skb_core
2.17% [kernel] [k] ip_forward
2.15% [kernel] [k] __build_skb
1.99% [kernel] [k] ip_route_input_rcu
1.70% [kernel] [k] mlx5e_txwqe_complete
1.54% [kernel] [k] dev_gro_receive
1.45% [kernel] [k] mlx5_cqwq_get_cqe
1.38% [kernel] [k] udp_v4_early_demux
1.35% [kernel] [k] netif_skb_features
1.33% [kernel] [k] inet_gro_receive
1.29% [kernel] [k] dev_hard_start_xmit
1.27% [kernel] [k] ip_rcv_finish
1.19% [kernel] [k] mlx5e_build_rx_skb
1.15% [kernel] [k] __netdev_pick_tx
1.11% [kernel] [k] kmem_cache_alloc
1.09% [kernel] [k] mlx5e_poll_tx_cq
1.07% [kernel] [k] mlx5e_txwqe_build_dsegs
1.00% [kernel] [k] vlan_dev_hard_start_xmit
0.90% [kernel] [k] __napi_alloc_skb
0.87% [kernel] [k] validate_xmit_skb
0.87% [kernel] [k] read_tsc
0.83% [kernel] [k] napi_gro_receive
0.79% [kernel] [k] skb_network_protocol
0.79% [kernel] [k] sch_direct_xmit
0.78% [kernel] [k] __local_bh_enable_ip
0.78% [kernel] [k] netdev_pick_tx
0.75% [kernel] [k] __udp4_lib_lookup
0.72% [kernel] [k] netif_receive_skb_internal
0.71% [kernel] [k] page_frag_free
0.71% [kernel] [k] deliver_ptype_list_skb
0.70% [kernel] [k] fib_validate_source
0.69% [kernel] [k] mlx5_cqwq_get_cqe
0.69% [kernel] [k] __netif_receive_skb
0.68% [kernel] [k] vlan_passthru_hard_header
0.61% [kernel] [k] rt_cache_valid
0.59% [kernel] [k] iptable_filter_hook
Without patch:
12,7Mpps forwarded (32cores active for two mlx5 nics)
100% cpu load ( rx drops on receiving side )
TX is about 13.05Mpps from pktgen
bwm-ng v0.6.1 (probing every 1.000s), press 'h' for help
input: /proc/net/dev type: rate
/ iface Rx Tx Total
==============================================================================
vlan1004: 0.00 P/s 589709.31 P/s 589709.31 P/s
lo: 0.00 P/s 0.00 P/s
0.00 P/s
vlan1016: 0.00 P/s 589495.50 P/s 589495.50 P/s
vlan1020: 0.00 P/s 589968.06 P/s 589968.06 P/s
vlan1018: 0.00 P/s 589896.12 P/s 589896.12 P/s
vlan1014: 0.00 P/s 589496.50 P/s 589496.50 P/s
vlan1005: 0.00 P/s 589502.50 P/s 589502.50 P/s
enp2s0f0: 42.96 P/s 2.00 P/s
44.96 P/s
vlan1017: 0.00 P/s 589508.50 P/s 589508.50 P/s
enp132s0: 12700689.00 P/s 0.00 P/s 12700689.00 P/s
vlan1000: 0.00 P/s 587671.38 P/s 587671.38 P/s
vlan1010: 0.00 P/s 589330.69 P/s 589330.69 P/s
vlan1019: 0.00 P/s 589808.19 P/s 589808.19 P/s
vlan1008: 0.00 P/s 589283.75 P/s 589283.75 P/s
vlan1011: 0.00 P/s 589482.56 P/s 589482.56 P/s
vlan1001: 0.00 P/s 589971.06 P/s 589971.06 P/s
vlan1006: 0.00 P/s 589785.25 P/s 589785.25 P/s
vlan1012: 0.00 P/s 589494.50 P/s 589494.50 P/s
vlan1013: 0.00 P/s 589495.50 P/s 589495.50 P/s
enp4s0: 0.00 P/s 12601841.00 P/s 12601841.00 P/s
vlan1007: 0.00 P/s 589537.50 P/s 589537.50 P/s
vlan1002: 0.00 P/s 589943.06 P/s 589943.06 P/s
vlan1009: 0.00 P/s 589306.69 P/s 589306.69 P/s
enp2s0f1: 99.90 P/s 0.00 P/s
99.90 P/s
vlan1015: 0.00 P/s 589553.44 P/s 589553.44 P/s
vlan1003: 0.00 P/s 589823.19 P/s 589823.19 P/s
------------------------------------------------------------------------------
total: 12700832.00 P/s 24981906.00 P/s 37682740.00 P/s
PerfTop: 124056 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
cycles], (all, 32 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
7.77% [kernel] [k] fib_table_lookup
7.37% [kernel] [k] do_raw_spin_lock
5.43% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
3.84% [kernel] [k] mlx5e_xmit
3.52% [kernel] [k] rt_cache_valid
3.46% [kernel] [k] ip_finish_output2
3.45% [kernel] [k] skb_dst_force
2.96% [kernel] [k] dst_release
2.62% [kernel] [k] __dev_queue_xmit
2.54% [kernel] [k] ip_rcv
2.26% [kernel] [k] ipt_do_table
1.97% [kernel] [k] __build_skb
1.94% [kernel] [k] __netif_receive_skb_core
1.94% [kernel] [k] ip_route_input_rcu
1.83% [kernel] [k] ip_forward
1.52% [kernel] [k] mlx5e_txwqe_complete
1.42% [kernel] [k] dev_gro_receive
1.31% [kernel] [k] mlx5_cqwq_get_cqe
1.28% [kernel] [k] netif_skb_features
1.21% [kernel] [k] ip_rcv_finish
1.18% [kernel] [k] inet_gro_receive
1.17% [kernel] [k] udp_v4_early_demux
1.14% [kernel] [k] dev_hard_start_xmit
1.03% [kernel] [k] mlx5e_txwqe_build_dsegs
1.02% [kernel] [k] mlx5e_poll_tx_cq
1.02% [kernel] [k] mlx5e_build_rx_skb
1.00% [kernel] [k] kmem_cache_alloc
0.95% [kernel] [k] __netdev_pick_tx
0.93% [kernel] [k] vlan_dev_hard_start_xmit
0.82% [kernel] [k] __napi_alloc_skb
0.79% [kernel] [k] read_tsc
0.76% [kernel] [k] validate_xmit_skb
0.75% [kernel] [k] napi_gro_receive
0.71% [kernel] [k] __local_bh_enable_ip
0.70% [kernel] [k] sch_direct_xmit
0.67% [kernel] [k] page_frag_free
0.66% [kernel] [k] skb_network_protocol
0.66% [kernel] [k] nf_hook_slow
0.63% [kernel] [k] netif_receive_skb_internal
0.62% [kernel] [k] vlan_passthru_hard_header
0.62% [kernel] [k] deliver_ptype_list_skb
0.62% [kernel] [k] __netif_receive_skb
0.61% [kernel] [k] fib_validate_source
0.61% [kernel] [k] mlx5_cqwq_get_cqe
0.55% [kernel] [k] eth_type_trans
0.55% [kernel] [k] iptable_filter_hook
0.53% [kernel] [k] __udp4_lib_lookup
0.53% [kernel] [k] udp_gro_receive
0.51% [kernel] [k] eth_header
0.49% [kernel] [k] fib_lookup.constprop.49
0.48% [kernel] [k] ip_output
0.44% [kernel] [k] netdev_pick_tx
0.43% [kernel] [k] validate_xmit_skb_list
0.42% [kernel] [k] swiotlb_map_page
0.41% [kernel] [k] dma_sync_single_for_cpu.constprop.36
0.41% [kernel] [k] mlx5_cqwq_get_wqe
0.39% [kernel] [k] eth_type_vlan
0.38% [kernel] [k] udp4_gro_receive
0.37% [kernel] [k] __jhash_nwords
0.37% [kernel] [k] ip_forward_finish
0.33% [kernel] [k] ip_finish_output
0.33% [kernel] [k] neigh_connected_output
0.31% [kernel] [k] mlx5e_features_check
0.29% [kernel] [k] ktime_get_with_offset
0.27% [kernel] [k] __udp4_lib_lookup_skb
0.27% [kernel] [k] napi_consume_skb
0.26% [kernel] [k] page_frag_alloc
0.26% [kernel] [k] get_dma_ops
0.25% [kernel] [k] udp4_portaddr_hash.isra.29
0.24% [kernel] [k] skb_dst_drop.isra.76
0.24% [kernel] [k] neigh_resolve_output
0.24% [kernel] [k] skb_release_data
0.22% [kernel] [k] kmem_cache_free_bulk
0.22% [kernel] [k] get_dma_ops
0.22% [kernel] [k] _kfree_skb_defer
0.21% [kernel] [k] ip_skb_dst_mtu
0.21% [kernel] [k] is_swiotlb_buffer
0.21% [kernel] [k] find_exception
0.21% [kernel] [k] compound_head
0.21% [kernel] [k] __net_timestamp.isra.89
W dniu 2017-09-11 o 18:57, Paweł Staszewski pisze:
> Tested with connectx-5
>
> Without patch
>
> 10Mpps - > 16 cores used
>
> PerfTop: 66258 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
> cycles], (all, 32 CPUs)
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>
> 10.12% [kernel] [k] do_raw_spin_lock
> 6.31% [kernel] [k] fib_table_lookup
> 6.12% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
> 4.90% [kernel] [k] rt_cache_valid
> 3.99% [kernel] [k] mlx5e_xmit
> 3.03% [kernel] [k] ip_rcv
> 2.68% [kernel] [k] __netif_receive_skb_core
> 2.54% [kernel] [k] skb_dst_force
> 2.41% [kernel] [k] ip_finish_output2
> 2.21% [kernel] [k] __build_skb
> 2.03% [kernel] [k] __dev_queue_xmit
> 1.96% [kernel] [k] mlx5e_txwqe_complete
> 1.79% [kernel] [k] ipt_do_table
> 1.78% [kernel] [k] inet_gro_receive
> 1.69% [kernel] [k] ip_forward
> 1.66% [kernel] [k] udp_v4_early_demux
> 1.65% [kernel] [k] dst_release
> 1.56% [kernel] [k] ip_rcv_finish
> 1.45% [kernel] [k] dev_gro_receive
> 1.45% [kernel] [k] netif_skb_features
> 1.39% [kernel] [k] mlx5e_poll_tx_cq
> 1.35% [kernel] [k] mlx5e_txwqe_build_dsegs
> 1.35% [kernel] [k] ip_route_input_rcu
> 1.15% [kernel] [k] dev_hard_start_xmit
> 1.12% [kernel] [k] napi_gro_receive
> 1.07% [kernel] [k] netif_receive_skb_internal
> 0.98% [kernel] [k] sch_direct_xmit
> 0.95% [kernel] [k] kmem_cache_alloc
> 0.89% [kernel] [k] read_tsc
> 0.88% [kernel] [k] mlx5e_build_rx_skb
> 0.86% [kernel] [k] mlx5_cqwq_get_cqe
> 0.82% [kernel] [k] page_frag_free
> 0.78% [kernel] [k] __local_bh_enable_ip
> 0.69% [kernel] [k] skb_network_protocol
> 0.68% [kernel] [k] __netif_receive_skb
> 0.67% [kernel] [k] vlan_dev_hard_start_xmit
> 0.65% [kernel] [k] mlx5e_poll_rx_cq
> 0.65% [kernel] [k] validate_xmit_skb
> 0.60% [kernel] [k] eth_type_trans
> 0.60% [kernel] [k] deliver_ptype_list_skb
> 0.60% [kernel] [k] fib_validate_source
> 0.55% [kernel] [k] eth_header
> 0.53% [kernel] [k] netdev_pick_tx
> 0.53% [kernel] [k] __napi_alloc_skb
> 0.51% [kernel] [k] __udp4_lib_lookup
> 0.50% [kernel] [k] eth_type_vlan
> 0.49% [kernel] [k] ip_output
> 0.49% [kernel] [k] page_frag_alloc
> 0.49% [kernel] [k] ip_finish_output
> 0.48% [kernel] [k] neigh_connected_output
> 0.45% [kernel] [k] nf_hook_slow
> 0.44% [kernel] [k] udp4_gro_receive
> 0.39% [kernel] [k] mlx5e_features_check
> 0.39% [kernel] [k] mlx5e_napi_poll
> 0.37% [kernel] [k] __jhash_nwords
> 0.37% [kernel] [k] udp_gro_receive
> 0.36% [kernel] [k] swiotlb_map_page
> 0.33% [kernel] [k] mlx5_cqwq_get_wqe
> 0.33% [kernel] [k] __netdev_pick_tx
> 0.29% [kernel] [k] ktime_get_with_offset
> 0.29% [kernel] [k] get_dma_ops
> 0.29% [kernel] [k] validate_xmit_skb_list
> 0.26% [kernel] [k] vlan_passthru_hard_header
> 0.26% [kernel] [k] __udp4_lib_lookup_skb
> 0.24% [kernel] [k] get_dma_ops
> 0.24% [kernel] [k] skb_release_data
> 0.23% [kernel] [k] ip_forward_finish
> 0.23% [kernel] [k] kmem_cache_free_bulk
> 0.23% [kernel] [k] timekeeping_get_ns
> 0.22% [kernel] [k] ip_skb_dst_mtu
> 0.21% [kernel] [k] compound_head
> 0.20% [kernel] [k] skb_gro_reset_offset
> 0.20% [kernel] [k] is_swiotlb_buffer
> 0.19% [kernel] [k] __net_timestamp.isra.90
> 0.19% [kernel] [k] dst_metric.constprop.61
> 0.18% [kernel] [k] skb_orphan_frags.constprop.126
> 0.18% [kernel] [k] _kfree_skb_defer
> 0.18% [kernel] [k] irq_entries_start
> 0.17% [kernel] [k] dev_hard_header.constprop.54
> 0.17% [kernel] [k] dma_mapping_error
> 0.17% [kernel] [k] neigh_resolve_output
>
>
>
>
> With patch
>
> 12Mpps -> 16 cores
>
> PerfTop: 66209 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
> cycles], (all, 32 CPUs)
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>
> 10.67% [kernel] [k] do_raw_spin_lock
> 6.96% [kernel] [k] fib_table_lookup
> 6.53% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
> 4.17% [kernel] [k] mlx5e_xmit
> 3.22% [kernel] [k] ip_rcv
> 3.07% [kernel] [k] __netif_receive_skb_core
> 2.86% [kernel] [k] __dev_queue_xmit
> 2.36% [kernel] [k] __build_skb
> 2.33% [kernel] [k] ip_forward
> 2.05% [kernel] [k] mlx5e_txwqe_complete
> 2.02% [kernel] [k] ip_finish_output2
> 2.00% [kernel] [k] ipt_do_table
> 1.84% [kernel] [k] ip_rcv_finish
> 1.83% [kernel] [k] inet_gro_receive
> 1.80% [kernel] [k] udp_v4_early_demux
> 1.61% [kernel] [k] dev_gro_receive
> 1.55% [kernel] [k] netif_skb_features
> 1.52% [kernel] [k] mlx5e_txwqe_build_dsegs
> 1.47% [kernel] [k] mlx5e_poll_tx_cq
> 1.39% [kernel] [k] ip_route_input_rcu
> 1.38% [kernel] [k] dev_hard_start_xmit
> 1.17% [kernel] [k] netif_receive_skb_internal
> 1.16% [kernel] [k] napi_gro_receive
> 1.03% [kernel] [k] kmem_cache_alloc
> 1.02% [kernel] [k] sch_direct_xmit
> 0.97% [kernel] [k] read_tsc
> 0.94% [kernel] [k] page_frag_free
> 0.91% [kernel] [k] mlx5_cqwq_get_cqe
> 0.90% [kernel] [k] mlx5e_build_rx_skb
> 0.89% [kernel] [k] skb_network_protocol
> 0.83% [kernel] [k] __local_bh_enable_ip
> 0.79% [kernel] [k] validate_xmit_skb
> 0.77% [kernel] [k] vlan_dev_hard_start_xmit
> 0.74% [kernel] [k] __netif_receive_skb
> 0.72% [kernel] [k] mlx5e_poll_rx_cq
> 0.70% [kernel] [k] netdev_pick_tx
> 0.69% [kernel] [k] eth_type_vlan
> 0.68% [kernel] [k] __netdev_pick_tx
> 0.66% [kernel] [k] nf_hook_slow
> 0.65% [kernel] [k] deliver_ptype_list_skb
> 0.62% [kernel] [k] fib_validate_source
> 0.61% [kernel] [k] eth_header
> 0.60% [kernel] [k] eth_type_trans
> 0.59% [kernel] [k] __udp4_lib_lookup
> 0.58% [kernel] [k] __napi_alloc_skb
> 0.53% [kernel] [k] ip_finish_output
> 0.51% [kernel] [k] neigh_connected_output
> 0.50% [kernel] [k] ip_output
> 0.50% [kernel] [k] rt_cache_valid
> 0.44% [kernel] [k] udp4_gro_receive
> 0.43% [kernel] [k] mlx5e_napi_poll
> 0.40% [kernel] [k] udp_gro_receive
> 0.40% [kernel] [k] page_frag_alloc
> 0.40% [kernel] [k] __jhash_nwords
> 0.39% [kernel] [k] swiotlb_map_page
> 0.38% [kernel] [k] mlx5_cqwq_get_wqe
> 0.36% [kernel] [k] mlx5e_features_check
> 0.32% [kernel] [k] get_dma_ops
> 0.31% [kernel] [k] ktime_get_with_offset
> 0.31% [kernel] [k] validate_xmit_skb_list
> 0.28% [kernel] [k] vlan_passthru_hard_header
> 0.28% [kernel] [k] get_dma_ops
> 0.27% [kernel] [k] __udp4_lib_lookup_skb
> 0.26% [kernel] [k] skb_gro_reset_offset
> 0.25% [kernel] [k] skb_release_data
> 0.25% [kernel] [k] timekeeping_get_ns
> 0.24% [kernel] [k] kmem_cache_free_bulk
> 0.24% [kernel] [k] ip_forward_finish
> 0.23% [kernel] [k] compound_head
> 0.23% [kernel] [k] ip_skb_dst_mtu
> 0.22% [kernel] [k] __net_timestamp.isra.90
> 0.22% [kernel] [k] is_swiotlb_buffer
> 0.21% [kernel] [k] neigh_resolve_output
> 0.21% [kernel] [k] dst_metric.constprop.61
> 0.20% [kernel] [k] skb_orphan_frags.constprop.126
> 0.20% [kernel] [k] irq_entries_start
> 0.19% [kernel] [k] mlx5e_calc_min_inline
> 0.19% [kernel] [k] dev_hard_header.constprop.54
> 0.19% [kernel] [k] _kfree_skb_defer
> 0.18% [kernel] [k] _raw_spin_lock
> 0.18% [kernel] [k] ip_route_input_noref
>
>
>
>
>
> W dniu 2017-09-09 o 11:03, Paweł Staszewski pisze:
>> Hi
>>
>>
>> Are there any plans to have this fix normally in kernel ?
>>
>> Or it is mostly only hack - not longterm fix and need to be different ?
>>
>>
>> All tests that was done shows that without this patch there is about
>> 20-30% network forwarding performance degradation when using vlan
>> interfaces
>>
>>
>> Thanks
>> Paweł
>>
>>
>>
>> W dniu 2017-08-15 o 03:17, Eric Dumazet pisze:
>>> On Mon, 2017-08-14 at 18:07 -0700, Eric Dumazet wrote:
>>>
>>>> Or try to hack the IFF_XMIT_DST_RELEASE flag on the vlan netdev.
>>> Something like :
>>>
>>> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
>>> index
>>> 5e831de3103e2f7092c7fa15534def403bc62fb4..9472de846d5c0960996261cb2843032847fa4bf7
>>> 100644
>>> --- a/net/8021q/vlan_netlink.c
>>> +++ b/net/8021q/vlan_netlink.c
>>> @@ -143,6 +143,7 @@ static int vlan_newlink(struct net *src_net,
>>> struct net_device *dev,
>>> vlan->vlan_proto = proto;
>>> vlan->vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
>>> vlan->real_dev = real_dev;
>>> + dev->priv_flags |= (real_dev->priv_flags & IFF_XMIT_DST_RELEASE);
>>> vlan->flags = VLAN_FLAG_REORDER_HDR;
>>> err = vlan_check_real_dev(real_dev, vlan->vlan_proto,
>>> vlan->vlan_id);
>>>
>>>
>>>
>>>
>>
>>
>
>
^ 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