Hello,
> >
> > > + else {
> > > + struct ifnet *destifp;
> > > +
> > > + destifp = if_get(rt->rt_ifidx);
> > > + if (destifp != NULL)
> > > + destmtu = destifp->if_mtu;
> > > + if_put(destifp);
> > > + }
> >
> > your code potentially leaves destmtu set to 0 in case we deal with invalid
> > ipforward_rt. I wonder how icmp_error() we are going to call further below
> > (at line 1544 in the old code) is going to deal with it. May be we
> > should just give up on sending ICMP_UNREACH message in this case.
> > find my small improvement to your patch further below.
>
> Note that my change should only matter for IPSEC. This whole MTU mess is
> scary. I can't even tell if it makes sense to send an ICMP if the ifp
> disappeared. The use of if_get(9) makes it clear that the MTU handling
> is insane.
>
> So I'm not opposed to your change but I think you should either commit
> it separately or keep it inside the "if (ipforward_rt.ro_rt) {" block,
> because it's a behavior change.
>
IMO we should not let IPSEC send ICMP with 0 MTU proposal. We are probably
better to send nothing in this case and let sender to retransmit.
Also if I understand the code well enough the proposed MTU could not be zero
before your change. However given the likelihood someone triggers the case
of invalidated route is very low (more academical than practical problem...),
I agree with follow up commit.
We should probably check with mikeb here.
regards
sasha
