From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kunihiro Ishiguro Subject: Re: [PATCH] IPv6 IPsec support Date: Tue, 18 Feb 2003 21:57:39 -0800 Sender: netdev-bounce@oss.sgi.com Message-ID: <87znos3j8s.wl@ipinfusion.com> References: <20030219134850.5f203ea7.Kazunori.Miyazawa@jp.yokogawa.com> Mime-Version: 1.0 (generated by SEMI 1.14.3 - "Ushinoya") Content-Type: text/plain; charset=US-ASCII Cc: netdev@oss.sgi.com, usagi-core@linux-ipv6.org, davem@redhat.com, kuznet@ms2.inr.ac.ru Return-path: To: Kazunori MIyazawa In-Reply-To: <20030219134850.5f203ea7.Kazunori.Miyazawa@jp.yokogawa.com> Errors-to: netdev-bounce@oss.sgi.com List-Id: netdev.vger.kernel.org I just look through the patch. Here is my quick comments. I think no need of broadcasting my comments to kernel ML, so I took it of from CC:. netdev guys will be interested in right? So I kept it. 1. Do we really need to remove AH header from skb? In case of IPv4 we modify iph->protocol for further processing thus AH header is removed. But in case of IPv6, we just simply authenticate the packet then process next header. So do we really need to remove AH header in IPv6? Remaining AH header does not harm... 2. Easy kmalloc()... It seems there are some easy kmalloc(). Yes I'm stingy with memory. Let's say no AH mutable option field in IPv6 extention headers (actually it is very common case). We just need char work_buf[8] to retain IPv6 header mutable field. But all the time the patch allocate complete copy of the header including extention header then keep it in the chamber.... + int hdr_len = skb->h.raw - skb->nh.raw; ... + tmp_hdr = kmalloc(hdr_len, GFP_ATOMIC); I think we should provision the need of mutation then allocate exactly required memory. If there no need of allocation, that's good news. Let me provide code for it. 3. xfrm6_policy_lookup() + if (pol->family != AF_INET6) continue); Last paren ;-). Well, I'll find more. Maybe we should be offline and come up with a single patch. -- Kunihiro Ishiguro