Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 3/3] net: phy: use phy_modify_changed in genphy_config_advert
From: Heiner Kallweit @ 2019-02-10 18:59 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <a9d88fc1-6ff7-5805-a7d3-7aad7391b4d8@gmail.com>

Use phy_modify_changed() to simplify the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 43 +++++++++++++-----------------------
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8573d17ec..3d14e48ae 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1516,7 +1516,7 @@ EXPORT_SYMBOL(phy_reset_after_clk_enable);
 static int genphy_config_advert(struct phy_device *phydev)
 {
 	u32 advertise;
-	int oldadv, adv, bmsr;
+	int bmsr, adv;
 	int err, changed = 0;
 
 	/* Only allow advertising what this PHY supports */
@@ -1529,22 +1529,14 @@ static int genphy_config_advert(struct phy_device *phydev)
 			    phydev->advertising);
 
 	/* Setup standard advertisement */
-	adv = phy_read(phydev, MII_ADVERTISE);
-	if (adv < 0)
-		return adv;
-
-	oldadv = adv;
-	adv &= ~(ADVERTISE_ALL | ADVERTISE_100BASE4 | ADVERTISE_PAUSE_CAP |
-		 ADVERTISE_PAUSE_ASYM);
-	adv |= ethtool_adv_to_mii_adv_t(advertise);
-
-	if (adv != oldadv) {
-		err = phy_write(phydev, MII_ADVERTISE, adv);
-
-		if (err < 0)
-			return err;
+	err = phy_modify_changed(phydev, MII_ADVERTISE,
+				 ADVERTISE_ALL | ADVERTISE_100BASE4 |
+				 ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
+				 ethtool_adv_to_mii_adv_t(advertise));
+	if (err < 0)
+		return err;
+	if (err > 0)
 		changed = 1;
-	}
 
 	bmsr = phy_read(phydev, MII_BMSR);
 	if (bmsr < 0)
@@ -1558,25 +1550,20 @@ static int genphy_config_advert(struct phy_device *phydev)
 		return changed;
 
 	/* Configure gigabit if it's supported */
-	adv = phy_read(phydev, MII_CTRL1000);
-	if (adv < 0)
-		return adv;
-
-	oldadv = adv;
-	adv &= ~(ADVERTISE_1000FULL | ADVERTISE_1000HALF);
-
+	adv = 0;
 	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
 			      phydev->supported) ||
 	    linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
 			      phydev->supported))
-		adv |= ethtool_adv_to_mii_ctrl1000_t(advertise);
+		adv = ethtool_adv_to_mii_ctrl1000_t(advertise);
 
-	if (adv != oldadv)
-		changed = 1;
-
-	err = phy_write(phydev, MII_CTRL1000, adv);
+	err = phy_modify_changed(phydev, MII_CTRL1000,
+				 ADVERTISE_1000FULL | ADVERTISE_1000HALF,
+				 adv);
 	if (err < 0)
 		return err;
+	if (err > 0)
+		changed = 1;
 
 	return changed;
 }
-- 
2.20.1



^ permalink raw reply related

* Re: [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr_get()
From: Ido Schimmel @ 2019-02-10 19:05 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devel@driverdev.osuosl.org, bridge@lists.linux-foundation.org,
	Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190210175105.31629-7-f.fainelli@gmail.com>

On Sun, Feb 10, 2019 at 09:50:55AM -0800, Florian Fainelli wrote:
> Now that all switchdev drivers have been converted to checking the
> bridge port flags during the prepare phase of the
> switchdev_port_attr_set(), we can move straight to trying to set the
> desired flag through SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS.
> 
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  net/bridge/br_switchdev.c | 20 +++-----------------
>  1 file changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index db9e8ab96d48..939f300522c5 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -64,29 +64,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
>  {
>  	struct switchdev_attr attr = {
>  		.orig_dev = p->dev,
> -		.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
> +		.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
> +		.flags = SWITCHDEV_F_DEFER,

How does this work? You defer the operation, so the driver cannot veto
it. This is why we have SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
which is not deferred.

> +		.u.brport_flags = flags,
>  	};
>  	int err;
>  
>  	if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
>  		return 0;
>  
> -	err = switchdev_port_attr_get(p->dev, &attr);
> -	if (err == -EOPNOTSUPP)
> -		return 0;
> -	if (err)
> -		return err;
> -
> -	/* Check if specific bridge flag attribute offload is supported */
> -	if (!(attr.u.brport_flags_support & mask)) {
> -		br_warn(p->br, "bridge flag offload is not supported %u(%s)\n",
> -			(unsigned int)p->port_no, p->dev->name);
> -		return -EOPNOTSUPP;
> -	}
> -
> -	attr.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS;
> -	attr.flags = SWITCHDEV_F_DEFER;
> -	attr.u.brport_flags = flags;
>  	err = switchdev_port_attr_set(p->dev, &attr);
>  	if (err) {
>  		br_warn(p->br, "error setting offload flag on port %u(%s)\n",
> -- 
> 2.19.1
> 

^ permalink raw reply

* Re: [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr_get()
From: Florian Fainelli @ 2019-02-10 19:34 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devel@driverdev.osuosl.org, bridge@lists.linux-foundation.org,
	Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190210190501.GA26726@splinter>

Le 2/10/19 à 11:05 AM, Ido Schimmel a écrit :
> On Sun, Feb 10, 2019 at 09:50:55AM -0800, Florian Fainelli wrote:
>> Now that all switchdev drivers have been converted to checking the
>> bridge port flags during the prepare phase of the
>> switchdev_port_attr_set(), we can move straight to trying to set the
>> desired flag through SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS.
>>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  net/bridge/br_switchdev.c | 20 +++-----------------
>>  1 file changed, 3 insertions(+), 17 deletions(-)
>>
>> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
>> index db9e8ab96d48..939f300522c5 100644
>> --- a/net/bridge/br_switchdev.c
>> +++ b/net/bridge/br_switchdev.c
>> @@ -64,29 +64,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
>>  {
>>  	struct switchdev_attr attr = {
>>  		.orig_dev = p->dev,
>> -		.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
>> +		.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
>> +		.flags = SWITCHDEV_F_DEFER,
> 
> How does this work? You defer the operation, so the driver cannot veto
> it. This is why we have SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
> which is not deferred.

I missed that indeed, how would you feel about splitting the attribute
setting into different phases:

- checking that the attribute setting is supported (caller context, so
possibly atomic)
- allocating and committing resources (deferred context)

For pretty much any DSA driver, we will have to be in sleepable context
anyway because of MDIO, SPI, I2C, whatever transport layer.

Not sure how to best approach this now...
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 1/3] net: phy: add register modifying helpers returning 1 on change
From: Andrew Lunn @ 2019-02-10 19:43 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <f454fd0e-199b-992e-f47f-6118101ac6ba@gmail.com>

On Sun, Feb 10, 2019 at 07:57:56PM +0100, Heiner Kallweit wrote:
> When modifying registers there are scenarios where we need to know
> whether the register content actually changed. This patch adds
> new helpers to not break users of the current ones, phy_modify() etc.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH net-next 2/3] net: phy: marvell10g: fix usage of new MMD modifying helpers
From: Andrew Lunn @ 2019-02-10 19:44 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <f6705472-58c2-5bad-be0a-b44518a4e72d@gmail.com>

On Sun, Feb 10, 2019 at 07:58:49PM +0100, Heiner Kallweit wrote:
> When replacing mv3310_modify() with phy_modify_mmd() we missed that
> they behave differently, mv3310_modify() returns 1 on a changed
> register value whilst phy_modify_mmd() returns 0. Fix this by replacing
> phy_modify_mmd() with phy_modify_mmd_changed() where needed.
> 
> Fixes: b52c018ddccf ("net: phy: make use of new MMD accessors")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Sorry, my bad.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH net-next 3/3] net: phy: use phy_modify_changed in genphy_config_advert
From: Andrew Lunn @ 2019-02-10 19:46 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <affccf1b-9c2f-2c5a-df8f-43f11ca8ccdc@gmail.com>

On Sun, Feb 10, 2019 at 07:59:57PM +0100, Heiner Kallweit wrote:
> Use phy_modify_changed() to simplify the code.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* linux-next: Fixes tag needs some work in the net tree
From: Stephen Rothwell @ 2019-02-10 20:14 UTC (permalink / raw)
  To: David Miller, Networking
  Cc: Linux Next Mailing List, Linux Kernel Mailing List, Ursula Braun

[-- Attachment #1: Type: text/plain, Size: 286 bytes --]

Hi all,

In commit

  ccc8ca9b90ac ("net/smc: fix byte_order for rx_curs_confirmed")

Fixes tag

  Fixes: b8649efad879 ("net/smc: fix sender_free computation") (net-tree)

has these problem(s):

  - The trailing '(net-tree)' is unexpected

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH net] sctp: make sctp_setsockopt_events() less strict about the option length
From: Marcelo Ricardo Leitner @ 2019-02-10 20:15 UTC (permalink / raw)
  To: David Miller
  Cc: julien, netdev, linux-sctp, linux-kernel, nhorman, vyasevich,
	lucien.xin
In-Reply-To: <20190210124616.GG13621@localhost.localdomain>

On Sun, Feb 10, 2019 at 10:46:16AM -0200, Marcelo Ricardo Leitner wrote:
> On Sat, Feb 09, 2019 at 03:12:17PM -0800, David Miller wrote:
> > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > Date: Wed, 6 Feb 2019 18:37:54 -0200
> > 
> > > On Wed, Feb 06, 2019 at 12:14:30PM -0800, Julien Gomes wrote:
> > >> Make sctp_setsockopt_events() able to accept sctp_event_subscribe
> > >> structures longer than the current definitions.
> > >> 
> > >> This should prevent unjustified setsockopt() failures due to struct
> > >> sctp_event_subscribe extensions (as in 4.11 and 4.12) when using
> > >> binaries that should be compatible, but were built with later kernel
> > >> uapi headers.
> > > 
> > > Not sure if we support backwards compatibility like this?
> > 
> > What a complete mess we have here.
> > 
> > Use new socket option numbers next time, do not change the size and/or
> > layout of existing socket options.
> 
> What about reusing the same socket option, but defining a new struct?
> Say, MYSOCKOPT supports struct mysockopt, struct mysockopt2, struct
> mysockopt3...
> 
> That way we have a clear definition of the user's intent.
> 
> > 
> > This whole thread, if you read it, is basically "if we compatability
> > this way, that breaks, and if we do compatability this other way oh
> > shit this other thing doesn't work."
> > 
> > I think we really need to specifically check for the difference sizes
> > that existed one by one, clear out the part not given by the user, and
> > backport this as far back as possible in a way that in the older kernels
> > we see if the user is actually trying to use the new features and if so
> > error out.
> 
> I'm afraid clearing out may not be enough, though seems it's the best
> we can do so far. If the struct is allocated but not fully initialized
> via a memset, but by setting its fields one by one, the remaining new
> fields will be left uninitinialized.

Need to clarify the "clearing out", I think it was meant differently.
It was more about on how to ensure that the 16-bytes long of the v3
supplied to a v1-only kernel is compatible with the 12-bytes long v1.
The kernel would have to check the trailing 4 bytes after v1-size and
make sure they are all zeroed in order for the old kernel to accept it
as a v1. But, as I said above, there are situations that this will not
be enough.

> 
> > 
> > Which, btw, is terrible behavior.  Newly compiled apps should work on
> > older kernels if they don't try to use the new features, and if they
> 
> One use case here is: a given distro is using kernel X and app Foo is
> built against it. Then upgrades to X+1, Foo is patched to fix an issue
> and is rebuilt against X+1. The user upgrades Foo package but for
> whatever reason, doesn't upgrade kernel or reboot the system. Here,
> Foo doesn't work anymore until the new kernel is also running.
> 
> > can the ones that want to try to use the new features should be able
> > to fall back when that feature isn't available in a non-ambiguous
> > and precisely defined way.
> > 
> > The fact that the use of the new feature is hidden in the new
> > structure elements is really rotten.
> > 
> > This patch, at best, needs some work and definitely a longer and more
> > detailed commit message.
> 

We have issues on read path too. 52ccb8e90c0a ("[SCTP]: Update
SCTP_PEER_ADDR_PARAMS socket option to the latest api draft.")
extended struct sctp_paddrparams and its getsockopt goes with:

sctp_getsockopt_peer_addr_params()
...
        if (len < sizeof(struct sctp_paddrparams))
                return -EINVAL;
        len = sizeof(struct sctp_paddrparams);

By then, we didn't have the /uapi/ folder yet. There may other cases.

^ permalink raw reply

* Re: [PATCH net-next 1/3] net: phy: add register modifying helpers returning 1 on change
From: Florian Fainelli @ 2019-02-10 20:27 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <f454fd0e-199b-992e-f47f-6118101ac6ba@gmail.com>

Le 2/10/19 à 10:57 AM, Heiner Kallweit a écrit :
> When modifying registers there are scenarios where we need to know
> whether the register content actually changed. This patch adds
> new helpers to not break users of the current ones, phy_modify() etc.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 2/3] net: phy: marvell10g: fix usage of new MMD modifying helpers
From: Florian Fainelli @ 2019-02-10 20:27 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <f6705472-58c2-5bad-be0a-b44518a4e72d@gmail.com>

Le 2/10/19 à 10:58 AM, Heiner Kallweit a écrit :
> When replacing mv3310_modify() with phy_modify_mmd() we missed that
> they behave differently, mv3310_modify() returns 1 on a changed
> register value whilst phy_modify_mmd() returns 0. Fix this by replacing
> phy_modify_mmd() with phy_modify_mmd_changed() where needed.
> 
> Fixes: b52c018ddccf ("net: phy: make use of new MMD accessors")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 3/3] net: phy: use phy_modify_changed in genphy_config_advert
From: Florian Fainelli @ 2019-02-10 20:28 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <affccf1b-9c2f-2c5a-df8f-43f11ca8ccdc@gmail.com>

Le 2/10/19 à 10:59 AM, Heiner Kallweit a écrit :
> Use phy_modify_changed() to simplify the code.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH for-next 2/4]  devlink: fix print of uint64_t
From: Stephen Hemminger @ 2019-02-10 20:34 UTC (permalink / raw)
  To: Aya Levin
  Cc: David Ahern, netdev, David S. Miller, Jiri Pirko, Moshe Shemesh,
	Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-3-git-send-email-ayal@mellanox.com>

On Sun, 10 Feb 2019 20:28:47 +0200
Aya Levin <ayal@mellanox.com> wrote:

>  This patch prints uint64_t with its corresponding format and avoid implicit
>  cast to uint32_t.
> 
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> Reported-by: Maria Pasechnik <mariap@mellanox.com>
> ---
>  devlink/devlink.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index a05755385a49..46e2e41c5dfd 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -1628,7 +1628,14 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
>  	if (val == (uint64_t) -1)
>  		return pr_out_str(dl, name, "unlimited");
>  
> -	return pr_out_uint(dl, name, val);
> +	if (dl->json_output) {
> +		jsonw_u64_field(dl->jw, name, val);
> +	} else {
> +		if (g_indent_newline)
> +			pr_out("%s %lu", name, val);
> +		else
> +			pr_out(" %s %lu", name, val);
> +	}
>  }
>  
>  static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)

