netdev - [PATCH net-next 11/13] nfp: abm: reset RED's child based on limit

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20181112225819.29823-12-jakub.kicinski@netronome.com>
Date:   Mon, 12 Nov 2018 14:58:17 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     davem@...emloft.net
Cc:     oss-drivers@...ronome.com, netdev@...r.kernel.org,
        jiri@...nulli.us, xiyou.wangcong@...il.com, jhs@...atatu.com,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: [PATCH net-next 11/13] nfp: abm: reset RED's child based on limit

RED qdisc will replace its child Qdisc with a new FIFO queue if
it is reconfigured and the limit parameter is not 0.

This means that when it's created with limit of 0 it will have no FIFO,
and all packets will be dropped.  If it's changed and limit is specified
it will loose its existing child (implicit graft).  Make sure we mark
RED Qdisc child as NFP_QDISC_UNTRACKED if its not the expected FIFO.

nfp_abm_qdisc_replace() will return 1 if Qdisc already existed.

Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
Reviewed-by: John Hurley <john.hurley@...ronome.com>
---
 .../net/ethernet/netronome/nfp/abm/qdisc.c    | 27 +++++++++++++++----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 151d2dafbc76..1b3c0b5b52bf 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -196,7 +196,7 @@ nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
 	if (*qdisc) {
 		if (WARN_ON((*qdisc)->type != type))
 			return -EINVAL;
-		return 0;
+		return 1;
 	}
 
 	*qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle, handle,
@@ -357,11 +357,24 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
 	i = nfp_abm_red_find(alink, opt);
 	existing = i >= 0;
 
-	if (ret) {
+	if (ret < 0) {
 		err = ret;
 		goto err_destroy;
 	}
 
+	/* If limit != 0 child gets reset */
+	if (opt->set.limit) {
+		if (nfp_abm_qdisc_child_valid(qdisc, 0))
+			qdisc->children[0]->use_cnt--;
+		qdisc->children[0] = NULL;
+	} else {
+		/* Qdisc was just allocated without a limit will use noop_qdisc,
+		 * i.e. a block hole.
+		 */
+		if (!ret)
+			qdisc->children[0] = NFP_QDISC_UNTRACKED;
+	}
+
 	if (!nfp_abm_red_check_params(alink, opt)) {
 		err = -EINVAL;
 		goto err_destroy;
@@ -533,10 +546,14 @@ nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
 		  struct tc_mq_qopt_offload *opt)
 {
 	struct nfp_qdisc *qdisc;
+	int ret;
 
-	return nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
-				     TC_H_ROOT, opt->handle,
-				     alink->total_queues, &qdisc);
+	ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
+				    TC_H_ROOT, opt->handle, alink->total_queues,
+				    &qdisc);
+	if (ret < 0)
+		return ret;
+	return 0;
 }
 
 int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
-- 
2.17.1