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]

Date:   Thu, 6 Aug 2020 22:51:23 +0530
From:   satish dhote <sdhote926@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: Question about TC filter

Hi Cong,

I tried adding below patch i.e. "return cl == 0 ? q->block : NULL;"
but after this I'm not able to see any output using "tc filter show... "
command. Looks like the filter is not getting configured.

If this is a bug, then do I need to file a new ticket for this?

Thanks
Satish

On Thu, Aug 6, 2020 at 5:36 AM Cong Wang <xiyou.wangcong@...il.com> wrote:
>
> 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.