Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 5/5] net: dsa: Stop accessing ds->dst->cpu_dp in drivers
From: Florian Fainelli @ 2017-06-07 19:21 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, Andrew Lunn, John Crispin, David Miller
In-Reply-To: <87ink7oh1h.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

On 06/07/2017 10:15 AM, Vivien Didelot wrote:
> Hi Florian,
> 
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>>> So as I said in v2, now that a driver is guaranteed that dp->cpu_dp is
>>> correctly assigned at setup time, isn't better (especially for future
>>> multi-CPU support) to provide an helper which returns the CPU port for a
>>> given port? i.e. dsa_get_cpu_port(struct dsa_switch *ds, int port).
>>>
>>> Or is there something blocking? I might be wrong.
>>
>> mt7530.c needs access to the CPU port at ops->setup() time which is
>> why this is still here.
> 
> Yes, mt7530 is the only one doing this and has an hardcoded CPU port. So
> what I meant was, shouldn't we have this instead:
> 
>     struct dsa_port *dsa_get_cpu_port(struct dsa_switch *ds, int port)
>     {
>         return ds->ports[port].cpu_dp;
>     }

We don't actually have a CPU port point to itself:

+
+		for (i = 0; i < ds->num_ports; i++) {
+			p = &ds->ports[i];
+			if (!dsa_port_is_valid(p) ||
+			    i == index) <=============
+				continue;
+
+			p->cpu_dp = port;
+		}
 	}

> 
> And:
> 
> -       dn = ds->dst->cpu_dp->netdev->dev.of_node->parent;
> +       cpu_dp = dsa_get_cpu_port(ds, MT7530_CPU_PORT);
> +       dn = cpu_dp->netdev->dev.of_node->parent;

If we are giving the port number to get its cpu_dp pointer back, that
seems a bit pointless.

I still think the helper with fls(ds->cpu_port_mask) - 1 is better in
that it will return what you have configured from Device Tree/platform
data. MT7530 does allow the CPU port being arbitrary, and it would
disable MTK tags in that case.

Thanks!
-- 
Florian

^ permalink raw reply

* Re: [PATCH 1/9] net: mvmdio: reorder headers alphabetically
From: Florian Fainelli @ 2017-06-07 19:24 UTC (permalink / raw)
  To: Antoine Tenart, davem, jason, andrew, gregory.clement,
	sebastian.hesselbarth
  Cc: thomas.petazzoni, mw, linux, netdev, linux-arm-kernel
In-Reply-To: <20170607083810.30922-2-antoine.tenart@free-electrons.com>

On 06/07/2017 01:38 AM, Antoine Tenart wrote:
> Cosmetic fix reordering headers alphabetically.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>

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

^ permalink raw reply

* Re: [PATCH 2/9] net: mvmdio: use tabs for defines
From: Florian Fainelli @ 2017-06-07 19:24 UTC (permalink / raw)
  To: Antoine Tenart, davem, jason, andrew, gregory.clement,
	sebastian.hesselbarth
  Cc: thomas.petazzoni, mw, linux, netdev, linux-arm-kernel
In-Reply-To: <20170607083810.30922-3-antoine.tenart@free-electrons.com>

On 06/07/2017 01:38 AM, Antoine Tenart wrote:
> Cosmetic patch replacing spaces by tabs for defined values.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>

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

^ permalink raw reply

* Re: [PATCH 3/9] net: mvmdio: use GENMASK for masks
From: Florian Fainelli @ 2017-06-07 19:24 UTC (permalink / raw)
  To: Antoine Tenart, davem, jason, andrew, gregory.clement,
	sebastian.hesselbarth
  Cc: thomas.petazzoni, mw, linux, netdev, linux-arm-kernel
In-Reply-To: <20170607083810.30922-4-antoine.tenart@free-electrons.com>

On 06/07/2017 01:38 AM, Antoine Tenart wrote:
> Cosmetic patch to use the GENMASK helper for masks.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>

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

^ permalink raw reply

* Re: [PATCH 6/9] net: mvmdio: put the poll intervals in the private structure
From: Florian Fainelli @ 2017-06-07 19:25 UTC (permalink / raw)
  To: Antoine Tenart, davem, jason, andrew, gregory.clement,
	sebastian.hesselbarth
  Cc: thomas.petazzoni, mw, linux, netdev, linux-arm-kernel
In-Reply-To: <20170607083810.30922-7-antoine.tenart@free-electrons.com>

On 06/07/2017 01:38 AM, Antoine Tenart wrote:
> Put the two poll intervals (min and max) in the driver's private
> structure. This is needed to add the xmdio support later.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>

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

^ permalink raw reply

* Re: [net-next] vxlan: use a more suitable function when assigning NULL
From: David Miller @ 2017-06-07 19:26 UTC (permalink / raw)
  To: markb; +Cc: jbenc, stephen, netdev
In-Reply-To: <1496835418-22723-1-git-send-email-markb@mellanox.com>

From: Mark Bloch <markb@mellanox.com>
Date: Wed,  7 Jun 2017 14:36:58 +0300

> When stopping the vxlan interface we detach it from the socket.
> Use RCU_INIT_POINTER() and not rcu_assign_pointer() to do so.
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Mark Bloch <markb@mellanox.com>

Applied, thanks Mark.

^ permalink raw reply

* Re: [PATCH net] bpf, arm64: use separate register for state in stxr
From: David Miller @ 2017-06-07 19:27 UTC (permalink / raw)
  To: daniel; +Cc: will.deacon, ast, zlim.lnx, netdev, linux-arm-kernel
In-Reply-To: <530443d87bd51b3682a1b60ea19b2030d7d21409.1496834547.git.daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed,  7 Jun 2017 13:45:37 +0200

> Will reported that in BPF_XADD we must use a different register in stxr
> instruction for the status flag due to otherwise CONSTRAINED UNPREDICTABLE
> behavior per architecture. Reference manual says [1]:
> 
>   If s == t, then one of the following behaviors must occur:
> 
>    * The instruction is UNDEFINED.
>    * The instruction executes as a NOP.
>    * The instruction performs the store to the specified address, but
>      the value stored is UNKNOWN.
> 
> Thus, use a different temporary register for the status flag to fix it.
> 
> Disassembly extract from test 226/STX_XADD_DW from test_bpf.ko:
> 
>   [...]
>   0000003c:  c85f7d4b  ldxr x11, [x10]
>   00000040:  8b07016b  add x11, x11, x7
>   00000044:  c80c7d4b  stxr w12, x11, [x10]
>   00000048:  35ffffac  cbnz w12, 0x0000003c
>   [...]
> 
>   [1] https://static.docs.arm.com/ddi0487/b/DDI0487B_a_armv8_arm.pdf, p.6132
> 
> Fixes: 85f68fe89832 ("bpf, arm64: implement jiting of BPF_XADD")
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Applied, thanks Daniel.

^ permalink raw reply

* Re: [PATCH 5/9] net: mvmdio: introduce an ops structure
From: Florian Fainelli @ 2017-06-07 19:28 UTC (permalink / raw)
  To: Antoine Tenart, davem, jason, andrew, gregory.clement,
	sebastian.hesselbarth
  Cc: thomas.petazzoni, mw, linux, netdev, linux-arm-kernel
In-Reply-To: <20170607083810.30922-6-antoine.tenart@free-electrons.com>

On 06/07/2017 01:38 AM, Antoine Tenart wrote:
> Introduce an ops structure to add an indirection on functions accessing
> the registers. This is needed to add the xMDIO support later.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/mvmdio.c | 65 +++++++++++++++++++++++++++--------
>  1 file changed, 51 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
> index 56bbe3990590..8a71ef93a61b 100644
> --- a/drivers/net/ethernet/marvell/mvmdio.c
> +++ b/drivers/net/ethernet/marvell/mvmdio.c
> @@ -62,6 +62,15 @@ struct orion_mdio_dev {
>  	 */
>  	int err_interrupt;
>  	wait_queue_head_t smi_busy_wait;
> +	struct orion_mdio_ops *ops;
> +};
> +
> +struct orion_mdio_ops {
> +	int (*is_done)(struct orion_mdio_dev *);
> +	int (*is_read_valid)(struct orion_mdio_dev *);
> +	void (*start_read)(struct orion_mdio_dev *, int, int);
> +	u16 (*read)(struct orion_mdio_dev *);
> +	void (*write)(struct orion_mdio_dev *, int, int, u16);
>  };
>  
>  static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
> @@ -74,6 +83,30 @@ static int orion_mdio_smi_is_read_valid(struct orion_mdio_dev *dev)
>  	return !(readl(dev->regs) & MVMDIO_SMI_READ_VALID);
>  }
>  
> +static void orion_mdio_start_read_op(struct orion_mdio_dev *dev, int mii_id,
> +				     int regnum)
> +{
> +	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
> +		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
> +		MVMDIO_SMI_READ_OPERATION),
> +	       dev->regs);
> +}
> +
> +static u16 orion_mdio_read_op(struct orion_mdio_dev *dev)
> +{
> +	return readl(dev->regs) & GENMASK(15,0);
> +}
> +
> +static void orion_mdio_write_op(struct orion_mdio_dev *dev, int mii_id,
> +				int regnum, u16 value)
> +{
> +	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
> +		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
> +		MVMDIO_SMI_WRITE_OPERATION            |
> +		(value << MVMDIO_SMI_DATA_SHIFT)),
> +	       dev->regs);
> +}
> +
>  /* Wait for the SMI unit to be ready for another operation
>   */
>  static int orion_mdio_wait_ready(struct mii_bus *bus)
> @@ -84,7 +117,7 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
>  	int timedout = 0;
>  
>  	while (1) {
> -	        if (orion_mdio_smi_is_done(dev))
> +	        if (dev->ops->is_done(dev))

Nit: you could actually keep this function (and all of them that have a
corresponding dev->ops function pointer) and just make it a static
inline that calls into dev->ops->is_done()

That would limit the delta to review, and it would still be within the
namespace of the driver (orion_mdio_*).

Your call.

>  			return 0;
>  	        else if (timedout)
>  			break;
> @@ -103,8 +136,7 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
>  			if (timeout < 2)
>  				timeout = 2;
>  			wait_event_timeout(dev->smi_busy_wait,
> -				           orion_mdio_smi_is_done(dev),
> -				           timeout);
> +				           dev->ops->is_done(dev), timeout);
>  
>  			++timedout;
>  	        }
> @@ -126,22 +158,19 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id,
>  	if (ret < 0)
>  		goto out;
>  
> -	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
> -		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
> -		MVMDIO_SMI_READ_OPERATION),
> -	       dev->regs);
> +	dev->ops->start_read(dev, mii_id, regnum);
>  
>  	ret = orion_mdio_wait_ready(bus);
>  	if (ret < 0)
>  		goto out;
>  
> -	if (orion_mdio_smi_is_read_valid(dev)) {
> +	if (dev->ops->is_read_valid(dev)) {
>  		dev_err(bus->parent, "SMI bus read not valid\n");
>  		ret = -ENODEV;
>  		goto out;
>  	}
>  
> -	ret = readl(dev->regs) & GENMASK(15,0);
> +	ret = dev->ops->read(dev);
>  out:
>  	mutex_unlock(&dev->lock);
>  	return ret;
> @@ -159,11 +188,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
>  	if (ret < 0)
>  		goto out;
>  
> -	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
> -		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
> -		MVMDIO_SMI_WRITE_OPERATION            |
> -		(value << MVMDIO_SMI_DATA_SHIFT)),
> -	       dev->regs);
> +	dev->ops->write(dev, mii_id, regnum, value);
>  
>  out:
>  	mutex_unlock(&dev->lock);
> @@ -190,6 +215,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  	struct resource *r;
>  	struct mii_bus *bus;
>  	struct orion_mdio_dev *dev;
> +	struct orion_mdio_ops *ops;
>  	int i, ret;
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -249,6 +275,17 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  
>  	mutex_init(&dev->lock);
>  
> +	ops = devm_kzalloc(&pdev->dev, sizeof(*ops), GFP_KERNEL);
> +	if (!ops)
> +		return -ENOMEM;
> +
> +	ops->is_done = orion_mdio_smi_is_done;
> +	ops->is_read_valid = orion_mdio_smi_is_read_valid;
> +	ops->start_read = orion_mdio_start_read_op;
> +	ops->read = orion_mdio_read_op;
> +	ops->write = orion_mdio_write_op;
> +	dev->ops = ops;
> +
>  	if (pdev->dev.of_node)
>  		ret = of_mdiobus_register(bus, pdev->dev.of_node);
>  	else
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next v2 5/5] net: dsa: Stop accessing ds->dst->cpu_dp in drivers
From: Vivien Didelot @ 2017-06-07 19:27 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, Andrew Lunn, John Crispin, David Miller
In-Reply-To: <c8ecc6c3-2780-cf63-24f6-b546e837430e@gmail.com>

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 06/07/2017 10:15 AM, Vivien Didelot wrote:
>> Hi Florian,
>> 
>> Florian Fainelli <f.fainelli@gmail.com> writes:
>> 
>>>> So as I said in v2, now that a driver is guaranteed that dp->cpu_dp is
>>>> correctly assigned at setup time, isn't better (especially for future
>>>> multi-CPU support) to provide an helper which returns the CPU port for a
>>>> given port? i.e. dsa_get_cpu_port(struct dsa_switch *ds, int port).
>>>>
>>>> Or is there something blocking? I might be wrong.
>>>
>>> mt7530.c needs access to the CPU port at ops->setup() time which is
>>> why this is still here.
>> 
>> Yes, mt7530 is the only one doing this and has an hardcoded CPU port. So
>> what I meant was, shouldn't we have this instead:
>> 
>>     struct dsa_port *dsa_get_cpu_port(struct dsa_switch *ds, int port)
>>     {
>>         return ds->ports[port].cpu_dp;
>>     }
>
> We don't actually have a CPU port point to itself:
>
> +
> +		for (i = 0; i < ds->num_ports; i++) {
> +			p = &ds->ports[i];
> +			if (!dsa_port_is_valid(p) ||
> +			    i == index) <=============
> +				continue;
> +
> +			p->cpu_dp = port;
> +		}
>  	}
>
>> 
>> And:
>> 
>> -       dn = ds->dst->cpu_dp->netdev->dev.of_node->parent;
>> +       cpu_dp = dsa_get_cpu_port(ds, MT7530_CPU_PORT);
>> +       dn = cpu_dp->netdev->dev.of_node->parent;
>
> If we are giving the port number to get its cpu_dp pointer back, that
> seems a bit pointless.
>
> I still think the helper with fls(ds->cpu_port_mask) - 1 is better in
> that it will return what you have configured from Device Tree/platform
> data. MT7530 does allow the CPU port being arbitrary, and it would
> disable MTK tags in that case.

