Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 24 Mar 2020 11:19:21 +0100
From:   Jiri Pirko <jiri@resnulli.us>
To:     Po Liu <Po.Liu@nxp.com>
Cc:     davem@davemloft.net, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, vinicius.gomes@intel.com,
        claudiu.manoil@nxp.com, vladimir.oltean@nxp.com,
        alexandru.marginean@nxp.com, xiaoliang.yang_1@nxp.com,
        roy.zang@nxp.com, mingkai.hu@nxp.com, jerry.huang@nxp.com,
        leoyang.li@nxp.com, michael.chan@broadcom.com, vishal@chelsio.com,
        saeedm@mellanox.com, leon@kernel.org, jiri@mellanox.com,
        idosch@mellanox.com, alexandre.belloni@bootlin.com,
        UNGLinuxDriver@microchip.com, kuba@kernel.org, jhs@mojatatu.com,
        xiyou.wangcong@gmail.com, simon.horman@netronome.com,
        pablo@netfilter.org, moshe@mellanox.com, m-karicheri2@ti.com,
        andre.guedes@linux.intel.com, stephen@networkplumber.org
Subject: Re: [v1,net-next  2/5] net: qos: introduce a gate control flow action
Message-ID: <20200324101921.GS11304@nanopsycho.orion>
References: <20200306125608.11717-11-Po.Liu@nxp.com>
 <20200324034745.30979-1-Po.Liu@nxp.com>
 <20200324034745.30979-3-Po.Liu@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200324034745.30979-3-Po.Liu@nxp.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Tue, Mar 24, 2020 at 04:47:40AM CET, Po.Liu@nxp.com wrote:
>Introduce a ingress frame gate control flow action. tc create a gate
>action would provide a gate list to control when open/close state. when
>the gate open state, the flow could pass but not when gate state is
>close. The driver would repeat the gate list cyclically. User also could
>assign a time point to start the gate list by the basetime parameter. if
>the basetime has passed current time, start time would calculate by the
>cycletime of the gate list.

Cannot decypher this either :/ Seriously, please make the patch
descriptions readable.

Also, a sentence starts with capital letter.


>The action gate behavior try to keep according to the IEEE 802.1Qci spec.
>For the software simulation, require the user input the clock type.
>
>Below is the setting example in user space. Tc filter a stream source ip
>address is 192.168.0.20 and gate action own two time slots. One is last
>200ms gate open let frame pass another is last 100ms gate close let
>frames dropped. When the passed total frames over 8000000 bytes, it will
>dropped in one 200000000ns time slot.
>
>> tc qdisc add dev eth0 ingress
>
>> tc filter add dev eth0 parent ffff: protocol ip \
>	   flower src_ip 192.168.0.20 \
>	   action gate index 2 clockid CLOCK_TAI \
>	   sched-entry OPEN 200000000 -1 8000000 \
>	   sched-entry CLOSE 100000000 -1 -1

The rest of the commands do not use capitals. Please lowercase these.


>
>> tc chain del dev eth0 ingress chain 0
>
>"sched-entry" follow the name taprio style. gate state is
>"OPEN"/"CLOSE". Follow the period nanosecond. Then next item is internal
>priority value means which ingress queue should put. "-1" means
>wildcard. The last value optional specifies the maximum number of
>MSDU octets that are permitted to pass the gate during the specified
>time interval.
>Base-time is not set will be as 0 as default, as result start time would
>be ((N + 1) * cycletime) which is the minimal of future time.
>
>Below example shows filtering a stream with destination mac address is
>10:00:80:00:00:00 and ip type is ICMP, follow the action gate. The gate
>action would run with one close time slot which means always keep close.
>The time cycle is total 200000000ns. The base-time would calculate by:
>
> 1357000000000 + (N + 1) * cycletime
>
>When the total value is the future time, it will be the start time.
>The cycletime here would be 200000000ns for this case.
>
>> tc filter add dev eth0 parent ffff:  protocol ip \
>	   flower skip_hw ip_proto icmp dst_mac 10:00:80:00:00:00 \
>	   action gate index 12 base-time 1357000000000 \
>	   sched-entry CLOSE 200000000 -1 -1 \
>	   clockid CLOCK_TAI
>
>NOTE: This software simulator version not separate the admin/operation
>state machine. Update setting would overwrite stop the previos setting
>and waiting new cycle start.
>

[...]


>diff --git a/net/sched/Kconfig b/net/sched/Kconfig
>index bfbefb7bff9d..320471a0a21d 100644
>--- a/net/sched/Kconfig
>+++ b/net/sched/Kconfig
>@@ -981,6 +981,21 @@ config NET_ACT_CT
> 	  To compile this code as a module, choose M here: the
> 	  module will be called act_ct.
> 
>+config NET_ACT_GATE
>+	tristate "Frame gate list control tc action"
>+	depends on NET_CLS_ACT
>+	help
>+	  Say Y here to allow the control the ingress flow by the gate list

"to control"?


>+	  control. The frame policing by the time gate list control open/close

Incomplete sentence.


>+	  cycle time. The manipulation will simulate the IEEE 802.1Qci stream
>+	  gate control behavior. The action could be offload by the tc flower
>+	  to hardware driver which the hardware own the capability of IEEE
>+	  802.1Qci.

We do not mention offload for the other actions. I suggest to not to
mention it here either.


>+
>+	  If unsure, say N.
>+	  To compile this code as a module, choose M here: the
>+	  module will be called act_gate.
>+
> config NET_IFE_SKBMARK
> 	tristate "Support to encoding decoding skb mark on IFE action"
> 	depends on NET_ACT_IFE

[...]