On (08/10/09 10:27), Sebastien Roy wrote:
>
> > - Instead of doing pairs of dladm_open/dladm_close, suggest having
> > a global dladm_handle that is opened once in main and closed on exit.
> > dladm_link_open() can use that handle, and it can be reused for
> > "ifconfig -a <..>" commands like "ifconfig -a ether" and "ifconfig -a
> > plumb".
>
> REJECT: I'd rather not; ifconfig currently doesn't depend on having the
> ability to open a dladm handle except for some commands, and moving this
> logic to a common location in the code introduces the risk that we'd be
> causing ifconfig to fail unnecessarily (like from within shared stack
> zones for example).
This is not very efficient for something like rcm_daemon which exec's ifconfig,
though I have admittedly not checked to see if rcm_daemon ever invokes ifconfig
to do an operation on more than one interface at any time.
The case that you point to (ifconfig failing from shared stack zone) can
be handled by having ifconfig`main treat dladm_open() as a soft error
except when the handle itself is needed (in dladm_link_open)
> >
> > usr/src/uts/common/inet/ip/ip_ndp.c
> > - do you need symmetric changes to ndp_lookup_then_add_v4 that also add
> > the new ipv4 nce with hwaddr == ill_dest_addr for tunnel interfaces?
>
> No. The IPv4 code is totally different. There is no hardware address
> passed in to ndp_lookup_then_add_v4(), but rather a "src_nce" which
> comes by way of a long trip through a giant call-chain that originates
> in a call to ire_create() in ip_newroute() (I pray that
> datapath-refactoring simplifies this.) For IRE_IF_NORESOLVER, a NULL
> src_nce is passed in, and the NULL src_nce gets passed in to
> ndp_add_v4() which does the right thing given the ill_resolver_mp. I'd
> have to consult with a Surya project team member to get the details on
> why they didn't make the IPv4 and IPv6 code more uniform. ;-)
the src_nce was used to handle the ipmp case (which may well be broken
for ipv6 in onnv- I've not checked). However, for the particular
issue at hand, the iptun-cr ws tries to set the hwaddr of nce's over
tunnel interfaces to the ill_dest_addr. This should be handled internally
in ndp_andd_v4 at
3672 } else if (ill->ill_net_type == IRE_IF_NORESOLVER) {
3673 /*
3674 * NORESOLVER entries are always created in the
REACHABLE
3675 * state.
3676 */
3677 if ((template = copyb(ill->ill_resolver_mp)) == NULL) {
3678 err = ENOMEM;
3679 goto err_ret;
3680 }
3681 state = ND_REACHABLE;
3682 }
This section, if it detects IFF_POINTOPOINT, should copy the
ill_dest_addr to nce->nce_res_mp->b_rptr + NCE_LL_ADDR_OFFSET(ill)
It's not merely an issue of symmetry: presumably the ndp_add_v6 code is
painstakingly doing nce_set_ll for some good reason (not sure where
the nce_res_mp gets uesd for ipv6)- don't we need to have the ipv4
nce's also setup in a similar way?
> >
> > usr/src/uts/common/inet/iptun/iptun.c
> > - why not just do a msgpullup at line 2043.
>
> IPsec re-inserts the outer IP header as a single MBLK on input, and
> since that IP header will simply be removed by GLDv3 when the packet
> gets passed up, doing a msgpullup() is a needless copy of data that will
> just get immediately discarded anyway.
Ok, would be useful to add a comment about this.
>
> > And if we consider that a tunnel tun1
> > with outer header (A -> B) and inner header (C -> B) is recursive [#]
> > then what about a tunnel which has outer/inner header
> > (A -> B) / (C -> D) where the route for B points at a tunnel tun2
> > that sets up (C1 -> D) as the outer header?
>
> You mean a packet that has those headers, and not a tunnel, right? A
yes.
> detect other simple cases of such misconfigurations, I agree that the
> check is kind of strange. I will remove it.
Ok.
> > (and even more confusing:
> > we drop the packet on the input path when inner_dst == outer_dst even
> > if both of these are locally hosted addresses? I would expect a host
> > that receives such a packet to accept it, assuming everything else
> > checks out).
>
> I'm afraid I'm not following you on this one. Where is this check?
sorry, my mistake.. I think I got my cscope confused. iptun_input
doesn't use the output from iptun_find_headers() in the same way
as iptun_output.
--Sowmini