OK looks good then!

^ permalink raw reply

* Re: [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add
From: Florian Fainelli @ 2017-06-07 19:31 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170606205631.22880-2-vivien.didelot@savoirfairelinux.com>

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> Define the target port membership of the VLAN entry in
> mv88e6xxx_port_vlan_add where ds is scoped.
> 
> Allow the DSA core to call later the port_vlan_add operation for CPU or
> DSA ports, by using the Unmodified membership for these ports, as in the
> current behavior.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: mv88e6xxx: Have 6161/6123 use EDSA tags
From: David Miller @ 2017-06-07 19:32 UTC (permalink / raw)
  To: andrew; +Cc: netdev, cphealy, vivien.didelot, nikita.yoush
In-Reply-To: <1496840779-4260-1-git-send-email-andrew@lunn.ch>

From: Andrew Lunn <andrew@lunn.ch>
Date: Wed,  7 Jun 2017 15:06:19 +0200

> The mv88e6161 and mv88e6123 are capable of using EDSA tags when
> passing frames from the host to the switch and back.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 0/3] mlx4 drivers: version update
From: David Miller @ 2017-06-07 19:33 UTC (permalink / raw)
  To: tariqt; +Cc: netdev, linux-rdma, eranbe, leonro
In-Reply-To: <1496841975-7728-1-git-send-email-tariqt@mellanox.com>

From: Tariq Toukan <tariqt@mellanox.com>
Date: Wed,  7 Jun 2017 16:26:12 +0300

> This patchset contains version updates for the MLX4 drivers:
> Core, EN, and IB.
> 
> Just like we've done in mlx5, we modify the outdated driver
> version (reported in ethtool for example).
> This better reflects the current driver state, and removes the
> redundant date string.
> We are not going to change this frequently or even use it.
> 
> I include the IB patch in this series as it has similar subject
> and content.
> It does not cause any kind of conflict with Doug's tree.
> The rdma mailing list is CCed.
> Please let me know if I need to submit this differently.

Ok, series applied.

Doug, let me know if you want to handle this differently or want
me to change something.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 4/5] net: dsa: mv88e6xxx: exclude all ports in new VLAN
From: Florian Fainelli @ 2017-06-07 19:34 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170606205631.22880-5-vivien.didelot@savoirfairelinux.com>

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> Now that the DSA core adds the CPU and DSA ports itself to the new VLAN
> entry, there is no need to include them as members of this VLAN when
> initializing a new VTU entry.
> 
> As of now, initialize a new VTU entry with all ports excluded.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

^ permalink raw reply

* Re: [PATCH 1/4] net: qrtr: Refactor packet allocation
From: David Miller @ 2017-06-07 19:35 UTC (permalink / raw)
  To: bjorn.andersson; +Cc: aneela, netdev, linux-arm-msm, linux-kernel, ccavin
In-Reply-To: <20170607185501.6000-1-bjorn.andersson@linaro.org>


A proper patch series must have a "[PATCH 0/N] ..." header posting
which explains what the patch series is doing at a high level, how
it is doing it, and why it is doing it that way.

^ permalink raw reply

* Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry
From: Florian Fainelli @ 2017-06-07 19:37 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170606205631.22880-6-vivien.didelot@savoirfairelinux.com>

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> The mv88e6xxx driver currently tries to be smart and remove by itself a
> VLAN entry from the VTU when the driven switch sees no user ports as
> members of the VLAN.
> 
> This is bad in a multi-chip switch fabric, since a chip in between
> others may have no bridge port members, but still needs to be aware of
> the VID in order to correctly pass frames in the data path.
> 
> Remove the code purging a VTU entry when updating a port membership.

In that case the switch sitting between two other chips and passing
traffic would still have at least two of its DSA ports be part of a VTU
entry, right?

So could not we just do ....

> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 522f023bb17e..64c0f88f9e79 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1325,9 +1325,8 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
>  static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>  				    int port, u16 vid)
>  {
> -	struct dsa_switch *ds = chip->ds;
>  	struct mv88e6xxx_vtu_entry vlan;
> -	int i, err;
> +	int err;
>  
>  	err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
>  	if (err)
> @@ -1339,18 +1338,6 @@ static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>  
>  	vlan.member[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
>  
> -	/* keep the VLAN unless all ports are excluded */
> -	vlan.valid = false;
> -	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
> -		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
> -			continue;

... break the loop here?

> -
> -		if (vlan.member[i] != GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER) {
> -			vlan.valid = true;
> -			break;
> -		}
> -	}
> -
>  	err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
>  	if (err)
>  		return err;
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members
From: Florian Fainelli @ 2017-06-07 19:38 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170606205631.22880-4-vivien.didelot@savoirfairelinux.com>

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> In a multi-chip switch fabric, it is currently the responsibility of the
> driver to add the CPU or DSA (interconnecting chips together) ports as
> members of a new VLAN entry. This makes the drivers more complicated.
> 
> We want the DSA drivers to be stupid and the DSA core being the one
> responsible for caring about the abstracted switch logic and topology.
> 
> Make the DSA core program the CPU and DSA ports as part of the VLAN.
> 
> This makes all chips of the data path to be aware of VIDs spanning the
> the whole fabric and thus, seamlessly add support for cross-chip VLAN.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

^ permalink raw reply

* Re: [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch
From: Florian Fainelli @ 2017-06-07 19:39 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170606205631.22880-3-vivien.didelot@savoirfairelinux.com>

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> Now that the VLAN object is propagated to every switch chip of the
> switch fabric, we can easily ensure that they all support the required
> VLAN operations before modifying an entry on a single switch.
> 
> To achieve that, remove the condition skipping other target switches,
> and add a bitmap of VLAN members, eventually containing the target port,
> if we are programming the switch target.

You could add in the commit message that with this commit, there is not
actually a functional change yet because we have one (and only one) bit
set in the members bitmap.

> 
> This will allow us to easily add other VLAN members, such as the DSA or
> CPU ports (to introduce cross-chip VLAN support) or the other port
> members if we want to reduce hardware accesses later.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

^ permalink raw reply

* [PATCH] net: Fix inconsistent teardown and release of private netdev state.
From: David Miller @ 2017-06-07 19:54 UTC (permalink / raw)
  To: netdev


Network devices can allocate reasources and private memory using
netdev_ops->ndo_init().  However, the release of these resources
can occur in one of two different places.

Either netdev_ops->ndo_uninit() or netdev->destructor().

The decision of which operation frees the resources depends upon
whether it is necessary for all netdev refs to be released before it
is safe to perform the freeing.

netdev_ops->ndo_uninit() presumably can occur right after the
NETDEV_UNREGISTER notifier completes and the unicast and multicast
address lists are flushed.

netdev->destructor(), on the other hand, does not run until the
netdev references all go away.

Further complicating the situation is that netdev->destructor()
almost universally does also a free_netdev().

This creates a problem for the logic in register_netdevice().
Because all callers of register_netdevice() manage the freeing
of the netdev, and invoke free_netdev(dev) if register_netdevice()
fails.

If netdev_ops->ndo_init() succeeds, but something else fails inside
of register_netdevice(), it does call ndo_ops->ndo_uninit().  But
it is not able to invoke netdev->destructor().

This is because netdev->destructor() will do a free_netdev() and
then the caller of register_netdevice() will do the same.

However, this means that the resources that would normally be released
by netdev->destructor() will not be.

Over the years drivers have added local hacks to deal with this, by
invoking their destructor parts by hand when register_netdevice()
fails.

Many drivers do not try to deal with this, and instead we have leaks.

Let's close this hole by formalizing the distinction between what
private things need to be freed up by netdev->destructor() and whether
the driver needs unregister_netdevice() to perform the free_netdev().

netdev->priv_destructor() performs all actions to free up the private
resources that used to be freed by netdev->destructor(), except for
free_netdev().

netdev->needs_free_netdev is a boolean that indicates whether
free_netdev() should be done at the end of unregister_netdevice().

Now, register_netdevice() can sanely release all resources after
ndo_ops->ndo_init() succeeds, by invoking both ndo_ops->ndo_uninit()
and netdev->priv_destructor().

And at the end of unregister_netdevice(), we invoke
netdev->priv_destructor() and optionally call free_netdev().

Signed-off-by: David S. Miller <davem@davemloft.net>
---

This is from a few weeks ago, pushed to 'net' and queued up for
-stable.

 drivers/net/bonding/bond_main.c                             | 6 +++---
 drivers/net/caif/caif_hsi.c                                 | 2 +-
 drivers/net/caif/caif_serial.c                              | 2 +-
 drivers/net/caif/caif_spi.c                                 | 2 +-
 drivers/net/caif/caif_virtio.c                              | 2 +-
 drivers/net/can/slcan.c                                     | 7 +++----
 drivers/net/can/vcan.c                                      | 2 +-
 drivers/net/can/vxcan.c                                     | 2 +-
 drivers/net/dummy.c                                         | 4 ++--
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c             | 2 +-
 drivers/net/geneve.c                                        | 2 +-
 drivers/net/gtp.c                                           | 2 +-
 drivers/net/hamradio/6pack.c                                | 2 +-
 drivers/net/hamradio/bpqether.c                             | 2 +-
 drivers/net/ifb.c                                           | 4 ++--
 drivers/net/ipvlan/ipvlan_main.c                            | 2 +-
 drivers/net/loopback.c                                      | 4 ++--
 drivers/net/macsec.c                                        | 4 ++--
 drivers/net/macvlan.c                                       | 2 +-
 drivers/net/nlmon.c                                         | 2 +-
 drivers/net/slip/slip.c                                     | 7 +++----
 drivers/net/team/team.c                                     | 4 ++--
 drivers/net/tun.c                                           | 4 ++--
 drivers/net/usb/cdc-phonet.c                                | 2 +-
 drivers/net/usb/qmi_wwan.c                                  | 2 +-
 drivers/net/veth.c                                          | 4 ++--
 drivers/net/vrf.c                                           | 2 +-
 drivers/net/vsockmon.c                                      | 2 +-
 drivers/net/vxlan.c                                         | 2 +-
 drivers/net/wan/dlci.c                                      | 2 +-
 drivers/net/wan/hdlc_fr.c                                   | 2 +-
 drivers/net/wan/lapbether.c                                 | 2 +-
 drivers/net/wireless/ath/ath6kl/main.c                      | 2 +-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 1 -
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c     | 3 ++-
 drivers/net/wireless/intersil/hostap/hostap_main.c          | 2 +-
 drivers/net/wireless/mac80211_hwsim.c                       | 2 +-
 drivers/net/wireless/marvell/mwifiex/main.c                 | 2 +-
 drivers/staging/rtl8188eu/os_dep/mon.c                      | 2 +-
 drivers/usb/gadget/function/f_phonet.c                      | 2 +-
 include/linux/netdevice.h                                   | 7 ++++---
 net/8021q/vlan_dev.c                                        | 4 ++--
 net/batman-adv/soft-interface.c                             | 5 ++---
 net/bluetooth/6lowpan.c                                     | 2 +-
 net/bridge/br_device.c                                      | 2 +-
 net/caif/chnl_net.c                                         | 4 ++--
 net/core/dev.c                                              | 8 ++++++--
 net/hsr/hsr_device.c                                        | 4 ++--
 net/ieee802154/6lowpan/core.c                               | 2 +-
 net/ipv4/ip_tunnel.c                                        | 4 ++--
 net/ipv4/ipmr.c                                             | 2 +-
 net/ipv6/ip6_gre.c                                          | 9 +++++----
 net/ipv6/ip6_tunnel.c                                       | 8 ++++----
 net/ipv6/ip6_vti.c                                          | 8 ++++----
 net/ipv6/ip6mr.c                                            | 2 +-
 net/ipv6/sit.c                                              | 6 +++---
 net/irda/irlan/irlan_eth.c                                  | 2 +-
 net/l2tp/l2tp_eth.c                                         | 2 +-
 net/mac80211/iface.c                                        | 6 +++---
 net/mac802154/iface.c                                       | 7 +++----
 net/openvswitch/vport-internal_dev.c                        | 4 ++--
 net/phonet/pep-gprs.c                                       | 2 +-
 62 files changed, 105 insertions(+), 103 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2be7880..d507b68 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4185,7 +4185,6 @@ static void bond_destructor(struct net_device *bond_dev)
 	struct bonding *bond = netdev_priv(bond_dev);
 	if (bond->wq)
 		destroy_workqueue(bond->wq);
-	free_netdev(bond_dev);
 }
 
 void bond_setup(struct net_device *bond_dev)
@@ -4205,7 +4204,8 @@ void bond_setup(struct net_device *bond_dev)
 	bond_dev->netdev_ops = &bond_netdev_ops;
 	bond_dev->ethtool_ops = &bond_ethtool_ops;
 
-	bond_dev->destructor = bond_destructor;
+	bond_dev->needs_free_netdev = true;
+	bond_dev->priv_destructor = bond_destructor;
 
 	SET_NETDEV_DEVTYPE(bond_dev, &bond_type);
 
@@ -4730,7 +4730,7 @@ int bond_create(struct net *net, const char *name)
 
 	rtnl_unlock();
 	if (res < 0)
-		bond_destructor(bond_dev);
+		free_netdev(bond_dev);
 	return res;
 }
 
diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index ddabce7..71a7c3b 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -1121,7 +1121,7 @@ static void cfhsi_setup(struct net_device *dev)
 	dev->flags = IFF_POINTOPOINT | IFF_NOARP;
 	dev->mtu = CFHSI_MAX_CAIF_FRAME_SZ;
 	dev->priv_flags |= IFF_NO_QUEUE;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	dev->netdev_ops = &cfhsi_netdevops;
 	for (i = 0; i < CFHSI_PRIO_LAST; ++i)
 		skb_queue_head_init(&cfhsi->qhead[i]);
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index c2dea49..76e1d35 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -428,7 +428,7 @@ static void caifdev_setup(struct net_device *dev)
 	dev->flags = IFF_POINTOPOINT | IFF_NOARP;
 	dev->mtu = CAIF_MAX_MTU;
 	dev->priv_flags |= IFF_NO_QUEUE;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	skb_queue_head_init(&serdev->head);
 	serdev->common.link_select = CAIF_LINK_LOW_LATENCY;
 	serdev->common.use_frag = true;
diff --git a/drivers/net/caif/caif_spi.c b/drivers/net/caif/caif_spi.c
index 3a529fb..fc21afe 100644
--- a/drivers/net/caif/caif_spi.c
+++ b/drivers/net/caif/caif_spi.c
@@ -712,7 +712,7 @@ static void cfspi_setup(struct net_device *dev)
 	dev->flags = IFF_NOARP | IFF_POINTOPOINT;
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->mtu = SPI_MAX_PAYLOAD_SIZE;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	skb_queue_head_init(&cfspi->qhead);
 	skb_queue_head_init(&cfspi->chead);
 	cfspi->cfdev.link_select = CAIF_LINK_HIGH_BANDW;
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index bc0eb47..8bffd25 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -617,7 +617,7 @@ static void cfv_netdev_setup(struct net_device *netdev)
 	netdev->tx_queue_len = 100;
 	netdev->flags = IFF_POINTOPOINT | IFF_NOARP;
 	netdev->mtu = CFV_DEF_MTU_SIZE;
-	netdev->destructor = free_netdev;
+	netdev->needs_free_netdev = true;
 }
 
 /* Create debugfs counters for the device */
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index eb71737..6a6e896 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -417,7 +417,7 @@ static int slc_open(struct net_device *dev)
 static void slc_free_netdev(struct net_device *dev)
 {
 	int i = dev->base_addr;
-	free_netdev(dev);
+
 	slcan_devs[i] = NULL;
 }
 
@@ -436,7 +436,8 @@ static const struct net_device_ops slc_netdev_ops = {
 static void slc_setup(struct net_device *dev)
 {
 	dev->netdev_ops		= &slc_netdev_ops;
-	dev->destructor		= slc_free_netdev;
+	dev->needs_free_netdev	= true;
+	dev->priv_destructor	= slc_free_netdev;
 
 	dev->hard_header_len	= 0;
 	dev->addr_len		= 0;
@@ -761,8 +762,6 @@ static void __exit slcan_exit(void)
 		if (sl->tty) {
 			printk(KERN_ERR "%s: tty discipline still running\n",
 			       dev->name);
-			/* Intentionally leak the control block. */
-			dev->destructor = NULL;
 		}
 
 		unregister_netdev(dev);
diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
index facca33..0eda1b3 100644
--- a/drivers/net/can/vcan.c
+++ b/drivers/net/can/vcan.c
@@ -163,7 +163,7 @@ static void vcan_setup(struct net_device *dev)
 		dev->flags |= IFF_ECHO;
 
 	dev->netdev_ops		= &vcan_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 }
 
 static struct rtnl_link_ops vcan_link_ops __read_mostly = {
diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
index 7fbb247..30cf236 100644
--- a/drivers/net/can/vxcan.c
+++ b/drivers/net/can/vxcan.c
@@ -156,7 +156,7 @@ static void vxcan_setup(struct net_device *dev)
 	dev->tx_queue_len	= 0;
 	dev->flags		= (IFF_NOARP|IFF_ECHO);
 	dev->netdev_ops		= &vxcan_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 }
 
 /* forward declaration for rtnl_create_link() */
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 149244a..9905b52 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -328,7 +328,6 @@ static void dummy_free_netdev(struct net_device *dev)
 	struct dummy_priv *priv = netdev_priv(dev);
 
 	kfree(priv->vfinfo);
-	free_netdev(dev);
 }
 
 static void dummy_setup(struct net_device *dev)
@@ -338,7 +337,8 @@ static void dummy_setup(struct net_device *dev)
 	/* Initialize the device structure. */
 	dev->netdev_ops = &dummy_netdev_ops;
 	dev->ethtool_ops = &dummy_ethtool_ops;
