RE: [EXT] Re: [v3,net-next 1/4] net: qos: introduce a gate control flow action

From: Po Liu
Date: Wed Apr 22 2020 - 23:30:06 EST


Hi Dave That,


> -----Original Message-----
> From: Dave Taht <dave.taht@xxxxxxxxx>
> Sent: 2020å4æ23æ 4:06
> To: Vladimir Oltean <olteanv@xxxxxxxxx>
> Cc: Allan W. Nielsen <allan.nielsen@xxxxxxxxxxxxx>; Po Liu
> <po.liu@xxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; lkml <linux-
> kernel@xxxxxxxxxxxxxxx>; netdev <netdev@xxxxxxxxxxxxxxx>; Vinicius Costa
> Gomes <vinicius.gomes@xxxxxxxxx>; Claudiu Manoil
> <claudiu.manoil@xxxxxxx>; Vladimir Oltean <vladimir.oltean@xxxxxxx>;
> Alexandru Marginean <alexandru.marginean@xxxxxxx>;
> michael.chan@xxxxxxxxxxxx; vishal@xxxxxxxxxxx; Saeed Mahameed
> <saeedm@xxxxxxxxxxxx>; leon@xxxxxxxxxx; Jiri Pirko
> <jiri@xxxxxxxxxxxx>; Ido Schimmel <idosch@xxxxxxxxxxxx>; Alexandre
> Belloni <alexandre.belloni@xxxxxxxxxxx>; Microchip Linux Driver Support
> <UNGLinuxDriver@xxxxxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>;
> Jamal Hadi Salim <jhs@xxxxxxxxxxxx>; Cong Wang
> <xiyou.wangcong@xxxxxxxxx>; simon.horman@xxxxxxxxxxxxx; Pablo
> Neira Ayuso <pablo@xxxxxxxxxxxxx>; moshe@xxxxxxxxxxxx; Murali
> Karicheri <m-karicheri2@xxxxxx>; Andre Guedes
> <andre.guedes@xxxxxxxxxxxxxxx>; Stephen Hemminger
> <stephen@xxxxxxxxxxxxxxxxxx>
> Subject: [EXT] Re: [v3,net-next 1/4] net: qos: introduce a gate control flow
> action
>
> Caution: EXT Email
>
> On Wed, Apr 22, 2020 at 12:31 PM Vladimir Oltean <olteanv@xxxxxxxxx>
> wrote:
> >
> > Hi Allan,
> >
> > On Wed, 22 Apr 2020 at 22:20, Allan W. Nielsen
> > <allan.nielsen@xxxxxxxxxxxxx> wrote:
> > >
> > > Hi Po,
> > >
> > > Nice to see even more work on the TSN standards in the upstream
> kernel.
> > >
> > > On 22.04.2020 10:48, Po Liu wrote:
> > > >EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > >know the content is safe
> > > >
> > > >Introduce a ingress frame gate control flow action.
> > > >Tc gate action does the work like this:
> > > >Assume there is a gate allow specified ingress frames can be passed
> > > >at specific time slot, and be dropped at specific time slot. Tc
> > > >filter chooses the ingress frames, and tc gate action would specify
> > > >what slot does these frames can be passed to device and what time
> > > >slot would be dropped.
> > > >Tc gate action would provide an entry list to tell how much time
> > > >gate keep open and how much time gate keep state close. Gate
> action
> > > >also assign a start time to tell when the entry list start. Then
> > > >driver would repeat the gate entry list cyclically.
> > > >For the software simulation, gate action requires the user assign a
> > > >time 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 frames have passed
> > > >total frames over 8000000 bytes, frames will be 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
> > >
> > > First of all, it is a long time since I read the 802.1Qci and when I
> > > did it, it was a draft. So please let me know if I'm completly off here.
> > >
> > > I know you are focusing on the gate control in this patch serie, but
> > > I assume that you later will want to do the policing and flow-meter
> > > as well. And it could make sense to consider how all of this work
> > > toghether.
> > >
> > > A common use-case for the policing is to have multiple rules
> > > pointing at the same policing instance. Maybe you want the sum of
> > > the traffic on 2 ports to be limited to 100mbit. If you specify such
> > > action on the individual rule (like done with the gate), then you
> > > can not have two rules pointing at the same policer instance.
> > >
> > > Long storry short, have you considered if it would be better to do
> > > something like:
> > >
> > > tc filter add dev eth0 parent ffff: protocol ip \
> > > flower src_ip 192.168.0.20 \
> > > action psfp-id 42
> > >
> > > And then have some other function to configure the properties of
> > > psfp-id 42?
> > >
> > >
> > > /Allan
> > >
> >
> > It is very good that you brought it up though, since in my opinion too
> > it is a rather important aspect, and it seems that the fact this
> > feature is already designed-in was a bit too subtle.
> >
> > "psfp-id" is actually his "index" argument.
> >
> > You can actually do this:
> > tc filter add dev eth0 ingress \
> > flower skip_hw dst_mac 01:80:c2:00:00:0e \
> > action gate index 1 clockid CLOCK_TAI \
> > base-time 200000000000 \
> > sched-entry OPEN 200000000 -1 -1 \
> > sched-entry CLOSE 100000000 -1 -1 tc filter add dev eth0
> > ingress \
> > flower skip_hw dst_mac 01:80:c2:00:00:0f \
> > action gate index 1
> >
> > Then 2 filters get created with the same action:
> >
> > tc -s filter show dev swp2 ingress
> > filter protocol all pref 49151 flower chain 0 filter protocol all pref
> > 49151 flower chain 0 handle 0x1
> > dst_mac 01:80:c2:00:00:0f
> > skip_hw
> > not_in_hw
> > action order 1:
> > priority wildcard clockid TAI flags 0x6404f
> > base-time 200000000000 cycle-time 300000000
> > cycle-time-ext 0
> > number 0 gate-state open interval 200000000
> > ipv wildcard max-octets wildcard
> > number 1 gate-state close interval 100000000
> > ipv wildcard max-octets wildcard
> > pipe
> > index 2 ref 2 bind 2 installed 168 sec used 168 sec
> > Action statistics:
> > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> > backlog 0b 0p requeues 0
> >
> > filter protocol all pref 49152 flower chain 0 filter protocol all pref
> > 49152 flower chain 0 handle 0x1
> > dst_mac 01:80:c2:00:00:0e
> > skip_hw
> > not_in_hw
> > action order 1:
> > priority wildcard clockid TAI flags 0x6404f
> > base-time 200000000000 cycle-time 300000000
> > cycle-time-ext 0
> > number 0 gate-state open interval 200000000
> > ipv wildcard max-octets wildcard
> > number 1 gate-state close interval 100000000
> > ipv wildcard max-octets wildcard
> > pipe
> > index 2 ref 2 bind 2 installed 168 sec used 168 sec
> > Action statistics:
> > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> > backlog 0b 0p requeues 0
> >
> > Actually my only concern is that maybe this mechanism should (?) have
> > been more generic. At the moment, this patch series implements it via
> > a TCA_GATE_ENTRY_INDEX netlink attribute, so every action which
> wants
> > to be shared across filters needs to reinvent this wheel.
> >
> > Thoughts, everyone?
>
> I don't have anything valuable to add, aside from commenting this whole
> thing makes my brain hurt.

Thanks for express your thoughts.

>
> > Thanks,
> > -Vladimir
>
>
>
> --
> Make Music, Not War

Thanks!

>
> Dave TÃht
> CTO, TekLibre, LLC
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .teklibre.com%2F&amp;data=02%7C01%7CPo.Liu%40nxp.com%7C09cf63bb
> 73ee4fb2e08108d7e6f8929f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C637231827599217479&amp;sdata=u3ILmBlK6RsVYDLy9gtFythp%2
> FbG2%2Bw40xea2N1sJvr4%3D&amp;reserved=0
> Tel: 1-831-435-0729



Br,
Po Liu