Re: [PATCH] netfilter: xt_ecn: Add missing hotdrop mark.

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Sat, Jul 29, 2017 at 07:33:00PM +0900, Taehee Yoo wrote:
> If the netfilter can't get L4 header, the netfilter
> marks hotdrop value. then {ip, ip6, arp, eb}t_do_table() drops
> that packet immediately. but xt_ecn doesn't mark hotdrop value.
> 
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
>  net/netfilter/xt_ecn.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
> index 3c831a8..c58db1d 100644
> --- a/net/netfilter/xt_ecn.c
> +++ b/net/netfilter/xt_ecn.c
> @@ -37,8 +37,10 @@ static bool match_tcp(const struct sk_buff *skb, struct xt_action_param *par)
>  	 * be good citizens.
>  	 */
>  	th = skb_header_pointer(skb, par->thoff, sizeof(_tcph), &_tcph);
> -	if (th == NULL)
> +	if (!th) {
> +		par->hotdrop = true;
>  		return false;
> +	}

Is it that we always - consistenly - drop packets that has no
information that we need.

I would say it's better to do this via policy, it's more flexible,
rather than assuming that accessing a packet that doesn't contain the
information that we need means a drop.

Another concern for me regarding this is the fact that probably this
has been the default behaviour for long time, if that's the case, I
would be reluctant to change this at this point.

Let me know, thanks!