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

@@ -24,6 +24,7 @@
 
 #include <haproxy/api-t.h>
 #include <haproxy/buf-t.h>
+#include <haproxy/filters-t.h>
 #include <haproxy/show_flags-t.h>
 
 /* 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 */
 };
 
 

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 */

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;
 }

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);
 		}
 	}

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);