On 20/11/15(Fri) 16:50, Alexandr Nedvedicky wrote:
> Hello,
>
> thanks for detailed explanation.
Your welcome.
>
> > + 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.
> Index: ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.261
> diff -u -p -r1.261 ip_input.c
> --- ip_input.c 14 Nov 2015 15:40:40 -0000 1.261
> +++ ip_input.c 20 Nov 2015 15:48:47 -0000
> @@ -1516,11 +1516,24 @@ ip_forward(struct mbuf *m, struct ifnet
>
> if (rt->rt_rmx.rmx_mtu)
> destmtu = rt->rt_rmx.rmx_mtu;
> - else
> - destmtu = ipforward_rt.ro_rt->rt_ifp->if_mtu;
> + else {
> + struct ifnet *destifp;
> +
> + destifp = if_get(rt->rt_ifidx);
> + if (destifp != NULL)
> + destmtu = destifp->if_mtu;
> + if_put(destifp);
> + }
> }
> #endif /*IPSEC*/
> ipstat.ips_cantfrag++;
> +
> + /*
> + * route to destniation no longer exists, we should revert
> code
> + * back to host unreachable.
> + */
> + if (destmtu == 0)
> + code = ICMP_UNREACH_HOST;
> break;
>
> case EACCES:
>
>
>
