netdev - Re: Question about TC filter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM_iQpWhwQc4yHvfFh-UWtEU2caMzXFXs4JM4gwQaRf=B0JG5g@mail.gmail.com>
Date:   Wed, 5 Aug 2020 17:05:48 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     satish dhote <sdhote926@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: Question about TC filter

Hello,

On Tue, Aug 4, 2020 at 10:39 PM satish dhote <sdhote926@...il.com> wrote:
>
> Hi Team,
>
> I have a question regarding tc filter behavior. I tried to look
> for the answer over the web and netdev FAQ but didn't get the
> answer. Hence I'm looking for your help.
>
> I added ingress qdisc for interface enp0s25 and then configured the
> tc filter as shown below, but after adding filters I realize that
> rule is reflected as a result of both ingress and egress filter
> command?  Is this the expected behaviour? or a bug? Why should the
> same filter be reflected in both ingress and egress path?

I am not very sure but I am feeling this is a bug. Let's Cc Daniel who
introduced this.

With the introduction of clsact qdisc, the keywords "ingress" and
"egress" were introduced too to match clsact qdisc. However, its
major/minor handle is kinda confusing:

TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS);
TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_EGRESS);
#define TC_H_CLSACT TC_H_INGRESS

They could match the ingress qdisc (ffff:) too.


>
> I understand that policy is always configured for ingress traffic,
> so I believe that filters should not be reflected with egress.
> Behaviour is same when I offloaded ovs flow to the tc software
> datapath.

I believe so too, otherwise it would be too confusing to users.

If you are able to test kernel patch, does the following one-line
change fix this problem? If not, I will try it on my side.

diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index 84838128b9c5..4d9c1bb15545 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -49,7 +49,7 @@ static struct tcf_block *ingress_tcf_block(struct
Qdisc *sch, unsigned long cl,
 {
        struct ingress_sched_data *q = qdisc_priv(sch);

-       return q->block;
+       return cl == 0 ? q->block : NULL;
 }

 static void clsact_chain_head_change(struct tcf_proto *tp_head, void *priv)


Thanks.