| Message ID | 20220810160840.311628-1-alexander.mikhalitsyn@virtuozzo.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | neighbour: fix possible DoS due to net iface start/stop loop | expand |
On Wed, 10 Aug 2022 19:08:38 +0300 Alexander Mikhalitsyn wrote: > include/net/neighbour.h | 1 + > net/core/neighbour.c | 46 +++++++++++++++++++++++++++++++++-------- > 2 files changed, 38 insertions(+), 9 deletions(-) Which tree are these based on? They don't seem to apply cleanly
Hi, Jakub On Thu, Aug 11, 2022 at 5:46 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 10 Aug 2022 19:08:38 +0300 Alexander Mikhalitsyn wrote: > > include/net/neighbour.h | 1 + > > net/core/neighbour.c | 46 +++++++++++++++++++++++++++++++++-------- > > 2 files changed, 38 insertions(+), 9 deletions(-) > > Which tree are these based on? They don't seem to apply cleanly It's based on 5.19 tree, but I can easily resent it based on net-next. Regards, Alex
On Thu, 11 Aug 2022 17:51:32 +0300 Alexander Mikhalitsyn wrote: > On Thu, Aug 11, 2022 at 5:46 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 10 Aug 2022 19:08:38 +0300 Alexander Mikhalitsyn wrote: > > > include/net/neighbour.h | 1 + > > > net/core/neighbour.c | 46 +++++++++++++++++++++++++++++++++-------- > > > 2 files changed, 38 insertions(+), 9 deletions(-) > > > > Which tree are these based on? They don't seem to apply cleanly > > It's based on 5.19 tree, but I can easily resent it based on net-next. netdev/net would be the most appropriate tree for a fix. Not that it differs much from net-next at this stage of the merge window.
On Thu, Aug 11, 2022 at 5:53 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 11 Aug 2022 17:51:32 +0300 Alexander Mikhalitsyn wrote: > > On Thu, Aug 11, 2022 at 5:46 PM Jakub Kicinski <kuba@kernel.org> wrote: > > > On Wed, 10 Aug 2022 19:08:38 +0300 Alexander Mikhalitsyn wrote: > > > > include/net/neighbour.h | 1 + > > > > net/core/neighbour.c | 46 +++++++++++++++++++++++++++++++++-------- > > > > 2 files changed, 38 insertions(+), 9 deletions(-) > > > > > > Which tree are these based on? They don't seem to apply cleanly > > > > It's based on 5.19 tree, but I can easily resent it based on net-next. > > netdev/net would be the most appropriate tree for a fix. > Not that it differs much from net-next at this stage of > the merge window. Yes, thanks Jakub. I'm a newbie here ;) Sorry for the inconvenience. Will rebase and send patches soon.
Dear friends, Recently one of OpenVZ users reported that they have issues with network availability of some containers. It was discovered that the reason is absence of ARP replies from the Host Node on the requests about container IPs. Of course, we started from tcpdump analysis and noticed that ARP requests successfuly comes to the problematic node external interface. So, something was wrong from the kernel side. I've played a lot with arping and perf in attempts to understand what's happening. And the key observation was that we experiencing issues only with ARP requests with broadcast source ip (skb->pkt_type == PACKET_BROADCAST). But for packets skb->pkt_type == PACKET_HOST everything works flawlessly. Let me show a small piece of code: static int arp_process(struct sock *sk, struct sk_buff *skb) ... if (NEIGH_CB(skb)->flags & LOCALLY_ENQUEUED || skb->pkt_type == PACKET_HOST || NEIGH_VAR(in_dev->arp_parms, PROXY_DELAY) == 0) { // reply instantly arp_send_dst(ARPOP_REPLY, ETH_P_ARP, sip, dev, tip, sha, dev->dev_addr, sha, reply_dst); } else { pneigh_enqueue(&arp_tbl, // reply with delay in_dev->arp_parms, skb); goto out_free_dst; } The problem was that for PACKET_BROADCAST packets we delaying replies and use pneigh_enqueue() function. For some reason, queued packets were lost almost all the time! The reason for such behaviour is pneigh_queue_purge() function which cleanups all the queue, and this function called everytime once some network device in the system gets link down. neigh_ifdown -> pneigh_queue_purge Now imagine that we have a node with 500+ containers with microservices. And some of that microservices are buggy and always restarting... in this case, pneigh_queue_purge function will be called very frequently. This problem is reproducible only with so-called "host routed" setup. The classical scheme bridge + veth is not affected. Minimal reproducer Suppose that we have a network 172.29.1.1/16 brd 172.29.255.255 and we have free-to-use IP, let it be 172.29.128.3 1. Network configuration. I showing the minimal configuration, it makes no sense as we have both veth devices stay at the same net namespace, but for demonstation and simplicity sake it's okay. ip l a veth31427 type veth peer name veth314271 ip l s veth31427 up ip l s veth314271 up # setup static arp entry and publish it arp -Ds -i br0 172.29.128.3 veth31427 pub # setup static route for this address route add 172.29.128.3/32 dev veth31427 2. "attacker" side (kubernetes pod with buggy microservice :) ) unshare -n ip l a type veth ip l s veth0 up ip l s veth1 up for i in {1..100000}; do ip link set veth0 down; sleep 0.01; ip link set veth0 up; done This will totaly block ARP replies for 172.29.128.3 address. Just try # arping -I eth0 172.29.128.3 -c 4 Our proposal is simple: 1. Let's cleanup queue partially. Remove only skb's that related to the net namespace of the adapter which link is down. 2. Let's account proxy_queue limit properly per-device. Current limitation looks not fully correct because we comparing per-device configurable limit with the "global" qlen of proxy_queue. Thanks, Alex v2: - only ("neigh: fix possible DoS due to net iface start/stop") is changed do del_timer_sync() if queue is empty after pneigh_queue_purge() Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: David Ahern <dsahern@kernel.org> Cc: Yajun Deng <yajun.deng@linux.dev> Cc: Roopa Prabhu <roopa@nvidia.com> Cc: Christian Brauner <brauner@kernel.org> Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: Denis V. Lunev <den@openvz.org> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> Cc: Konstantin Khorenko <khorenko@virtuozzo.com> Cc: Pavel Tikhomirov <ptikhomirov@virtuozzo.com> Cc: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com> Cc: kernel@openvz.org Cc: devel@openvz.org Signed-off-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com> Alexander Mikhalitsyn (1): neighbour: make proxy_queue.qlen limit per-device Denis V. Lunev (1): neigh: fix possible DoS due to net iface start/stop loop include/net/neighbour.h | 1 + net/core/neighbour.c | 46 +++++++++++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 9 deletions(-)