[RFC net-next] ipv4: Don't promote secondaries when flushing addresses

[RFC net-next] ipv4: Don't promote secondaries when flushing addresses

[Jakub Sitnicki]

Promoting secondary addresses on address removal makes flushing all
addresses from a device with 1000's of them slow. This is because we
cannot take down the secondary addresses when we are removing the
primary one, which would make it faster.

However, the userspace, when performing a flush, will in the end remove
all the addresses regardless of secondary address promotion taking
place. Unfortunately the kernel currently cannot distinguish between a
single address removal and a flush of all addresses.

To help with this case introduce a IFA_F_FLUSH flag that can be used by
userspace to signal that a removal operation is being done because of a
flush. When the flag is set, don't bother with secondary address
promotion as we expect that secondary addresses will be removed soon as
well.

Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>

---

A benchmark involving a flush of 40,000 addresses from a dummy device
shows a x4 speed-up of the 'flush' operation. 'ip' had to be modified to
set the IFA_F_FLUSH flag for RTM_DELADDR requests issued for the
'flush':

  # time $IP -stats addr flush dev dum0

Before:

  real    0m30.596s
  user    0m0.000s
  sys     0m30.567s

After:

  real    0m7.601s
  user    0m0.000s
  sys     0m7.569s

It's also worth noting that promote_secondaries sysctl param is enabled by
default since systemd 216 thus making it the new "normal" on some distros.


 include/uapi/linux/if_addr.h |  1 +
 net/ipv4/devinet.c           | 14 ++++++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index ebaf5701c9db..19aab9a9cec5 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -54,6 +54,7 @@ enum {
 #define IFA_F_NOPREFIXROUTE	0x200
 #define IFA_F_MCAUTOJOIN	0x400
 #define IFA_F_STABLE_PRIVACY	0x800
+#define IFA_F_FLUSH		0x1000

 struct ifa_cacheinfo {
 	__u32	ifa_prefered;
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index d7585ab1a77a..1f436e1e5222 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -331,13 +331,14 @@ int inet_addr_onlink(struct in_device *in_dev, __be32 a, __be32 b)
 }

 static void __inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
-			 int destroy, struct nlmsghdr *nlh, u32 portid)
+			   int destroy, struct nlmsghdr *nlh, u32 portid,
+			   bool flush)
 {
 	struct in_ifaddr *promote = NULL;
 	struct in_ifaddr *ifa, *ifa1 = *ifap;
 	struct in_ifaddr *last_prim = in_dev->ifa_list;
 	struct in_ifaddr *prev_prom = NULL;
-	int do_promote = IN_DEV_PROMOTE_SECONDARIES(in_dev);
+	int do_promote = IN_DEV_PROMOTE_SECONDARIES(in_dev) && !flush;

 	ASSERT_RTNL();

@@ -437,7 +438,7 @@ static void __inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
 static void inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
 			 int destroy)
 {
-	__inet_del_ifa(in_dev, ifap, destroy, NULL, 0);
+	__inet_del_ifa(in_dev, ifap, destroy, NULL, 0, false);
 }

 static void check_lifetime(struct work_struct *work);
@@ -607,6 +608,7 @@ static int inet_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct in_device *in_dev;
 	struct ifaddrmsg *ifm;
 	struct in_ifaddr *ifa, **ifap;
+	bool flush = false;
 	int err = -EINVAL;

 	ASSERT_RTNL();
@@ -623,6 +625,9 @@ static int inet_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto errout;
 	}