More conditional code adds bloat and is a nuisance.
Why not use print_u64 that already exists.

I wish devlink used json_print in a more standard manner.

^ permalink raw reply

* Re: [PATCH for-next 3/4] devlink: fix boolean JSON print
From: Stephen Hemminger @ 2019-02-10 20:34 UTC (permalink / raw)
  To: Aya Levin
  Cc: David Ahern, netdev, David S. Miller, Jiri Pirko, Moshe Shemesh,
	Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-4-git-send-email-ayal@mellanox.com>

On Sun, 10 Feb 2019 20:28:48 +0200
Aya Levin <ayal@mellanox.com> wrote:

> This patch removes the inverted commas from boolean values in JSON
> format: true/false instead of "true"/"false".
> 
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> ---
>  devlink/devlink.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index 46e2e41c5dfd..a433883f1b2b 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -1605,10 +1605,10 @@ static void pr_out_str(struct dl *dl, const char *name, const char *val)
>  
>  static void pr_out_bool(struct dl *dl, const char *name, bool val)
>  {
> -	if (val)
> -		pr_out_str(dl, name, "true");
> +	if (dl->json_output)
> +		jsonw_bool_field(dl->jw, name, val);
>  	else
> -		pr_out_str(dl, name, "false");
> +		pr_out_str(dl, name, val ? "true" : "false");
>  }
>  
>  static void pr_out_uint(struct dl *dl, const char *name, unsigned int val)

There is already print_bool in json_print why is that not used?

^ permalink raw reply

* Re: [PATCH for-next 4/4] devlink: add health command support
From: Stephen Hemminger @ 2019-02-10 20:42 UTC (permalink / raw)
  To: Aya Levin
  Cc: David Ahern, netdev, David S. Miller, Jiri Pirko, Moshe Shemesh,
	Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-5-git-send-email-ayal@mellanox.com>

On Sun, 10 Feb 2019 20:28:49 +0200
Aya Levin <ayal@mellanox.com> wrote:

> +
> +static void cmd_health_help(void)
> +{
> +	pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
> +	pr_err("Usage: devlink health recover DEV reporter REPORTER_NAME\n");
> +	pr_err("Usage: devlink health diagnose DEV reporter REPORTER_NAME\n");
> +	pr_err("Usage: devlink health dump show DEV reporter REPORTER_NAME\n");
> +	pr_err("Usage: devlink health dump clear DEV reporter REPORTER_NAME\n");
> +	pr_err("Usage: devlink health set DEV reporter REPORTER_NAME NAME VALUE\n");
> +}
> +

Minor nit:
I prefer that all code and outputs in iproute2 look the same for ease
of maintenance and constituency of user experience.

Why does devlink not use:

static void cmd_health_help(void)
{
	fprintf(stderr, "Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
	fprintf(stderr, "       devlink health recover DEV reporter REPORTER_NAME\n");
	fprintf(stderr, "       devlink health diagnose DEV reporter REPORTER_NAME\n");

...

Or 
	fprintf(stderr, "Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n"
			"       devlink health recover DEV reporter REPORTER_NAME\n"	
			"       devlink health diagnose DEV reporter REPORTER_NAME\n");

^ permalink raw reply

* Re: [PATCH net-next v2 00/16] net: Remove switchdev_ops
From: Florian Fainelli @ 2019-02-10 20:44 UTC (permalink / raw)
  To: netdev; +Cc: idosch, linux-kernel, devel, bridge, jiri, andrew, vivien.didelot
In-Reply-To: <20190210175105.31629-1-f.fainelli@gmail.com>

Le 2/10/19 à 9:50 AM, Florian Fainelli a écrit :
> Hi all,
> 
> This patch series finishes by the removal of switchdev_ops. To get there
> we need to do a few things:
> 
> - get rid of the one and only call to switchdev_port_attr_get() which is
>   used to fetch the device's bridge port flags capabilities, instead we
>   can just check what flags are being programmed during the prepare
>   phase
> 
> - once we get rid of getting switchdev port attributes we convert the
>   setting of such attributes using a blocking notifier
> 
> And then remove switchdev_ops completely.
> 
> Please review and let me know what you think!

I am going to submit a v3 which moves the port_attr_{get,set} to a
switchdev notifier, but does not yet get rid of
switchdev_port_attr_get() entirely since there is not a clear path yet
to split the setting between non-sleeping and sleeping context.

> 
> Changes in v2:
> - fixed bisectability issues in patch #15
> - added Acked-by from Jiri where necessary
> - fixed a few minor issues according to Jiri's feedback:
> 	- rename port_attr_event -> port_attr_set_event
> 	- moved SWITCHDEV_PORT_ATTR_SET closer to other blocking events
> 
> Florian Fainelli (16):
>   Documentation: networking: switchdev: Update port parent ID section
>   mlxsw: spectrum: Check bridge flags during prepare phase
>   staging: fsl-dpaa2: ethsw: Check bridge port flags during set
>   net: dsa: Add setter for SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS
>   rocker: Check bridge flags during prepare phase
>   net: bridge: Stop calling switchdev_port_attr_get()
>   net: Remove SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
>   net: Get rid of switchdev_port_attr_get()
>   switchdev: Add SWITCHDEV_PORT_ATTR_SET
>   rocker: Handle SWITCHDEV_PORT_ATTR_SET
>   net: dsa: Handle SWITCHDEV_PORT_ATTR_SET
>   mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_SET
>   net: mscc: ocelot: Handle SWITCHDEV_PORT_ATTR_SET
>   staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_SET
>   net: switchdev: Replace port attr set SDO with a notification
>   net: Remove switchdev_ops
> 
>  Documentation/networking/switchdev.txt        | 15 ++-
>  .../net/ethernet/mellanox/mlxsw/spectrum.c    | 12 ---
>  .../net/ethernet/mellanox/mlxsw/spectrum.h    |  2 -
>  .../mellanox/mlxsw/spectrum_switchdev.c       | 69 ++++----------
>  drivers/net/ethernet/mscc/ocelot.c            | 21 +++-
>  drivers/net/ethernet/rocker/rocker_main.c     | 95 ++++++++-----------
>  drivers/staging/fsl-dpaa2/ethsw/ethsw.c       | 48 ++++------
>  include/linux/netdevice.h                     |  3 -
>  include/net/switchdev.h                       | 36 ++-----
>  net/bridge/br_switchdev.c                     | 20 +---
>  net/dsa/dsa_priv.h                            |  3 +
>  net/dsa/port.c                                | 10 ++
>  net/dsa/slave.c                               | 40 ++++----
>  net/switchdev/switchdev.c                     | 92 +++++-------------
>  14 files changed, 170 insertions(+), 296 deletions(-)
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next v2 00/16] net: Remove switchdev_ops
From: David Miller @ 2019-02-10 20:51 UTC (permalink / raw)
  To: f.fainelli
  Cc: netdev, idosch, linux-kernel, devel, bridge, jiri, andrew,
	vivien.didelot
In-Reply-To: <746bce08-5b5f-70c9-efb7-33b067929198@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sun, 10 Feb 2019 12:44:47 -0800

> I am going to submit a v3 which moves the port_attr_{get,set} to a
> switchdev notifier, but does not yet get rid of
> switchdev_port_attr_get() entirely since there is not a clear path yet
> to split the setting between non-sleeping and sleeping context.

Ok.

^ permalink raw reply

* [PATCH v2 bpf] bpf: fix lockdep false positive in stackmap
From: Alexei Starovoitov @ 2019-02-10 20:52 UTC (permalink / raw)
  To: davem; +Cc: daniel, peterz, edumazet, longman, jannh, netdev, kernel-team

Lockdep warns about false positive:
[   11.211460] ------------[ cut here ]------------
[   11.211936] DEBUG_LOCKS_WARN_ON(depth <= 0)
[   11.211985] WARNING: CPU: 0 PID: 141 at ../kernel/locking/lockdep.c:3592 lock_release+0x1ad/0x280
[   11.213134] Modules linked in:
[   11.214954] RIP: 0010:lock_release+0x1ad/0x280
[   11.223508] Call Trace:
[   11.223705]  <IRQ>
[   11.223874]  ? __local_bh_enable+0x7a/0x80
[   11.224199]  up_read+0x1c/0xa0
[   11.224446]  do_up_read+0x12/0x20
[   11.224713]  irq_work_run_list+0x43/0x70
[   11.225030]  irq_work_run+0x26/0x50
[   11.225310]  smp_irq_work_interrupt+0x57/0x1f0
[   11.225662]  irq_work_interrupt+0xf/0x20

since rw_semaphore is released in a different task vs task that locked the sema.
It is expected behavior.
Fix the warning with up_read_non_owner() and rwsem_release() annotation.

Fixes: bae77c5eb5b2 ("bpf: enable stackmap with build_id in nmi context")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/stackmap.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index d43b14535827..950ab2f28922 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -44,7 +44,7 @@ static void do_up_read(struct irq_work *entry)
 	struct stack_map_irq_work *work;
 
 	work = container_of(entry, struct stack_map_irq_work, irq_work);
-	up_read(work->sem);
+	up_read_non_owner(work->sem);
 	work->sem = NULL;
 }
 
@@ -338,6 +338,12 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
 	} else {
 		work->sem = &current->mm->mmap_sem;
 		irq_work_queue(&work->irq_work);
+		/*
+		 * The irq_work will release the mmap_sem with
+		 * up_read_non_owner(). The rwsem_release() is called
+		 * here to release the lock from lockdep's perspective.
+		 */
+		rwsem_release(&current->mm->mmap_sem.dep_map, 1, _RET_IP_);
 	}
 }
 
-- 
2.20.0


^ permalink raw reply related

* Re: [PATCH net-next 0/3] net: phy: add and use register modifying helpers returning 1 on change
From: David Miller @ 2019-02-10 20:53 UTC (permalink / raw)
  To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <a9d88fc1-6ff7-5805-a7d3-7aad7391b4d8@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 10 Feb 2019 19:56:47 +0100

> Add and use register modifying helpers returning 1 on change.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH v2 net 0/2] r8169: revert two commits due to a regression
From: David Miller @ 2019-02-10 20:57 UTC (permalink / raw)
  To: hkallweit1; +Cc: nic_swsd, linux, netdev
In-Reply-To: <63e9e767-f78b-8865-fe25-41d77e4e7a58@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 10 Feb 2019 15:24:37 +0100

> Sander reported a regression (kernel panic, see[1]), therefore let's
> revert these commits. Removal of the barriers doesn't seem to
> contribute to the issue, the patch just overlaps with the problematic
> one and only reverting both patches was tested.
> 
> [1] https://marc.info/?t=154965066400001&r=1&w=2
> 
> v2:
> - improve commit message

Series applied, thanks.

^ permalink raw reply

* [PATCH net-next] ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs
From: Callum Sinclair @ 2019-02-10 22:12 UTC (permalink / raw)
  To: davem, kuznet, yoshfuji, nikolay, netdev, linux-kernel; +Cc: Callum Sinclair

Created a way to clear the multicast forwarding cache on a socket
without having to either remove the entries manually using the delete
entry socket option or destroy and recreate the multicast socket.

Using the flags MRT_FLUSH_ENTRIES and MRT_FLUSH_VIFS, all multicast
entries can be cleared, all multicast interfaces can be closed or both
can be cleared using one sockopt call.

Patch Set 2:
  - Created new flags to flush static entries or vifs

Callum Sinclair (1):
  ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs

 include/uapi/linux/mroute.h  |  9 ++++-
 include/uapi/linux/mroute6.h |  9 ++++-
 net/ipv4/ipmr.c              | 71 ++++++++++++++++++++-------------
 net/ipv6/ip6mr.c             | 76 +++++++++++++++++++++++-------------
 4 files changed, 108 insertions(+), 57 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next] ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs
From: Callum Sinclair @ 2019-02-10 22:12 UTC (permalink / raw)
  To: davem, kuznet, yoshfuji, nikolay, netdev, linux-kernel; +Cc: Callum Sinclair
In-Reply-To: <20190210221222.28215-1-callum.sinclair@alliedtelesis.co.nz>

Currently the only way to clear the forwarding cache was to delete the
entries one by one using the MRT_DEL_MFC socket option or to destroy and
recreate the socket.

Create a new socket option which will clear the multicast forwarding
cache on the socket without destroying the socket. The new socket option
MRT_FLUSH_ENTRIES will clear all multicast entries on the sockets table
and the MRT_FLUSH_VIFS will delete all multicast vifs on the socket
table.

Signed-off-by: Callum Sinclair <callum.sinclair@alliedtelesis.co.nz>
---
 include/uapi/linux/mroute.h  |  9 ++++-
 include/uapi/linux/mroute6.h |  9 ++++-
 net/ipv4/ipmr.c              | 71 ++++++++++++++++++++-------------
 net/ipv6/ip6mr.c             | 76 +++++++++++++++++++++++-------------
 4 files changed, 108 insertions(+), 57 deletions(-)

diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
index 5d37a9ccce63..6b556ed7e252 100644
--- a/include/uapi/linux/mroute.h
+++ b/include/uapi/linux/mroute.h
@@ -28,12 +28,19 @@
 #define MRT_TABLE	(MRT_BASE+9)	/* Specify mroute table ID		*/
 #define MRT_ADD_MFC_PROXY	(MRT_BASE+10)	/* Add a (*,*|G) mfc entry	*/
 #define MRT_DEL_MFC_PROXY	(MRT_BASE+11)	/* Del a (*,*|G) mfc entry	*/
-#define MRT_MAX		(MRT_BASE+11)
+#define MRT_FLUSH	(MRT_BASE+12)	/* Flush all mfc entries and/or vifs	*/
+#define MRT_MAX		(MRT_BASE+12)
 
 #define SIOCGETVIFCNT	SIOCPROTOPRIVATE	/* IP protocol privates */
 #define SIOCGETSGCNT	(SIOCPROTOPRIVATE+1)
 #define SIOCGETRPF	(SIOCPROTOPRIVATE+2)
 
+/* MRT_FLUSH optional flags */
+#define MRT_FLUSH_MFC	1	/* Flush multicast entries */
+#define MRT_FLUSH_MFC_STATIC	2	/* Flush static multicast entries */
+#define MRT_FLUSH_VIFS	3	/* Flush multicast vifs */
+#define MRT_FLUSH_VIFS_STATIC	4	/* Flush static multicast vifs */
+
 #define MAXVIFS		32
 typedef unsigned long vifbitmap_t;	/* User mode code depends on this lot */
 typedef unsigned short vifi_t;
diff --git a/include/uapi/linux/mroute6.h b/include/uapi/linux/mroute6.h
index 9999cc006390..47a32c78cbea 100644
--- a/include/uapi/linux/mroute6.h
+++ b/include/uapi/linux/mroute6.h
@@ -31,12 +31,19 @@
 #define MRT6_TABLE	(MRT6_BASE+9)	/* Specify mroute table ID		*/
 #define MRT6_ADD_MFC_PROXY	(MRT6_BASE+10)	/* Add a (*,*|G) mfc entry	*/
 #define MRT6_DEL_MFC_PROXY	(MRT6_BASE+11)	/* Del a (*,*|G) mfc entry	*/
-#define MRT6_MAX	(MRT6_BASE+11)
+#define MRT6_FLUSH	(MRT6_BASE+12)	/* Flush all mfc entries and/or vifs	*/
+#define MRT6_MAX	(MRT6_BASE+12)
 
 #define SIOCGETMIFCNT_IN6	SIOCPROTOPRIVATE	/* IP protocol privates */
 #define SIOCGETSGCNT_IN6	(SIOCPROTOPRIVATE+1)
 #define SIOCGETRPF	(SIOCPROTOPRIVATE+2)
 
+/* MRT6_FLUSH optional flags */
+#define MRT6_FLUSH_MFC	1	/* Flush multicast entries */
+#define MRT6_FLUSH_MFC_STATIC	2	/* Flush static multicast entries */
+#define MRT6_FLUSH_VIFS	3	/* Flushing multicast vifs */
+#define MRT6_FLUSH_VIFS_STATIC	4	/* Flush static multicast vifs */
+
 #define MAXMIFS		32
 typedef unsigned long mifbitmap_t;	/* User mode code depends on this lot */
 typedef unsigned short mifi_t;
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index e536970557dd..0cbd0ed4ffff 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -110,7 +110,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
 static void mroute_netlink_event(struct mr_table *mrt, struct mfc_cache *mfc,
 				 int cmd);
 static void igmpmsg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt);
-static void mroute_clean_tables(struct mr_table *mrt, bool all);
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags);
 static void ipmr_expire_process(struct timer_list *t);
 
 #ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
@@ -415,7 +415,7 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id)
 static void ipmr_free_table(struct mr_table *mrt)
 {
 	del_timer_sync(&mrt->ipmr_expire_timer);
-	mroute_clean_tables(mrt, true);
+	mroute_clean_tables(mrt, true, MRT_FLUSH_VIFS | MRT_FLUSH_MFC);
 	rhltable_destroy(&mrt->mfc_hash);
 	kfree(mrt);
 }
@@ -1296,7 +1296,7 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
 }
 
 /* Close the multicast socket, and clear the vif tables etc */
-static void mroute_clean_tables(struct mr_table *mrt, bool all)
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags)
 {
 	struct net *net = read_pnet(&mrt->net);
 	struct mr_mfc *c, *tmp;
@@ -1305,35 +1305,41 @@ static void mroute_clean_tables(struct mr_table *mrt, bool all)
 	int i;
 
 	/* Shut down all active vif entries */
-	for (i = 0; i < mrt->maxvif; i++) {
-		if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
-			continue;
-		vif_delete(mrt, i, 0, &list);
+	if (flags & MRT_FLUSH_VIFS) {
+		for (i = 0; i < mrt->maxvif; i++) {
+			if ((mrt->vif_table[i].flags & VIFF_STATIC) &&
+			   !(all && (flags & MRT_FLUSH_VIFS_STATIC)))
+				continue;
+			vif_delete(mrt, i, 0, &list);
+		}
+		unregister_netdevice_many(&list);
 	}
-	unregister_netdevice_many(&list);
 
 	/* Wipe the cache */
-	list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
-		if (!all && (c->mfc_flags & MFC_STATIC))
-			continue;
-		rhltable_remove(&mrt->mfc_hash, &c->mnode, ipmr_rht_params);
-		list_del_rcu(&c->list);
-		cache = (struct mfc_cache *)c;
-		call_ipmr_mfc_entry_notifiers(net, FIB_EVENT_ENTRY_DEL, cache,
-					      mrt->id);
-		mroute_netlink_event(mrt, cache, RTM_DELROUTE);
-		mr_cache_put(c);
-	}
-
-	if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
-		spin_lock_bh(&mfc_unres_lock);
-		list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
-			list_del(&c->list);
+	if (flags & MRT_FLUSH_MFC) {
+		list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
+			if ((c->mfc_flags & MFC_STATIC) &&
+				!(all && (flags & MRT_FLUSH_MFC_STATIC)))
+				continue;
+			rhltable_remove(&mrt->mfc_hash, &c->mnode, ipmr_rht_params);
+			list_del_rcu(&c->list);
 			cache = (struct mfc_cache *)c;
+			call_ipmr_mfc_entry_notifiers(net, FIB_EVENT_ENTRY_DEL, cache,
+										  mrt->id);
 			mroute_netlink_event(mrt, cache, RTM_DELROUTE);
-			ipmr_destroy_unres(mrt, cache);
+			mr_cache_put(c);
+		}
+
+		if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
+			spin_lock_bh(&mfc_unres_lock);
+			list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
+				list_del(&c->list);
+				cache = (struct mfc_cache *)c;
+				mroute_netlink_event(mrt, cache, RTM_DELROUTE);
+				ipmr_destroy_unres(mrt, cache);
+			}
+			spin_unlock_bh(&mfc_unres_lock);
 		}
-		spin_unlock_bh(&mfc_unres_lock);
 	}
 }
 
@@ -1354,7 +1360,7 @@ static void mrtsock_destruct(struct sock *sk)
 						    NETCONFA_IFINDEX_ALL,
 						    net->ipv4.devconf_all);
 			RCU_INIT_POINTER(mrt->mroute_sk, NULL);
-			mroute_clean_tables(mrt, false);
+			mroute_clean_tables(mrt, false, MRT_FLUSH_VIFS | MRT_FLUSH_MFC);
 		}
 	}
 	rtnl_unlock();
@@ -1479,6 +1485,17 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval,
 					   sk == rtnl_dereference(mrt->mroute_sk),
 					   parent);
 		break;
+	case MRT_FLUSH:
+		if (optlen != sizeof(val)) {
+			ret = -EINVAL;
+			break;
+		}
+		if (get_user(val, (int __user *)optval)) {
+			ret = -EFAULT;
+			break;
+		}
+		mroute_clean_tables(mrt, false, val);
+		break;
 	/* Control PIM assert. */
 	case MRT_ASSERT:
 		if (optlen != sizeof(val)) {
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index cc01aa3f2b5e..31041d4332bc 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -97,7 +97,7 @@ static void mr6_netlink_event(struct mr_table *mrt, struct mfc6_cache *mfc,
 static void mrt6msg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt);
 static int ip6mr_rtm_dumproute(struct sk_buff *skb,
 			       struct netlink_callback *cb);
-static void mroute_clean_tables(struct mr_table *mrt, bool all);
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags);
 static void ipmr_expire_process(struct timer_list *t);
 
 #ifdef CONFIG_IPV6_MROUTE_MULTIPLE_TABLES
@@ -393,7 +393,7 @@ static struct mr_table *ip6mr_new_table(struct net *net, u32 id)
 static void ip6mr_free_table(struct mr_table *mrt)
 {
 	del_timer_sync(&mrt->ipmr_expire_timer);
-	mroute_clean_tables(mrt, true);
+	mroute_clean_tables(mrt, true, MRT6_FLUSH_VIFS | MRT6_FLUSH_MFC);
 	rhltable_destroy(&mrt->mfc_hash);
 	kfree(mrt);
 }