-	dev->destructor = dummy_free_netdev;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = dummy_free_netdev;
 
 	/* Fill in device structure with ethernet-generic values. */
 	dev->flags |= IFF_NOARP;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index c12c4a3..2649a06e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -4537,7 +4537,7 @@ static void dummy_setup(struct net_device *dev)
 	/* Initialize the device structure. */
 	dev->netdev_ops = &cxgb4_mgmt_netdev_ops;
 	dev->ethtool_ops = &cxgb4_mgmt_ethtool_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 }
 
 static int config_mgmt_dev(struct pci_dev *pdev)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index dec5d56..4d44c02 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1007,7 +1007,7 @@ static void geneve_setup(struct net_device *dev)
 
 	dev->netdev_ops = &geneve_netdev_ops;
 	dev->ethtool_ops = &geneve_ethtool_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 
 	SET_NETDEV_DEVTYPE(dev, &geneve_type);
 
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 4fea1b3..abb82ff 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -611,7 +611,7 @@ static const struct net_device_ops gtp_netdev_ops = {
 static void gtp_link_setup(struct net_device *dev)
 {
 	dev->netdev_ops		= &gtp_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 
 	dev->hard_header_len = 0;
 	dev->addr_len = 0;
diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 922bf44..021a8ec 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -311,7 +311,7 @@ static void sp_setup(struct net_device *dev)
 {
 	/* Finish setting up the DEVICE info. */
 	dev->netdev_ops		= &sp_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 	dev->mtu		= SIXP_MTU;
 	dev->hard_header_len	= AX25_MAX_HEADER_LEN;
 	dev->header_ops 	= &ax25_header_ops;
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index f62e7f3..78a6414 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -476,7 +476,7 @@ static const struct net_device_ops bpq_netdev_ops = {
 static void bpq_setup(struct net_device *dev)
 {
 	dev->netdev_ops	     = &bpq_netdev_ops;
-	dev->destructor	     = free_netdev;
+	dev->needs_free_netdev = true;
 
 	memcpy(dev->broadcast, &ax25_bcast, AX25_ADDR_LEN);
 	memcpy(dev->dev_addr,  &ax25_defaddr, AX25_ADDR_LEN);
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 312fce7..144ea5a 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -207,7 +207,6 @@ static void ifb_dev_free(struct net_device *dev)
 		__skb_queue_purge(&txp->tq);
 	}
 	kfree(dp->tx_private);
-	free_netdev(dev);
 }
 
 static void ifb_setup(struct net_device *dev)
@@ -230,7 +229,8 @@ static void ifb_setup(struct net_device *dev)
 	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	netif_keep_dst(dev);
 	eth_hw_addr_random(dev);
-	dev->destructor = ifb_dev_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = ifb_dev_free;
 }
 
 static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 618ed88..7c7680c 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -632,7 +632,7 @@ void ipvlan_link_setup(struct net_device *dev)
 	dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
 	dev->priv_flags |= IFF_UNICAST_FLT | IFF_NO_QUEUE;
 	dev->netdev_ops = &ipvlan_netdev_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	dev->header_ops = &ipvlan_header_ops;
 	dev->ethtool_ops = &ipvlan_ethtool_ops;
 }
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 224f65c..3061249 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -159,7 +159,6 @@ static void loopback_dev_free(struct net_device *dev)
 {
 	dev_net(dev)->loopback_dev = NULL;
 	free_percpu(dev->lstats);
-	free_netdev(dev);
 }
 
 static const struct net_device_ops loopback_ops = {
@@ -196,7 +195,8 @@ static void loopback_setup(struct net_device *dev)
 	dev->ethtool_ops	= &loopback_ethtool_ops;
 	dev->header_ops		= &eth_header_ops;
 	dev->netdev_ops		= &loopback_ops;
-	dev->destructor		= loopback_dev_free;
+	dev->needs_free_netdev	= true;
+	dev->priv_destructor	= loopback_dev_free;
 }
 
 /* Setup and register the loopback device. */
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index cdc347b..7941167 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2996,7 +2996,6 @@ static void macsec_free_netdev(struct net_device *dev)
 	free_percpu(macsec->secy.tx_sc.stats);
 
 	dev_put(real_dev);
-	free_netdev(dev);
 }
 
 static void macsec_setup(struct net_device *dev)
@@ -3006,7 +3005,8 @@ static void macsec_setup(struct net_device *dev)
 	dev->max_mtu = ETH_MAX_MTU;
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->netdev_ops = &macsec_netdev_ops;
-	dev->destructor = macsec_free_netdev;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = macsec_free_netdev;
 	SET_NETDEV_DEVTYPE(dev, &macsec_type);
 
 	eth_zero_addr(dev->broadcast);
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index b34eaaa..b8cec52 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1089,7 +1089,7 @@ void macvlan_common_setup(struct net_device *dev)
 	netif_keep_dst(dev);
 	dev->priv_flags	       |= IFF_UNICAST_FLT;
 	dev->netdev_ops		= &macvlan_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 	dev->header_ops		= &macvlan_hard_header_ops;
 	dev->ethtool_ops	= &macvlan_ethtool_ops;
 }
diff --git a/drivers/net/nlmon.c b/drivers/net/nlmon.c
index b916038..c4b3362 100644
--- a/drivers/net/nlmon.c
+++ b/drivers/net/nlmon.c
@@ -113,7 +113,7 @@ static void nlmon_setup(struct net_device *dev)
 
 	dev->netdev_ops	= &nlmon_ops;
 	dev->ethtool_ops = &nlmon_ethtool_ops;
-	dev->destructor	= free_netdev;
+	dev->needs_free_netdev = true;
 
 	dev->features = NETIF_F_SG | NETIF_F_FRAGLIST |
 			NETIF_F_HIGHDMA | NETIF_F_LLTX;
diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index 1da31dc..74b9072 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -629,7 +629,7 @@ static void sl_uninit(struct net_device *dev)
 static void sl_free_netdev(struct net_device *dev)
 {
 	int i = dev->base_addr;
-	free_netdev(dev);
+
 	slip_devs[i] = NULL;
 }
 
@@ -651,7 +651,8 @@ static const struct net_device_ops sl_netdev_ops = {
 static void sl_setup(struct net_device *dev)
 {
 	dev->netdev_ops		= &sl_netdev_ops;
-	dev->destructor		= sl_free_netdev;
+	dev->needs_free_netdev	= true;
+	dev->priv_destructor	= sl_free_netdev;
 
 	dev->hard_header_len	= 0;
 	dev->addr_len		= 0;
@@ -1369,8 +1370,6 @@ static void __exit slip_exit(void)
 		if (sl->tty) {
 			printk(KERN_ERR "%s: tty discipline still running\n",
 			       dev->name);
-			/* Intentionally leak the control block. */
-			dev->destructor = NULL;
 		}
 
 		unregister_netdev(dev);
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 6c5d5ef..fba8c13 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1643,7 +1643,6 @@ static void team_destructor(struct net_device *dev)
 	struct team *team = netdev_priv(dev);
 
 	free_percpu(team->pcpu_stats);
-	free_netdev(dev);
 }
 
 static int team_open(struct net_device *dev)
@@ -2079,7 +2078,8 @@ static void team_setup(struct net_device *dev)
 
 	dev->netdev_ops = &team_netdev_ops;
 	dev->ethtool_ops = &team_ethtool_ops;
-	dev->destructor	= team_destructor;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = team_destructor;
 	dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->priv_flags |= IFF_TEAM;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index bbd707b..9ee7d42 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1560,7 +1560,6 @@ static void tun_free_netdev(struct net_device *dev)
 	free_percpu(tun->pcpu_stats);
 	tun_flow_uninit(tun);
 	security_tun_dev_free_security(tun->security);
-	free_netdev(dev);
 }
 
 static void tun_setup(struct net_device *dev)
@@ -1571,7 +1570,8 @@ static void tun_setup(struct net_device *dev)
 	tun->group = INVALID_GID;
 
 	dev->ethtool_ops = &tun_ethtool_ops;
-	dev->destructor = tun_free_netdev;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = tun_free_netdev;
 	/* We prefer our own queue length */
 	dev->tx_queue_len = TUN_READQ_SIZE;
 }
diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
index eb52de8..c7a350b 100644
--- a/drivers/net/usb/cdc-phonet.c
+++ b/drivers/net/usb/cdc-phonet.c
@@ -298,7 +298,7 @@ static void usbpn_setup(struct net_device *dev)
 	dev->addr_len		= 1;
 	dev->tx_queue_len	= 3;
 
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 }
 
 /*
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index d716576..e76f988 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -123,7 +123,7 @@ static void qmimux_setup(struct net_device *dev)
 	dev->addr_len        = 0;
 	dev->flags           = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
 	dev->netdev_ops      = &qmimux_netdev_ops;
-	dev->destructor      = free_netdev;
+	dev->needs_free_netdev = true;
 }
 
 static struct net_device *qmimux_find_dev(struct usbnet *dev, u8 mux_id)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 38f0f03..0156fe8 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -222,7 +222,6 @@ static int veth_dev_init(struct net_device *dev)
 static void veth_dev_free(struct net_device *dev)
 {
 	free_percpu(dev->vstats);
-	free_netdev(dev);
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -317,7 +316,8 @@ static void veth_setup(struct net_device *dev)
 			       NETIF_F_HW_VLAN_STAG_TX |
 			       NETIF_F_HW_VLAN_CTAG_RX |
 			       NETIF_F_HW_VLAN_STAG_RX);
-	dev->destructor = veth_dev_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = veth_dev_free;
 	dev->max_mtu = ETH_MAX_MTU;
 
 	dev->hw_features = VETH_FEATURES;
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index ceda586..de3dcbb 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1347,7 +1347,7 @@ static void vrf_setup(struct net_device *dev)
 	dev->netdev_ops = &vrf_netdev_ops;
 	dev->l3mdev_ops = &vrf_l3mdev_ops;
 	dev->ethtool_ops = &vrf_ethtool_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 
 	/* Fill in device structure with ethernet-generic values. */
 	eth_hw_addr_random(dev);
diff --git a/drivers/net/vsockmon.c b/drivers/net/vsockmon.c
index 7f0136f..c28bdce 100644
--- a/drivers/net/vsockmon.c
+++ b/drivers/net/vsockmon.c
@@ -135,7 +135,7 @@ static void vsockmon_setup(struct net_device *dev)
 
 	dev->netdev_ops	= &vsockmon_ops;
 	dev->ethtool_ops = &vsockmon_ethtool_ops;
-	dev->destructor	= free_netdev;
+	dev->needs_free_netdev = true;
 
 	dev->features = NETIF_F_SG | NETIF_F_FRAGLIST |
 			NETIF_F_HIGHDMA | NETIF_F_LLTX;
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 328b471..927c6a9 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2584,7 +2584,7 @@ static void vxlan_setup(struct net_device *dev)
 	eth_hw_addr_random(dev);
 	ether_setup(dev);
 
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	SET_NETDEV_DEVTYPE(dev, &vxlan_type);
 
 	dev->features	|= NETIF_F_LLTX;
