zebra: avoid pbr iptable added twice when used with flowspec

The usage of zebra dplane makes the job asyncronous which implies
that a given job will try to add an iptable, while the second job
will not know that its iptable is the same as the former one.

The below exabgp rules stand for two bgp flowspec rules sent to
the bgp device:

flow {
route {match {
source 185.228.172.73/32;
destination 0.0.0.0/0;
source-port >=49156&<=49159;
}then {redirect 213.242.114.113;}}
route {match {
source 185.228.172.73/32;
destination 0.0.0.0/0;
source-port >=49160&<=49163;
}then {redirect 213.242.114.113;}}
}

This rule creates a single iptable, but in fact, the same iptable
name is appended twice. This results in duplicated entries in the
iptables context. This also results in contexts not flushed, when
BGP session or 'flush' operation is performed.

iptables-save:
[..]
-A PREROUTING -m set --match-set match0x55baf4c25cb0 src,src -g match0x55baf4c25cb0
-A PREROUTING -m set --match-set match0x55baf4c25cb0 src,src -g match0x55baf4c25cb0
-A match0x55baf4c25cb0 -j MARK --set-xmark 0x100/0xffffffff
-A match0x55baf4c25cb0 -j ACCEPT
-A match0x55baf4c25cb0 -j MARK --set-xmark 0x100/0xffffffff
-A match0x55baf4c25cb0 -j ACCEPT
[..]

This commit addresses this issue, by checking that an iptable
context is not already being processed. A flag is added in the
original iptable context, and a check is done if the iptable
context is not already being processed for install or uinstall.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This commit is contained in:
Philippe Guibert 2022-05-23 10:21:16 +02:00
parent ac425bfc2b
commit c9250e28e8
4 changed files with 42 additions and 5 deletions

View File

@ -876,9 +876,24 @@ void zsend_iptable_notify_owner(const struct zebra_dplane_ctx *ctx,
struct stream *s;
struct zebra_pbr_iptable ipt;
uint16_t cmd = ZEBRA_IPTABLE_NOTIFY_OWNER;
struct zebra_pbr_iptable *ipt_hash;
enum dplane_op_e op = dplane_ctx_get_op(ctx);
dplane_ctx_get_pbr_iptable(ctx, &ipt);
ipt_hash = hash_lookup(zrouter.iptable_hash, &ipt);
if (ipt_hash) {
if (op == DPLANE_OP_IPTABLE_ADD &&
CHECK_FLAG(ipt_hash->internal_flags,
IPTABLE_INSTALL_QUEUED))
UNSET_FLAG(ipt_hash->internal_flags,
IPTABLE_INSTALL_QUEUED);
else if (op == DPLANE_OP_IPTABLE_DELETE &&
CHECK_FLAG(ipt_hash->internal_flags,
IPTABLE_UNINSTALL_QUEUED))
UNSET_FLAG(ipt_hash->internal_flags,
IPTABLE_UNINSTALL_QUEUED);
}
if (IS_ZEBRA_DEBUG_PACKET)
zlog_debug("%s: Notifying %s id %u note %u", __func__,
zserv_command_string(cmd), ipt.unique, note);

View File

@ -4550,6 +4550,17 @@ iptable_update_internal(enum dplane_op_e op, struct zebra_pbr_iptable *iptable)
struct zebra_dplane_ctx *ctx;
int ret;
if ((op == DPLANE_OP_IPTABLE_ADD &&
CHECK_FLAG(iptable->internal_flags, IPTABLE_INSTALL_QUEUED)) ||
(op == DPLANE_OP_IPTABLE_DELETE &&
CHECK_FLAG(iptable->internal_flags, IPTABLE_UNINSTALL_QUEUED))) {
if (IS_ZEBRA_DEBUG_DPLANE_DETAIL)
zlog_debug(
"update dplane ctx %s: iptable %s already in progress",
dplane_op2str(op), iptable->ipset_name);
return result;
}
ctx = dplane_ctx_alloc();
ret = dplane_ctx_iptable_init(ctx, op, iptable);
@ -4562,14 +4573,19 @@ done:
atomic_fetch_add_explicit(&zdplane_info.dg_iptable_in, 1,
memory_order_relaxed);
if (ret == AOK)
if (ret == AOK) {
result = ZEBRA_DPLANE_REQUEST_QUEUED;
else {
if (op == DPLANE_OP_IPTABLE_ADD)
SET_FLAG(iptable->internal_flags,
IPTABLE_INSTALL_QUEUED);
else
SET_FLAG(iptable->internal_flags,
IPTABLE_UNINSTALL_QUEUED);
} else {
atomic_fetch_add_explicit(&zdplane_info.dg_iptable_errors, 1,
memory_order_relaxed);
dplane_ctx_free(&ctx);
}
return result;
}

View File

@ -812,8 +812,11 @@ static void *pbr_iptable_alloc_intern(void *arg)
void zebra_pbr_add_iptable(struct zebra_pbr_iptable *iptable)
{
(void)hash_get(zrouter.iptable_hash, iptable, pbr_iptable_alloc_intern);
(void)dplane_pbr_iptable_add(iptable);
struct zebra_pbr_iptable *ipt_hash;
ipt_hash = hash_get(zrouter.iptable_hash, iptable,
pbr_iptable_alloc_intern);
(void)dplane_pbr_iptable_add(ipt_hash);
}
void zebra_pbr_del_iptable(struct zebra_pbr_iptable *iptable)

View File

@ -171,6 +171,9 @@ struct zebra_pbr_iptable {
struct list *interface_name_list;
#define IPTABLE_INSTALL_QUEUED 1 << 1
#define IPTABLE_UNINSTALL_QUEUED 1 << 2
uint8_t internal_flags;
char ipset_name[ZEBRA_IPSET_NAME_SIZE];
};