From: Petr Machata <petrm@nvidia.com>
To: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Cc: <netdev@vger.kernel.org>,
Horatiu Vultur <horatiu.vultur@microchip.com>,
Jiri Pirko <jiri@mellanox.com>,
Florian Fainelli <f.fainelli@gmail.com>,
"Petr Machata" <petrm@mellanox.com>,
Ido Schimmel <idosch@mellanox.com>,
"David S . Miller" <davem@davemloft.net>,
Ivan Vecera <ivecera@redhat.com>,
"Jakub Kicinski" <kuba@kernel.org>
Subject: Re: [PATCH resend net] net: switchdev: don't set port_obj_info->handled true when -EOPNOTSUPP
Date: Fri, 22 Jan 2021 14:42:45 +0100 [thread overview]
Message-ID: <87y2glay0a.fsf@nvidia.com> (raw)
In-Reply-To: <20210121234317.65936-1-rasmus.villemoes@prevas.dk>
Rasmus Villemoes <rasmus.villemoes@prevas.dk> writes:
> 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 - br_mrp_set_ring_role() always ends
> up setting mrp->ring_role_offloaded to 1, despite not a single
> mainline driver implementing any of the SWITCHDEV_OBJ_ID*_MRP. So
> since the MRP code thinks the generation of MRP test frames has been
> offloaded, no such frames are actually put on the wire.
>
> So, 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: f30f0601eb93 ("switchdev: Add helpers to aid traversal through lower devices")
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Looks good.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Thanks!
prev parent reply other threads:[~2021-01-22 13:43 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 23:43 [PATCH resend net] net: switchdev: don't set port_obj_info->handled true when -EOPNOTSUPP Rasmus Villemoes
2021-01-22 9:05 ` Horatiu Vultur
2021-01-22 13:36 ` Rasmus Villemoes
2021-01-23 20:44 ` Jakub Kicinski
2021-01-22 13:42 ` Petr Machata [this message]
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=87y2glay0a.fsf@nvidia.com \
--to=petrm@nvidia.com \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=horatiu.vultur@microchip.com \
--cc=idosch@mellanox.com \
--cc=ivecera@redhat.com \
--cc=jiri@mellanox.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=petrm@mellanox.com \
--cc=rasmus.villemoes@prevas.dk \
/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.