Re: [RFC net-next] ipv4: Don't promote secondaries when flushing addresses
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
Re: [RFC net-next] ipv4: Don't promote secondaries when flushing addresses
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 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
On Thu, 7 Jun 2018 13:00:29 +0200 Michal Kubecek 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
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
[RFC net-next] ipv4: Don't promote secondaries when flushing addresses
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
---
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:
real0m30.596s
user0m0.000s
sys 0m30.567s
After:
real0m7.601s
user0m0.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_NOPREFIXROUTE0x200
#define IFA_F_MCAUTOJOIN 0x400
#define IFA_F_STABLE_PRIVACY 0x800
+#define IFA_F_FLUSH0x1000
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 = _dev->ifa_list; (ifa = *ifap) != NULL;
ifap = >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;
}
--
2.14.4