diff --git a/drivers/net/wan/dlci.c b/drivers/net/wan/dlci.c
index 65ee2a6..a0d76f7 100644
--- a/drivers/net/wan/dlci.c
+++ b/drivers/net/wan/dlci.c
@@ -475,7 +475,7 @@ static void dlci_setup(struct net_device *dev)
 	dev->flags		= 0;
 	dev->header_ops		= &dlci_header_ops;
 	dev->netdev_ops		= &dlci_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 
 	dlp->receive		= dlci_receive;
 
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index eb91528..78596e4 100644
--- a/drivers/net/wan/hdlc_fr.c
+++ b/drivers/net/wan/hdlc_fr.c
@@ -1106,7 +1106,7 @@ static int fr_add_pvc(struct net_device *frad, unsigned int dlci, int type)
 		return -EIO;
 	}
 
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	*get_dev_p(pvc, type) = dev;
 	if (!used) {
 		state(hdlc)->dce_changed = 1;
diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c
index 9df9ed6..63f7490 100644
--- a/drivers/net/wan/lapbether.c
+++ b/drivers/net/wan/lapbether.c
@@ -306,7 +306,7 @@ static const struct net_device_ops lapbeth_netdev_ops = {
 static void lapbeth_setup(struct net_device *dev)
 {
 	dev->netdev_ops	     = &lapbeth_netdev_ops;
-	dev->destructor	     = free_netdev;
+	dev->needs_free_netdev = true;
 	dev->type            = ARPHRD_X25;
 	dev->hard_header_len = 3;
 	dev->mtu             = 1000;
diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index 91ee542..b90c77e 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -1287,7 +1287,7 @@ void init_netdev(struct net_device *dev)
 	struct ath6kl *ar = ath6kl_priv(dev);
 
 	dev->netdev_ops = &ath6kl_netdev_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	dev->watchdog_timeo = ATH6KL_TX_TIMEOUT;
 
 	dev->needed_headroom = ETH_HLEN;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index cd1d673..617199c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5225,7 +5225,6 @@ void brcmf_cfg80211_free_netdev(struct net_device *ndev)
 
 	if (vif)
 		brcmf_free_vif(vif);
-	free_netdev(ndev);
 }
 
 static bool brcmf_is_linkup(const struct brcmf_event_msg *e)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index a3d8236..511d190 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -624,7 +624,8 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
 		if (!ndev)
 			return ERR_PTR(-ENOMEM);
 
-		ndev->destructor = brcmf_cfg80211_free_netdev;
+		ndev->needs_free_netdev = true;
+		ndev->priv_destructor = brcmf_cfg80211_free_netdev;
 		ifp = netdev_priv(ndev);
 		ifp->ndev = ndev;
 		/* store mapping ifidx to bsscfgidx */
diff --git a/drivers/net/wireless/intersil/hostap/hostap_main.c b/drivers/net/wireless/intersil/hostap/hostap_main.c
index 544fc09..1372b20 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_main.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_main.c
@@ -73,7 +73,7 @@ struct net_device * hostap_add_interface(struct local_info *local,
 	dev->mem_end = mdev->mem_end;
 
 	hostap_setup_dev(dev, local, type);
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 
 	sprintf(dev->name, "%s%s", prefix, name);
 	if (!rtnl_locked)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 87444af..a659104 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2855,7 +2855,7 @@ static const struct net_device_ops hwsim_netdev_ops = {
 static void hwsim_mon_setup(struct net_device *dev)
 {
 	dev->netdev_ops = &hwsim_netdev_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	ether_setup(dev);
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->type = ARPHRD_IEEE80211_RADIOTAP;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index dd87b9f..39b6b5e 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1280,7 +1280,7 @@ void mwifiex_init_priv_params(struct mwifiex_private *priv,
 			      struct net_device *dev)
 {
 	dev->netdev_ops = &mwifiex_netdev_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	/* Initialize private structure */
 	priv->current_key_index = 0;
 	priv->media_connected = false;
diff --git a/drivers/staging/rtl8188eu/os_dep/mon.c b/drivers/staging/rtl8188eu/os_dep/mon.c
index cfe37eb..859d0d6 100644
--- a/drivers/staging/rtl8188eu/os_dep/mon.c
+++ b/drivers/staging/rtl8188eu/os_dep/mon.c
@@ -152,7 +152,7 @@ static const struct net_device_ops mon_netdev_ops = {
 static void mon_setup(struct net_device *dev)
 {
 	dev->netdev_ops = &mon_netdev_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	ether_setup(dev);
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->type = ARPHRD_IEEE80211;
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index b4058f0..6a1ce6a 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -281,7 +281,7 @@ static void pn_net_setup(struct net_device *dev)
 	dev->tx_queue_len	= 1;
 
 	dev->netdev_ops		= &pn_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 	dev->header_ops		= &phonet_header_ops;
 }
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9c23bd2..2ead32a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1596,8 +1596,8 @@ enum netdev_priv_flags {
  *	@rtnl_link_state:	This enum represents the phases of creating
  *				a new link
  *
- *	@destructor:		Called from unregister,
- *				can be used to call free_netdev
+ *	@needs_free_netdev:	Should unregister perform free_netdev?
+ *	@priv_destructor:	Called from unregister
  *	@npinfo:		XXX: need comments on this one
  * 	@nd_net:		Network namespace this network device is inside
  *
@@ -1858,7 +1858,8 @@ struct net_device {
 		RTNL_LINK_INITIALIZING,
 	} rtnl_link_state:16;
 
-	void (*destructor)(struct net_device *dev);
+	bool needs_free_netdev;
+	void (*priv_destructor)(struct net_device *dev);
 
 #ifdef CONFIG_NETPOLL
 	struct netpoll_info __rcu	*npinfo;
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 9ee5787..33885dc 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -806,7 +806,6 @@ static void vlan_dev_free(struct net_device *dev)
 
 	free_percpu(vlan->vlan_pcpu_stats);
 	vlan->vlan_pcpu_stats = NULL;
-	free_netdev(dev);
 }
 
 void vlan_setup(struct net_device *dev)
@@ -819,7 +818,8 @@ void vlan_setup(struct net_device *dev)
 	netif_keep_dst(dev);
 
 	dev->netdev_ops		= &vlan_netdev_ops;
-	dev->destructor		= vlan_dev_free;
+	dev->needs_free_netdev	= true;
+	dev->priv_destructor	= vlan_dev_free;
 	dev->ethtool_ops	= &vlan_ethtool_ops;
 
 	dev->min_mtu		= 0;
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index b25789a..10f7edf 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -1034,8 +1034,6 @@ static void batadv_softif_free(struct net_device *dev)
 	 * netdev and its private data (bat_priv)
 	 */
 	rcu_barrier();
-
-	free_netdev(dev);
 }
 
 /**
@@ -1047,7 +1045,8 @@ static void batadv_softif_init_early(struct net_device *dev)
 	ether_setup(dev);
 
 	dev->netdev_ops = &batadv_netdev_ops;
-	dev->destructor = batadv_softif_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = batadv_softif_free;
 	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_NETNS_LOCAL;
 	dev->priv_flags |= IFF_NO_QUEUE;
 
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 6089599..ab3b654 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -598,7 +598,7 @@ static void netdev_setup(struct net_device *dev)
 
 	dev->netdev_ops		= &netdev_ops;
 	dev->header_ops		= &header_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 }
 
 static struct device_type bt_type = {
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 430b53e..f0f3447 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -379,7 +379,7 @@ void br_dev_setup(struct net_device *dev)
 	ether_setup(dev);
 
 	dev->netdev_ops = &br_netdev_ops;
-	dev->destructor = free_netdev;
+	dev->needs_free_netdev = true;
 	dev->ethtool_ops = &br_ethtool_ops;
 	SET_NETDEV_DEVTYPE(dev, &br_type);
 	dev->priv_flags = IFF_EBRIDGE | IFF_NO_QUEUE;
diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 1816fc9..fe3c53e 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -392,14 +392,14 @@ static void chnl_net_destructor(struct net_device *dev)
 {
 	struct chnl_net *priv = netdev_priv(dev);
 	caif_free_client(&priv->chnl);
-	free_netdev(dev);
 }
 
 static void ipcaif_net_setup(struct net_device *dev)
 {
 	struct chnl_net *priv;
 	dev->netdev_ops = &netdev_ops;
-	dev->destructor = chnl_net_destructor;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = chnl_net_destructor;
 	dev->flags |= IFF_NOARP;
 	dev->flags |= IFF_POINTOPOINT;
 	dev->mtu = GPRS_PDP_MTU;
diff --git a/net/core/dev.c b/net/core/dev.c
index d07aa5f..7507fea 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7485,6 +7485,8 @@ int register_netdevice(struct net_device *dev)
 err_uninit:
 	if (dev->netdev_ops->ndo_uninit)
 		dev->netdev_ops->ndo_uninit(dev);
+	if (dev->priv_destructor)
+		dev->priv_destructor(dev);
 	goto out;
 }
 EXPORT_SYMBOL(register_netdevice);
@@ -7692,8 +7694,10 @@ void netdev_run_todo(void)
 		WARN_ON(rcu_access_pointer(dev->ip6_ptr));
 		WARN_ON(dev->dn_ptr);
 
-		if (dev->destructor)
-			dev->destructor(dev);
+		if (dev->priv_destructor)
+			dev->priv_destructor(dev);
+		if (dev->needs_free_netdev)
+			free_netdev(dev);
 
 		/* Report a network device has been unregistered */
 		rtnl_lock();
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index c73160f..0a0a392 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -378,7 +378,6 @@ static void hsr_dev_destroy(struct net_device *hsr_dev)
 	del_timer_sync(&hsr->announce_timer);
 
 	synchronize_rcu();
-	free_netdev(hsr_dev);
 }
 
 static const struct net_device_ops hsr_device_ops = {
@@ -404,7 +403,8 @@ void hsr_dev_setup(struct net_device *dev)
 	SET_NETDEV_DEVTYPE(dev, &hsr_type);
 	dev->priv_flags |= IFF_NO_QUEUE;
 
-	dev->destructor = hsr_dev_destroy;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = hsr_dev_destroy;
 
 	dev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
 			   NETIF_F_GSO_MASK | NETIF_F_HW_CSUM |
diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index d7efbf0..0a866f33 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -107,7 +107,7 @@ static void lowpan_setup(struct net_device *ldev)
 
 	ldev->netdev_ops	= &lowpan_netdev_ops;
 	ldev->header_ops	= &lowpan_header_ops;
-	ldev->destructor	= free_netdev;
+	ldev->needs_free_netdev	= true;
 	ldev->features		|= NETIF_F_NETNS_LOCAL;
 }
 
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index b878ecb..b436d07 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -967,7 +967,6 @@ static void ip_tunnel_dev_free(struct net_device *dev)
 	gro_cells_destroy(&tunnel->gro_cells);
 	dst_cache_destroy(&tunnel->dst_cache);
 	free_percpu(dev->tstats);
-	free_netdev(dev);
 }
 
 void ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
@@ -1155,7 +1154,8 @@ int ip_tunnel_init(struct net_device *dev)
 	struct iphdr *iph = &tunnel->parms.iph;
 	int err;
 
-	dev->destructor	= ip_tunnel_dev_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = ip_tunnel_dev_free;
 	dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
 	if (!dev->tstats)
 		return -ENOMEM;
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 3a02d52..200b65a 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -501,7 +501,7 @@ static void reg_vif_setup(struct net_device *dev)
 	dev->mtu		= ETH_DATA_LEN - sizeof(struct iphdr) - 8;
 	dev->flags		= IFF_NOARP;
 	dev->netdev_ops		= &reg_vif_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 	dev->features		|= NETIF_F_NETNS_LOCAL;
 }
 
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 8d128ba..c8c386a 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -990,13 +990,13 @@ static void ip6gre_dev_free(struct net_device *dev)
 
 	dst_cache_destroy(&t->dst_cache);
 	free_percpu(dev->tstats);
-	free_netdev(dev);
 }
 
 static void ip6gre_tunnel_setup(struct net_device *dev)
 {
 	dev->netdev_ops = &ip6gre_netdev_ops;
-	dev->destructor = ip6gre_dev_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = ip6gre_dev_free;
 
 	dev->type = ARPHRD_IP6GRE;
 
@@ -1147,7 +1147,7 @@ static int __net_init ip6gre_init_net(struct net *net)
 	return 0;
 
 err_reg_dev:
-	ip6gre_dev_free(ign->fb_tunnel_dev);
+	free_netdev(ign->fb_tunnel_dev);
 err_alloc_dev:
 	return err;
 }
@@ -1299,7 +1299,8 @@ static void ip6gre_tap_setup(struct net_device *dev)
 	ether_setup(dev);
 
 	dev->netdev_ops = &ip6gre_tap_netdev_ops;
-	dev->destructor = ip6gre_dev_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = ip6gre_dev_free;
 
 	dev->features |= NETIF_F_NETNS_LOCAL;
 	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 6eb2ae5..883b652 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -254,7 +254,6 @@ static void ip6_dev_free(struct net_device *dev)
 	gro_cells_destroy(&t->gro_cells);
 	dst_cache_destroy(&t->dst_cache);
 	free_percpu(dev->tstats);
-	free_netdev(dev);
 }
 
 static int ip6_tnl_create2(struct net_device *dev)
@@ -322,7 +321,7 @@ static struct ip6_tnl *ip6_tnl_create(struct net *net, struct __ip6_tnl_parm *p)
 	return t;
 
 failed_free:
-	ip6_dev_free(dev);
+	free_netdev(dev);
 failed:
 	return ERR_PTR(err);
 }
