From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Abeni Subject: Re: [PATCH net-next 3/4] net/tc: introduce TC_ACT_MIRRED. Date: Tue, 24 Jul 2018 08:48:19 +0200 Message-ID: References: <78e96c4fd40645bb4bce45b6502ed34cf502356f.1531941678.git.pabeni@redhat.com> <202e02c5fa084874e48b9347f09b256f23e63d91.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Jiri Pirko , Linux Kernel Network Developers , Jamal Hadi Salim , Daniel Borkmann , Marcelo Ricardo Leitner , Eyal Birger To: Cong Wang Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53314 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2388351AbeGXHxR (ORCPT ); Tue, 24 Jul 2018 03:53:17 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Hi, On Mon, 2018-07-23 at 14:12 -0700, Cong Wang wrote: > On Fri, Jul 20, 2018 at 2:54 AM Paolo Abeni 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