All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@mellanox.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: "open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"moderated list:ETHERNET BRIDGE"
	<bridge@lists.linux-foundation.org>,
	open list <linux-kernel@vger.kernel.org>,
	"vivien.didelot@gmail.com" <vivien.didelot@gmail.com>,
	Jiri Pirko <jiri@mellanox.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Bridge] [PATCH net-next v3 7/8] net: switchdev: Replace port attr set SDO with a notification
Date: Wed, 27 Feb 2019 20:16:31 +0000	[thread overview]
Message-ID: <20190227201628.GA7022@splinter> (raw)
In-Reply-To: <20190227194432.725-8-f.fainelli@gmail.com>

On Wed, Feb 27, 2019 at 11:44:31AM -0800, Florian Fainelli wrote:
> Drop switchdev_ops.switchdev_port_attr_set. Drop the uses of this field
> from all clients, which were migrated to use switchdev notification in
> the previous patches.
> 
> Add a new function switchdev_port_attr_notify() that sends the switchdev
> notifications SWITCHDEV_PORT_ATTR_SET and calls the blocking (process)
> notifier chain.
> 
> We have one odd case within net/bridge/br_switchdev.c with the
> SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS attribute identifier that
> requires executing from atomic context, we deal with that one
> specifically.
> 
> Drop __switchdev_port_attr_set() and update switchdev_port_attr_set()
> likewise.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>

One small nit that you can address in a follow-up:

> @@ -67,12 +67,18 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
>  		.id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
>  		.u.brport_flags = mask,
>  	};
> +	struct switchdev_notifier_port_attr_info info = {
> +		.attr = &attr,
> +	};
>  	int err;
>  
>  	if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
>  		return 0;
>  
> -	err = switchdev_port_attr_set(p->dev, &attr);
> +	/* We run from atomic context here */
> +	err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
> +				       &info.info, NULL);
> +	err = notifier_to_errno(err);
>  	if (err == -EOPNOTSUPP)
>  		return 0;

This check can be removed. The code below checks `err` and fails the
operation in case of error.

WARNING: multiple messages have this Message-ID (diff)
From: Ido Schimmel <idosch@mellanox.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>,
	"moderated list:ETHERNET BRIDGE"
	<bridge@lists.linux-foundation.org>,
	Jiri Pirko <jiri@mellanox.com>, "andrew@lunn.ch" <andrew@lunn.ch>,
	"vivien.didelot@gmail.com" <vivien.didelot@gmail.com>
Subject: Re: [PATCH net-next v3 7/8] net: switchdev: Replace port attr set SDO with a notification
Date: Wed, 27 Feb 2019 20:16:31 +0000	[thread overview]
Message-ID: <20190227201628.GA7022@splinter> (raw)
In-Reply-To: <20190227194432.725-8-f.fainelli@gmail.com>

On Wed, Feb 27, 2019 at 11:44:31AM -0800, Florian Fainelli wrote:
> Drop switchdev_ops.switchdev_port_attr_set. Drop the uses of this field
> from all clients, which were migrated to use switchdev notification in
> the previous patches.
> 
> Add a new function switchdev_port_attr_notify() that sends the switchdev
> notifications SWITCHDEV_PORT_ATTR_SET and calls the blocking (process)
> notifier chain.
> 
> We have one odd case within net/bridge/br_switchdev.c with the
> SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS attribute identifier that
> requires executing from atomic context, we deal with that one
> specifically.
> 
> Drop __switchdev_port_attr_set() and update switchdev_port_attr_set()
> likewise.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>

One small nit that you can address in a follow-up:

> @@ -67,12 +67,18 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
>  		.id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
>  		.u.brport_flags = mask,
>  	};
> +	struct switchdev_notifier_port_attr_info info = {
> +		.attr = &attr,
> +	};
>  	int err;
>  
>  	if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
>  		return 0;
>  
> -	err = switchdev_port_attr_set(p->dev, &attr);
> +	/* We run from atomic context here */
> +	err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
> +				       &info.info, NULL);
> +	err = notifier_to_errno(err);
>  	if (err == -EOPNOTSUPP)
>  		return 0;

This check can be removed. The code below checks `err` and fails the
operation in case of error.

  reply	other threads:[~2019-02-27 20:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-27 19:44 [Bridge] [PATCH net-next v3 0/8] net: Remove switchdev_ops Florian Fainelli
2019-02-27 19:44 ` Florian Fainelli
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 1/8] switchdev: Add SWITCHDEV_PORT_ATTR_SET Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 2/8] rocker: Handle SWITCHDEV_PORT_ATTR_SET Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 3/8] net: dsa: " Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 4/8] mlxsw: spectrum_switchdev: " Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 5/8] net: mscc: ocelot: " Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 6/8] staging: fsl-dpaa2: ethsw: " Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 7/8] net: switchdev: Replace port attr set SDO with a notification Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 20:16   ` Ido Schimmel [this message]
2019-02-27 20:16     ` Ido Schimmel
2019-02-27 19:44 ` [Bridge] [PATCH net-next v3 8/8] net: Remove switchdev_ops Florian Fainelli
2019-02-27 19:44   ` Florian Fainelli
2019-02-27 20:16   ` [Bridge] " Ido Schimmel
2019-02-27 20:16     ` Ido Schimmel
2019-02-27 20:18 ` [Bridge] [PATCH net-next v3 0/8] " Ido Schimmel
2019-02-27 20:18   ` Ido Schimmel
2019-02-27 20:40 ` [Bridge] " David Miller
2019-02-27 20:40   ` David Miller

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=20190227201628.GA7022@splinter \
    --to=idosch@mellanox.com \
    --cc=andrew@lunn.ch \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=f.fainelli@gmail.com \
    --cc=jiri@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@gmail.com \
    /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.