From d71e2e73eacfaaa41e2b7b098c63f9ef3568ec35 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Thu, 5 Feb 2026 13:55:36 +0100 Subject: [PATCH] MEDIUM: filters: use per-channel filter list when relevant In the historical implementation, all filter related information where stored at the stream level (using struct strm_flt * context), and filters iteration was performed at the stream level also. We identified that this was not ideal and would make the implementation of future filters more complex since filters ordering should be handled in a different order during request and response handling for decompression for instance. To make such thing possible, in this commit we migrate some channel specific filter contexts in the channel directly (request or response), and we implement 2 additional filter lists, one on the request channel and another on the response channel. The historical stream filter list is kept as-is because in some contexts only the stream is available and we have to iterate on all filters. But for functions where we only are interested in request side or response side filters, we now use dedicated channel filters list instead. The only overhead is that the "struct filter" was expanded by two "struct list". For now, no change of behavior is expected. --- include/haproxy/channel-t.h | 2 ++ include/haproxy/filters-t.h | 22 ++++++++++------ include/haproxy/filters.h | 51 +++++++++++++++++++++---------------- src/filters.c | 22 +++++++++++++--- src/stream.c | 8 +++--- 5 files changed, 67 insertions(+), 38 deletions(-) diff --git a/include/haproxy/channel-t.h b/include/haproxy/channel-t.h index 20afe7d65..a2a11a561 100644 --- a/include/haproxy/channel-t.h +++ b/include/haproxy/channel-t.h @@ -24,6 +24,7 @@ #include #include +#include #include /* The CF_* macros designate Channel Flags, which may be ORed in the bit field @@ -205,6 +206,7 @@ struct channel { unsigned char xfer_large; /* number of consecutive large xfers */ unsigned char xfer_small; /* number of consecutive small xfers */ int analyse_exp; /* expiration date for current analysers (if set) */ + struct chn_flt flt; /* current state of filters active on this channel */ }; diff --git a/include/haproxy/filters-t.h b/include/haproxy/filters-t.h index 6c1879fbe..19658d847 100644 --- a/include/haproxy/filters-t.h +++ b/include/haproxy/filters-t.h @@ -232,22 +232,28 @@ struct filter { * 0: request channel, 1: response channel */ unsigned int pre_analyzers; /* bit field indicating analyzers to pre-process */ unsigned int post_analyzers; /* bit field indicating analyzers to post-process */ - struct list list; /* Next filter for the same proxy/stream */ + struct list list; /* Filter list for the stream */ + /* req_list and res_list are exactly equivalent, except the order may differ */ + struct list req_list; /* Filter list for request channel */ + struct list res_list; /* Filter list for response channel */ }; /* * Structure reprensenting the "global" state of filters attached to a stream. + * Doesn't hold much information, as the channel themselves hold chn_flt struct + * which contains the per-channel members. */ struct strm_flt { struct list filters; /* List of filters attached to a stream */ - struct filter *current[2]; /* From which filter resume processing, for a specific channel. - * This is used for resumable callbacks only, - * If NULL, we start from the first filter. - * 0: request channel, 1: response channel */ unsigned short flags; /* STRM_FL_* */ - unsigned char nb_req_data_filters; /* Number of data filters registered on the request channel */ - unsigned char nb_rsp_data_filters; /* Number of data filters registered on the response channel */ - unsigned long long offset[2]; +}; + +/* structure holding filter state for some members that are channel oriented */ +struct chn_flt { + struct list filters; /* List of filters attached to a channel */ + struct filter *current; /* From which filter resume processing, for a specific channel. */ + unsigned char nb_data_filters; /* Number of data filters registered on channel */ + unsigned long long offset; }; #endif /* _HAPROXY_FILTERS_T_H */ diff --git a/include/haproxy/filters.h b/include/haproxy/filters.h index 6a3a7e35b..d2c5dd407 100644 --- a/include/haproxy/filters.h +++ b/include/haproxy/filters.h @@ -40,13 +40,13 @@ extern const char *fcgi_flt_id; /* Useful macros to access per-channel values. It can be safely used inside * filters. */ #define CHN_IDX(chn) (((chn)->flags & CF_ISRESP) == CF_ISRESP) -#define FLT_STRM_OFF(s, chn) (strm_flt(s)->offset[CHN_IDX(chn)]) +#define FLT_STRM_OFF(s, chn) (chn->flt.offset) #define FLT_OFF(flt, chn) ((flt)->offset[CHN_IDX(chn)]) #define HAS_FILTERS(strm) ((strm)->strm_flt.flags & STRM_FLT_FL_HAS_FILTERS) -#define HAS_REQ_DATA_FILTERS(strm) ((strm)->strm_flt.nb_req_data_filters != 0) -#define HAS_RSP_DATA_FILTERS(strm) ((strm)->strm_flt.nb_rsp_data_filters != 0) +#define HAS_REQ_DATA_FILTERS(strm) ((strm)->req.flt.nb_data_filters != 0) +#define HAS_RSP_DATA_FILTERS(strm) ((strm)->res.flt.nb_data_filters != 0) #define HAS_DATA_FILTERS(strm, chn) (((chn)->flags & CF_ISRESP) ? HAS_RSP_DATA_FILTERS(strm) : HAS_REQ_DATA_FILTERS(strm)) #define IS_REQ_DATA_FILTER(flt) ((flt)->flags & FLT_FL_IS_REQ_DATA_FILTER) @@ -137,14 +137,11 @@ static inline void register_data_filter(struct stream *s, struct channel *chn, struct filter *filter) { if (!IS_DATA_FILTER(filter, chn)) { - if (chn->flags & CF_ISRESP) { + if (chn->flags & CF_ISRESP) filter->flags |= FLT_FL_IS_RSP_DATA_FILTER; - strm_flt(s)->nb_rsp_data_filters++; - } - else { + else filter->flags |= FLT_FL_IS_REQ_DATA_FILTER; - strm_flt(s)->nb_req_data_filters++; - } + chn->flt.nb_data_filters++; } } @@ -153,15 +150,11 @@ static inline void unregister_data_filter(struct stream *s, struct channel *chn, struct filter *filter) { if (IS_DATA_FILTER(filter, chn)) { - if (chn->flags & CF_ISRESP) { + if (chn->flags & CF_ISRESP) filter->flags &= ~FLT_FL_IS_RSP_DATA_FILTER; - strm_flt(s)->nb_rsp_data_filters--; - - } - else { + else filter->flags &= ~FLT_FL_IS_REQ_DATA_FILTER; - strm_flt(s)->nb_req_data_filters--; - } + chn->flt.nb_data_filters--; } } @@ -186,9 +179,16 @@ static inline struct filter *flt_list_start(struct stream *strm, struct channel { struct filter *filter; - filter = LIST_NEXT(&strm_flt(strm)->filters, struct filter *, list); - if (&filter->list == &strm_flt(strm)->filters) - filter = NULL; /* empty list */ + if (chn->flags & CF_ISRESP) { + filter = LIST_NEXT(&chn->flt.filters, struct filter *, res_list); + if (&filter->res_list == &chn->flt.filters) + filter = NULL; /* empty list */ + } + else { + filter = LIST_NEXT(&chn->flt.filters, struct filter *, req_list); + if (&filter->req_list == &chn->flt.filters) + filter = NULL; /* empty list */ + } return filter; } @@ -196,9 +196,16 @@ static inline struct filter *flt_list_start(struct stream *strm, struct channel static inline struct filter *flt_list_next(struct stream *strm, struct channel *chn, struct filter *filter) { - filter = LIST_NEXT(&filter->list, struct filter *, list); - if (&filter->list == &strm_flt(strm)->filters) - filter = NULL; /* end of list */ + if (chn->flags & CF_ISRESP) { + filter = LIST_NEXT(&filter->res_list, struct filter *, res_list); + if (&filter->res_list == &chn->flt.filters) + filter = NULL; /* end of list */ + } + else { + filter = LIST_NEXT(&filter->req_list, struct filter *, req_list); + if (&filter->req_list == &chn->flt.filters) + filter = NULL; /* end of list */ + } return filter; } diff --git a/src/filters.c b/src/filters.c index 0596d9a38..f12f16e2e 100644 --- a/src/filters.c +++ b/src/filters.c @@ -67,9 +67,9 @@ static inline struct filter *resume_filter_list_start(struct stream *strm, struc { struct filter *filter; - if (strm_flt(strm)->current[CHN_IDX(chn)]) { - filter = strm_flt(strm)->current[CHN_IDX(chn)]; - strm_flt(strm)->current[CHN_IDX(chn)] = NULL; + if (chn->flt.current) { + filter = chn->flt.current; + chn->flt.current = NULL; if (!(chn_prod(chn)->flags & SC_FL_ERROR) && !(chn->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT))) { (strm)->waiting_entity.type = STRM_ENTITY_NONE; @@ -100,7 +100,7 @@ static inline void resume_filter_list_break(struct stream *strm, struct channel strm->last_entity.type = STRM_ENTITY_FILTER; strm->last_entity.ptr = filter; } - strm_flt(strm)->current[CHN_IDX(chn)] = filter; + chn->flt.current = filter; } /* List head of all known filter keywords */ @@ -455,6 +455,14 @@ flt_stream_add_filter(struct stream *s, struct flt_conf *fconf, unsigned int fla } LIST_APPEND(&strm_flt(s)->filters, &f->list); + + /* for now f->req_list == f->res_list to preserve + * historical behavior, but the ordering will change + * in the future + */ + LIST_APPEND(&s->req.flt.filters, &f->req_list); + LIST_APPEND(&s->res.flt.filters, &f->res_list); + strm_flt(s)->flags |= STRM_FLT_FL_HAS_FILTERS; return 0; } @@ -470,6 +478,10 @@ flt_stream_init(struct stream *s) memset(strm_flt(s), 0, sizeof(*strm_flt(s))); LIST_INIT(&strm_flt(s)->filters); + memset(&s->req.flt, 0, sizeof(s->req.flt)); + LIST_INIT(&s->req.flt.filters); + memset(&s->res.flt, 0, sizeof(s->res.flt)); + LIST_INIT(&s->res.flt.filters); list_for_each_entry(fconf, &strm_fe(s)->filter_configs, list) { if (flt_stream_add_filter(s, fconf, 0) < 0) return -1; @@ -494,6 +506,8 @@ flt_stream_release(struct stream *s, int only_backend) if (FLT_OPS(filter)->detach) FLT_OPS(filter)->detach(s, filter); LIST_DELETE(&filter->list); + LIST_DELETE(&filter->req_list); + LIST_DELETE(&filter->res_list); pool_free(pool_head_filter, filter); } } diff --git a/src/stream.c b/src/stream.c index f77090729..891377301 100644 --- a/src/stream.c +++ b/src/stream.c @@ -3771,8 +3771,8 @@ static void __strm_dump_to_buffer(struct buffer *buf, const struct show_sess_ctx htx, htx->flags, htx->size, htx->data, htx_nbblks(htx), (htx->tail >= htx->head) ? "NO" : "YES"); } - if (HAS_FILTERS(strm) && strm->strm_flt.current[0]) { - const struct filter *flt = strm->strm_flt.current[0]; + if (HAS_FILTERS(strm) && strm->req.flt.current) { + const struct filter *flt = strm->req.flt.current; chunk_appendf(buf, "%s current_filter=%p (id=\"%s\" flags=0x%x pre=0x%x post=0x%x) \n", pfx, flt, flt->config->id, flt->flags, flt->pre_analyzers, flt->post_analyzers); @@ -3804,8 +3804,8 @@ static void __strm_dump_to_buffer(struct buffer *buf, const struct show_sess_ctx (htx->tail >= htx->head) ? "NO" : "YES"); } - if (HAS_FILTERS(strm) && strm->strm_flt.current[1]) { - const struct filter *flt = strm->strm_flt.current[1]; + if (HAS_FILTERS(strm) && strm->res.flt.current) { + const struct filter *flt = strm->res.flt.current; chunk_appendf(buf, "%s current_filter=%p (id=\"%s\" flags=0x%x pre=0x%x post=0x%x) \n", pfx, flt, flt->config->id, flt->flags, flt->pre_analyzers, flt->post_analyzers);