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
|