LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH net-next 06/12] ipvs: optimize dst usage for real server

To: David Miller <davem@xxxxxxxxxxxxx>
Subject: Re: [PATCH net-next 06/12] ipvs: optimize dst usage for real server
Cc: horms@xxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx
From: Julian Anastasov <ja@xxxxxx>
Date: Wed, 6 Mar 2013 23:58:09 +0200 (EET)
        Hello,

On Wed, 6 Mar 2013, David Miller wrote:

> From: Julian Anastasov <ja@xxxxxx>
> Date: Wed,  6 Mar 2013 10:42:16 +0200
> 
> >     Currently when forwarding requests to real servers
> > we use dst_lock and atomic operations when cloning the
> > dst_cache value. As the dst_cache value does not change
> > most of the time it is better to use RCU and to lock
> > dst_lock only when we need to replace the obsoleted dst.
> > For this to work we keep dst_cache in new structure protected
> > by RCU. For packets to remote real servers we will use noref
> > version of dst_cache, it will be valid while we are in RCU
> > read-side critical section because now dst_release for replaced
> > dsts will be invoked after the grace period. NAT-ed packets
> > via loopback that are not sent but are passed to local stack
> > with NF_ACCEPT need a dst clone (skb_dst_force).
> > 
> > Signed-off-by: Julian Anastasov <ja@xxxxxx>
> 
> It think it's much cleaner to have the callers of route lookups do
> skb_set_dst_noref().

        This was my first thought but commit 27b75c95f1
(net: avoid RCU for NOCACHE dst) uses dst_hold for
DST_NOCACHE dsts. IPVS wants to cache DST_NOCACHE entries
(due to known-NH reasons) and later to use them for
noref dsts...

        Also, in the IPVS case we can not set skb_dst
at the time when route is looked up because due to
MTU and other checks we can fail, functions like
icmp_send() will need the original skb_dst in
such case.

        There are two choices: __ip_vs_get_out_rt
to return refdst as implemented in this patchset or
to return dst with additional 'bool *noref' argument,
so that caller can decide between skb_dst_set or
skb_set_dst_noref. Then may be we will need just
a new skb_dst_set_noref_{always,force} func that will
contain the old skb_set_dst_noref code, i.e. without
dst_hold? Not sure which variant sounds better.

> Then you don't need any new interfaces, and you therefore don't need
> to expose bits of the noref implementation and semantics at all.

Regards

--
Julian Anastasov <ja@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>