lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 20 May 2014 12:44:01 +0800 From: Jason Wang <jasowang@...hat.com> To: Eric Dumazet <eric.dumazet@...il.com> CC: Xi Wang <xii@...gle.com>, "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org, "Michael S. Tsirkin" <mst@...hat.com>, Maxim Krasnyansky <maxk@....qualcomm.com>, Neal Cardwell <ncardwell@...gle.com>, Eric Dumazet <edumazet@...gle.com> Subject: Re: [PATCH v2] net-tun: restructure tun_do_read for better sleep/wakeup efficiency On 05/19/2014 10:09 PM, Eric Dumazet wrote: > On Mon, 2014-05-19 at 17:27 +0800, Jason Wang wrote: > >> Still a little bit difference. We check the reg_state after we're sure >> there's nothing left in sk_receive_queue. But this patch returns -EIO >> before trying to dequeue skb. > Do you think its a problem, other than a simple difference of behavior ? I just point out the possible issues. > > Is this -EIO thing a hard requirement, or a side effect ? Probably not, but it's really not hard to keep this. > > Either we try to converge things, or we keep this driver a pile of > copy/pasted stuff from other locations, missing improvements we did > in core networking stack, because of some supposed differences. Agree, but we'd better separate the unrelated changes into other patches to simplify review and speed up convergence. > > About the sk_data_ready() and wake_up_all(), you missed the whole part > of the patch I think. > > Check how sock_def_readable() does everything properly and efficiently, > including the async part. But this changes (sk_data_ready()) has nothing related to switching to use __skb_recv_datagram() The async part may not work since tun has its own implementation of fasync. > > static void sock_def_readable(struct sock *sk) > { > struct socket_wq *wq; > > rcu_read_lock(); > wq = rcu_dereference(sk->sk_wq); > if (wq_has_sleeper(wq)) > wake_up_interruptible_sync_poll(&wq->wait, POLLIN | POLLPRI | > POLLRDNORM | POLLRDBAND); > sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN); > rcu_read_unlock(); > } > > Using sk_data_ready() is the way to reach sock_def_readable() > as its not an EXPORT_SYMBOL. > > If this driver was using the common interface, we would not have > these false sharing problems, that were solved a long time ago. Yes, but better with a separate patch. We can probably rework the tun_fasync() to use sock_fasync(), but using sk_data_ready() in tun_detach_all() is still questionable. We may not want to send EIO to userspace when tun device is destroying. > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists