On Wed, Jun 10, 2015 at 2:47 PM, Scott Feldman <sfel...@gmail.com> wrote: > On Wed, Jun 10, 2015 at 2:25 PM, David Ahern <dsah...@gmail.com> wrote: >> On 6/10/15 2:56 PM, sfel...@gmail.com wrote: >>> >>> From: Scott Feldman <sfel...@gmail.com> >>> >>> Fix a BUG() where CONFIG_NET_SWITCHDEV is set but the driver for a bridged >>> port does not support switchdec_port_attr_set op. Don't BUG() if >>> -EOPNOTSUPP is returned. >>> >>> Signed-off-by: Scott Feldman <sfel...@gmail.com> >>> Reported-by: Brenden Blanco <bbla...@plumgrid.com> >>> --- >>> net/switchdev/switchdev.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c >>> index e008057..99bced4 100644 >>> --- a/net/switchdev/switchdev.c >>> +++ b/net/switchdev/switchdev.c >>> @@ -103,7 +103,7 @@ static void switchdev_port_attr_set_work(struct >>> work_struct *work) >>> >>> rtnl_lock(); >>> err = switchdev_port_attr_set(asw->dev, &asw->attr); >>> - BUG_ON(err); >>> + BUG_ON(err && err != -EOPNOTSUPP); >>> rtnl_unlock(); >>> >>> dev_put(asw->dev); >>> >> >> Should that be WARN_ON instead of BUG_ON? > > I think I had it as WARN when we were working on the initial patches, > but we changed it to BUG_ON because we should only get an error here > if the driver screwed something up between PREPARE phase and COMMIT > phase, so it should be considered a driver bug which needs fixing.
Actually, ignore what I said above. I was confusing this BUG_ON with the one in switchdev_port_attr_set(). Perhaps this BUG_ON() you're commenting on should be WARN(). A driver could return an err in PREPARE phase and that shouldn't be a BUG_ON situation; seems WARN would be better. It the case where the driver returns an err in COMMIT phase but didn't return an err in PREPARE phase we want to BUG_ON(). Maybe that case doesn't justify BUG_ON either, based on the link you posted. Jiri, IIRC, you suggested the BUG_ON(). Does it still sound right based on the point David is raising? -scott -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
