On Wed, Feb 21, 2024 at 03:49:52PM +0100, Claudio Jeker wrote:
> On Wed, Feb 21, 2024 at 03:31:24PM +0100, Alexander Bluhm wrote:
> > On Wed, Feb 21, 2024 at 11:14:52AM +0100, Claudio Jeker wrote:
> > > On Tue, Feb 20, 2024 at 10:54:20PM +0100, Alexander Bluhm wrote:
> > > > Hi,
> > > > 
> > > > This makes the route cache aware of multipath routing.  If source
> > > > address or multipath flags change, discard cached route.
> > > > 
> > > > ok?
> > > 
> > > I wonder about the RTF_MPATH check. Isn't the generation number preventing
> > > this already? You can't add or clear RTF_MPATH (which is done by the
> > > kernel) without adding or removing a route.
> > 
> > +                   !ISSET(ro->ro_rt->rt_flags, RTF_MPATH) ||
> > 
> > This one is needed to ignore src for routes that do not have RTF_MPATH
> > set.  If route_cache() is called with a src that does not match the
> > src in the route cache, use the non-multipath route anyway.
> 
> Do you have numbers that show that the cache hit rate is significantly
> lower because of this and the one below?

No.  Currently the cache performance is not good.  We need it in
more places first.  And we need persistent caches bewteen IP input
invocations.  Currently I am working on fixing correctness.

Why should I put a different route in the cache each time the src
address changes?  rt_hash() checks RTF_MPATH and ignores routes
without.  To keep the cache consistent with lookoup we should do
the same.

> > If ipmultipath is false, all routes are valid, no matter what src
> > is.  The check is there, to ignore ro->ro_srcin.s_addr == src->s_addr
> > in this case.
> 
> See above.

rt_hash() checks ipmultipath.  The cache should have the same logic
as lookup.

bluhm