Hi,

For a chance to get this included in 4.1 (although it'd be off by default)
can you test the attached patch with real traffic and report back?

Please note this patch has the accept_filter feature enabled so there is no
need to do anything besides ensuring it works as expected.

Thanks.


On Fri, Sep 4, 2015 at 3:12 PM, Federico Schwindt <[email protected]> wrote:

>
>
> On Fri, Sep 4, 2015 at 2:26 PM, Rafael Zalamena <[email protected]>
> wrote:
>
>> Em Fri, 4 Sep 2015 13:42:40 +0100
>> Federico Schwindt <[email protected]> escreveu:
>>
>> > Diff aside looking at the code my impression is that the
>> VTCP_filter_http()
>> > function is meant to be compiled in always so erroring out if it's not
>> > supported might be wrong here, or at least not when errno is EOPNOTSUPP/
>> > multiple times.
>>
>> People without it must have a way to find it out. The old way was not
>> including any code and avoid it at all, but then we had a code that
>> behaves
>> differently according to the system which seems better than not giving
>> any clues.
>>
>
> The older code will always include VTCP_filter_http() for Linux, it was
> not calling it though.
>
>
>>
>> > On 4 Sep 2015 1:35 pm, "Federico Schwindt" <[email protected]> wrote:
>> >
>> > > Did you see my second diff? :)
>> > --- SNIPPED ---
>>
>> Yes, I read your second diff.
>>
>> I agree with you that it would be better to kill the accept_filter
>> param ifdef guards to keep it avaliable in every system, but I don't
>> think it's a good idea to break the old behavior because people might
>> be expecting it. That's why I suggested the TCP_DEFER_ACCEPT detection
>> to make the code more portable instead of just looking for __linux.
>>
>
> My second diff doesn't change any behaviour so nothing will break unless
> you enable it.
>
>
diff --git a/include/tbl/params.h b/include/tbl/params.h
index d3a2982..c7f1479 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -30,21 +30,23 @@
 
 /*lint -save -e525 -e539 */
 
-#ifdef HAVE_ACCEPT_FILTERS
 PARAM(
 	/* name */	accept_filter,
 	/* typ */	bool,
 	/* min */	NULL,
 	/* max */	NULL,
+#if defined(HAVE_ACCEPT_FILTERS) || defined(__linux)
 	/* default */	"on",
+#else
+	/* default */	"off",
+#endif /* HAVE_ACCEPT_FILTERS || __linux */
 	/* units */	"bool",
 	/* flags */	MUST_RESTART,
 	/* s-text */
-	"Enable kernel accept-filters, (if available in the kernel).",
+	"Enable kernel accept-filters (if available in the kernel).",
 	/* l-text */	NULL,
 	/* func */	NULL
 )
-#endif /* HAVE_ACCEPT_FILTERS */
 
 PARAM(
 	/* name */	acceptor_sleep_decay,
_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reply via email to