From: Philippe Guibert Date: Mon, 23 May 2022 08:21:16 +0000 (+0200) Subject: zebra: avoid pbr iptable added twice when used with flowspec X-Git-Tag: base_8.3~25^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=refs%2Fpull%2F11244%2Fhead;p=mirror%2Ffrr.git 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 --- diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 408d420be6..9a30c2b78f 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -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); diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 4e753c9d1a..0da44e3c4e 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -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; } diff --git a/zebra/zebra_pbr.c b/zebra/zebra_pbr.c index e66d7aaf5c..bb963bcc23 100644 --- a/zebra/zebra_pbr.c +++ b/zebra/zebra_pbr.c @@ -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) diff --git a/zebra/zebra_pbr.h b/zebra/zebra_pbr.h index c5102df4fa..33b8162a88 100644 --- a/zebra/zebra_pbr.h +++ b/zebra/zebra_pbr.h @@ -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]; };