+	if (tb[IFA_FLAGS])
+		flush = !!(nla_get_u32(tb[IFA_FLAGS]) & IFA_F_FLUSH);
+
 	for (ifap = &in_dev->ifa_list; (ifa = *ifap) != NULL;
 	     ifap = &ifa->ifa_next) {
 		if (tb[IFA_LOCAL] &&
@@ -639,7 +644,8 @@ static int inet_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,

 		if (ipv4_is_multicast(ifa->ifa_address))
 			ip_mc_config(net->ipv4.mc_autojoin_sk, false, ifa);
-		__inet_del_ifa(in_dev, ifap, 1, nlh, NETLINK_CB(skb).portid);
+		__inet_del_ifa(in_dev, ifap, 1, nlh, NETLINK_CB(skb).portid,
+			       flush);
 		return 0;
 	}

Re: [RFC net-next] ipv4: Don't promote secondaries when flushing addresses

[Michal Kubecek]

On Thu, Jun 07, 2018 at 12:13:01PM +0200, Jakub Sitnicki wrote:
> Promoting secondary addresses on address removal makes flushing all
> addresses from a device with 1000's of them slow. This is because we
> cannot take down the secondary addresses when we are removing the
> primary one, which would make it faster.
> 
> However, the userspace, when performing a flush, will in the end remove
> all the addresses regardless of secondary address promotion taking
> place. Unfortunately the kernel currently cannot distinguish between a
> single address removal and a flush of all addresses.
> 
> To help with this case introduce a IFA_F_FLUSH flag that can be used by
> userspace to signal that a removal operation is being done because of a
> flush. When the flag is set, don't bother with secondary address
> promotion as we expect that secondary addresses will be removed soon as
> well.

Unless you intend to use the flag to allow deleting a specific address
with its secondaries (overriding promote_secondaries), maybe it would
be more practical to go even further and delete all addresses on the
interface if IFA_F_FLUSH is set so that userspace could delete all
addresses with one request.

Michal Kubecek

Re: [RFC net-next] ipv4: Don't promote secondaries when flushing addresses

[Jakub Sitnicki]

On Thu, 7 Jun 2018 13:00:29 +0200
Michal Kubecek <mkubecek@suse.cz> wrote:

> On Thu, Jun 07, 2018 at 12:13:01PM +0200, Jakub Sitnicki wrote:
> > Promoting secondary addresses on address removal makes flushing all
> > addresses from a device with 1000's of them slow. This is because we
> > cannot take down the secondary addresses when we are removing the
> > primary one, which would make it faster.
> > 
> > However, the userspace, when performing a flush, will in the end remove
> > all the addresses regardless of secondary address promotion taking
> > place. Unfortunately the kernel currently cannot distinguish between a
> > single address removal and a flush of all addresses.
> > 
> > To help with this case introduce a IFA_F_FLUSH flag that can be used by
> > userspace to signal that a removal operation is being done because of a
> > flush. When the flag is set, don't bother with secondary address
> > promotion as we expect that secondary addresses will be removed soon as
> > well.  
> 
> Unless you intend to use the flag to allow deleting a specific address
> with its secondaries (overriding promote_secondaries), maybe it would
> be more practical to go even further and delete all addresses on the
> interface if IFA_F_FLUSH is set so that userspace could delete all
> addresses with one request.

Thanks for input, Michal. The intend as I understand it is to make
flushing all the addresses fast(er). Let me see if I can rework it
according to your suggestion. It does make more sense to do it like
that to me too.

Thanks,
Jakub

Re: [RFC net-next] ipv4: Don't promote secondaries when flushing addresses

[Phil Sutter]

Hi Jakub,

On Thu, Jun 07, 2018 at 02:17:50PM +0200, Jakub Sitnicki wrote:
> On Thu, 7 Jun 2018 13:00:29 +0200
> Michal Kubecek <mkubecek@suse.cz> wrote:
> 
> > On Thu, Jun 07, 2018 at 12:13:01PM +0200, Jakub Sitnicki wrote:
> > > Promoting secondary addresses on address removal makes flushing all
> > > addresses from a device with 1000's of them slow. This is because we
> > > cannot take down the secondary addresses when we are removing the
> > > primary one, which would make it faster.
> > > 
> > > However, the userspace, when performing a flush, will in the end remove
> > > all the addresses regardless of secondary address promotion taking
> > > place. Unfortunately the kernel currently cannot distinguish between a
> > > single address removal and a flush of all addresses.
> > > 
> > > To help with this case introduce a IFA_F_FLUSH flag that can be used by
> > > userspace to signal that a removal operation is being done because of a
> > > flush. When the flag is set, don't bother with secondary address
> > > promotion as we expect that secondary addresses will be removed soon as
> > > well.  
> > 
> > Unless you intend to use the flag to allow deleting a specific address
> > with its secondaries (overriding promote_secondaries), maybe it would
> > be more practical to go even further and delete all addresses on the
> > interface if IFA_F_FLUSH is set so that userspace could delete all
> > addresses with one request.
> 
> Thanks for input, Michal. The intend as I understand it is to make
> flushing all the addresses fast(er). Let me see if I can rework it
> according to your suggestion. It does make more sense to do it like
> that to me too.

Yes, I agree with Michal. IIRC, flushing a specific primary along with
all it's secondaries from an interface is not even supported by
iproute2, so no need to optimize for that I guess. OTOH, if your
solution allowed to get rid of that nasty loop in ipaddr_flush(), I owe
you one extra beer at the next occasion. :)

Thanks for holding on to this old ticket!

Cheers, Phil

Re: [RFC net-next] ipv4: Don't promote secondaries when flushing addresses

[Michal Kubecek]

On Thu, Jun 07, 2018 at 02:35:39PM +0200, Phil Sutter wrote:
> Yes, I agree with Michal. IIRC, flushing a specific primary along with
> all it's secondaries from an interface is not even supported by
> iproute2, so no need to optimize for that I guess. OTOH, if your
> solution allowed to get rid of that nasty loop in ipaddr_flush(), I owe
> you one extra beer at the next occasion. :)

I'm afraid it will have to stay as fallback for older kernels not
supporting flush requests. But there would be no need to actually use
it. If we know RTM_DELADDR request for zero address is guaranteed to
fail with current and older kernels, we could do

  - use RTM_DELADDR with IFA_F_FLUSH and zero address
  - if it fails, get the list and run the loop

If not, it could still be

  - use RTM_DELADDR with IFA_F_FLUSH and zero address
  - get the list of addresses (empty if first step worked)
  - run the loop

Michal Kubecek