Revert "platform: add bpf filter to ignore routes from routing daemons"

The socket BPF filter operates on the entire message (skb). One netlink
message (for RTM_NEWROUTE) usually contains a list of routes (struct
nlmsghdr), but the BPF filter was only looking at the first route to decide
whether to throw away the entire message. That is wrong.

This causes that we miss routes. It also means that the response to our
RTM_GETROUTE dump request could be filtered and we poll/wait (unsuccessfully)
for it:

  <trace> [1641829930.4111] platform-linux: netlink: read: wait for ACK for sequence number 26...

To get this right, the BPF filter would have to look at all routes in the
list to find out whether there are any we want to receive (and if
there are any, to pass the entire message, regardless that is might also
contain routes we want to ignore). As BPF does not support loops, that
is not trivial. Arguably, we still could do something where we look at a
bounded, unrolled number of routes, in the hope that there are not more
routes in one message that we support. The problem is that this BPF
filter is supposed to filter out massive amounts of routes, so most
messages will be filled to the brim with routes and we would have to
expect a high number of routes in the RTM_NEWROUTE that need checking.

We could still ignore routes in _new_from_nl_route(), to enter the
cache. That means, we would catch them later than with the BPF filter,
but still before a lot of the overhead of handling them. That probably
will be done next, but for now revert the commit.

This reverts commit e9ca5583e5.

https://bugzilla.redhat.com/show_bug.cgi?id=2037411
This commit is contained in:
Thomas Haller 2022-01-14 09:35:53 +01:00
parent 50dc71323b
commit c37a21ea29
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
3 changed files with 0 additions and 61 deletions

View file

@ -3409,11 +3409,6 @@ _new_from_nl_route(struct nlmsghdr *nlh, gboolean id_only)
if (!NM_IN_SET(rtm->rtm_type, RTN_UNICAST, RTN_LOCAL))
return NULL;
/* A BPF filter on the netlink socket already enforces this, but
* check it also here in case the filter couldn't be installed. */
if (rtm->rtm_protocol > RTPROT_STATIC && !NM_IN_SET(rtm->rtm_protocol, RTPROT_DHCP, RTPROT_RA))
return NULL;
if (nlmsg_parse_arr(nlh, sizeof(struct rtmsg), tb, policy) < 0)
return NULL;
@ -9548,10 +9543,6 @@ constructed(GObject *_object)
nm_assert(!nle);
}
nle = nl_socket_set_rtprot_filter(priv->nlh);
if (nle)
_LOGW("error adding routing protocol filter: %s (%d)", nm_strerror(nle), -nle);
fd = nl_socket_get_fd(priv->nlh);
_LOGD("Netlink socket for events established: port=%u, fd=%d",

View file

@ -9,7 +9,6 @@
#include <unistd.h>
#include <fcntl.h>
#include <linux/filter.h>
/*****************************************************************************/
@ -1046,55 +1045,6 @@ nl_socket_set_ext_ack(struct nl_sock *sk, gboolean enable)
return 0;
}
int
nl_socket_set_rtprot_filter(struct nl_sock *sk)
{
struct sock_filter filter[] = {
/*
* if (h->nlmsg_type != RTM_NEWROUTE && h->nlmsg_type != RTM_DELROUTE)
* return KEEP;
*/
BPF_STMT(BPF_LD | BPF_ABS | BPF_H, offsetof(struct nlmsghdr, nlmsg_type)),
BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, htons(RTM_NEWROUTE), 2, 0),
BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, htons(RTM_DELROUTE), 1, 0),
BPF_STMT(BPF_RET | BPF_K, 0xFFFF),
/*
* if (rtm->rtm_protocol <= RTPROT_STATIC)
* return KEEP;
*/
BPF_STMT(BPF_LD | BPF_ABS | BPF_B,
sizeof(struct nlmsghdr) + offsetof(struct rtmsg, rtm_protocol)),
BPF_JUMP(BPF_JMP | BPF_JGT | BPF_K, RTPROT_STATIC, 1, 0),
BPF_STMT(BPF_RET | BPF_K, 0xFFFF),
/*
* if (rtm->rtm_protocol == RTPROT_DHCP || rtm->rtm_protocol == RTPROT_RA)
* return KEEP;
*/
BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, RTPROT_DHCP, 1, 0),
BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, RTPROT_RA, 0, 1),
BPF_STMT(BPF_RET | BPF_K, 0xFFFF),
/* return DROP; */
BPF_STMT(BPF_RET | BPF_K, 0),
};
struct sock_fprog prog = {
.len = sizeof(filter) / sizeof(filter[0]),
.filter = filter,
};
int err;
if (sk->s_fd == -1)
return -NME_NL_BAD_SOCK;
err = setsockopt(sk->s_fd, SOL_SOCKET, SO_ATTACH_FILTER, &prog, sizeof(prog));
if (err < 0)
return -nm_errno_from_native(errno);
return 0;
}
void
nl_socket_disable_msg_peek(struct nl_sock *sk)
{

View file

@ -568,8 +568,6 @@ int nl_wait_for_ack(struct nl_sock *sk, const struct nl_cb *cb);
int nl_socket_set_ext_ack(struct nl_sock *sk, gboolean enable);
int nl_socket_set_rtprot_filter(struct nl_sock *sk);
/*****************************************************************************/
void *genlmsg_put(struct nl_msg *msg,