@@ -1769,7 +1768,8 @@ static const struct net_device_ops ip6_tnl_netdev_ops = {
 static void ip6_tnl_dev_setup(struct net_device *dev)
 {
 	dev->netdev_ops = &ip6_tnl_netdev_ops;
-	dev->destructor = ip6_dev_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = ip6_dev_free;
 
 	dev->type = ARPHRD_TUNNEL6;
 	dev->flags |= IFF_NOARP;
@@ -2216,7 +2216,7 @@ static int __net_init ip6_tnl_init_net(struct net *net)
 	return 0;
 
 err_register:
-	ip6_dev_free(ip6n->fb_tnl_dev);
+	free_netdev(ip6n->fb_tnl_dev);
 err_alloc_dev:
 	return err;
 }
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index d67ef56..837ea1e 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -180,7 +180,6 @@ vti6_tnl_unlink(struct vti6_net *ip6n, struct ip6_tnl *t)
 static void vti6_dev_free(struct net_device *dev)
 {
 	free_percpu(dev->tstats);
-	free_netdev(dev);
 }
 
 static int vti6_tnl_create2(struct net_device *dev)
@@ -235,7 +234,7 @@ static struct ip6_tnl *vti6_tnl_create(struct net *net, struct __ip6_tnl_parm *p
 	return t;
 
 failed_free:
-	vti6_dev_free(dev);
+	free_netdev(dev);
 failed:
 	return NULL;
 }
@@ -842,7 +841,8 @@ static const struct net_device_ops vti6_netdev_ops = {
 static void vti6_dev_setup(struct net_device *dev)
 {
 	dev->netdev_ops = &vti6_netdev_ops;
-	dev->destructor = vti6_dev_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = vti6_dev_free;
 
 	dev->type = ARPHRD_TUNNEL6;
 	dev->hard_header_len = LL_MAX_HEADER + sizeof(struct ipv6hdr);
@@ -1100,7 +1100,7 @@ static int __net_init vti6_init_net(struct net *net)
 	return 0;
 
 err_register:
-	vti6_dev_free(ip6n->fb_tnl_dev);
+	free_netdev(ip6n->fb_tnl_dev);
 err_alloc_dev:
 	return err;
 }
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 374997d..2ecb39b 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -733,7 +733,7 @@ static void reg_vif_setup(struct net_device *dev)
 	dev->mtu		= 1500 - sizeof(struct ipv6hdr) - 8;
 	dev->flags		= IFF_NOARP;
 	dev->netdev_ops		= &reg_vif_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 	dev->features		|= NETIF_F_NETNS_LOCAL;
 }
 
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 61e5902..2378503 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -265,7 +265,7 @@ static struct ip_tunnel *ipip6_tunnel_locate(struct net *net,
 	return nt;
 
 failed_free:
-	ipip6_dev_free(dev);
+	free_netdev(dev);
 failed:
 	return NULL;
 }
@@ -1336,7 +1336,6 @@ static void ipip6_dev_free(struct net_device *dev)
 
 	dst_cache_destroy(&tunnel->dst_cache);
 	free_percpu(dev->tstats);
-	free_netdev(dev);
 }
 
 #define SIT_FEATURES (NETIF_F_SG	   | \
@@ -1351,7 +1350,8 @@ static void ipip6_tunnel_setup(struct net_device *dev)
 	int t_hlen = tunnel->hlen + sizeof(struct iphdr);
 
 	dev->netdev_ops		= &ipip6_netdev_ops;
-	dev->destructor		= ipip6_dev_free;
+	dev->needs_free_netdev	= true;
+	dev->priv_destructor	= ipip6_dev_free;
 
 	dev->type		= ARPHRD_SIT;
 	dev->hard_header_len	= LL_MAX_HEADER + t_hlen;
diff --git a/net/irda/irlan/irlan_eth.c b/net/irda/irlan/irlan_eth.c
index 74d09f9..3be8528 100644
--- a/net/irda/irlan/irlan_eth.c
+++ b/net/irda/irlan/irlan_eth.c
@@ -65,7 +65,7 @@ static void irlan_eth_setup(struct net_device *dev)
 	ether_setup(dev);
 
 	dev->netdev_ops		= &irlan_eth_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 	dev->min_mtu		= 0;
 	dev->max_mtu		= ETH_MAX_MTU;
 
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 8b21af7..f7c54ec 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -141,7 +141,7 @@ static void l2tp_eth_dev_setup(struct net_device *dev)
 	dev->priv_flags		&= ~IFF_TX_SKB_SHARING;
 	dev->features		|= NETIF_F_LLTX;
 	dev->netdev_ops		= &l2tp_eth_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 }
 
 static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb, int data_len)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 3bd5b81..267e965 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1213,7 +1213,6 @@ static const struct net_device_ops ieee80211_monitorif_ops = {
 static void ieee80211_if_free(struct net_device *dev)
 {
 	free_percpu(dev->tstats);
-	free_netdev(dev);
 }
 
 static void ieee80211_if_setup(struct net_device *dev)
@@ -1221,7 +1220,8 @@ static void ieee80211_if_setup(struct net_device *dev)
 	ether_setup(dev);
 	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	dev->netdev_ops = &ieee80211_dataif_ops;
-	dev->destructor = ieee80211_if_free;
+	dev->needs_free_netdev = true;
+	dev->priv_destructor = ieee80211_if_free;
 }
 
 static void ieee80211_if_setup_no_queue(struct net_device *dev)
@@ -1914,7 +1914,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 
 		ret = register_netdevice(ndev);
 		if (ret) {
-			ieee80211_if_free(ndev);
+			free_netdev(ndev);
 			return ret;
 		}
 	}
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 06019db..bd88a9b 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -526,8 +526,6 @@ static void mac802154_wpan_free(struct net_device *dev)
 	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
 
 	mac802154_llsec_destroy(&sdata->sec);
-
-	free_netdev(dev);
 }
 
 static void ieee802154_if_setup(struct net_device *dev)
@@ -593,7 +591,8 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
 					sdata->dev->dev_addr);
 
 		sdata->dev->header_ops = &mac802154_header_ops;
-		sdata->dev->destructor = mac802154_wpan_free;
+		sdata->dev->needs_free_netdev = true;
+		sdata->dev->priv_destructor = mac802154_wpan_free;
 		sdata->dev->netdev_ops = &mac802154_wpan_ops;
 		sdata->dev->ml_priv = &mac802154_mlme_wpan;
 		wpan_dev->promiscuous_mode = false;
@@ -608,7 +607,7 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
 
 		break;
 	case NL802154_IFTYPE_MONITOR:
-		sdata->dev->destructor = free_netdev;
+		sdata->dev->needs_free_netdev = true;
 		sdata->dev->netdev_ops = &mac802154_monitor_ops;
 		wpan_dev->promiscuous_mode = true;
 		break;
diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 89193a6..04a3128 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -94,7 +94,6 @@ static void internal_dev_destructor(struct net_device *dev)
 	struct vport *vport = ovs_internal_dev_get_vport(dev);
 
 	ovs_vport_free(vport);
-	free_netdev(dev);
 }
 
 static void
@@ -156,7 +155,8 @@ static void do_setup(struct net_device *netdev)
 	netdev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_OPENVSWITCH |
 			      IFF_PHONY_HEADROOM | IFF_NO_QUEUE;
-	netdev->destructor = internal_dev_destructor;
+	netdev->needs_free_netdev = true;
+	netdev->priv_destructor = internal_dev_destructor;
 	netdev->ethtool_ops = &internal_dev_ethtool_ops;
 	netdev->rtnl_link_ops = &internal_dev_link_ops;
 
diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
index 21c28b5..2c93379 100644
--- a/net/phonet/pep-gprs.c
+++ b/net/phonet/pep-gprs.c
@@ -236,7 +236,7 @@ static void gprs_setup(struct net_device *dev)
 	dev->tx_queue_len	= 10;
 
 	dev->netdev_ops		= &gprs_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->needs_free_netdev	= true;
 }
 
 /*
-- 
2.4.11

^ permalink raw reply related

* Re: [net-intel-i40e] question about assignment overwrite
From: Mauro Rodrigues @ 2017-06-07 19:55 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: Gustavo A. R. Silva, intel-wired-lan, netdev, linux-kernel
In-Reply-To: <1495086551.46489.1.camel@intel.com>

On Wed, May 17, 2017 at 10:49:11PM -0700, Jeff Kirsher wrote:
> On Wed, 2017-05-17 at 15:48 -0500, Gustavo A. R. Silva wrote:
> > While looking into Coverity ID 1408956 I ran into the following
> > piece  
> > of code at drivers/net/ethernet/intel/i40e/i40e_main.c:8807:
> > 
> > 8807        if (pf->hw.mac.type == I40E_MAC_X722) {
> > 8808                pf->flags |= I40E_FLAG_RSS_AQ_CAPABLE
> > 8809                             | I40E_FLAG_128_QP_RSS_CAPABLE
> > 8810                             | I40E_FLAG_HW_ATR_EVICT_CAPABLE
> > 8811                             | I40E_FLAG_OUTER_UDP_CSUM_CAPABLE
> > 8812                             | I40E_FLAG_WB_ON_ITR_CAPABLE
> > 8813                             |
> > I40E_FLAG_MULTIPLE_TCP_UDP_RSS_PCTYPE
> > 8814                             | I40E_FLAG_NO_PCI_LINK_CHECK
> > 8815                             | I40E_FLAG_USE_SET_LLDP_MIB
> > 8816                             | I40E_FLAG_GENEVE_OFFLOAD_CAPABLE
> > 8817                             | I40E_FLAG_PTP_L4_CAPABLE
> > 8818                             | I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE;
> > 8819        } else if ((pf->hw.aq.api_maj_ver > 1) ||
> > 8820                   ((pf->hw.aq.api_maj_ver == 1) &&
> > 8821                    (pf->hw.aq.api_min_ver > 4))) {
> > 8822                /* Supported in FW API version higher than 1.4 */
> > 8823                pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
> > 8824                pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> > 8825        } else {
> > 8826                pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> > 8827        }
> > 
> > The issue here is that the assignment at line 8823 is overwritten
> > by  
> > the code at line 8824.
> > 
> > I'm suspicious that line 8824 should be remove and a patch like the  
> > following can be applied:
> > 
> > index d5c9c9e..48ffa73 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > @@ -8821,7 +8821,6 @@ static int i40e_sw_init(struct i40e_pf *pf)
> >                      (pf->hw.aq.api_min_ver > 4))) {
> >                  /* Supported in FW API version higher than 1.4 */
> >                  pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
> > -               pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> >          } else {
> >                  pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> >          }
> > 
> > What do you think?
> 
> This issue is already fixed in my dev-queue branch on my next-queue
> tree.

Hi Jeff, are you planning to push this fix into net branch anytime soon?

Thanks,

Mauro

^ permalink raw reply

* Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry
From: Vivien Didelot @ 2017-06-07 19:59 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <ae09e920-fcea-ac29-3b2d-51275e0fec56@gmail.com>

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 06/06/2017 01:56 PM, Vivien Didelot wrote:
>> The mv88e6xxx driver currently tries to be smart and remove by itself a
>> VLAN entry from the VTU when the driven switch sees no user ports as
>> members of the VLAN.
>> 
>> This is bad in a multi-chip switch fabric, since a chip in between
>> others may have no bridge port members, but still needs to be aware of
>> the VID in order to correctly pass frames in the data path.
>> 
>> Remove the code purging a VTU entry when updating a port membership.
>
> In that case the switch sitting between two other chips and passing
> traffic would still have at least two of its DSA ports be part of a VTU
> entry, right?

That is correct.

