All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Petr Machata <petrm@nvidia.com>,
	Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
	Jakub Kicinski <kuba@kernel.org>
Subject: [PATCH 5.4 02/32] net: switchdev: dont set port_obj_info->handled true when -EOPNOTSUPP
Date: Fri,  5 Feb 2021 15:07:17 +0100	[thread overview]
Message-ID: <20210205140652.456963490@linuxfoundation.org> (raw)
In-Reply-To: <20210205140652.348864025@linuxfoundation.org>

From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

commit 20776b465c0c249f5e5b5b4fe077cd24ef1cda86 upstream.

It's not true that switchdev_port_obj_notify() only inspects the
->handled field of "struct switchdev_notifier_port_obj_info" if
call_switchdev_blocking_notifiers() returns 0 - there's a WARN_ON()
triggering for a non-zero return combined with ->handled not being
true. But the real problem here is that -EOPNOTSUPP is not being
properly handled.

The wrapper functions switchdev_handle_port_obj_add() et al change a
return value of -EOPNOTSUPP to 0, and the treatment of ->handled in
switchdev_port_obj_notify() seems to be designed to change that back
to -EOPNOTSUPP in case nobody actually acted on the notifier (i.e.,
everybody returned -EOPNOTSUPP).

Currently, as soon as some device down the stack passes the check_cb()
check, ->handled gets set to true, which means that
switchdev_port_obj_notify() cannot actually ever return -EOPNOTSUPP.

This, for example, means that the detection of hardware offload
support in the MRP code is broken: switchdev_port_obj_add() used by
br_mrp_switchdev_send_ring_test() always returns 0, so since the MRP
code thinks the generation of MRP test frames has been offloaded, no
such frames are actually put on the wire. Similarly,
br_mrp_switchdev_set_ring_role() also always returns 0, causing
mrp->ring_role_offloaded to be set to 1.

To fix this, continue to set ->handled true if any callback returns
success or any error distinct from -EOPNOTSUPP. But if all the
callbacks return -EOPNOTSUPP, make sure that ->handled stays false, so
the logic in switchdev_port_obj_notify() can propagate that
information.

Fixes: 9a9f26e8f7ea ("bridge: mrp: Connect MRP API with the switchdev API")
Fixes: f30f0601eb93 ("switchdev: Add helpers to aid traversal through lower devices")
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Link: https://lore.kernel.org/r/20210125124116.102928-1-rasmus.villemoes@prevas.dk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/switchdev/switchdev.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -461,10 +461,11 @@ static int __switchdev_handle_port_obj_a
 	extack = switchdev_notifier_info_to_extack(&port_obj_info->info);
 
 	if (check_cb(dev)) {
-		/* This flag is only checked if the return value is success. */
-		port_obj_info->handled = true;
-		return add_cb(dev, port_obj_info->obj, port_obj_info->trans,
-			      extack);
+		err = add_cb(dev, port_obj_info->obj, port_obj_info->trans,
+			     extack);
+		if (err != -EOPNOTSUPP)
+			port_obj_info->handled = true;
+		return err;
 	}
 
 	/* Switch ports might be stacked under e.g. a LAG. Ignore the
@@ -513,9 +514,10 @@ static int __switchdev_handle_port_obj_d
 	int err = -EOPNOTSUPP;
 
 	if (check_cb(dev)) {
-		/* This flag is only checked if the return value is success. */
-		port_obj_info->handled = true;
-		return del_cb(dev, port_obj_info->obj);
+		err = del_cb(dev, port_obj_info->obj);
+		if (err != -EOPNOTSUPP)
+			port_obj_info->handled = true;
+		return err;
 	}
 
 	/* Switch ports might be stacked under e.g. a LAG. Ignore the
@@ -563,9 +565,10 @@ static int __switchdev_handle_port_attr_
 	int err = -EOPNOTSUPP;
 
 	if (check_cb(dev)) {
-		port_attr_info->handled = true;
-		return set_cb(dev, port_attr_info->attr,
-			      port_attr_info->trans);
+		err = set_cb(dev, port_attr_info->attr, port_attr_info->trans);
+		if (err != -EOPNOTSUPP)
+			port_attr_info->handled = true;
+		return err;
 	}
 
 	/* Switch ports might be stacked under e.g. a LAG. Ignore the



  parent reply	other threads:[~2021-02-05 21:35 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-05 14:07 [PATCH 5.4 00/32] 5.4.96-rc1 review Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 01/32] net: dsa: bcm_sf2: put device node before return Greg Kroah-Hartman
2021-02-07  9:25   ` Pavel Machek
2021-02-05 14:07 ` Greg Kroah-Hartman [this message]
2021-02-05 14:07 ` [PATCH 5.4 03/32] ibmvnic: Ensure that CRQ entry read are correctly ordered Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 04/32] Revert "Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"" Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 05/32] ACPI: thermal: Do not call acpi_thermal_check() directly Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 06/32] arm64: Fix kernel address detection of __is_lm_address() Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 07/32] arm64: Do not pass tagged addresses to __is_lm_address() Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 08/32] tcp: make TCP_USER_TIMEOUT accurate for zero window probes Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 09/32] btrfs: backref, only collect file extent items matching backref offset Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 10/32] btrfs: backref, dont add refs from shared block when resolving normal backref Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 11/32] btrfs: backref, only search backref entries from leaves of the same root Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 12/32] btrfs: backref, use correct count to resolve normal data refs Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 13/32] net_sched: gen_estimator: support large ewma log Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 14/32] phy: cpcap-usb: Fix warning for missing regulator_disable Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 15/32] platform/x86: touchscreen_dmi: Add swap-x-y quirk for Goodix touchscreen on Estar Beauty HD tablet Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 16/32] platform/x86: intel-vbtn: Support for tablet mode on Dell Inspiron 7352 Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 17/32] x86: __always_inline __{rd,wr}msr() Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 18/32] scsi: scsi_transport_srp: Dont block target in failfast state Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 19/32] scsi: libfc: Avoid invoking response handler twice if ep is already completed Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 20/32] scsi: fnic: Fix memleak in vnic_dev_init_devcmd2 Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 21/32] ASoC: SOF: Intel: hda: Resume codec to do jack detection Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 22/32] mac80211: fix fast-rx encryption check Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 23/32] scsi: ibmvfc: Set default timeout to avoid crash during migration Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 24/32] ALSA: hda: Add Cometlake-R PCI ID Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 25/32] udf: fix the problem that the disc content is not displayed Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 26/32] nvme: check the PRINFO bit before deciding the host buffer length Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 27/32] selftests/powerpc: Only test lwm/stmw on big endian Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 28/32] drm/amd/display: Update dram_clock_change_latency for DCN2.1 Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 29/32] drm/amd/display: Change function decide_dp_link_settings to avoid infinite looping Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 30/32] objtool: Dont fail on missing symbol table Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 31/32] kthread: Extract KTHREAD_IS_PER_CPU Greg Kroah-Hartman
2021-02-05 14:07 ` [PATCH 5.4 32/32] workqueue: Restrict affinity change to rescuer Greg Kroah-Hartman
2021-02-05 20:58 ` [PATCH 5.4 00/32] 5.4.96-rc1 review Igor
2021-02-05 21:16 ` Florian Fainelli
2021-02-07 14:36   ` Greg Kroah-Hartman
2021-02-06 14:27 ` Naresh Kamboju
2021-02-06 16:02 ` Guenter Roeck
2021-02-07 15:34 ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210205140652.456963490@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=petrm@nvidia.com \
    --cc=rasmus.villemoes@prevas.dk \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.