@@ -1496,42 +1496,48 @@ static int ip6mr_mfc_add(struct net *net, struct mr_table *mrt,
  *	Close the multicast socket, and clear the vif tables etc
  */
 
-static void mroute_clean_tables(struct mr_table *mrt, bool all)
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags)
 {
 	struct mr_mfc *c, *tmp;
 	LIST_HEAD(list);
 	int i;
 
 	/* Shut down all active vif entries */
-	for (i = 0; i < mrt->maxvif; i++) {
-		if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
-			continue;
-		mif6_delete(mrt, i, 0, &list);
+	if (flags & MRT6_FLUSH_VIFS) {
+		for (i = 0; i < mrt->maxvif; i++) {
+			if ((mrt->vif_table[i].flags & VIFF_STATIC) &&
+			    !(all && (flags & MRT6_FLUSH_VIFS_STATIC)))
+				continue;
+			mif6_delete(mrt, i, 0, &list);
+		}
+		unregister_netdevice_many(&list);
 	}
-	unregister_netdevice_many(&list);
 
 	/* Wipe the cache */
-	list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
-		if (!all && (c->mfc_flags & MFC_STATIC))
-			continue;
-		rhltable_remove(&mrt->mfc_hash, &c->mnode, ip6mr_rht_params);
-		list_del_rcu(&c->list);
-		call_ip6mr_mfc_entry_notifiers(read_pnet(&mrt->net),
-					       FIB_EVENT_ENTRY_DEL,
-					       (struct mfc6_cache *)c, mrt->id);
-		mr6_netlink_event(mrt, (struct mfc6_cache *)c, RTM_DELROUTE);
-		mr_cache_put(c);
-	}
+	if (flags & MRT6_FLUSH_MFC) {
+		list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
+			if ((c->mfc_flags & MFC_STATIC) &&
+				!(all && (flags & MRT6_FLUSH_MFC_STATIC)))
+				continue;
+			rhltable_remove(&mrt->mfc_hash, &c->mnode, ip6mr_rht_params);
+			list_del_rcu(&c->list);
+			call_ip6mr_mfc_entry_notifiers(read_pnet(&mrt->net),
+										   FIB_EVENT_ENTRY_DEL,
+										   (struct mfc6_cache *)c, mrt->id);
+			mr6_netlink_event(mrt, (struct mfc6_cache *)c, RTM_DELROUTE);
+			mr_cache_put(c);
+		}
 
-	if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
-		spin_lock_bh(&mfc_unres_lock);
-		list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
-			list_del(&c->list);
-			mr6_netlink_event(mrt, (struct mfc6_cache *)c,
-					  RTM_DELROUTE);
-			ip6mr_destroy_unres(mrt, (struct mfc6_cache *)c);
+		if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
+			spin_lock_bh(&mfc_unres_lock);
+			list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
+				list_del(&c->list);
+				mr6_netlink_event(mrt, (struct mfc6_cache *)c,
+								  RTM_DELROUTE);
+				ip6mr_destroy_unres(mrt, (struct mfc6_cache *)c);
+			}
+			spin_unlock_bh(&mfc_unres_lock);
 		}
-		spin_unlock_bh(&mfc_unres_lock);
 	}
 }
 
@@ -1587,7 +1593,7 @@ int ip6mr_sk_done(struct sock *sk)
 						     NETCONFA_IFINDEX_ALL,
 						     net->ipv6.devconf_all);
 
-			mroute_clean_tables(mrt, false);
+			mroute_clean_tables(mrt, false, MRT6_FLUSH_VIFS | MRT6_FLUSH_MFC);
 			err = 0;
 			break;
 		}
@@ -1703,6 +1709,20 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, uns
 		rtnl_unlock();
 		return ret;
 
+	case MRT6_FLUSH:
+	{
+		int flags;
+
+		if (optlen != sizeof(flags))
+			return -EINVAL;
+		if (get_user(flags, (int __user *)optval))
+			return -EFAULT;
+		rtnl_lock();
+		mroute_clean_tables(mrt, true, flags);
+		rtnl_unlock();
+		return 0;
+	}
+
 	/*
 	 *	Control PIM assert (to activate pim will activate assert)
 	 */
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next v5 12/12] sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW
From: Michael Ellerman @ 2019-02-10 22:12 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: David S. Miller, Linux Kernel Mailing List,
	Linux Network Devel Mailing List, Arnd Bergmann,
	y2038 Mailman List, ccaulfie, Helge Deller, Paul Mackerras,
	Ralf Baechle, Richard Henderson, cluster-devel, linuxppc-dev,
	linux-alpha, linux-arch, linux-mips, Parisc List, sparclinux
In-Reply-To: <CABeXuvqpexo4g7xQihKPoOd4ce4rLq0Agy-jYMELYvOAnqmXJA@mail.gmail.com>

Deepa Dinamani <deepa.kernel@gmail.com> writes:

>> You touched powerpc in the previous patch but not this one.
>>
>> That's because we use the asm-generic version I assume.
>
> That is correct.
>
>> Would be good to mention in the change log though to avoid any confusion.
>
> I'm not sure how to do that now. It looks like the series has already
> been applied to net-next with a couple of merge conflicts fixed.

That's fine, it's not that important.

cheers

^ permalink raw reply

* Re: [PATCH net-next] ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs
From: Nikolay Aleksandrov @ 2019-02-10 22:55 UTC (permalink / raw)
  To: Callum Sinclair, davem, kuznet, yoshfuji, netdev, linux-kernel,
	Nicolas Dichtel
In-Reply-To: <20190210221222.28215-2-callum.sinclair@alliedtelesis.co.nz>

On 2/11/19 12:12 AM, Callum Sinclair wrote:
> Currently the only way to clear the forwarding cache was to delete the
> entries one by one using the MRT_DEL_MFC socket option or to destroy and
> recreate the socket.
> 
> Create a new socket option which will clear the multicast forwarding
> cache on the socket without destroying the socket. The new socket option
> MRT_FLUSH_ENTRIES will clear all multicast entries on the sockets table
> and the MRT_FLUSH_VIFS will delete all multicast vifs on the socket
> table.
> 
> Signed-off-by: Callum Sinclair <callum.sinclair@alliedtelesis.co.nz>
> ---
>  include/uapi/linux/mroute.h  |  9 ++++-
>  include/uapi/linux/mroute6.h |  9 ++++-
>  net/ipv4/ipmr.c              | 71 ++++++++++++++++++++-------------
>  net/ipv6/ip6mr.c             | 76 +++++++++++++++++++++++-------------
>  4 files changed, 108 insertions(+), 57 deletions(-)
> 

Hi,
How could 3 be a flag ? How was this tested exactly ?

I think you can remove the "all" argument altogether and just use the
flags. Also for each iteration of this patch please add a version (as in
v1, v2 etc after net-next in the subject) and in the commit message explain what
has changed between versions like v1 -> v2: fixed blah.
And what happens if we only specify VIFF_STATIC without VIFF or just MFC_STATIC without MFC ?
I see how these work as kind of sub-flags, but there is no explanation of the intended
behaviour nor justification, in fact the commit message is wrong - MRT_FLUSH_ENTRIES
will not clean all and actually there is no MRT_FLUSH_ENTRIES flag or call.

Also please CC all of the people who reviewed previous versions like
Nicolas Dichtel.

A few more comments inline below.

Thanks,
 Nik

> diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
> index 5d37a9ccce63..6b556ed7e252 100644
> --- a/include/uapi/linux/mroute.h
> +++ b/include/uapi/linux/mroute.h
> @@ -28,12 +28,19 @@
>  #define MRT_TABLE	(MRT_BASE+9)	/* Specify mroute table ID		*/
>  #define MRT_ADD_MFC_PROXY	(MRT_BASE+10)	/* Add a (*,*|G) mfc entry	*/
>  #define MRT_DEL_MFC_PROXY	(MRT_BASE+11)	/* Del a (*,*|G) mfc entry	*/
> -#define MRT_MAX		(MRT_BASE+11)
> +#define MRT_FLUSH	(MRT_BASE+12)	/* Flush all mfc entries and/or vifs	*/
> +#define MRT_MAX		(MRT_BASE+12)
>  
>  #define SIOCGETVIFCNT	SIOCPROTOPRIVATE	/* IP protocol privates */
>  #define SIOCGETSGCNT	(SIOCPROTOPRIVATE+1)
>  #define SIOCGETRPF	(SIOCPROTOPRIVATE+2)
>  
> +/* MRT_FLUSH optional flags */
> +#define MRT_FLUSH_MFC	1	/* Flush multicast entries */
> +#define MRT_FLUSH_MFC_STATIC	2	/* Flush static multicast entries */
> +#define MRT_FLUSH_VIFS	3	/* Flush multicast vifs */
> +#define MRT_FLUSH_VIFS_STATIC	4	/* Flush static multicast vifs */
> +
>  #define MAXVIFS		32
>  typedef unsigned long vifbitmap_t;	/* User mode code depends on this lot */
>  typedef unsigned short vifi_t;
> diff --git a/include/uapi/linux/mroute6.h b/include/uapi/linux/mroute6.h
> index 9999cc006390..47a32c78cbea 100644
> --- a/include/uapi/linux/mroute6.h
> +++ b/include/uapi/linux/mroute6.h
> @@ -31,12 +31,19 @@
>  #define MRT6_TABLE	(MRT6_BASE+9)	/* Specify mroute table ID		*/
>  #define MRT6_ADD_MFC_PROXY	(MRT6_BASE+10)	/* Add a (*,*|G) mfc entry	*/
>  #define MRT6_DEL_MFC_PROXY	(MRT6_BASE+11)	/* Del a (*,*|G) mfc entry	*/
> -#define MRT6_MAX	(MRT6_BASE+11)
> +#define MRT6_FLUSH	(MRT6_BASE+12)	/* Flush all mfc entries and/or vifs	*/
> +#define MRT6_MAX	(MRT6_BASE+12)
>  
>  #define SIOCGETMIFCNT_IN6	SIOCPROTOPRIVATE	/* IP protocol privates */
>  #define SIOCGETSGCNT_IN6	(SIOCPROTOPRIVATE+1)
>  #define SIOCGETRPF	(SIOCPROTOPRIVATE+2)
>  
> +/* MRT6_FLUSH optional flags */
> +#define MRT6_FLUSH_MFC	1	/* Flush multicast entries */
> +#define MRT6_FLUSH_MFC_STATIC	2	/* Flush static multicast entries */
> +#define MRT6_FLUSH_VIFS	3	/* Flushing multicast vifs */
> +#define MRT6_FLUSH_VIFS_STATIC	4	/* Flush static multicast vifs */
> +
>  #define MAXMIFS		32
>  typedef unsigned long mifbitmap_t;	/* User mode code depends on this lot */
>  typedef unsigned short mifi_t;
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index e536970557dd..0cbd0ed4ffff 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -110,7 +110,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
>  static void mroute_netlink_event(struct mr_table *mrt, struct mfc_cache *mfc,
>  				 int cmd);
>  static void igmpmsg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt);
> -static void mroute_clean_tables(struct mr_table *mrt, bool all);
> +static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags);
>  static void ipmr_expire_process(struct timer_list *t);
>  
>  #ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
> @@ -415,7 +415,7 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id)
>  static void ipmr_free_table(struct mr_table *mrt)
>  {
>  	del_timer_sync(&mrt->ipmr_expire_timer);
> -	mroute_clean_tables(mrt, true);
> +	mroute_clean_tables(mrt, true, MRT_FLUSH_VIFS | MRT_FLUSH_MFC);
>  	rhltable_destroy(&mrt->mfc_hash);
>  	kfree(mrt);
>  }
> @@ -1296,7 +1296,7 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
>  }
>  
>  /* Close the multicast socket, and clear the vif tables etc */
> -static void mroute_clean_tables(struct mr_table *mrt, bool all)
> +static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags)
>  {
>  	struct net *net = read_pnet(&mrt->net);
>  	struct mr_mfc *c, *tmp;
> @@ -1305,35 +1305,41 @@ static void mroute_clean_tables(struct mr_table *mrt, bool all)
>  	int i;
>  
>  	/* Shut down all active vif entries */
> -	for (i = 0; i < mrt->maxvif; i++) {
> -		if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
> -			continue;
> -		vif_delete(mrt, i, 0, &list);
> +	if (flags & MRT_FLUSH_VIFS) {
> +		for (i = 0; i < mrt->maxvif; i++) {
> +			if ((mrt->vif_table[i].flags & VIFF_STATIC) &&
> +			   !(all && (flags & MRT_FLUSH_VIFS_STATIC)))

nit: the ! must be 1 more space further (below the second bracket after if).


> +				continue;
> +			vif_delete(mrt, i, 0, &list);
> +		}
> +		unregister_netdevice_many(&list);
>  	}
> -	unregister_netdevice_many(&list);
>  
>  	/* Wipe the cache */
> -	list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
> -		if (!all && (c->mfc_flags & MFC_STATIC))
> -			continue;
> -		rhltable_remove(&mrt->mfc_hash, &c->mnode, ipmr_rht_params);
> -		list_del_rcu(&c->list);
> -		cache = (struct mfc_cache *)c;
> -		call_ipmr_mfc_entry_notifiers(net, FIB_EVENT_ENTRY_DEL, cache,
> -					      mrt->id);
> -		mroute_netlink_event(mrt, cache, RTM_DELROUTE);
> -		mr_cache_put(c);
> -	}
> -
> -	if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
> -		spin_lock_bh(&mfc_unres_lock);
> -		list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
> -			list_del(&c->list);
> +	if (flags & MRT_FLUSH_MFC) {
> +		list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
> +			if ((c->mfc_flags & MFC_STATIC) &&
> +				!(all && (flags & MRT_FLUSH_MFC_STATIC)))

again wrong indentation, the ! must be below the second bracket after the if (the
first character of the condition).

> +				continue;
> +			rhltable_remove(&mrt->mfc_hash, &c->mnode, ipmr_rht_params);
> +			list_del_rcu(&c->list);
>  			cache = (struct mfc_cache *)c;
> +			call_ipmr_mfc_entry_notifiers(net, FIB_EVENT_ENTRY_DEL, cache,
> +										  mrt->id);

again wrong indentation

>  			mroute_netlink_event(mrt, cache, RTM_DELROUTE);
> -			ipmr_destroy_unres(mrt, cache);
> +			mr_cache_put(c);
> +		}
> +
> +		if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
> +			spin_lock_bh(&mfc_unres_lock);
> +			list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
> +				list_del(&c->list);
> +				cache = (struct mfc_cache *)c;
> +				mroute_netlink_event(mrt, cache, RTM_DELROUTE);
> +				ipmr_destroy_unres(mrt, cache);
> +			}
> +			spin_unlock_bh(&mfc_unres_lock);
>  		}
> -		spin_unlock_bh(&mfc_unres_lock);
>  	}
>  }
>  
> @@ -1354,7 +1360,7 @@ static void mrtsock_destruct(struct sock *sk)
>  						    NETCONFA_IFINDEX_ALL,
>  						    net->ipv4.devconf_all);
>  			RCU_INIT_POINTER(mrt->mroute_sk, NULL);
> -			mroute_clean_tables(mrt, false);
> +			mroute_clean_tables(mrt, false, MRT_FLUSH_VIFS | MRT_FLUSH_MFC);
>  		}
>  	}
>  	rtnl_unlock();
> @@ -1479,6 +1485,17 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval,
>  					   sk == rtnl_dereference(mrt->mroute_sk),
>  					   parent);
>  		break;
> +	case MRT_FLUSH:
> +		if (optlen != sizeof(val)) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +		if (get_user(val, (int __user *)optval)) {
> +			ret = -EFAULT;
> +			break;
> +		}
> +		mroute_clean_tables(mrt, false, val);
> +		break;
>  	/* Control PIM assert. */
>  	case MRT_ASSERT:
>  		if (optlen != sizeof(val)) {
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index cc01aa3f2b5e..31041d4332bc 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -97,7 +97,7 @@ static void mr6_netlink_event(struct mr_table *mrt, struct mfc6_cache *mfc,
>  static void mrt6msg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt);
>  static int ip6mr_rtm_dumproute(struct sk_buff *skb,
>  			       struct netlink_callback *cb);
> -static void mroute_clean_tables(struct mr_table *mrt, bool all);
> +static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags);
>  static void ipmr_expire_process(struct timer_list *t);
>  
>  #ifdef CONFIG_IPV6_MROUTE_MULTIPLE_TABLES
> @@ -393,7 +393,7 @@ static struct mr_table *ip6mr_new_table(struct net *net, u32 id)
>  static void ip6mr_free_table(struct mr_table *mrt)
>  {
>  	del_timer_sync(&mrt->ipmr_expire_timer);
> -	mroute_clean_tables(mrt, true);
> +	mroute_clean_tables(mrt, true, MRT6_FLUSH_VIFS | MRT6_FLUSH_MFC);
>  	rhltable_destroy(&mrt->mfc_hash);
>  	kfree(mrt);
>  }
> @@ -1496,42 +1496,48 @@ static int ip6mr_mfc_add(struct net *net, struct mr_table *mrt,
>   *	Close the multicast socket, and clear the vif tables etc
>   */
>  
> -static void mroute_clean_tables(struct mr_table *mrt, bool all)
> +static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags)
>  {
>  	struct mr_mfc *c, *tmp;
>  	LIST_HEAD(list);
>  	int i;
>  
>  	/* Shut down all active vif entries */
> -	for (i = 0; i < mrt->maxvif; i++) {
> -		if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
> -			continue;
> -		mif6_delete(mrt, i, 0, &list);
> +	if (flags & MRT6_FLUSH_VIFS) {
> +		for (i = 0; i < mrt->maxvif; i++) {
> +			if ((mrt->vif_table[i].flags & VIFF_STATIC) &&
> +			    !(all && (flags & MRT6_FLUSH_VIFS_STATIC)))
> +				continue;
> +			mif6_delete(mrt, i, 0, &list);
> +		}
> +		unregister_netdevice_many(&list);
>  	}
> -	unregister_netdevice_many(&list);
>  
>  	/* Wipe the cache */
> -	list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
> -		if (!all && (c->mfc_flags & MFC_STATIC))
> -			continue;
> -		rhltable_remove(&mrt->mfc_hash, &c->mnode, ip6mr_rht_params);
> -		list_del_rcu(&c->list);
> -		call_ip6mr_mfc_entry_notifiers(read_pnet(&mrt->net),
> -					       FIB_EVENT_ENTRY_DEL,
> -					       (struct mfc6_cache *)c, mrt->id);
> -		mr6_netlink_event(mrt, (struct mfc6_cache *)c, RTM_DELROUTE);
> -		mr_cache_put(c);
> -	}
> +	if (flags & MRT6_FLUSH_MFC) {
> +		list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
> +			if ((c->mfc_flags & MFC_STATIC) &&
> +				!(all && (flags & MRT6_FLUSH_MFC_STATIC)))

wrong indentation

> +				continue;
> +			rhltable_remove(&mrt->mfc_hash, &c->mnode, ip6mr_rht_params);
> +			list_del_rcu(&c->list);
> +			call_ip6mr_mfc_entry_notifiers(read_pnet(&mrt->net),
> +										   FIB_EVENT_ENTRY_DEL,
> +										   (struct mfc6_cache *)c, mrt->id);

wrong indentation

> +			mr6_netlink_event(mrt, (struct mfc6_cache *)c, RTM_DELROUTE);
> +			mr_cache_put(c);
> +		}
>  
> -	if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
> -		spin_lock_bh(&mfc_unres_lock);
> -		list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
> -			list_del(&c->list);
> -			mr6_netlink_event(mrt, (struct mfc6_cache *)c,
> -					  RTM_DELROUTE);
> -			ip6mr_destroy_unres(mrt, (struct mfc6_cache *)c);
> +		if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
> +			spin_lock_bh(&mfc_unres_lock);
> +			list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
> +				list_del(&c->list);
> +				mr6_netlink_event(mrt, (struct mfc6_cache *)c,
> +								  RTM_DELROUTE);

wrong indentation

> +				ip6mr_destroy_unres(mrt, (struct mfc6_cache *)c);
> +			}
> +			spin_unlock_bh(&mfc_unres_lock);
>  		}
> -		spin_unlock_bh(&mfc_unres_lock);
>  	}
>  }
>  
> @@ -1587,7 +1593,7 @@ int ip6mr_sk_done(struct sock *sk)
>  						     NETCONFA_IFINDEX_ALL,
>  						     net->ipv6.devconf_all);
>  
> -			mroute_clean_tables(mrt, false);
> +			mroute_clean_tables(mrt, false, MRT6_FLUSH_VIFS | MRT6_FLUSH_MFC);
>  			err = 0;
>  			break;
>  		}
> @@ -1703,6 +1709,20 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, uns
>  		rtnl_unlock();
>  		return ret;
>  
> +	case MRT6_FLUSH:
> +	{
> +		int flags;
> +
> +		if (optlen != sizeof(flags))
> +			return -EINVAL;
> +		if (get_user(flags, (int __user *)optval))
> +			return -EFAULT;
> +		rtnl_lock();
> +		mroute_clean_tables(mrt, true, flags);
> +		rtnl_unlock();
> +		return 0;
> +	}
> +
>  	/*
>  	 *	Control PIM assert (to activate pim will activate assert)
>  	 */
> 


