netdev - Re: [PATCH net-next 3/4] net/tc: introduce TC_ACT

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] [day] [month] [year] [list]

Date:   Tue, 24 Jul 2018 08:48:19 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Jiri Pirko <jiri@...nulli.us>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Eyal Birger <eyal.birger@...il.com>
Subject: Re: [PATCH net-next 3/4] net/tc: introduce TC_ACT_MIRRED.

Hi,

On Mon, 2018-07-23 at 14:12 -0700, Cong Wang wrote:
> On Fri, Jul 20, 2018 at 2:54 AM Paolo Abeni <pabeni@...hat.com> wrote:
> > Note this is what already happens with TC_ACT_REDIRECT: currently the
> > user space uses it freely, even if only {cls,act}_bpf can return such
> > value in a meaningful way, and only from the ingress and the egress
> > hooks.
>
> Yes, my question is why do we give user such a freedom?
> 
> In other words, what do you want users to choose here? To scrub or not
> to scrub? To clone or not to clone?
> 
> From my understanding of your whole patchset, your goal is to get rid
> of clone, and users definitely don't care about clone or not clone for
> redirections, this is why I insist it doesn't need to be visible to user.

Thank you for your kind reply!

No, my intention is not to expose to the user-space another option. I
added the  additional tcfa_action value in response to concerns exposed
vs the v1 version of this series (it changed the act_mirred behaviour
and possibly broke some use-case).

When assembling the v2 I did not implemented the (deserved) isolation
vs user-space because of the already existing TC_ACT_REDIRECT: its
current implementation fooled me to think such considerations were not
relevant.

> If your goal is not just skipping clone, but also, let's say, scrub or not
> scrub, then it should be visible to users. However, I don't see why
> users care about scrub or not, they have to understand what scrub
> is at least, it is a purely kernel-internal behavior.

I agree to hide TC_ACT_REINJECT and any choice about scrubbing to user-
space, as per the code chunk I  posted before. I'll send a v3
implementing such schema.

Cheers,

Paolo