From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nogah Frankel Subject: Re: [PATCH net-next 05/14] nfp: abm: add simple RED offload Date: Mon, 28 May 2018 18:49:51 +0300 Message-ID: <63246dc9-740d-ea72-e15e-599487b3a845@gmail.com> References: <20180526045338.10993-1-jakub.kicinski@netronome.com> <20180526045338.10993-6-jakub.kicinski@netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: jiri@resnulli.us, xiyou.wangcong@gmail.com, john.fastabend@gmail.com, netdev@vger.kernel.org, oss-drivers@netronome.com, alexei.starovoitov@gmail.com, nogahf@mellanox.com, yuvalm@mellanox.com, gerlitz.or@gmail.com To: Jakub Kicinski , davem@davemloft.net Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:34883 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1034957AbeE1Ptz (ORCPT ); Mon, 28 May 2018 11:49:55 -0400 Received: by mail-wm0-f65.google.com with SMTP id o78-v6so33296828wmg.0 for ; Mon, 28 May 2018 08:49:54 -0700 (PDT) In-Reply-To: <20180526045338.10993-6-jakub.kicinski@netronome.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 26-May-18 7:53 AM, Jakub Kicinski wrote: > Offload simple RED configurations. For now support only DCTCP > like scenarios where min and max are the same. > > Signed-off-by: Jakub Kicinski > --- > drivers/net/ethernet/netronome/nfp/abm/main.c | 82 +++++++++++++++++++ > drivers/net/ethernet/netronome/nfp/abm/main.h | 10 +++ > 2 files changed, 92 insertions(+) > > diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c > index 28a18ac62040..22251d88c958 100644 > --- a/drivers/net/ethernet/netronome/nfp/abm/main.c > +++ b/drivers/net/ethernet/netronome/nfp/abm/main.c > @@ -38,6 +38,8 @@ > #include > #include > #include > +#include > +#include > > #include "../nfpcore/nfp.h" > #include "../nfpcore/nfp_cpp.h" > @@ -55,6 +57,84 @@ static u32 nfp_abm_portid(enum nfp_repr_type rtype, unsigned int id) > FIELD_PREP(NFP_ABM_PORTID_ID, id); > } > > +static void > +nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink, > + u32 handle) > +{ > + struct nfp_port *port = nfp_port_from_netdev(netdev); > + > + if (handle != alink->qdiscs[0].handle) > + return; > + > + alink->qdiscs[0].handle = TC_H_UNSPEC; > + port->tc_offload_cnt = 0; > + nfp_abm_ctrl_set_all_q_lvls(alink, ~0); > +} > + > +static int > +nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink, > + struct tc_red_qopt_offload *opt) > +{ > + struct nfp_port *port = nfp_port_from_netdev(netdev); > + int err; > + > + if (opt->set.min != opt->set.max || !opt->set.is_ecn) { I am a bit worried about the min == max. sch_red doesn't really support it. It will calculate incorrect delta value. (And that only if tc_red_eval_P in iproute2 won't reject it). You might maybe use max = min+1, because in real life it will probably act the same but without this problem. Nogah Frankel (from a new mail address) > + nfp_warn(alink->abm->app->cpp, > + "RED offload failed - unsupported parameters\n"); > + err = -EINVAL; > + goto err_destroy; > + } > + err = nfp_abm_ctrl_set_all_q_lvls(alink, opt->set.min); > + if (err) > + goto err_destroy; > + > + alink->qdiscs[0].handle = opt->handle; > + port->tc_offload_cnt = 1; > + > + return 0; > +err_destroy: > + if (alink->qdiscs[0].handle != TC_H_UNSPEC) > + nfp_abm_red_destroy(netdev, alink, alink->qdiscs[0].handle); > + return err; > +} > + > +static int > +nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink, > + struct tc_red_qopt_offload *opt) > +{ > + if (opt->parent != TC_H_ROOT) > + return -EOPNOTSUPP; > + > + switch (opt->command) { > + case TC_RED_REPLACE: > + return nfp_abm_red_replace(netdev, alink, opt); > + case TC_RED_DESTROY: > + nfp_abm_red_destroy(netdev, alink, opt->handle); > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int > +nfp_abm_setup_tc(struct nfp_app *app, struct net_device *netdev, > + enum tc_setup_type type, void *type_data) > +{ > + struct nfp_repr *repr = netdev_priv(netdev); > + struct nfp_port *port; > + > + port = nfp_port_from_netdev(netdev); > + if (!port || port->type != NFP_PORT_PF_PORT) > + return -EOPNOTSUPP; > + > + switch (type) { > + case TC_SETUP_QDISC_RED: > + return nfp_abm_setup_tc_red(netdev, repr->app_priv, type_data); > + default: > + return -EOPNOTSUPP; > + } > +} > + > static struct net_device *nfp_abm_repr_get(struct nfp_app *app, u32 port_id) > { > enum nfp_repr_type rtype; > @@ -403,6 +483,8 @@ const struct nfp_app_type app_abm = { > .vnic_alloc = nfp_abm_vnic_alloc, > .vnic_free = nfp_abm_vnic_free, > > + .setup_tc = nfp_abm_setup_tc, > + > .eswitch_mode_get = nfp_abm_eswitch_mode_get, > .eswitch_mode_set = nfp_abm_eswitch_mode_set, > > diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h > index 1ac651cdc140..979f98fb808b 100644 > --- a/drivers/net/ethernet/netronome/nfp/abm/main.h > +++ b/drivers/net/ethernet/netronome/nfp/abm/main.h > @@ -58,18 +58,28 @@ struct nfp_abm { > const struct nfp_rtsym *q_lvls; > }; > > +/** > + * struct nfp_red_qdisc - representation of single RED Qdisc > + * @handle: handle of currently offloaded RED Qdisc > + */ > +struct nfp_red_qdisc { > + u32 handle; > +}; > + > /** > * struct nfp_abm_link - port tuple of a ABM NIC > * @abm: back pointer to nfp_abm > * @vnic: data vNIC > * @id: id of the data vNIC > * @queue_base: id of base to host queue within PCIe (not QC idx) > + * @qdiscs: array of qdiscs > */ > struct nfp_abm_link { > struct nfp_abm *abm; > struct nfp_net *vnic; > unsigned int id; > unsigned int queue_base; > + struct nfp_red_qdisc qdiscs[1]; > }; > > void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink); >