Erik, On Fri, 2009-08-07 at 15:40 +0200, Erik Nordmark wrote: > I've looked at uts/common/inet except the iptun* files.
Thanks very much for the review, responses inline: > > ip.h: > You can remove the definition of TUN_CMD; I don't think it is used any more. ACCEPT > > ip.c: Shouldn't be bump InDelivers when we call connp->conn_recv to send > to iptun? ACCEPT: Yes, and we'll need to pass in the ill that received the packet into ip_iptun_input() order to bump the right mib. > > ip6.c: Ditto. ACCEPT > > ip_ndp.c: You've removed the special handling if IP_ADDR_LEN around line > 3682. With this change how does a point-to-multipoint device driver tell > the nexthop that the packet should be sent to? Perhaps 6to4 tunnels > don't need this because the nexthop is always extracted from the IPv6 > destination address. But I don't see how other point-to-multipoint > devices (IP over X.25, Fibrechannel, etc) can work with this removed. DISCUSS: 6to4 tunnels would also benefit from this point-to-multipoint mechanism, but it currently can't because the GLDv3 framework can't currently cope with processing a DL_UNITDATA_REQ that has a destination address of a different length than the length of the link's registered link-layer address. This could be made to work in the future for 6to4. For other types of links, I can bring that code back, but it won't work as-is. The reason is that this will inhibit tunnel links from negotiating proper fast-path headers. What I can do is bring the code back, but only for link types that are not DL_IPV4, DL_IPV6, and DL_6TO4. Does that make sense to you? > > ipclassifier.c: > The new ipcl_iptun_classify functions do not check tsol_receive_local(). > Should they? Currently in onnv-gate we do check since the tunnel fanout > happens through ip_fanout_proto which continues looking for a matching > conn_t if tsol_receive_local fails. > (Is this what "XXX KEBE ASKS - What about TX and labels?" is asking about?) ACCEPT: Yes, that's what KEBE was asking about, and this needs to be done. I'll consult with Will Young or Ken Powell about this and fix this. > > spd.c: Typo at > 5862 * NOTE: Even if our policy is trasnport mode, set the ACCEPT > > At L5933 what prevents inner_ipv* from pointing past b_wptr? The reason > I'm asking is that you have removed the asserts that checked for this. ACCEPT: It would be good to verify what danmcd intended here since this was his change (and I don't fully comprehend the intricacies of the IPsec "frag cache" that these packets come from), but I'll add the ASSERT() calls back in, they can't possibly hurt. Thanks! -Seb _______________________________________________ networking-discuss mailing list [email protected]