>
> So could not we just do ....
>
>> 
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>>  drivers/net/dsa/mv88e6xxx/chip.c | 15 +--------------
>>  1 file changed, 1 insertion(+), 14 deletions(-)
>> 
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index 522f023bb17e..64c0f88f9e79 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -1325,9 +1325,8 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
>>  static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>>  				    int port, u16 vid)
>>  {
>> -	struct dsa_switch *ds = chip->ds;
>>  	struct mv88e6xxx_vtu_entry vlan;
>> -	int i, err;
>> +	int err;
>>  
>>  	err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
>>  	if (err)
>> @@ -1339,18 +1338,6 @@ static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>>  
>>  	vlan.member[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
>>  
>> -	/* keep the VLAN unless all ports are excluded */
>> -	vlan.valid = false;
>> -	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
>> -		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
>> -			continue;
>
> ... break the loop here?

I can remove only the dsa_is_{cpu,dsa}_port condition above, this will
make the code ready for when the DSA core will remove VLAN on DSA ports.

Thanks!

        Vivien

^ permalink raw reply

* Re: [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch
From: Vivien Didelot @ 2017-06-07 20:03 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <8d6edbe3-a80b-6cc7-b7ec-d8f1578dea27@gmail.com>

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 06/06/2017 01:56 PM, Vivien Didelot wrote:
>> Now that the VLAN object is propagated to every switch chip of the
>> switch fabric, we can easily ensure that they all support the required
>> VLAN operations before modifying an entry on a single switch.
>> 
>> To achieve that, remove the condition skipping other target switches,
>> and add a bitmap of VLAN members, eventually containing the target port,
>> if we are programming the switch target.
>
> You could add in the commit message that with this commit, there is not
> actually a functional change yet because we have one (and only one) bit
> set in the members bitmap.

Hum there is a small functional change though: if one interconnected
switch of the fabric does not support the VLAN operations, -EOPNOTSUPP
is returned (even though the target switch is VLAN capable.)

Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH net] net: ipv6: Release route when device is unregistering
From: Roopa Prabhu @ 2017-06-07 20:14 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev@vger.kernel.org
In-Reply-To: <20170607182623.20734-1-dsahern@gmail.com>

On Wed, Jun 7, 2017 at 11:26 AM, David Ahern <dsahern@gmail.com> wrote:
> Roopa reported attempts to delete a bond device that is referenced in a
> multipath route is hanging:
>
> $ ifdown bond2    # ifupdown2 command that deletes virtual devices
> unregister_netdevice: waiting for bond2 to become free. Usage count = 2
>
> Steps to reproduce:
>     echo 1 > /proc/sys/net/ipv6/conf/all/ignore_routes_with_linkdown
>     ip link add dev bond12 type bond
>     ip link add dev bond13 type bond
>     ip addr add 2001:db8:2::0/64 dev bond12
>     ip addr add 2001:db8:3::0/64 dev bond13
>     ip route add 2001:db8:33::0/64 nexthop via 2001:db8:2::2 nexthop via 2001:db8:3::2
>     ip link del dev bond12
>     ip link del dev bond13
>
> The root cause is the recent change to keep routes on a linkdown. Update
> the check to detect when the device is unregistering and release the
> route for that case.
>
> Fixes: a1a22c12060e4 ("net: ipv6: Keep nexthop of multipath route on admin down")
> Reported-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---

Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch fixes the issue for me. Thanks David.

^ permalink raw reply

* [PATCH] mISDN: remove unnecessary variable assignments
From: Gustavo A. R. Silva @ 2017-06-07 20:15 UTC (permalink / raw)
  To: Karsten Keil; +Cc: netdev, linux-kernel, Gustavo A. R. Silva

Remove unnecessary variable assignments.

Addresses-Coverity-ID: 1226917
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/isdn/hardware/mISDN/mISDNipac.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/isdn/hardware/mISDN/mISDNipac.c b/drivers/isdn/hardware/mISDN/mISDNipac.c
index 6742b0d..e240010 100644
--- a/drivers/isdn/hardware/mISDN/mISDNipac.c
+++ b/drivers/isdn/hardware/mISDN/mISDNipac.c
@@ -364,8 +364,8 @@ isac_mos_irq(struct isac_hw *isac)
 			WriteISAC(isac, ISAC_MOCR, isac->mocr);
 			if (isac->mon_txc && (isac->mon_txp >= isac->mon_txc)) {
 				if (isac->monitor)
-					ret = isac->monitor(isac->dch.hw,
-							    MONITOR_TX_0, NULL, 0);
+					isac->monitor(isac->dch.hw,
+						      MONITOR_TX_0, NULL, 0);
 			}
 			kfree(isac->mon_tx);
 			isac->mon_tx = NULL;
@@ -375,8 +375,8 @@ isac_mos_irq(struct isac_hw *isac)
 		}
 		if (isac->mon_txc && (isac->mon_txp >= isac->mon_txc)) {
 			if (isac->monitor)
-				ret = isac->monitor(isac->dch.hw,
-						    MONITOR_TX_0, NULL, 0);
+				isac->monitor(isac->dch.hw,
+					      MONITOR_TX_0, NULL, 0);
 			kfree(isac->mon_tx);
 			isac->mon_tx = NULL;
 			isac->mon_txc = 0;
@@ -397,8 +397,8 @@ isac_mos_irq(struct isac_hw *isac)
 			WriteISAC(isac, ISAC_MOCR, isac->mocr);
 			if (isac->mon_txc && (isac->mon_txp >= isac->mon_txc)) {
 				if (isac->monitor)
-					ret = isac->monitor(isac->dch.hw,
-							    MONITOR_TX_1, NULL, 0);
+					isac->monitor(isac->dch.hw,
+						      MONITOR_TX_1, NULL, 0);
 			}
 			kfree(isac->mon_tx);
 			isac->mon_tx = NULL;
@@ -408,8 +408,8 @@ isac_mos_irq(struct isac_hw *isac)
 		}
 		if (isac->mon_txc && (isac->mon_txp >= isac->mon_txc)) {
 			if (isac->monitor)
-				ret = isac->monitor(isac->dch.hw,
-						    MONITOR_TX_1, NULL, 0);
+				isac->monitor(isac->dch.hw,
+					      MONITOR_TX_1, NULL, 0);
 			kfree(isac->mon_tx);
 			isac->mon_tx = NULL;
 			isac->mon_txc = 0;
-- 
2.5.0

^ permalink raw reply related

* Re: [PATCH net-next] tcp: add TCPMemoryPressuresChrono counter
From: Eric Dumazet @ 2017-06-07 20:23 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20170607.145448.152856288455996460.davem@davemloft.net>

On Wed, 2017-06-07 at 14:54 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 06 Jun 2017 12:19:27 -0700
> 
> > @@ -320,18 +320,37 @@ struct tcp_splice_state {
> >   * All the __sk_mem_schedule() is of this nature: accounting
> >   * is strict, actions are advisory and have some latency.
> >   */
> > -int tcp_memory_pressure __read_mostly;
> > +unsigned long tcp_memory_pressure __read_mostly;
> >  EXPORT_SYMBOL(tcp_memory_pressure);
> 
> I know it is not strictly related to your change, but if you are
> adding new exports in this area let's do it properly.
> 
> The existing tcp_memory_pressure export, as well as these two:
> 
> ...
> >  EXPORT_SYMBOL(tcp_enter_memory_pressure);
> ...
> > +EXPORT_SYMBOL(tcp_leave_memory_pressure);
> 
> ought to be EXPORT_SYMBOL_GPL().  They are exported for the sake of
> ipv6 TCP support, and nothing else.
> 
> I'll apply this with that fixed.
> 
> Thanks Eric.

Sure, I will send a V2 with the two EXPORT_SYMBOL_GPL()

Thanks.

^ permalink raw reply

* [PATCH v2 net-next] tcp: add TCPMemoryPressuresChrono counter
From: Eric Dumazet @ 2017-06-07 20:29 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <1496776767.736.11.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <edumazet@google.com>

DRAM supply shortage and poor memory pressure tracking in TCP
stack makes any change in SO_SNDBUF/SO_RCVBUF (or equivalent autotuning
limits) and tcp_mem[] quite hazardous.

TCPMemoryPressures SNMP counter is an indication of tcp_mem sysctl
limits being hit, but only tracking number of transitions.

If TCP stack behavior under stress was perfect :
1) It would maintain memory usage close to the limit.
2) Memory pressure state would be entered for short times.

We certainly prefer 100 events lasting 10ms compared to one event
lasting 200 seconds.

This patch adds a new SNMP counter tracking cumulative duration of
memory pressure events, given in ms units.

$ cat /proc/sys/net/ipv4/tcp_mem
3088    4117    6176
$ grep TCP /proc/net/sockstat
TCP: inuse 180 orphan 0 tw 2 alloc 234 mem 4140
$ nstat -n ; sleep 10 ; nstat |grep Pressure
TcpExtTCPMemoryPressures        1700
TcpExtTCPMemoryPressuresChrono  5209

v2: Used EXPORT_SYMBOL_GPL() instead of EXPORT_SYMBOL() as David
instructed.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/sock.h        |   22 ++--------------------
 include/net/tcp.h         |    3 ++-
 include/uapi/linux/snmp.h |    1 +
 net/core/sock.c           |   20 ++++++++++++++++++++
 net/decnet/af_decnet.c    |    2 +-
 net/ipv4/proc.c           |    1 +
 net/ipv4/tcp.c            |   31 +++++++++++++++++++++++++------
 net/ipv4/tcp_ipv4.c       |    1 +
 net/ipv6/tcp_ipv6.c       |    1 +
 net/sctp/socket.c         |    2 +-
 10 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 3467d9e89e7dba1c35fa44a6268a28735f795319..858891c36f94ad2577726d6d21cf871dbcd55d98 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1080,6 +1080,7 @@ struct proto {
 	bool			(*stream_memory_free)(const struct sock *sk);
 	/* Memory pressure */
 	void			(*enter_memory_pressure)(struct sock *sk);
+	void			(*leave_memory_pressure)(struct sock *sk);
 	atomic_long_t		*memory_allocated;	/* Current allocated memory. */
 	struct percpu_counter	*sockets_allocated;	/* Current number of sockets. */
 	/*
@@ -1088,7 +1089,7 @@ struct proto {
 	 * All the __sk_mem_schedule() is of this nature: accounting
 	 * is strict, actions are advisory and have some latency.
 	 */
-	int			*memory_pressure;
+	unsigned long		*memory_pressure;
 	long			*sysctl_mem;
 	int			*sysctl_wmem;
 	int			*sysctl_rmem;
@@ -1193,25 +1194,6 @@ static inline bool sk_under_memory_pressure(const struct sock *sk)
 	return !!*sk->sk_prot->memory_pressure;
 }
 
-static inline void sk_leave_memory_pressure(struct sock *sk)
-{
-	int *memory_pressure = sk->sk_prot->memory_pressure;
-
-	if (!memory_pressure)
-		return;
-
-	if (*memory_pressure)
-		*memory_pressure = 0;
-}
-
-static inline void sk_enter_memory_pressure(struct sock *sk)
-{
-	if (!sk->sk_prot->enter_memory_pressure)
-		return;
-
-	sk->sk_prot->enter_memory_pressure(sk);
-}
-
 static inline long
 sk_memory_allocated(const struct sock *sk)
 {
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 28b577a35786ddc9b223b54dd387e59910d9c521..9b7cf93392d7f850d539f8b51333f2ba13e8856f 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -279,7 +279,7 @@ extern int sysctl_tcp_pacing_ca_ratio;
 
 extern atomic_long_t tcp_memory_allocated;
 extern struct percpu_counter tcp_sockets_allocated;
-extern int tcp_memory_pressure;
+extern unsigned long tcp_memory_pressure;
 
 /* optimized version of sk_under_memory_pressure() for TCP sockets */
 static inline bool tcp_under_memory_pressure(const struct sock *sk)
@@ -1322,6 +1322,7 @@ extern void tcp_openreq_init_rwin(struct request_sock *req,
 				  const struct dst_entry *dst);
 
 void tcp_enter_memory_pressure(struct sock *sk);
+void tcp_leave_memory_pressure(struct sock *sk);
 
 static inline int keepalive_intvl_when(const struct tcp_sock *tp)
 {
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index 95cffcb21dfdba7c974706131d0f43e21435e82d..d8569329579816213255169d0c183f4400835f7b 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -228,6 +228,7 @@ enum
 	LINUX_MIB_TCPABORTONLINGER,		/* TCPAbortOnLinger */
 	LINUX_MIB_TCPABORTFAILED,		/* TCPAbortFailed */
 	LINUX_MIB_TCPMEMORYPRESSURES,		/* TCPMemoryPressures */
+	LINUX_MIB_TCPMEMORYPRESSURESCHRONO,	/* TCPMemoryPressuresChrono */
 	LINUX_MIB_TCPSACKDISCARD,		/* TCPSACKDiscard */
 	LINUX_MIB_TCPDSACKIGNOREDOLD,		/* TCPSACKIgnoredOld */
 	LINUX_MIB_TCPDSACKIGNOREDNOUNDO,	/* TCPSACKIgnoredNoUndo */
diff --git a/net/core/sock.c b/net/core/sock.c
index bef844127e0182091678b9d57f7ec85c5241748d..ad8a4bc841267a442a1da3c56ef1cf074f9825b9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2076,6 +2076,26 @@ int sock_cmsg_send(struct sock *sk, struct msghdr *msg,
 }
 EXPORT_SYMBOL(sock_cmsg_send);
 
+static void sk_enter_memory_pressure(struct sock *sk)
+{
+	if (!sk->sk_prot->enter_memory_pressure)
+		return;
+
+	sk->sk_prot->enter_memory_pressure(sk);
+}
+
+static void sk_leave_memory_pressure(struct sock *sk)
+{
+	if (sk->sk_prot->leave_memory_pressure) {
+		sk->sk_prot->leave_memory_pressure(sk);
+	} else {
+		unsigned long *memory_pressure = sk->sk_prot->memory_pressure;
+
+		if (memory_pressure && *memory_pressure)
+			*memory_pressure = 0;
+	}
+}
+
 /* On 32bit arches, an skb frag is limited to 2^15 */
 #define SKB_FRAG_PAGE_ORDER	get_order(32768)
 
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 405483a07efc7ac2efcfe86e285a7673547c9691..73a0399dc7a277178b0a432a067172131dce99ee 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -447,7 +447,7 @@ static void dn_destruct(struct sock *sk)
 	dst_release(rcu_dereference_check(sk->sk_dst_cache, 1));
 }
 
-static int dn_memory_pressure;
+static unsigned long dn_memory_pressure;
 
 static void dn_enter_memory_pressure(struct sock *sk)
 {
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index fa44e752a9a3f8eb9957314149ae15e6df10465a..43eb6567b3a0a2add9a1d36019eae5b6d5caf657 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -250,6 +250,7 @@ static const struct snmp_mib snmp4_net_list[] = {
 	SNMP_MIB_ITEM("TCPAbortOnLinger", LINUX_MIB_TCPABORTONLINGER),
 	SNMP_MIB_ITEM("TCPAbortFailed", LINUX_MIB_TCPABORTFAILED),
 	SNMP_MIB_ITEM("TCPMemoryPressures", LINUX_MIB_TCPMEMORYPRESSURES),
+	SNMP_MIB_ITEM("TCPMemoryPressuresChrono", LINUX_MIB_TCPMEMORYPRESSURESCHRONO),
 	SNMP_MIB_ITEM("TCPSACKDiscard", LINUX_MIB_TCPSACKDISCARD),
 	SNMP_MIB_ITEM("TCPDSACKIgnoredOld", LINUX_MIB_TCPDSACKIGNOREDOLD),
 	SNMP_MIB_ITEM("TCPDSACKIgnoredNoUndo", LINUX_MIB_TCPDSACKIGNOREDNOUNDO),
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 87981fcdfcf20c6846ea3474dce1e640aea6e092..cc8fd8b747a47e9b66492ecdf27256ef6d879877 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -320,17 +320,36 @@ struct tcp_splice_state {
  * All the __sk_mem_schedule() is of this nature: accounting
  * is strict, actions are advisory and have some latency.
  */
-int tcp_memory_pressure __read_mostly;
-EXPORT_SYMBOL(tcp_memory_pressure);
+unsigned long tcp_memory_pressure __read_mostly;
+EXPORT_SYMBOL_GPL(tcp_memory_pressure);
 
 void tcp_enter_memory_pressure(struct sock *sk)
 {
-	if (!tcp_memory_pressure) {
+	unsigned long val;
+
+	if (tcp_memory_pressure)
+		return;
+	val = jiffies;
+
+	if (!val)
+		val--;
+	if (!cmpxchg(&tcp_memory_pressure, 0, val))
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMEMORYPRESSURES);
-		tcp_memory_pressure = 1;
-	}
 }
-EXPORT_SYMBOL(tcp_enter_memory_pressure);
+EXPORT_SYMBOL_GPL(tcp_enter_memory_pressure);
+
+void tcp_leave_memory_pressure(struct sock *sk)
+{
+	unsigned long val;
+
+	if (!tcp_memory_pressure)
+		return;
+	val = xchg(&tcp_memory_pressure, 0);
+	if (val)
+		NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPMEMORYPRESSURESCHRONO,
+			      jiffies_to_msecs(jiffies - val));
+}
+EXPORT_SYMBOL_GPL(tcp_leave_memory_pressure);
 
 /* Convert seconds to retransmits based on initial and max timeout */
 static u8 secs_to_retrans(int seconds, int timeout, int rto_max)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 191b2f78b19d2c8d62c59cc046bd608687679619..d19933949373ca019fd06fa310a506efd29718cb 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2387,6 +2387,7 @@ struct proto tcp_prot = {
 	.unhash			= inet_unhash,
 	.get_port		= inet_csk_get_port,
 	.enter_memory_pressure	= tcp_enter_memory_pressure,
+	.leave_memory_pressure	= tcp_leave_memory_pressure,
 	.stream_memory_free	= tcp_stream_memory_free,
 	.sockets_allocated	= &tcp_sockets_allocated,
 	.orphan_count		= &tcp_orphan_count,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 233edfabe1dbceaeb6cdd42a2bb379072aeee361..f6592790d9f93bf40f3e5432c945bca20b28ff34 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1909,6 +1909,7 @@ struct proto tcpv6_prot = {
 	.unhash			= inet_unhash,
 	.get_port		= inet_csk_get_port,
 	.enter_memory_pressure	= tcp_enter_memory_pressure,
+	.leave_memory_pressure	= tcp_leave_memory_pressure,
 	.stream_memory_free	= tcp_stream_memory_free,
 	.sockets_allocated	= &tcp_sockets_allocated,
 	.memory_allocated	= &tcp_memory_allocated,
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 0822046e4f3f5a1acd3f5382d915bf9004a25c1c..5f58dd03e3ace38b9c4babbe2d92f0a3f98a4b68 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -103,7 +103,7 @@ static int sctp_autobind(struct sock *sk);
 static void sctp_sock_migrate(struct sock *, struct sock *,
 			      struct sctp_association *, sctp_socket_type_t);
 
-static int sctp_memory_pressure;
+static unsigned long sctp_memory_pressure;
 static atomic_long_t sctp_memory_allocated;
 struct percpu_counter sctp_sockets_allocated;
 

^ 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