Message ID | 20180607101301.30439-1-jkbs@redhat.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
Series | [RFC,net-next] ipv4: Don't promote secondaries when flushing addresses | expand |
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
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
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
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
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; }
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(-) -- 2.14.4