Download raw body.
route cache mpath
On Tue, Feb 27, 2024 at 02:19:29PM +0100, Claudio Jeker wrote:
> On Tue, Feb 27, 2024 at 01:52:51PM +0100, Alexander Bluhm wrote:
> > On Mon, Feb 26, 2024 at 11:44:57AM +0100, Alexander Bluhm wrote:
> > > Also IP input should pass a struct route to IP forward. This is
> > > the same logic that is done when passing a route from IP forward
> > > to IP output. As a result the numbers of route cache lookups in
> > > netstat -s should be correct now.
> > >
> > > Finally I removed some inconsistencies between IPv4 and IPv4 and
> > > IP forward and IP output.
> >
> > > Or should I split the diff in smaller pieces?
> >
> > This is the other half of the diff.
> >
> > ok?
>
> Some bikesheds inline.
> In general the diff should go, OK claudio@
>
> --
> :wq Claudio
>
> > @@ -1482,26 +1480,23 @@ ip_forward(struct mbuf *m, struct ifnet
> > if (m->m_flags & (M_BCAST|M_MCAST) || in_canforward(ip->ip_dst) == 0) {
> > ipstat_inc(ips_cantforward);
> > m_freem(m);
> > - goto freecopy;
> > + goto done;
>
> Would have been good to do that as a seperate step since it adds a lot of
> noise to the diff.
Sure, here is the cleanup part.
> > + if (rt == NULL) {
> > + ip6stat_inc(ip6s_noroute);
> > + if (mcopy) {
>
> I would prefer to use if (mcopy != NULL) here. See more below.
Yes, let's make it consistent the other way around.
> Same here. I prefer if (mcopy != NULL) over if (mcopy) for pointers.
> Especially since a few lines below we have: if (mcopy == NULL)
Now I have changed all of them.
I have put a space in front of the goto labels. Then diff -p shows
the function name and not the label.
ok?
bluhm
Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
diff -u -p -r1.390 ip_input.c
--- netinet/ip_input.c 22 Feb 2024 14:25:58 -0000 1.390
+++ netinet/ip_input.c 27 Feb 2024 15:55:47 -0000
@@ -391,7 +391,10 @@ ip_input_if(struct mbuf **mp, int *offp,
struct rtentry *rt = NULL;
struct ip *ip;
int hlen;
- in_addr_t pfrdr = 0;
+#if NPF > 0
+ struct in_addr odst;
+#endif
+ int pfrdr = 0;
KASSERT(*offp == 0);
@@ -412,7 +415,7 @@ ip_input_if(struct mbuf **mp, int *offp,
/*
* Packet filter
*/
- pfrdr = ip->ip_dst.s_addr;
+ odst = ip->ip_dst;
if (pf_test(AF_INET, PF_IN, ifp, mp) != PF_PASS)
goto bad;
m = *mp;
@@ -420,7 +423,7 @@ ip_input_if(struct mbuf **mp, int *offp,
goto bad;
ip = mtod(m, struct ip *);
- pfrdr = (pfrdr != ip->ip_dst.s_addr);
+ pfrdr = odst.s_addr != ip->ip_dst.s_addr;
#endif
hlen = ip->ip_hl << 2;
@@ -1472,7 +1475,7 @@ const u_char inetctlerrmap[PRC_NCMDS] =
void
ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt)
{
- struct mbuf mfake, *mcopy = NULL;
+ struct mbuf mfake, *mcopy;
struct ip *ip = mtod(m, struct ip *);
struct route ro;
int error = 0, type = 0, code = 0, destmtu = 0, fake = 0, len;
@@ -1482,11 +1485,11 @@ ip_forward(struct mbuf *m, struct ifnet
if (m->m_flags & (M_BCAST|M_MCAST) || in_canforward(ip->ip_dst) == 0) {
ipstat_inc(ips_cantforward);
m_freem(m);
- goto freecopy;
+ goto done;
}
if (ip->ip_ttl <= IPTTLDEC) {
icmp_error(m, ICMP_TIMXCEED, ICMP_TIMXCEED_INTRANS, dest, 0);
- goto freecopy;
+ goto done;
}
ro.ro_rt = NULL;
@@ -1563,10 +1566,10 @@ ip_forward(struct mbuf *m, struct ifnet
if (type)
ipstat_inc(ips_redirectsent);
else
- goto freecopy;
+ goto done;
}
if (!fake)
- goto freecopy;
+ goto done;
switch (error) {
case 0: /* forwarded, but need redirect */
@@ -1590,7 +1593,7 @@ ip_forward(struct mbuf *m, struct ifnet
}
ipstat_inc(ips_cantfrag);
if (destmtu == 0)
- goto freecopy;
+ goto done;
break;
case EACCES:
@@ -1598,7 +1601,7 @@ ip_forward(struct mbuf *m, struct ifnet
* pf(4) blocked the packet. There is no need to send an ICMP
* packet back since pf(4) takes care of it.
*/
- goto freecopy;
+ goto done;
case ENOBUFS:
/*
@@ -1607,7 +1610,7 @@ ip_forward(struct mbuf *m, struct ifnet
* source quench could be a big problem under DoS attacks,
* or the underlying interface is rate-limited.
*/
- goto freecopy;
+ goto done;
case ENETUNREACH: /* shouldn't happen, checked above */
case EHOSTUNREACH:
@@ -1619,10 +1622,10 @@ ip_forward(struct mbuf *m, struct ifnet
break;
}
mcopy = m_copym(&mfake, 0, len, M_DONTWAIT);
- if (mcopy)
+ if (mcopy != NULL)
icmp_error(mcopy, type, code, dest, destmtu);
-freecopy:
+ done:
if (fake)
m_tag_delete_chain(&mfake);
rtfree(rt);
Index: netinet6/ip6_forward.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_forward.c,v
diff -u -p -r1.115 ip6_forward.c
--- netinet6/ip6_forward.c 22 Feb 2024 14:25:58 -0000 1.115
+++ netinet6/ip6_forward.c 27 Feb 2024 15:43:48 -0000
@@ -89,7 +89,7 @@ ip6_forward(struct mbuf *m, struct rtent
struct route ro;
struct ifnet *ifp = NULL;
int error = 0, type = 0, code = 0, destmtu = 0;
- struct mbuf *mcopy = NULL;
+ struct mbuf *mcopy;
#ifdef IPSEC
struct tdb *tdb = NULL;
#endif /* IPSEC */
@@ -121,13 +121,13 @@ ip6_forward(struct mbuf *m, struct rtent
m->m_pkthdr.ph_ifidx);
}
m_freem(m);
- goto out;
+ goto done;
}
if (ip6->ip6_hlim <= IPV6_HLIMDEC) {
icmp6_error(m, ICMP6_TIME_EXCEEDED,
ICMP6_TIME_EXCEED_TRANSIT, 0);
- goto out;
+ goto done;
}
ip6->ip6_hlim -= IPV6_HLIMDEC;
@@ -175,12 +175,12 @@ reroute:
m->m_pkthdr.ph_rtableid);
if (rt == NULL) {
ip6stat_inc(ip6s_noroute);
- if (mcopy) {
+ if (mcopy != NULL) {
icmp6_error(mcopy, ICMP6_DST_UNREACH,
ICMP6_DST_UNREACH_NOROUTE, 0);
}
m_freem(m);
- goto out;
+ goto done;
}
}
ro.ro_rt = rt;
@@ -211,11 +211,11 @@ reroute:
ip6->ip6_nxt,
m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
}
- if (mcopy)
+ if (mcopy != NULL)
icmp6_error(mcopy, ICMP6_DST_UNREACH,
ICMP6_DST_UNREACH_BEYONDSCOPE, 0);
m_freem(m);
- goto out;
+ goto done;
}
#ifdef IPSEC
@@ -270,11 +270,11 @@ reroute:
* type/code is based on suggestion by Rich Draves.
* not sure if it is the best pick.
*/
- if (mcopy)
+ if (mcopy != NULL)
icmp6_error(mcopy, ICMP6_DST_UNREACH,
ICMP6_DST_UNREACH_ADDR, 0);
m_freem(m);
- goto out;
+ goto done;
}
type = ND_REDIRECT;
}
@@ -327,18 +327,18 @@ reroute:
if (mcopy != NULL)
icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0, ifp->if_mtu);
m_freem(m);
- goto out;
+ goto done;
senderr:
if (mcopy == NULL)
- goto out;
+ goto done;
switch (error) {
case 0:
if (type == ND_REDIRECT) {
icmp6_redirect_output(mcopy, rt);
ip6stat_inc(ip6s_redirectsent);
- goto out;
+ goto done;
}
goto freecopy;
@@ -383,11 +383,11 @@ senderr:
break;
}
icmp6_error(mcopy, type, code, destmtu);
- goto out;
+ goto done;
-freecopy:
+ freecopy:
m_freem(mcopy);
-out:
+ done:
rtfree(rt);
if_put(ifp);
#ifdef IPSEC
Index: netinet6/ip6_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
diff -u -p -r1.258 ip6_input.c
--- netinet6/ip6_input.c 22 Feb 2024 14:25:58 -0000 1.258
+++ netinet6/ip6_input.c 27 Feb 2024 15:52:54 -0000
@@ -366,7 +366,7 @@ ip6_input_if(struct mbuf **mp, int *offp
#if NPF > 0
struct in6_addr odst;
#endif
- int srcrt = 0;
+ int pfrdr = 0;
KASSERT(*offp == 0);
@@ -413,7 +413,7 @@ ip6_input_if(struct mbuf **mp, int *offp
goto bad;
ip6 = mtod(m, struct ip6_hdr *);
- srcrt = !IN6_ARE_ADDR_EQUAL(&odst, &ip6->ip6_dst);
+ pfrdr = !IN6_ARE_ADDR_EQUAL(&odst, &ip6->ip6_dst);
#endif
/*
@@ -618,7 +618,7 @@ ip6_input_if(struct mbuf **mp, int *offp
}
#endif /* IPSEC */
- ip6_forward(m, rt, srcrt);
+ ip6_forward(m, rt, pfrdr);
*mp = NULL;
return IPPROTO_DONE;
bad:
Index: netinet6/ip6_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v
diff -u -p -r1.287 ip6_output.c
--- netinet6/ip6_output.c 22 Feb 2024 14:25:58 -0000 1.287
+++ netinet6/ip6_output.c 27 Feb 2024 15:50:45 -0000
@@ -748,8 +748,16 @@ reroute:
(error = if_output_ml(ifp, &ml, sin6tosa(dst), ro->ro_rt)))
goto done;
ip6stat_inc(ip6s_fragmented);
+ goto done;
-done:
+ freehdrs:
+ m_freem(exthdrs.ip6e_hbh); /* m_freem will check if mbuf is 0 */
+ m_freem(exthdrs.ip6e_dest1);
+ m_freem(exthdrs.ip6e_rthdr);
+ m_freem(exthdrs.ip6e_dest2);
+ bad:
+ m_freem(m);
+ done:
if (ro == &iproute && ro->ro_rt) {
rtfree(ro->ro_rt);
} else if (ro_pmtu == &iproute && ro_pmtu->ro_rt) {
@@ -760,16 +768,6 @@ done:
tdb_unref(tdb);
#endif /* IPSEC */
return (error);
-
-freehdrs:
- m_freem(exthdrs.ip6e_hbh); /* m_freem will check if mbuf is 0 */
- m_freem(exthdrs.ip6e_dest1);
- m_freem(exthdrs.ip6e_rthdr);
- m_freem(exthdrs.ip6e_dest2);
- /* FALLTHROUGH */
-bad:
- m_freem(m);
- goto done;
}
int
route cache mpath