^ permalink raw reply

* [PATCH net-next v3 8/9] net: switchdev: Replace port attr get/set SDO with a notification
From: Florian Fainelli @ 2019-02-10 23:40 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, idosch, linux-kernel, devel, bridge, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190210234007.16173-1-f.fainelli@gmail.com>

Drop switchdev_ops.switchdev_port_attr_get and _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_GET and _SET.

Update switchdev_port_attr_get() to dispatch to this new function. Drop
__switchdev_port_attr_set() and update switchdev_port_attr_set()
likewise.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/switchdev/switchdev.c | 107 +++++++++++++-------------------------
 1 file changed, 37 insertions(+), 70 deletions(-)

diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 7e1357db33d7..8fc3db2179f5 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -174,81 +174,31 @@ static int switchdev_deferred_enqueue(struct net_device *dev,
 	return 0;
 }
 
-/**
- *	switchdev_port_attr_get - Get port attribute
- *
- *	@dev: port device
- *	@attr: attribute to get
- */
-int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr *attr)
+static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
+				      struct net_device *dev,
+				      struct switchdev_attr *attr,
+				      struct switchdev_trans *trans)
 {
-	const struct switchdev_ops *ops = dev->switchdev_ops;
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	struct switchdev_attr first = {
-		.id = SWITCHDEV_ATTR_ID_UNDEFINED
-	};
-	int err = -EOPNOTSUPP;
+	int err;
+	int rc;
 
-	if (ops && ops->switchdev_port_attr_get)
-		return ops->switchdev_port_attr_get(dev, attr);
+	struct switchdev_notifier_port_attr_info attr_info = {
+		.attr = attr,
+		.trans = trans,
+		.handled = false,
+	};
 
-	if (attr->flags & SWITCHDEV_F_NO_RECURSE)
+	rc = call_switchdev_blocking_notifiers(nt, dev, &attr_info.info, NULL);
+	err = notifier_to_errno(rc);
+	if (err) {
+		WARN_ON(!attr_info.handled);
 		return err;
-
-	/* Switch device port(s) may be stacked under
-	 * bond/team/vlan dev, so recurse down to get attr on
-	 * each port.  Return -ENODATA if attr values don't
-	 * compare across ports.
-	 */
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = switchdev_port_attr_get(lower_dev, attr);
-		if (err)
-			break;
-		if (first.id == SWITCHDEV_ATTR_ID_UNDEFINED)
-			first = *attr;
-		else if (memcmp(&first, attr, sizeof(*attr)))
-			return -ENODATA;
-	}
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(switchdev_port_attr_get);
-
-static int __switchdev_port_attr_set(struct net_device *dev,
-				     const struct switchdev_attr *attr,
-				     struct switchdev_trans *trans)
-{
-	const struct switchdev_ops *ops = dev->switchdev_ops;
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	int err = -EOPNOTSUPP;
-
-	if (ops && ops->switchdev_port_attr_set) {
-		err = ops->switchdev_port_attr_set(dev, attr, trans);
-		goto done;
-	}
-
-	if (attr->flags & SWITCHDEV_F_NO_RECURSE)
-		goto done;
-
-	/* Switch device port(s) may be stacked under
-	 * bond/team/vlan dev, so recurse down to set attr on
-	 * each port.
-	 */
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = __switchdev_port_attr_set(lower_dev, attr, trans);
-		if (err)
-			break;
 	}
 
-done:
-	if (err == -EOPNOTSUPP && attr->flags & SWITCHDEV_F_SKIP_EOPNOTSUPP)
-		err = 0;
+	if (!attr_info.handled)
+		return -EOPNOTSUPP;
 
-	return err;
+	return 0;
 }
 
 static int switchdev_port_attr_set_now(struct net_device *dev,
@@ -267,7 +217,9 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
 	 */
 
 	trans.ph_prepare = true;
-	err = __switchdev_port_attr_set(dev, attr, &trans);
+	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET,
+					 dev, (struct switchdev_attr *)attr,
+					 &trans);
 	if (err) {
 		/* Prepare phase failed: abort the transaction.  Any
 		 * resources reserved in the prepare phase are
@@ -286,7 +238,9 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
 	 */
 
 	trans.ph_prepare = false;
-	err = __switchdev_port_attr_set(dev, attr, &trans);
+	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET,
+					 dev, (struct switchdev_attr *)attr,
+					 &trans);
 	WARN(err, "%s: Commit of attribute (id=%d) failed.\n",
 	     dev->name, attr->id);
 	switchdev_trans_items_warn_destroy(dev, &trans);
