RE: [patch net-next RFC 1/9] net_sch: red: Add offload ability to RED qdisc

[Nogah Frankel]

> -----Original Message-----
> From: Roman Mashak [mailto:m...@mojatatu.com]
> Sent: Monday, October 30, 2017 2:47 PM
> To: Jiri Pirko <j...@resnulli.us>
> Cc: netdev@vger.kernel.org; da...@davemloft.net; Nogah Frankel 
> <nog...@mellanox.com>;
> j...@mojatatu.com; xiyou.wangc...@gmail.com; mlxsw <ml...@mellanox.com>;
> and...@lunn.ch; vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> michael.c...@broadcom.com; ganes...@chelsio.com; Saeed Mahameed
> <sae...@mellanox.com>; Matan Barak <mat...@mellanox.com>; Leon Romanovsky
> <leo...@mellanox.com>; Ido Schimmel <ido...@mellanox.com>;
> jakub.kicin...@netronome.com; simon.hor...@netronome.com;
> pieter.jansenvanvuu...@netronome.com; john.hur...@netronome.com;
> alexander.h.du...@intel.com; Or Gerlitz <ogerl...@mellanox.com>;
> john.fastab...@gmail.com
> Subject: Re: [patch net-next RFC 1/9] net_sch: red: Add offload ability to 
> RED qdisc
> 
> Jiri Pirko <j...@resnulli.us> writes:
> 
> 
> [...]
> 
> > +static void red_unoffload(struct Qdisc *sch)
> > +{
> > +   struct net_device *dev = qdisc_dev(sch);
> > +   struct tc_red_qopt_offload opt = {
> > +           .handle = sch->handle,
> > +           .command = TC_RED_DESTROY,
> > +           .parent = sch->parent,
> > +   };
> > +
> > +   if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
> > +           return;
> > +
> > +   dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_RED,  &opt);
> > +}
> > +
> 
> > @@ -162,6 +179,28 @@ static const struct nla_policy red_policy[TCA_RED_MAX 
> > + 1] = {
> >     [TCA_RED_MAX_P] = { .type = NLA_U32 },
> >  };
> >
> > +static int red_offload(struct Qdisc *sch)
> > +{
> > +   struct red_sched_data *q = qdisc_priv(sch);
> > +   struct net_device *dev = qdisc_dev(sch);
> > +   struct tc_red_qopt_offload opt = {
> > +           .handle = sch->handle,
> > +           .command = TC_RED_REPLACE,
> > +           .parent = sch->parent,
> > +           .set = {
> > +                   .min = q->parms.qth_min >> q->parms.Wlog,
> > +                   .max = q->parms.qth_max >> q->parms.Wlog,
> > +                   .probability = q->parms.max_P,
> > +                   .is_ecn = red_use_ecn(q),
> > +           },
> > +   };
> > +
> > +   if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
> > +           return -EOPNOTSUPP;
> > +
> > +   return dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_RED, &opt);
> > +}
> > +
> 
> [...]
> 
> Can't red_unoffload() and red_offload() be unified in a single API? For
> example, red_offload(struct Qdisc *sch, bool enable) ?

I will unify them.
Thanks.