Re: [PATCH net-next 1/7] net: sched: Add a trap-and-forward action

From: Jamal Hadi Salim <jhs@mojatatu.com>
To: Petr Machata <petrm@nvidia.com>
Cc: netdev@vger.kernel.org, Jiri Pirko <jiri@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Ido Schimmel <idosch@nvidia.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>
Subject: Re: [PATCH net-next 1/7] net: sched: Add a trap-and-forward action
Date: Sun, 11 Apr 2021 15:23:02 -0400	[thread overview]
Message-ID: <137b2a5b-8a05-e699-1ac9-3dc072ea16a7@mojatatu.com> (raw)
In-Reply-To: <8735vz609x.fsf@nvidia.com>

On 2021-04-09 9:43 a.m., Petr Machata wrote:
> 
> Jamal Hadi Salim <jhs@mojatatu.com> writes:
> 

>>>> Does the spectrum not support multiple actions?
>>>> e.g with a policy like:
>>>>    match blah action trap action drop skip_sw
>>> Trap drops implicitly. We need a "trap, but don't drop". Expressed in
>>> terms of existing actions it would be "mirred egress redirect dev
>>> $cpu_port". But how to express $cpu_port except again by a HW-specific
>>> magic token I don't know.
> 
> (I meant mirred egress mirror, not redirect.)
>

Ok.

>> Note: mirred was originally intended to send redirect/mirror
>> packets to user space (the comment is still there in the code).
>> Infact there is a patch lying around somewhere that does that with
>> packet sockets (the author hasnt been serious about pushing it
>> upstream). In that case the semantics are redirecting to a file
>> descriptor. Could we have something like that here which points
>> to whatever representation $cpu_port has? Sounds like semantics
>> for "trap and forward" are just "mirror and forward".
> 
> Hmm, we have devlink ports, the CPU port is exposed there. But that's
> the only thing that comes to mind. Those are specific for the given
> device though, it doesn't look suitable...
> 

If it has an ifindex should be good enough for abstraction
purposes.

>> I think there is value in having something like trap action
>> which generalizes the combinations only to the fact that
>> it will make it easier to relay the info to the offload without
>> much transformation.
>> If i was to do it i would write one action configured by user space:
>> - to return DROP if you want action trap-and-drop semantics.
>> - to return STOLEN if you want trap
>> - to return PIPE if you want trap and forward. You will need a second
>> action composed to forward.
> 
> I think your STOLEN and PIPE are the same behavior. Both are "transfer
> the packet to the SW datapath, but keep it in the HW datapath".
> 
> In general I have no issue expressing this stuff as a new action,
> instead of an opcode. I'll take a look at this.
> 

Ok, thanks.

cheers,
jamal