@@ -338,6 +292,19 @@ int switchdev_port_attr_set(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(switchdev_port_attr_set);
 
+/**
+ *	switchdev_port_attr_get - Get port attribute
+ *
+ *	@dev: port device
+ *	@attr: attribute to get
+ */
+int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr *attr)
+{
+	return switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_GET, dev,
+					  attr, NULL);
+}
+EXPORT_SYMBOL_GPL(switchdev_port_attr_get);
+
 static size_t switchdev_obj_size(const struct switchdev_obj *obj)
 {
 	switch (obj->id) {
-- 
2.19.1


^ permalink raw reply related

* [PATCH net-next v3 9/9] net: Remove switchdev_ops
From: Florian Fainelli @ 2019-02-10 23:40 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, idosch, linux-kernel, devel, bridge, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190210234007.16173-1-f.fainelli@gmail.com>

Now that we have converted all possible callers to using a switchdev
notifier for attributes we do not have a need for implementing
switchdev_ops anymore, and this can be removed from all drivers the
net_device structure.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 12 ------------
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h |  2 --
 .../mellanox/mlxsw/spectrum_switchdev.c        | 13 -------------
 drivers/net/ethernet/mscc/ocelot.c             |  5 -----
 drivers/net/ethernet/rocker/rocker_main.c      |  6 ------
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c        |  6 ------
 include/linux/netdevice.h                      |  3 ---
 include/net/switchdev.h                        | 18 ------------------
 net/dsa/slave.c                                |  6 ------
 9 files changed, 71 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 7c9745cecbbd..619965abab43 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3220,7 +3220,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 	}
 	mlxsw_sp_port->default_vlan = mlxsw_sp_port_vlan;
 
-	mlxsw_sp_port_switchdev_init(mlxsw_sp_port);
 	mlxsw_sp->ports[local_port] = mlxsw_sp_port;
 	err = register_netdev(dev);
 	if (err) {
@@ -3237,7 +3236,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 
 err_register_netdev:
 	mlxsw_sp->ports[local_port] = NULL;
-	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_destroy(mlxsw_sp_port_vlan);
 err_port_vlan_create:
 err_port_pvid_set:
@@ -3280,7 +3278,6 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
 	mlxsw_core_port_clear(mlxsw_sp->core, local_port, mlxsw_sp);
 	unregister_netdev(mlxsw_sp_port->dev); /* This calls ndo_stop */
 	mlxsw_sp->ports[local_port] = NULL;
-	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_flush(mlxsw_sp_port, true);
 	mlxsw_sp_port_nve_fini(mlxsw_sp_port);
 	mlxsw_sp_tc_qdisc_fini(mlxsw_sp_port);
@@ -4001,12 +3998,6 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
 		goto err_span_init;
 	}
 
-	err = mlxsw_sp_switchdev_init(mlxsw_sp);
-	if (err) {
-		dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize switchdev\n");
-		goto err_switchdev_init;
-	}
-
 	err = mlxsw_sp_counter_pool_init(mlxsw_sp);
 	if (err) {
 		dev_err(mlxsw_sp->bus_info->dev, "Failed to init counter pool\n");
@@ -4077,8 +4068,6 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
 err_afa_init:
 	mlxsw_sp_counter_pool_fini(mlxsw_sp);
 err_counter_pool_init:
-	mlxsw_sp_switchdev_fini(mlxsw_sp);
-err_switchdev_init:
 	mlxsw_sp_span_fini(mlxsw_sp);
 err_span_init:
 	mlxsw_sp_lag_fini(mlxsw_sp);
@@ -4141,7 +4130,6 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
 	mlxsw_sp_nve_fini(mlxsw_sp);
 	mlxsw_sp_afa_fini(mlxsw_sp);
 	mlxsw_sp_counter_pool_fini(mlxsw_sp);
-	mlxsw_sp_switchdev_fini(mlxsw_sp);
 	mlxsw_sp_span_fini(mlxsw_sp);
 	mlxsw_sp_lag_fini(mlxsw_sp);
 	mlxsw_sp_buffers_fini(mlxsw_sp);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index ceebc91f4f1d..82e3e6dc81a1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -375,8 +375,6 @@ u32 mlxsw_sp_bytes_cells(const struct mlxsw_sp *mlxsw_sp, u32 bytes);
 /* spectrum_switchdev.c */
 int mlxsw_sp_switchdev_init(struct mlxsw_sp *mlxsw_sp);
 void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp);
-void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port);
-void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port);
 int mlxsw_sp_rif_fdb_op(struct mlxsw_sp *mlxsw_sp, const char *mac, u16 fid,
 			bool adding);
 void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index c6d7bb70e8f2..fafd582c2fdc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1957,11 +1957,6 @@ static struct mlxsw_sp_port *mlxsw_sp_lag_rep_port(struct mlxsw_sp *mlxsw_sp,
 	return NULL;
 }
 
-static const struct switchdev_ops mlxsw_sp_port_switchdev_ops = {
-	.switchdev_port_attr_get	= mlxsw_sp_port_attr_get,
-	.switchdev_port_attr_set	= mlxsw_sp_port_attr_set,
-};
-
 static int
 mlxsw_sp_bridge_8021q_port_join(struct mlxsw_sp_bridge_device *bridge_device,
 				struct mlxsw_sp_bridge_port *bridge_port,
@@ -3576,11 +3571,3 @@ void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp)
 	kfree(mlxsw_sp->bridge);
 }
 
-void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port)
-{
-	mlxsw_sp_port->dev->switchdev_ops = &mlxsw_sp_port_switchdev_ops;
-}
-
-void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port)
-{
-}
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 850a49033a30..21c73a0355d4 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1324,10 +1324,6 @@ static int ocelot_port_obj_del(struct net_device *dev,
 	return ret;
 }
 
-static const struct switchdev_ops ocelot_port_switchdev_ops = {
-	.switchdev_port_attr_set	= ocelot_port_attr_set,
-};
-
 static int ocelot_port_bridge_join(struct ocelot_port *ocelot_port,
 				   struct net_device *bridge)
 {
@@ -1654,7 +1650,6 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
 
 	dev->netdev_ops = &ocelot_port_netdev_ops;
 	dev->ethtool_ops = &ocelot_ethtool_ops;
-	dev->switchdev_ops = &ocelot_port_switchdev_ops;
 
 	dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXFCS;
 	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 591008c8fa74..16f045b06c55 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2147,11 +2147,6 @@ static int rocker_port_obj_del(struct net_device *dev,
 	return err;
 }
 
-static const struct switchdev_ops rocker_port_switchdev_ops = {
-	.switchdev_port_attr_get	= rocker_port_attr_get,
-	.switchdev_port_attr_set	= rocker_port_attr_set,
-};
-
 struct rocker_fib_event_work {
 	struct work_struct work;
 	union {
@@ -2605,7 +2600,6 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
 	rocker_port_dev_addr_init(rocker_port);
 	dev->netdev_ops = &rocker_port_netdev_ops;
 	dev->ethtool_ops = &rocker_port_ethtool_ops;
-	dev->switchdev_ops = &rocker_port_switchdev_ops;
 	netif_tx_napi_add(dev, &rocker_port->napi_tx, rocker_port_poll_tx,
 			  NAPI_POLL_WEIGHT);
 	netif_napi_add(dev, &rocker_port->napi_rx, rocker_port_poll_rx,
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index bc9e7de07200..44831990239c 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -932,11 +932,6 @@ static int swdev_port_obj_del(struct net_device *netdev,
 	return err;
 }
 
-static const struct switchdev_ops ethsw_port_switchdev_ops = {
-	.switchdev_port_attr_get	= swdev_port_attr_get,
-	.switchdev_port_attr_set	= swdev_port_attr_set,
-};
-
 /* For the moment, only flood setting needs to be updated */
 static int port_bridge_join(struct net_device *netdev,
 			    struct net_device *upper_dev)
@@ -1466,7 +1461,6 @@ static int ethsw_probe_port(struct ethsw_core *ethsw, u16 port_idx)
 	SET_NETDEV_DEV(port_netdev, dev);
 	port_netdev->netdev_ops = &ethsw_port_ops;
 	port_netdev->ethtool_ops = &ethsw_port_ethtool_ops;
-	port_netdev->switchdev_ops = &ethsw_port_switchdev_ops;
 
 	/* Set MTU limits */
 	port_netdev->min_mtu = ETH_MIN_MTU;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1fb733f38a47..a747456b9d23 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1836,9 +1836,6 @@ struct net_device {
 #endif
 	const struct net_device_ops *netdev_ops;
 	const struct ethtool_ops *ethtool_ops;
-#ifdef CONFIG_NET_SWITCHDEV
-	const struct switchdev_ops *switchdev_ops;
-#endif
 #ifdef CONFIG_NET_L3_MASTER_DEV
 	const struct l3mdev_ops	*l3mdev_ops;
 #endif
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index b8becabbef38..e9aa920aba57 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -113,21 +113,6 @@ void *switchdev_trans_item_dequeue(struct switchdev_trans *trans);
 
 typedef int switchdev_obj_dump_cb_t(struct switchdev_obj *obj);
 
-/**
- * struct switchdev_ops - switchdev operations
- *
- * @switchdev_port_attr_get: Get a port attribute (see switchdev_attr).
- *
- * @switchdev_port_attr_set: Set a port attribute (see switchdev_attr).
- */
-struct switchdev_ops {
-	int	(*switchdev_port_attr_get)(struct net_device *dev,
-					   struct switchdev_attr *attr);
-	int	(*switchdev_port_attr_set)(struct net_device *dev,
-					   const struct switchdev_attr *attr,
-					   struct switchdev_trans *trans);
-};
-
 enum switchdev_notifier_type {
 	SWITCHDEV_FDB_ADD_TO_BRIDGE = 1,
 	SWITCHDEV_FDB_DEL_TO_BRIDGE,
@@ -229,7 +214,6 @@ int switchdev_handle_port_obj_del(struct net_device *dev,
 			int (*del_cb)(struct net_device *dev,
 				      const struct switchdev_obj *obj));
 
-#define SWITCHDEV_SET_OPS(netdev, ops) ((netdev)->switchdev_ops = (ops))
 #else
 
 static inline void switchdev_deferred_process(void)
@@ -322,8 +306,6 @@ switchdev_handle_port_obj_del(struct net_device *dev,
 	return 0;
 }
 
-#define SWITCHDEV_SET_OPS(netdev, ops) do {} while (0)
-
 #endif
 
 #endif /* _LINUX_SWITCHDEV_H_ */
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2a14a38f5f93..236b6a72dba3 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1057,11 +1057,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_get_port_parent_id	= dsa_slave_get_port_parent_id,
 };
 
-static const struct switchdev_ops dsa_slave_switchdev_ops = {
-	.switchdev_port_attr_get	= dsa_slave_port_attr_get,
-	.switchdev_port_attr_set	= dsa_slave_port_attr_set,
-};
-
 static struct device_type dsa_type = {
 	.name	= "dsa",
 };
@@ -1321,7 +1316,6 @@ int dsa_slave_create(struct dsa_port *port)
 	eth_hw_addr_inherit(slave_dev, master);
 	slave_dev->priv_flags |= IFF_NO_QUEUE;
 	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
-	slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
 	slave_dev->min_mtu = 0;
 	slave_dev->max_mtu = ETH_MAX_MTU;
 	SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
-- 
2.19.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox