RE: [EXT] Re: [RFC,net-next 2/9] net: qos: introduce a gate control flow action

[Po Liu]

Hi Vinicius,


> -----Original Message-----
> From: Vinicius Costa Gomes <vinicius.gomes@xxxxxxxxx>
> Sent: 2020年3月13日 6:14
> >> 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 \
> >          sched-entry OPEN 200000000 -1 -1 \
> >          sched-entry CLOSE 100000000 -1 -1
> >
> >> tc chain del dev eth0 ingress chain 0
> 
> A more complex example, showing how it would work for more filters
> would be nice.
> 
> For example, filters matching 3 different source IPs (even better, using
> MAC addresses so it would work when you add the 'skip_sw' flag),
> assigning different priorities to them, also showing how the "base-time"
> could be used.

Ok, I would update with a more clear example.

> >
> > +static int tcf_gate_act(struct sk_buff *skb, const struct tc_action *a,
> > +                     struct tcf_result *res) {
> > +     struct tcf_gate *g = to_gate(a);
> > +     struct tcf_gate_params *p;
> > +     struct gate_action *gact;
> > +     int action;
> > +
> > +     tcf_lastuse_update(&g->tcf_tm);
> > +     tcf_action_update_bstats(&g->common, skb);
> > +
> 
> Please test this with lockdep enabled, I got the feeling that there's a
> missing rcu_read_lock() somewhere around here.

I will add lock here.

> 
> > +     gact->tk_offset = tk_offset;
> > +     spin_lock_init(&gact->entry_lock);
> > +     hrtimer_init(&gact->hitimer, clockid, HRTIMER_MODE_ABS);
> > +     gact->hitimer.function = gate_timer_func;
> 
> Hm, having an hrtimer per filter seems kind dangerous, in the sense that it
> seems very hard to configure right.

Per gate action exactly. 

> 
> But I don't have any alternative ideas for now.

Yes, hrtimer should only start when software gate simulator binding. But seems there is no way to link the skip_hw state with the gate action in current state. So hrtimer has to start at initial time when it is create.

> 
> --
> Vinicius

Br,
Po Liu