On Fri, Mar 04, 2016 at 09:01:39AM -0800, John Fastabend wrote:
> On 16-03-03 06:55 AM, Amir Vadai wrote:
> > This patch is based on a patch made by John Fastabend.
> > It adds support for offloading cls_flower.
> > when NETIF_F_HW_TC is on:
> > flags = 0 => Rule will be processed twice - by hardware, and if
> > still relevant, by software.
> > flags = SKIP_HW => Rull will be processed by software only
> >
> > If hardare fail/not capabale to apply the rule, operation will fail.
> >
> > Suggested-by: John Fastabend <john.r.fastab...@intel.com>
> > Signed-off-by: Amir Vadai <a...@vadai.me>
> > ---
>
> [...]
>
> > static bool fl_destroy(struct tcf_proto *tp, bool force)
> > {
> > struct cls_fl_head *head = rtnl_dereference(tp->root);
> > @@ -174,6 +220,7 @@ static bool fl_destroy(struct tcf_proto *tp, bool force)
> > return false;
> >
> > list_for_each_entry_safe(f, next, &head->filters, list) {
> > + fl_hw_destroy_filter(tp, (u64)f);
> > list_del_rcu(&f->list);
> > call_rcu(&f->rcu, fl_destroy_filter);
> > }
> > @@ -454,11 +501,13 @@ static int fl_change(struct net *net, struct sk_buff
> > *in_skb,
> > u32 handle, struct nlattr **tca,
> > unsigned long *arg, bool ovr)
> > {
> > + struct net_device *dev = tp->q->dev_queue->dev;
> > struct cls_fl_head *head = rtnl_dereference(tp->root);
> > struct cls_fl_filter *fold = (struct cls_fl_filter *) *arg;
> > struct cls_fl_filter *fnew;
> > struct nlattr *tb[TCA_FLOWER_MAX + 1];
> > struct fl_flow_mask mask = {};
> > + u32 flags = 0;
> > int err;
> >
> > if (!tca[TCA_OPTIONS])
> > @@ -486,6 +535,9 @@ static int fl_change(struct net *net, struct sk_buff
> > *in_skb,
> > }
> > fnew->handle = handle;
> >
> > + if (tb[TCA_FLOWER_FLAGS])
> > + flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
> > +
> > err = fl_set_parms(net, tp, fnew, &mask, base, tb, tca[TCA_RATE], ovr);
> > if (err)
> > goto errout;
> > @@ -498,9 +550,22 @@ static int fl_change(struct net *net, struct sk_buff
> > *in_skb,
> > head->ht_params);
> > if (err)
> > goto errout;
> > - if (fold)
> > +
> > + err = fl_hw_replace_filter(tp,
> > + &head->dissector,
> > + &mask.key,
> > + &fnew->key,
> > + &fnew->exts,
> > + (u64)fnew,
> > + flags);
> > + if (err)
> > + goto err_hash_remove;
> > +
>
> This behaviour is different than how I did u32 in the u32 case I just
> let the software case get loaded and do not throw any errors. The
> intent was if we required a HW entry we would explicitly state that
> with the SKIP_SW (to be implemented) flag. This error path seems
> to block the software filter when the hardware fails.
Makes sense.
>
> I think it would be best to do the same as u32 here and use the error
> path only if SKIP_SW is set. Or if you really want an error path on
> SW/HW loads then use another bit in the flag to specify STRICT or
> something along those lines.
I will do the same as u32. I won't add this STRICT flag, because I don't
have any use case for this mode in which processing is done in both SW
and HW.
>
>
> > + if (fold) {
> > rhashtable_remove_fast(&head->ht, &fold->ht_node,
> > head->ht_params);
> > + fl_hw_destroy_filter(tp, (u64)fold);
> > + }
> >
> > *arg = (unsigned long) fnew;
> >
> > @@ -514,6 +579,9 @@ static int fl_change(struct net *net, struct sk_buff
> > *in_skb,
> >
> > return 0;
> >
> > +err_hash_remove:
> > + rhashtable_remove_fast(&head->ht, &fnew->ht_node, head->ht_params);
> > +
> > errout:
> > kfree(fnew);
> > return err;
> > @@ -527,6 +595,7 @@ static int fl_delete(struct tcf_proto *tp, unsigned
> > long arg)
> > rhashtable_remove_fast(&head->ht, &f->ht_node,
> > head->ht_params);
> > list_del_rcu(&f->list);
> > + fl_hw_destroy_filter(tp, (u64)f);
> > tcf_unbind_filter(tp, &f->res);
> > call_rcu(&f->rcu, fl_destroy_filter);
> > return 0;
> >
>
