Re: [v1,net-next 2/5] net: qos: introduce a gate control flow action

[Jiri Pirko <jiri@resnulli.us>]

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

[...]