| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <63246dc9-740d-ea72-e15e-599487b3a845@gmail.com>
Date: Mon, 28 May 2018 18:49:51 +0300
From: Nogah Frankel <nogah.frankel@...il.com>
To: Jakub Kicinski <jakub.kicinski@...ronome.com>, davem@...emloft.net
Cc: jiri@...nulli.us, xiyou.wangcong@...il.com,
john.fastabend@...il.com, netdev@...r.kernel.org,
oss-drivers@...ronome.com, alexei.starovoitov@...il.com,
nogahf@...lanox.com, yuvalm@...lanox.com, gerlitz.or@...il.com
Subject: Re: [PATCH net-next 05/14] nfp: abm: add simple RED offload
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 <jakub.kicinski@...ronome.com>
> ---
> 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 <linux/netdevice.h>
> #include <linux/rcupdate.h>
> #include <linux/slab.h>
> +#include <net/pkt_cls.h>
> +#include <net/pkt_sched.h>
>
> #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);
>
Powered by blists - more mailing lists