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
| ||
|
Message-ID: <1271946805.7895.5658.camel@edumazet-laptop> Date: Thu, 22 Apr 2010 16:33:25 +0200 From: Eric Dumazet <eric.dumazet@...il.com> To: Changli Gao <xiaosuo@...il.com> Cc: "David S. Miller" <davem@...emloft.net>, jamal <hadi@...erus.ca>, Tom Herbert <therbert@...gle.com>, netdev@...r.kernel.org Subject: Re: [PATCH v5] net: batch skb dequeueing from softnet input_pkt_queue Le jeudi 22 avril 2010 à 21:06 +0800, Changli Gao a écrit : > On Thu, Apr 22, 2010 at 7:37 PM, Eric Dumazet <eric.dumazet@...il.com> wrote: > > > > Please reorder things better. > > > > Most likely this function is called for one packet. > > > > In your version you take twice the rps_lock()/rps_unlock() path, so > > it'll be slower. > > > > Once to 'transfert' one list to process list > > > > Once to be able to do the 'label out:' post processing. > > > > It doesn't make any difference. We have to hold rps_lock to update > input_pkt_queue_len, if we don't use another variable to record the > length of the process queue, or atomic variable. > It does make a difference, Damn it. I really really start to think you dont read what I wrote, or you dont care. Damn, cant you update all the things at once, taking this lock only once ? You focus having an ultra precise count of pkt_queue.len, but we dont care at all ! We only want a _limit_, or else the box can be killed by DOS. If in practice this limit can be 2*limit, thats OK. Cant you understand this ? > I think it is better that using another variable to record the length > of the process queue, and updating it before process_backlog() > returns. For one packet, there is only one locking/unlocking. There is > only one issue you concerned: cache miss due to sum the two queues' > length. I'll change softnet_data to: > > struct softnet_data { > struct Qdisc *output_queue; > struct list_head poll_list; > struct sk_buff *completion_queue; > struct sk_buff_head process_queue; > > #ifdef CONFIG_RPS > struct softnet_data *rps_ipi_list; > > /* Elements below can be accessed between CPUs for RPS */ > struct call_single_data csd ____cacheline_aligned_in_smp; > struct softnet_data *rps_ipi_next; > unsigned int cpu; > unsigned int input_queue_head; > #endif > unsigned int process_queue_len; > struct sk_buff_head input_pkt_queue; > struct napi_struct backlog; > }; > > For one packets, we have to update process_queue_len in any way. For > more packets, we only change process_queue_len just before > process_backlog() returns. It means that process_queue_len change is > batched. > We need one limit. Not two limits. I already told you how to do it, but you ignored me and started yet another convoluted thing. process_backlog() transfert the queue to its own queue and reset pkt_len to 0 (Only once) End of story. Maximum packet queued to this cpu softnet_data will be 2 * old_limit. So what ? -- 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