Netdev List
 help / color / mirror / Atom feed
* RE: [PATCH net] net: dsa: mv88e6060: fix switch MAC address
From: Woojung.Huh @ 2017-10-13 19:09 UTC (permalink / raw)
  To: vivien.didelot, netdev
  Cc: linux-kernel, kernel, davem, f.fainelli, andrew, David.Laight
In-Reply-To: <20171013173922.26536-1-vivien.didelot@savoirfairelinux.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Vivien Didelot
> Sent: Friday, October 13, 2017 1:39 PM
> To: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; kernel@savoirfairelinux.com; David S.
> Miller; Florian Fainelli; Andrew Lunn; David Laight; Vivien Didelot
> Subject: [PATCH net] net: dsa: mv88e6060: fix switch MAC address
> 
> The 88E6060 Ethernet switch always transmits the multicast bit of the
> switch MAC address as a zero. It re-uses the corresponding bit 8 of the
> register "Switch MAC Address Register Bytes 0 & 1" for "DiffAddr".
> 
> If the "DiffAddr" bit is 0, then all ports transmit the same source
> address. If it is set to 1, then bit 2:0 are used for the port number.
> 
> The mv88e6060 driver is currently wrongly shifting the MAC address byte
> 0 by 9. To fix this, shift it by 8 as usual and clear its bit 0.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---

Reviewed-by: Woojung Huh <woojung.huh@microchip.com>

- Woojung

^ permalink raw reply

* [Patch net] tun: call dev_get_valid_name() before register_netdevice()
From: Cong Wang @ 2017-10-13 18:58 UTC (permalink / raw)
  To: netdev; +Cc: avekceeb, Cong Wang, Jason Wang, Michael S. Tsirkin

register_netdevice() could fail early when we have an invalid
dev name, in which case ->ndo_uninit() is not called. For tun
device, this is a problem because a timer etc. are already
initialized and it expects ->ndo_uninit() to clean them up.

We could move these initializations into a ->ndo_init() so
that register_netdevice() knows better, however this is still
complicated due to the logic in tun_detach().

Therefore, I choose to just call dev_get_valid_name() before
register_netdevice(), which is quicker and much easier to audit.
And for this specific case, it is already enough.

Fixes: 96442e42429e ("tuntap: choose the txq based on rxq")
Reported-by: Dmitry Alexeev <avekceeb@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 drivers/net/tun.c         | 3 +++
 include/linux/netdevice.h | 3 +++
 net/core/dev.c            | 6 +++---
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5ce580f413b9..e21bf90b819f 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2027,6 +2027,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 
 		if (!dev)
 			return -ENOMEM;
+		err = dev_get_valid_name(net, dev, name);
+		if (err)
+			goto err_free_dev;
 
 		dev_net_set(dev, net);
 		dev->rtnl_link_ops = &tun_link_ops;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f535779d9dc1..2eaac7d75af4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3694,6 +3694,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 				    unsigned char name_assign_type,
 				    void (*setup)(struct net_device *),
 				    unsigned int txqs, unsigned int rxqs);
+int dev_get_valid_name(struct net *net, struct net_device *dev,
+		       const char *name);
+
 #define alloc_netdev(sizeof_priv, name, name_assign_type, setup) \
 	alloc_netdev_mqs(sizeof_priv, name, name_assign_type, setup, 1, 1)
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 588b473194a8..11596a302a26 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1147,9 +1147,8 @@ static int dev_alloc_name_ns(struct net *net,
 	return ret;
 }
 
-static int dev_get_valid_name(struct net *net,
-			      struct net_device *dev,
-			      const char *name)
+int dev_get_valid_name(struct net *net, struct net_device *dev,
+		       const char *name)
 {
 	BUG_ON(!net);
 
@@ -1165,6 +1164,7 @@ static int dev_get_valid_name(struct net *net,
 
 	return 0;
 }
+EXPORT_SYMBOL(dev_get_valid_name);
 
 /**
  *	dev_change_name - change name of a device
-- 
2.13.0

^ permalink raw reply related

* Re: [RFC] Support for UNARP (RFC 1868)
From: Girish Moodalbail @ 2017-10-13 18:23 UTC (permalink / raw)
  To: Mahesh Bandewar (महेश बंडेवार)
  Cc: Eric Dumazet, linux-netdev, David Miller, kuznet
In-Reply-To: <CAF2d9jg7A7fJfE-i4aSgYuvAE+_4VfBOMEtsY_urDNhO0qfoYA@mail.gmail.com>

On 10/12/17 5:53 PM, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Thu, Oct 12, 2017 at 4:06 PM, Girish Moodalbail
> <girish.moodalbail@oracle.com> wrote:
>> Hello Eric,
>>
>> The basic idea is to mark the ARP entry either FAILED or STALE as soon as we
>> can so that the subsequent packets that depend on that ARP entry will take
>> the slow path (neigh_resolve_output()).
>>
>> Say, if base_reachable_time is 30 seconds, then an ARP entry will be in
>> reachable state somewhere between 15 to 45 seconds. Assuming the worst case,
>> the ARP entry will be in REACHABLE state for 45 seconds and the packets
>> continue to traverse the network towards the source machine and gets dropped
>> their since the VM has moved to destination machine.
>>
>> Instead, based on the received UNARP packet if we mark the ARP entry
>>
>> (a) FAILED
>>     - we move to INCOMPLETE state and start the address resolution by sending
>>       out ARP packets (up to allowed maximum number) until we get ARP
>> response
>>       back at which point we move the ARP entry state to reachable.
>>
>> (b) STALE
>>     - we move to DELAY state and set the next timer to DELAY_PROBE_TIME
>>       (1 second) and continue to send queued packets in arp_queue.
>>     - After 1 sec we move to PROBE state and start the address resolution
>> like
>>       in the case(a) above.
>>
>> I was leaning towards (a).
> One could arbitrarily increase the stale timeout (by changing no of
> probes). So sender
> will continue sending traffic to something that has already gone away.
> STALE doesn't
> mean bad but here the sender is clearly indicating it's going away so
> FAILED seems to
> be the only logical option.

I agree.

>>
>>> Will TCP flows be terminated, instead
>>> of being smoothly migrated (TCP_REPAIR)
>>
>>
>> The TCP flows will not be terminated. Upon receiving UNARP packet, the ARP
>> entry will be marked FAILED. The subsequent TCP packets from the client
>> (towards that IP) will be queued (the first 3 packets in arp_queue and then
>> other packets get dropped) until the IP address is resolved again through
>> the slow path neigh_resolve_output().
>>
>> The slow path marks the entry as INCOMPLETE and will start sending several
>> ARP requests (ucast_solicit + app_solicit + mcast_solicit) to resolve the
>> IP. If the resolution is successful, then the TCP packets will be sent out.
>> If not, we will invalidate the ARP entry and call arp_error_report() on the
>> queued packets (which will end up sending ICMP_HOST_UNREACH error). This
>> behavior is same as what will occur if TCP server disappears in the middle
>> of a connection.
>>
>>>
>>> What about IPv6 ? Or maybe more abruptly, do we still need to add
>>> features to IPv4 in 2017,  22 years after this RFC came ? ;)
>>
>>
>> Legit question :). Well one thing I have seen in Networking is that an old
>> idea circles back around later and turns out to be useful in new contexts
>> and use cases. Like I enumerated in my initial email there are certain use
>> cases in Cloud that might benefit from UNARP.
>>
> It doesn't make sense to have this implemented only for IPv4. At this time if
> equivalent IPv6 feature is missing, I don't see it being useful / acceptable.

Obviously UNARP is IPv4 only. I am not aware of any standard way of doing the 
same for IPv6. If such a standard doesn't exist, then we will have to go through 
IETF to get one done? If that is the case, can we please do this in a phased 
manner? This will atleast help the Cloud that are IPv4 only.

thanks,
~Girish

^ permalink raw reply

* [PATCH net-next v3 2/5] net: dsa: mv88e6060: fix switch MAC address
From: Vivien Didelot @ 2017-10-13 18:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, David Laight, Vivien Didelot
In-Reply-To: <20171013181809.14627-1-vivien.didelot@savoirfairelinux.com>

The 88E6060 Ethernet switch always transmits the multicast bit of the
switch MAC address as a zero. It re-uses the corresponding bit 8 of the
register "Switch MAC Address Register Bytes 0 & 1" for "DiffAddr".

If the "DiffAddr" bit is 0, then all ports transmit the same source
address. If it is set to 1, then bit 2:0 are used for the port number.

The mv88e6060 driver is currently wrongly shifting the MAC address byte
0 by 9. To fix this, shift it by 8 as usual and clear its bit 0.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6060.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 621cdc46ad81..d64be2b83d3c 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -214,8 +214,14 @@ static int mv88e6060_setup(struct dsa_switch *ds)
 
 static int mv88e6060_set_addr(struct dsa_switch *ds, u8 *addr)
 {
-	/* Use the same MAC Address as FD Pause frames for all ports */
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, (addr[0] << 9) | addr[1]);
+	u16 val = addr[0] << 8 | addr[1];
+
+	/* The multicast bit is always transmitted as a zero, so the switch uses
+	 * bit 8 for "DiffAddr", where 0 means all ports transmit the same SA.
+	 */
+	val &= 0xfeff;
+
+	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, val);
 	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]);
 	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]);
 
-- 
2.14.2

^ permalink raw reply related

* [PATCH net-next v3 1/5] net: dsa: mv88e6xxx: setup random mac address
From: Vivien Didelot @ 2017-10-13 18:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, David Laight, Vivien Didelot
In-Reply-To: <20171013181809.14627-1-vivien.didelot@savoirfairelinux.com>

An Ethernet switch may support having a MAC address, which can be used
as the switch's source address in transmitted full-duplex Pause frames.

If a DSA switch supports the related .set_addr operation, the DSA core
sets the master's MAC address on the switch. This won't make sense
anymore in a multi-CPU ports system, because there won't be a unique
master device assigned to a switch tree.

Instead, setup the switch from within the Marvell driver with a random
MAC address, and remove the .set_addr implementation.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d74c7335c512..76cf383dcf90 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -932,6 +932,19 @@ static int mv88e6xxx_irl_setup(struct mv88e6xxx_chip *chip)
 	return 0;
 }
 
+static int mv88e6xxx_mac_setup(struct mv88e6xxx_chip *chip)
+{
+	if (chip->info->ops->set_switch_mac) {
+		u8 addr[ETH_ALEN];
+
+		eth_random_addr(addr);
+
+		return chip->info->ops->set_switch_mac(chip, addr);
+	}
+
+	return 0;
+}
+
 static int mv88e6xxx_pvt_map(struct mv88e6xxx_chip *chip, int dev, int port)
 {
 	u16 pvlan = 0;
@@ -2013,6 +2026,10 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	if (err)
 		goto unlock;
 
+	err = mv88e6xxx_mac_setup(chip);
+	if (err)
+		goto unlock;
+
 	err = mv88e6xxx_phy_setup(chip);
 	if (err)
 		goto unlock;
@@ -2043,21 +2060,6 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	return err;
 }
 
-static int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
-
-	if (!chip->info->ops->set_switch_mac)
-		return -EOPNOTSUPP;
-
-	mutex_lock(&chip->reg_lock);
-	err = chip->info->ops->set_switch_mac(chip, addr);
-	mutex_unlock(&chip->reg_lock);
-
-	return err;
-}
-
 static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 {
 	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
@@ -3785,7 +3787,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.probe			= mv88e6xxx_drv_probe,
 	.get_tag_protocol	= mv88e6xxx_get_tag_protocol,
 	.setup			= mv88e6xxx_setup,
-	.set_addr		= mv88e6xxx_set_addr,
 	.adjust_link		= mv88e6xxx_adjust_link,
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
-- 
2.14.2

^ permalink raw reply related

* [PATCH net-next v3 5/5] net: dsa: remove .set_addr
From: Vivien Didelot @ 2017-10-13 18:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, David Laight, Vivien Didelot
In-Reply-To: <20171013181809.14627-1-vivien.didelot@savoirfairelinux.com>

Now that there is no user for the .set_addr function, remove it from
DSA. If a switch supports this feature (like mv88e6xxx), the
implementation can be done in the driver setup.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h | 1 -
 net/dsa/dsa2.c    | 6 ------
 net/dsa/legacy.c  | 6 ------
 3 files changed, 13 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ce1d622734d7..2746741f74cf 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -291,7 +291,6 @@ struct dsa_switch_ops {
 	enum dsa_tag_protocol (*get_tag_protocol)(struct dsa_switch *ds);
 
 	int	(*setup)(struct dsa_switch *ds);
-	int	(*set_addr)(struct dsa_switch *ds, u8 *addr);
 	u32	(*get_phy_flags)(struct dsa_switch *ds, int port);
 
 	/*
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 54ed054777bd..6ac9e11d385c 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -336,12 +336,6 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 	if (err)
 		return err;
 
-	if (ds->ops->set_addr) {
-		err = ds->ops->set_addr(ds, dst->cpu_dp->netdev->dev_addr);
-		if (err < 0)
-			return err;
-	}
-
 	if (!ds->slave_mii_bus && ds->ops->phy_read) {
 		ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
 		if (!ds->slave_mii_bus)
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 19ff6e0a21dc..b0fefbffe082 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -172,12 +172,6 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 	if (ret)
 		return ret;
 
-	if (ops->set_addr) {
-		ret = ops->set_addr(ds, master->dev_addr);
-		if (ret < 0)
-			return ret;
-	}
-
 	if (!ds->slave_mii_bus && ops->phy_read) {
 		ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
 		if (!ds->slave_mii_bus)
-- 
2.14.2

^ permalink raw reply related

* [PATCH net-next v3 4/5] net: dsa: dsa_loop: remove .set_addr
From: Vivien Didelot @ 2017-10-13 18:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, David Laight, Vivien Didelot
In-Reply-To: <20171013181809.14627-1-vivien.didelot@savoirfairelinux.com>

The .set_addr function does nothing, remove the dsa_loop implementation
before getting rid of it completely in DSA.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/dsa_loop.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index d55051abf4ed..3a3f4f7ba364 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -110,13 +110,6 @@ static void dsa_loop_get_ethtool_stats(struct dsa_switch *ds, int port,
 		data[i] = ps->ports[port].mib[i].val;
 }
 
-static int dsa_loop_set_addr(struct dsa_switch *ds, u8 *addr)
-{
-	dev_dbg(ds->dev, "%s\n", __func__);
-
-	return 0;
-}
-
 static int dsa_loop_phy_read(struct dsa_switch *ds, int port, int regnum)
 {
 	struct dsa_loop_priv *ps = ds->priv;
@@ -263,7 +256,6 @@ static const struct dsa_switch_ops dsa_loop_driver = {
 	.get_strings		= dsa_loop_get_strings,
 	.get_ethtool_stats	= dsa_loop_get_ethtool_stats,
 	.get_sset_count		= dsa_loop_get_sset_count,
-	.set_addr		= dsa_loop_set_addr,
 	.phy_read		= dsa_loop_phy_read,
 	.phy_write		= dsa_loop_phy_write,
 	.port_bridge_join	= dsa_loop_port_bridge_join,
-- 
2.14.2

^ permalink raw reply related

* [PATCH net-next v3 3/5] net: dsa: mv88e6060: setup random mac address
From: Vivien Didelot @ 2017-10-13 18:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, David Laight, Vivien Didelot
In-Reply-To: <20171013181809.14627-1-vivien.didelot@savoirfairelinux.com>

As for mv88e6xxx, setup the switch from within the mv88e6060 driver with
a random MAC address, and remove the .set_addr implementation.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6060.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index d64be2b83d3c..6173be889d95 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/etherdevice.h>
 #include <linux/jiffies.h>
 #include <linux/list.h>
 #include <linux/module.h>
@@ -188,6 +189,27 @@ static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 	return 0;
 }
 
+static int mv88e6060_setup_addr(struct dsa_switch *ds)
+{
+	u8 addr[ETH_ALEN];
+	u16 val;
+
+	eth_random_addr(addr);
+
+	val = addr[0] << 8 | addr[1];
+
+	/* The multicast bit is always transmitted as a zero, so the switch uses
+	 * bit 8 for "DiffAddr", where 0 means all ports transmit the same SA.
+	 */
+	val &= 0xfeff;
+
+	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, val);
+	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]);
+	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]);
+
+	return 0;
+}
+
 static int mv88e6060_setup(struct dsa_switch *ds)
 {
 	int ret;
@@ -203,6 +225,10 @@ static int mv88e6060_setup(struct dsa_switch *ds)
 	if (ret < 0)
 		return ret;
 
+	ret = mv88e6060_setup_addr(ds);
+	if (ret < 0)
+		return ret;
+
 	for (i = 0; i < MV88E6060_PORTS; i++) {
 		ret = mv88e6060_setup_port(ds, i);
 		if (ret < 0)
@@ -212,22 +238,6 @@ static int mv88e6060_setup(struct dsa_switch *ds)
 	return 0;
 }
 
-static int mv88e6060_set_addr(struct dsa_switch *ds, u8 *addr)
-{
-	u16 val = addr[0] << 8 | addr[1];
-
-	/* The multicast bit is always transmitted as a zero, so the switch uses
-	 * bit 8 for "DiffAddr", where 0 means all ports transmit the same SA.
-	 */
-	val &= 0xfeff;
-
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, val);
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]);
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]);
-
-	return 0;
-}
-
 static int mv88e6060_port_to_phy_addr(int port)
 {
 	if (port >= 0 && port < MV88E6060_PORTS)
@@ -262,7 +272,6 @@ static const struct dsa_switch_ops mv88e6060_switch_ops = {
 	.get_tag_protocol = mv88e6060_get_tag_protocol,
 	.probe		= mv88e6060_drv_probe,
 	.setup		= mv88e6060_setup,
-	.set_addr	= mv88e6060_set_addr,
 	.phy_read	= mv88e6060_phy_read,
 	.phy_write	= mv88e6060_phy_write,
 };
-- 
2.14.2

^ permalink raw reply related

* [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Vivien Didelot @ 2017-10-13 18:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, David Laight, Vivien Didelot

An Ethernet switch may support having a MAC address, which can be used
as the switch's source address in transmitted full-duplex Pause frames.

If a DSA switch supports the related .set_addr operation, the DSA core
sets the master's MAC address on the switch.

This won't make sense anymore in a multi-CPU ports system, because there
won't be a unique master device assigned to a switch tree.

Moreover this operation is confusing because it makes the user think
that it could be used to program the switch with the MAC address of the
CPU/management port such that MAC address learning can be disabled on
said port, but in fact, that's not how it is currently used.

To fix this, assign a random MAC address at setup time in the mv88e6060
and mv88e6xxx drivers before removing .set_addr completely from DSA.

Changes in v3:
  - include fix for mv88e6060 switch MAC address setter.

Changes in v2:
  - remove .set_addr implementation from drivers and use a random MAC.

Vivien Didelot (5):
  net: dsa: mv88e6xxx: setup random mac address
  net: dsa: mv88e6060: fix switch MAC address
  net: dsa: mv88e6060: setup random mac address
  net: dsa: dsa_loop: remove .set_addr
  net: dsa: remove .set_addr

 drivers/net/dsa/dsa_loop.c       |  8 --------
 drivers/net/dsa/mv88e6060.c      | 37 ++++++++++++++++++++++++++-----------
 drivers/net/dsa/mv88e6xxx/chip.c | 33 +++++++++++++++++----------------
 include/net/dsa.h                |  1 -
 net/dsa/dsa2.c                   |  6 ------
 net/dsa/legacy.c                 |  6 ------
 6 files changed, 43 insertions(+), 48 deletions(-)

-- 
2.14.2

^ permalink raw reply

* RE: [PATCH net-next v2 2/4] net: dsa: mv88e6060: setup random mac address
From: Woojung.Huh @ 2017-10-13 17:59 UTC (permalink / raw)
  To: vivien.didelot, David.Laight, netdev
  Cc: linux-kernel, kernel, davem, f.fainelli, andrew
In-Reply-To: <871sm7j8ub.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

Hi Vivien,

> >> > +	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, (addr[0] << 9) |
> >> addr[1]);
> >>
> >> Is that supposed to be 9 ?
> >
> > Looks like it.
> > Check
> http://www.marvell.com/switching/assets/marvell_linkstreet_88E6060_data
> sheet.pdf
> 
> Hum, David is correct, there is a bug in the driver which needs to be
> addressed first. MAC address bit 40 is addr[0] & 0x1, thus we must
> shift byte 0 by 8 and mask it against 0xfe.
> 
> I'll respin this serie including a fix for both net and net-next.

Yes, you are right. Missed description about bit 40.

Thanks.
Woojung

^ permalink raw reply

* Re: [PATCH] tests: Remove bashisms (s/source/.)
From: Rustad, Mark D @ 2017-10-13 17:55 UTC (permalink / raw)
  To: Petr Vorel; +Cc: netdev@vger.kernel.org
In-Reply-To: <20171008143916.21813-1-petr.vorel@gmail.com>

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

> On Oct 8, 2017, at 7:39 AM, Petr Vorel <petr.vorel@gmail.com> wrote:
> 
> diff --git a/testsuite/tests/ip/route/add_default_route.t b/testsuite/tests/ip/route/add_default_route.t
> index e5ea6473..0b566f1f 100755
> --- a/testsuite/tests/ip/route/add_default_route.t
> +++ b/testsuite/tests/ip/route/add_default_route.t
> @@ -1,6 +1,6 @@
> -#!/bin/sh
> +#!/bin/bash

Funny - ^^^ choosing bash explicitly while
   vvvv removing the bashism?

> -source lib/generic.sh
> +. lib/generic.sh
> 
> ts_log "[Testing add default route]"

I noticed a couple other files already specified /bin/bash, yet you removed the bashisms. But the above struck me as something that would not seem to have been intended.

^ permalink raw reply

* [PATCH net] net: dsa: mv88e6060: fix switch MAC address
From: Vivien Didelot @ 2017-10-13 17:39 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, David Laight, Vivien Didelot

The 88E6060 Ethernet switch always transmits the multicast bit of the
switch MAC address as a zero. It re-uses the corresponding bit 8 of the
register "Switch MAC Address Register Bytes 0 & 1" for "DiffAddr".

If the "DiffAddr" bit is 0, then all ports transmit the same source
address. If it is set to 1, then bit 2:0 are used for the port number.

The mv88e6060 driver is currently wrongly shifting the MAC address byte
0 by 9. To fix this, shift it by 8 as usual and clear its bit 0.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6060.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 621cdc46ad81..d64be2b83d3c 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -214,8 +214,14 @@ static int mv88e6060_setup(struct dsa_switch *ds)
 
 static int mv88e6060_set_addr(struct dsa_switch *ds, u8 *addr)
 {
-	/* Use the same MAC Address as FD Pause frames for all ports */
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, (addr[0] << 9) | addr[1]);
+	u16 val = addr[0] << 8 | addr[1];
+
+	/* The multicast bit is always transmitted as a zero, so the switch uses
+	 * bit 8 for "DiffAddr", where 0 means all ports transmit the same SA.
+	 */
+	val &= 0xfeff;
+
+	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, val);
 	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]);
 	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]);
 
-- 
2.14.2

^ permalink raw reply related

* Re: [PATCH v2 5/5] fsl/fman: add dpaa in module names
From: Florian Fainelli @ 2017-10-13 17:39 UTC (permalink / raw)
  To: madalin.bucur, netdev, davem; +Cc: andrew, vivien.didelot, junote, linux-kernel
In-Reply-To: <1507906212-10076-6-git-send-email-madalin.bucur@nxp.com>

On 10/13/2017 07:50 AM, Madalin Bucur wrote:
> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>

You should provide a line or two to explain why are you making this
change, it is to resolve modular build configurations?

> ---
>  drivers/net/ethernet/freescale/fman/Makefile | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/Makefile b/drivers/net/ethernet/freescale/fman/Makefile
> index 2c38119..4ae524a 100644
> --- a/drivers/net/ethernet/freescale/fman/Makefile
> +++ b/drivers/net/ethernet/freescale/fman/Makefile
> @@ -1,9 +1,9 @@
>  subdir-ccflags-y +=  -I$(srctree)/drivers/net/ethernet/freescale/fman
>  
> -obj-$(CONFIG_FSL_FMAN) += fsl_fman.o
> -obj-$(CONFIG_FSL_FMAN) += fsl_fman_port.o
> -obj-$(CONFIG_FSL_FMAN) += fsl_mac.o
> +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman.o
> +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman_port.o
> +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_mac.o
>  
> -fsl_fman-objs	:= fman_muram.o fman.o fman_sp.o fman_keygen.o
> -fsl_fman_port-objs := fman_port.o
> -fsl_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o
> +fsl_dpaa_fman-objs	:= fman_muram.o fman.o fman_sp.o fman_keygen.o
> +fsl_dpaa_fman_port-objs := fman_port.o
> +fsl_dpaa_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o
> 


-- 
Florian

^ permalink raw reply

* Re: [Patch net-next v2] tcp: add a tracepoint for tcp_retransmit_skb()
From: Cong Wang @ 2017-10-13 17:33 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Linux Kernel Network Developers, Eric Dumazet,
	Hannes Frederic Sowa, Brendan Gregg, Neal Cardwell
In-Reply-To: <20171012231839.upwbtco3h524mmui@ast-mbp>

On Thu, Oct 12, 2017 at 4:18 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Oct 12, 2017 at 03:48:07PM -0700, Cong Wang wrote:
>> We need a real-time notification for tcp retransmission
>> for monitoring.
>>
>> Of course we could use ftrace to dynamically instrument this
>> kernel function too, however we can't retrieve the connection
>> information at the same time, for example perf-tools [1] reads
>> /proc/net/tcp for socket details, which is slow when we have
>> a lots of connections.
>>
>> Therefore, this patch adds a tracepoint for tcp_retransmit_skb()
>> and exposes src/dst IP addresses and ports of the connection.
>> This also makes it easier to integrate into perf.
>>
>> Note, I expose both IPv4 and IPv6 addresses at the same time:
>> for a IPv4 socket, v4 mapped address is used as IPv6 addresses,
>> for a IPv6 socket, LOOPBACK4_IPV6 is already filled by kernel.
>> Also, add sk and skb pointers as they are useful for BPF.
>>
>> 1. https://github.com/brendangregg/perf-tools/blob/master/net/tcpretrans
>>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
>> Cc: Neal Cardwell <ncardwell@google.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> ---
>>  include/trace/events/tcp.h | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>>  net/core/net-traces.c      |  1 +
>>  net/ipv4/tcp_output.c      |  3 ++
>>  3 files changed, 72 insertions(+)
>>  create mode 100644 include/trace/events/tcp.h
>>
>> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
>> new file mode 100644
>> index 000000000000..749f93c542ab
>> --- /dev/null
>> +++ b/include/trace/events/tcp.h
>> @@ -0,0 +1,68 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM tcp
>> +
>> +#if !defined(_TRACE_TCP_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_TCP_H
>> +
>> +#include <linux/ipv6.h>
>> +#include <linux/tcp.h>
>> +#include <linux/tracepoint.h>
>> +#include <net/ipv6.h>
>> +
>> +TRACE_EVENT(tcp_retransmit_skb,
>> +
>> +     TP_PROTO(struct sock *sk, struct sk_buff *skb, int segs),
>> +
>> +     TP_ARGS(sk, skb, segs),
>> +
>> +     TP_STRUCT__entry(
>> +             __field(void *, skbaddr)
>> +             __field(void *, skaddr)
>> +             __field(__u16, sport)
>> +             __field(__u16, dport)
>> +             __array(__u8, saddr, 4)
>> +             __array(__u8, daddr, 4)
>> +             __array(__u8, saddr_v6, 16)
>> +             __array(__u8, daddr_v6, 16)
>> +     ),
> ...
>>       if (likely(!err)) {
>>               TCP_SKB_CB(skb)->sacked |= TCPCB_EVER_RETRANS;
>> +             trace_tcp_retransmit_skb(sk, skb, segs);
>
> looks great to me, but why 'segs' is there?
> It's unused.

Ah, I copy-n-paste the tcp_retransmit_skb() prototype...
I will remove it.

Thanks.

^ permalink raw reply

* Re: Fw: [Bug 197213] New: panic in interrupt after ioctl to tun
From: Cong Wang @ 2017-10-13 17:31 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Linux Kernel Network Developers, avekceeb
In-Reply-To: <20171013081117.61e82817@xeon-e3>

On Fri, Oct 13, 2017 at 8:11 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> Hi,
>
> this is one more corner case found by syzkaller.
> I'm not sure that 'Networking' is the right category for this, but the panic
> was triggered by ioctl to /dev/net/tun...
>
>
> [   13.728009] BUG: unable to handle kernel NULL pointer dereference at
>   (null)
> [   13.728903] IP: run_timer_softirq+0x315/0x3f0
> [   13.729401] PGD 7bd8b067 P4D 7bd8b067 PUD 7bd7f067 PMD 0
> [   13.730040] Oops: 0002 [#1] SMP
> [   13.730400] Modules linked in:
> [   13.730747] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc4-with-tun #1
> [   13.731533] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.0.0-prebuilt.qemu-project.org 04/01/2014
> [   13.732672] task: ffffffffa280f480 task.stack: ffffffffa2800000
> [   13.733332] RIP: 0010:run_timer_softirq+0x315/0x3f0
> [   13.733883] RSP: 0018:ffff961b7fc03ed0 EFLAGS: 00010086
> [   13.734467] RAX: ffff961b7bf070c0 RBX: ffff961b7fc10cc0 RCX:
> 0000000000000000
> [   13.735265] RDX: dead000000000200 RSI: 00000000fffffe01 RDI:
> ffff961b7fc10cc0
> [   13.736059] RBP: ffff961b7fc03f50 R08: 00000000fffba1c0 R09:
> ffff961b7fc11168
> [   13.736857] R10: ffff961b7fc03ee8 R11: ffff961b7fc10d30 R12:
> ffff961b7fc03ee0
> [   13.737652] R13: dead000000000200 R14: 0000000000000001 R15:
> ffff961b7bf070c0
> [   13.738463] FS:  0000000000000000(0000) GS:ffff961b7fc00000(0000)
> knlGS:0000000000000000
> [   13.739017] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   13.739339] CR2: 0000000000000000 CR3: 000000007bcf8000 CR4:
> 00000000000006f0
> [   13.739741] Call Trace:
> [   13.739882]  <IRQ>
> [   13.740000]  ? ktime_get+0x3b/0x90
> [   13.740196]  ? lapic_next_event+0x18/0x20
> [   13.740413]  __do_softirq+0xcf/0x2a8
> [   13.740606]  irq_exit+0xab/0xb0
> [   13.740778]  smp_apic_timer_interrupt+0x64/0x110
> [   13.741025]  apic_timer_interrupt+0x90/0xa0
> [   13.741250]  </IRQ>
> [   13.741367] RIP: 0010:default_idle+0x18/0xf0
> [   13.741596] RSP: 0018:ffffffffa2803e60 EFLAGS: 00000246 ORIG_RAX:
> ffffffffffffff10
> [   13.741998] RAX: 0000000080000000 RBX: ffffffffa293f5e0 RCX:
> 0000000000000000
> [   13.742370] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
> 0000000000000000
> [   13.742750] RBP: ffffffffa2803e78 R08: 000000040a453dcd R09:
> ffff9c324031f930
> [   13.743128] R10: 0000000000000000 R11: 00000069d14f9aee R12:
> 0000000000000000
> [   13.743504] R13: 0000000000000000 R14: ffffffffa2a37780 R15:
> 0000000000000000
> [   13.743883]  arch_cpu_idle+0xa/0x10
> [   13.744072]  default_idle_call+0x1e/0x30
> [   13.744284]  do_idle+0x14f/0x1a0
> [   13.744458]  cpu_startup_entry+0x18/0x20
> [   13.744670]  rest_init+0xa9/0xb0
> [   13.744845]  start_kernel+0x3c6/0x3d3
> [   13.745043]  x86_64_start_reservations+0x24/0x26
> [   13.745291]  x86_64_start_kernel+0x6f/0x72
> [   13.745512]  secondary_startup_64+0xa5/0xa5
> [   13.745741] Code: 88 4c 39 65 88 0f 84 3b ff ff ff 49 8b 04 24 48 85 c0 74
> 56 4d 8b 3c 24 4c 89 7b 08 0f 1f 44 00 00 49 8b 17 49 8b 4f 08 48 85 d2 <48> 89
> 11 74 04 48 89 4a 08 41 f6 47 2a 20 49 c7 47 08 00 00 00
> [   13.746745] RIP: run_timer_softirq+0x315/0x3f0 RSP: ffff961b7fc03ed0
> [   13.747087] CR2: 0000000000000000
> [   13.747270] ---[ end trace 04d492145975c7cc ]---
> [   13.747516] Kernel panic - not syncing: Fatal exception in interrupt
> [   13.747946] Kernel Offset: 0x20a00000 from 0xffffffff81000000 (relocation
> range: 0xffffffff80000000-0xffffffffbfffffff)
> [   13.748515] ---[ end Kernel panic - not syncing: Fatal exception in
> interrupt
>
> Reproducer:
>
> #include <sys/syscall.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <fcntl.h>
>
> char addr[40] = {0xcf, 0x0b, 0x0b, 0x99, 0x22, 0x33, 0x96, 0xdf, 0xbd, 0x2e,
> 0x29, 0x1b, 0x4d, 0xc0, 0x2a, 0xee, 0x03};
>
> void test() {
>     int fd = -1;
>     fd = open("/dev/net/tun", 0, 0);
>     syscall(__NR_ioctl, fd, 0x400454caul, addr);
> }
>
> #define max_iter 10
> int main(void) {
>         int iter;
>         for (iter = 0; iter<max_iter; iter++) {
>         test();
>             printf("done %d of %d\n", iter+1, max_iter);
>         }
>     return 0;
> }

I just make a patch to fix this, however it uncovers another bug,
so I am trying to fix both of them (if not more)...


Thanks!

^ permalink raw reply

* [PATCH net] l2tp: check ps->sock before running pppol2tp_session_ioctl()
From: Guillaume Nault @ 2017-10-13 17:22 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman, Tom Parkin

When pppol2tp_session_ioctl() is called by pppol2tp_tunnel_ioctl(),
the session may be unconnected. That is, it was created by
pppol2tp_session_create() and hasn't been connected with
pppol2tp_connect(). In this case, ps->sock is NULL, so we need to check
for this case in order to avoid dereferencing a NULL pointer.

Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_ppp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index bc6e8bfc5be4..f50452b919d5 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -988,6 +988,9 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 		 session->name, cmd, arg);
 
 	sk = ps->sock;
+	if (!sk)
+		return -EBADR;
+
 	sock_hold(sk);
 
 	switch (cmd) {
-- 
2.15.0.rc0

^ permalink raw reply related

* Re: [PATCH] net: stmmac: dwmac_lib: fix interchanged sleep/timeout values in DMA reset function
From: David Miller @ 2017-10-13 17:20 UTC (permalink / raw)
  To: ingrassia; +Cc: peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20171012090047.GA4479@ingrassia.epigenesys.com>

From: Emiliano Ingrassia <ingrassia@epigenesys.com>
Date: Thu, 12 Oct 2017 11:00:47 +0200

> The DMA reset timeout, used in read_poll_timeout, is
> ten times shorter than the sleep time.
> This patch fixes these values interchanging them, as it was
> before the read_poll_timeout introduction.
> 
> Fixes: 8a70aeca80c2 ("net: stmmac: Use readl_poll_timeout")
> 
> Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com>

Applied.

^ permalink raw reply

* Re: [PATCH] [net] liquidio: fix timespec64_to_ns typo
From: David Miller @ 2017-10-13 17:19 UTC (permalink / raw)
  To: arnd
  Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
	intiyaz.basha, ricardo.farrington, veerasenareddy.burru, netdev,
	linux-kernel
In-Reply-To: <20171012094841.3178599-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 12 Oct 2017 11:48:31 +0200

> While experimenting with changes to the timekeeping code, I
> ran into a build error in the liquidio driver:
> 
> drivers/net/ethernet/cavium/liquidio/lio_main.c: In function 'liquidio_ptp_settime':
> drivers/net/ethernet/cavium/liquidio/lio_main.c:1850:22: error: passing argument 1 of 'timespec_to_ns' from incompatible pointer type [-Werror=incompatible-pointer-types]
> 
> The driver had a type mismatch since it was first merged, but
> this never caused problems because it is only built on 64-bit
> architectures that define timespec and timespec64 to the same
> type.
> 
> If we ever want to compile-test the driver on 32-bit or change
> the way that 64-bit timespec64 is defined, we need to fix it,
> so let's just do it now.
> 
> Fixes: f21fb3ed364b ("Add support of Cavium Liquidio ethernet adapters")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

^ permalink raw reply

* Re: [next-queue PATCH v7 4/6] net/sched: Introduce Credit Based Shaper (CBS) qdisc
From: Vinicius Costa Gomes @ 2017-10-13 17:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, intel-wired-lan, jhs, xiyou.wangcong, jiri, andre.guedes,
	ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
	richardcochran, henrik, levipearson, rodney.cummings
In-Reply-To: <1507862553.31614.53.camel@edumazet-glaptop3.roam.corp.google.com>

Hi,

Eric Dumazet <eric.dumazet@gmail.com> writes:

[...]

>
> Your mixing of s64 and u64 is disturbing.
>
> do_div() handles u64, not s64.
>
> div64_s64() might be needed in place of do_div()

I wasn't very comfortable about the signal juggling either. Didn't know
about div64_s64(), looks much better. Will fix, thanks.


Cheers,

^ permalink raw reply

* Re: [PATCH v1] pch_gbe: Switch to new PCI IRQ allocation API
From: Andy Shevchenko @ 2017-10-13 17:04 UTC (permalink / raw)
  To: David S. Miller, netdev
In-Reply-To: <20171013170221.70056-1-andriy.shevchenko@linux.intel.com>

On Fri, 2017-10-13 at 20:02 +0300, Andy Shevchenko wrote:
> This removes custom flag handling.

Please, discard this one, slipped to early.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |  3 +-
>  .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 42 +++++++++--
> -----------
>  2 files changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
> b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
> index 8d710a3b4db0..697e29dd4bd3 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
> @@ -613,7 +613,6 @@ struct pch_gbe_privdata {
>   * @rx_ring:		Pointer of Rx descriptor ring structure
>   * @rx_buffer_len:	Receive buffer length
>   * @tx_queue_len:	Transmit queue length
> - * @have_msi:		PCI MSI mode flag
>   * @pch_gbe_privdata:	PCI Device ID driver_data
>   */
>  
> @@ -623,6 +622,7 @@ struct pch_gbe_adapter {
>  	atomic_t irq_sem;
>  	struct net_device *netdev;
>  	struct pci_dev *pdev;
> +	int irq;
>  	struct net_device *polling_netdev;
>  	struct napi_struct napi;
>  	struct pch_gbe_hw hw;
> @@ -637,7 +637,6 @@ struct pch_gbe_adapter {
>  	struct pch_gbe_rx_ring *rx_ring;
>  	unsigned long rx_buffer_len;
>  	unsigned long tx_queue_len;
> -	bool have_msi;
>  	bool rx_stop_flag;
>  	int hwts_tx_en;
>  	int hwts_rx_en;
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> index 5ae9681a2da7..0284a3bc019c 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -781,11 +781,8 @@ static void pch_gbe_free_irq(struct
> pch_gbe_adapter *adapter)
>  {
>  	struct net_device *netdev = adapter->netdev;
>  
> -	free_irq(adapter->pdev->irq, netdev);
> -	if (adapter->have_msi) {
> -		pci_disable_msi(adapter->pdev);
> -		netdev_dbg(netdev, "call pci_disable_msi\n");
> -	}
> +	free_irq(adapter->irq, netdev);
> +	pci_free_irq_vectors(adapter->pdev);
>  }
>  
>  /**
> @@ -799,7 +796,7 @@ static void pch_gbe_irq_disable(struct
> pch_gbe_adapter *adapter)
>  	atomic_inc(&adapter->irq_sem);
>  	iowrite32(0, &hw->reg->INT_EN);
>  	ioread32(&hw->reg->INT_ST);
> -	synchronize_irq(adapter->pdev->irq);
> +	synchronize_irq(adapter->irq);
>  
>  	netdev_dbg(adapter->netdev, "INT_EN reg : 0x%08x\n",
>  		   ioread32(&hw->reg->INT_EN));
> @@ -1903,30 +1900,23 @@ static int pch_gbe_request_irq(struct
> pch_gbe_adapter *adapter)
>  {
>  	struct net_device *netdev = adapter->netdev;
>  	int err;
> -	int flags;
>  
> -	flags = IRQF_SHARED;
> -	adapter->have_msi = false;
> -	err = pci_enable_msi(adapter->pdev);
> -	netdev_dbg(netdev, "call pci_enable_msi\n");
> -	if (err) {
> -		netdev_dbg(netdev, "call pci_enable_msi - Error:
> %d\n", err);
> -	} else {
> -		flags = 0;
> -		adapter->have_msi = true;
> -	}
> -	err = request_irq(adapter->pdev->irq, &pch_gbe_intr,
> -			  flags, netdev->name, netdev);
> +	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> +	if (err < 0)
> +		return err;
> +
> +	adapter->irq = pci_irq_vector(pdev, 0);
> +
> +	err = request_irq(adapter->irq, &pch_gbe_intr, IRQF_SHARED,
> +			  netdev->name, netdev);
>  	if (err)
>  		netdev_err(netdev, "Unable to allocate interrupt
> Error: %d\n",
>  			   err);
> -	netdev_dbg(netdev,
> -		   "adapter->have_msi : %d  flags : 0x%04x  return :
> 0x%04x\n",
> -		   adapter->have_msi, flags, err);
> +	netdev_dbg(netdev, "have_msi : %d  return : 0x%04x\n",
> +		   pci_dev_msi_enabled(adapter->pdev), err);
>  	return err;
>  }
>  
> -
>  /**
>   * pch_gbe_up - Up GbE network device
>   * @adapter:  Board private structure
> @@ -2399,9 +2389,9 @@ static void pch_gbe_netpoll(struct net_device
> *netdev)
>  {
>  	struct pch_gbe_adapter *adapter = netdev_priv(netdev);
>  
> -	disable_irq(adapter->pdev->irq);
> -	pch_gbe_intr(adapter->pdev->irq, netdev);
> -	enable_irq(adapter->pdev->irq);
> +	disable_irq(adapter->irq);
> +	pch_gbe_intr(adapter->irq, netdev);
> +	enable_irq(adapter->irq);
>  }
>  #endif
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* [PATCH v1] pch_gbe: Switch to new PCI IRQ allocation API
From: Andy Shevchenko @ 2017-10-13 17:02 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Andy Shevchenko

This removes custom flag handling.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |  3 +-
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 42 +++++++++-------------
 2 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 8d710a3b4db0..697e29dd4bd3 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -613,7 +613,6 @@ struct pch_gbe_privdata {
  * @rx_ring:		Pointer of Rx descriptor ring structure
  * @rx_buffer_len:	Receive buffer length
  * @tx_queue_len:	Transmit queue length
- * @have_msi:		PCI MSI mode flag
  * @pch_gbe_privdata:	PCI Device ID driver_data
  */
 
@@ -623,6 +622,7 @@ struct pch_gbe_adapter {
 	atomic_t irq_sem;
 	struct net_device *netdev;
 	struct pci_dev *pdev;
+	int irq;
 	struct net_device *polling_netdev;
 	struct napi_struct napi;
 	struct pch_gbe_hw hw;
@@ -637,7 +637,6 @@ struct pch_gbe_adapter {
 	struct pch_gbe_rx_ring *rx_ring;
 	unsigned long rx_buffer_len;
 	unsigned long tx_queue_len;
-	bool have_msi;
 	bool rx_stop_flag;
 	int hwts_tx_en;
 	int hwts_rx_en;
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 5ae9681a2da7..0284a3bc019c 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -781,11 +781,8 @@ static void pch_gbe_free_irq(struct pch_gbe_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
 
-	free_irq(adapter->pdev->irq, netdev);
-	if (adapter->have_msi) {
-		pci_disable_msi(adapter->pdev);
-		netdev_dbg(netdev, "call pci_disable_msi\n");
-	}
+	free_irq(adapter->irq, netdev);
+	pci_free_irq_vectors(adapter->pdev);
 }
 
 /**
@@ -799,7 +796,7 @@ static void pch_gbe_irq_disable(struct pch_gbe_adapter *adapter)
 	atomic_inc(&adapter->irq_sem);
 	iowrite32(0, &hw->reg->INT_EN);
 	ioread32(&hw->reg->INT_ST);
-	synchronize_irq(adapter->pdev->irq);
+	synchronize_irq(adapter->irq);
 
 	netdev_dbg(adapter->netdev, "INT_EN reg : 0x%08x\n",
 		   ioread32(&hw->reg->INT_EN));
@@ -1903,30 +1900,23 @@ static int pch_gbe_request_irq(struct pch_gbe_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
 	int err;
-	int flags;
 
-	flags = IRQF_SHARED;
-	adapter->have_msi = false;
-	err = pci_enable_msi(adapter->pdev);
-	netdev_dbg(netdev, "call pci_enable_msi\n");
-	if (err) {
-		netdev_dbg(netdev, "call pci_enable_msi - Error: %d\n", err);
-	} else {
-		flags = 0;
-		adapter->have_msi = true;
-	}
-	err = request_irq(adapter->pdev->irq, &pch_gbe_intr,
-			  flags, netdev->name, netdev);
+	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (err < 0)
+		return err;
+
+	adapter->irq = pci_irq_vector(pdev, 0);
+
+	err = request_irq(adapter->irq, &pch_gbe_intr, IRQF_SHARED,
+			  netdev->name, netdev);
 	if (err)
 		netdev_err(netdev, "Unable to allocate interrupt Error: %d\n",
 			   err);
-	netdev_dbg(netdev,
-		   "adapter->have_msi : %d  flags : 0x%04x  return : 0x%04x\n",
-		   adapter->have_msi, flags, err);
+	netdev_dbg(netdev, "have_msi : %d  return : 0x%04x\n",
+		   pci_dev_msi_enabled(adapter->pdev), err);
 	return err;
 }
 
-
 /**
  * pch_gbe_up - Up GbE network device
  * @adapter:  Board private structure
@@ -2399,9 +2389,9 @@ static void pch_gbe_netpoll(struct net_device *netdev)
 {
 	struct pch_gbe_adapter *adapter = netdev_priv(netdev);
 
-	disable_irq(adapter->pdev->irq);
-	pch_gbe_intr(adapter->pdev->irq, netdev);
-	enable_irq(adapter->pdev->irq);
+	disable_irq(adapter->irq);
+	pch_gbe_intr(adapter->irq, netdev);
+	enable_irq(adapter->irq);
 }
 #endif
 
-- 
2.14.2

^ permalink raw reply related

* RE: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
From: Vivien Didelot @ 2017-10-13 16:50 UTC (permalink / raw)
  To: David Laight, 'Florian Fainelli', netdev@vger.kernel.org
  Cc: Andrew Lunn, kernel@savoirfairelinux.com,
	linux-kernel@vger.kernel.org, David S. Miller
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD0094FEF@AcuExch.aculab.com>

Hi David,

David Laight <David.Laight@ACULAB.COM> writes:

> From: Vivien Didelot
>> Sent: 13 October 2017 16:29
>> Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:
>> 
>> >>> How about using:
>> >>>
>> >>> 	union {
>> >>> 		struct net_device *master;
>> >>> 		struct net_device *slave;
>> >>> 	} netdev;
>> >> ...
>> >>
>> >> You can remove the 'netdev' all the compilers support unnamed unions.
>> >
>> > There are issues with older GCC versions, see the commit 42275bd8fcb3
>> > ("switchdev: don't use anonymous union on switchdev attr/obj structs")
>> >
>> > That's why I kept it in the v2 I sent.
>> 
>> At the same time, I can see that struct sk_buff uses anonym union a lot.
>> 
>> It seems weird that one raised a compiler issue for switchdev but not
>> for skbuff.h... Do you think it is viable to drop the name here then?
>
> I believe the problem is with initialisers for static structures
> that contain anonymous unions.

The dsa_port structures are dynamically allocated so this seems safe to
use an anonymous union here.

BTW v2 never left my computer in fact, so this will be fixed up in v2.


Thanks!

        Vivien

^ permalink raw reply

* [RFC PATCH] net: realtek: r8169: implement set_link_ksettings()
From: Tobias Jakobi @ 2017-10-13 16:44 UTC (permalink / raw)
  To: nic_swsd; +Cc: netdev, tremyfr, davem, Tobias Jakobi

Commit 6fa1ba61520576cf1346c4ff09a056f2950cb3bf partially
implemented the new ethtool API, by replacing get_settings()
with get_link_ksettings(). This breaks ethtool, since the
userspace tool (according to the new API specs) never tries
the legacy set() call, when the new get() call succeeds.

All attempts to chance some setting from userspace result in:
> Cannot set new settings: Operation not supported

Implement the missing set() call.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/net/ethernet/realtek/r8169.c | 40 +++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index e03fcf914690..24e8f7133038 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -2025,21 +2025,6 @@ static int rtl8169_set_speed(struct net_device *dev,
 	return ret;
 }
 
-static int rtl8169_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
-{
-	struct rtl8169_private *tp = netdev_priv(dev);
-	int ret;
-
-	del_timer_sync(&tp->timer);
-
-	rtl_lock_work(tp);
-	ret = rtl8169_set_speed(dev, cmd->autoneg, ethtool_cmd_speed(cmd),
-				cmd->duplex, cmd->advertising);
-	rtl_unlock_work(tp);
-
-	return ret;
-}
-
 static netdev_features_t rtl8169_fix_features(struct net_device *dev,
 	netdev_features_t features)
 {
@@ -2166,6 +2151,29 @@ static int rtl8169_get_link_ksettings(struct net_device *dev,
 	return rc;
 }
 
+static int rtl8169_set_link_ksettings(struct net_device *dev,
+				      const struct ethtool_link_ksettings *cmd)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+	int rc;
+	u32 advertising;
+
+	if (!ethtool_convert_link_mode_to_legacy_u32(&advertising,
+	    cmd->link_modes.advertising))
+		return -EINVAL;
+
+	del_timer_sync(&tp->timer);
+
+	rtl_lock_work(tp);
+
+	rc = rtl8169_set_speed(dev, cmd->base.autoneg, cmd->base.speed,
+			       cmd->base.duplex, advertising);
+
+	rtl_unlock_work(tp);
+
+	return rc;
+}
+
 static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			     void *p)
 {
@@ -2367,7 +2375,6 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.get_drvinfo		= rtl8169_get_drvinfo,
 	.get_regs_len		= rtl8169_get_regs_len,
 	.get_link		= ethtool_op_get_link,
-	.set_settings		= rtl8169_set_settings,
 	.get_msglevel		= rtl8169_get_msglevel,
 	.set_msglevel		= rtl8169_set_msglevel,
 	.get_regs		= rtl8169_get_regs,
@@ -2379,6 +2386,7 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.get_ts_info		= ethtool_op_get_ts_info,
 	.nway_reset		= rtl8169_nway_reset,
 	.get_link_ksettings	= rtl8169_get_link_ksettings,
+	.set_link_ksettings	= rtl8169_set_link_ksettings,
 };
 
 static void rtl8169_get_mac_version(struct rtl8169_private *tp,
-- 
2.13.5

^ permalink raw reply related

* RE: [PATCH net-next v2 2/4] net: dsa: mv88e6060: setup random mac address
From: Vivien Didelot @ 2017-10-13 16:44 UTC (permalink / raw)
  To: Woojung.Huh, David.Laight, netdev
  Cc: linux-kernel, kernel, davem, f.fainelli, andrew
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40B512EB@CHN-SV-EXMX02.mchp-main.com>

Hi David, Woojung,

Woojung.Huh@microchip.com writes:

>> From: Vivien Didelot
>> > Sent: 13 October 2017 02:41
>> > As for mv88e6xxx, setup the switch from within the mv88e6060 driver with
>> > a random MAC address, and remove the .set_addr implementation.
>> >
>> > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> > ---
>> >  drivers/net/dsa/mv88e6060.c | 30 +++++++++++++++++++-----------
>> >  1 file changed, 19 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
>> > index 621cdc46ad81..2f9d5e6a0f97 100644
>> > --- a/drivers/net/dsa/mv88e6060.c
>> > +++ b/drivers/net/dsa/mv88e6060.c
>> ...
>> > +	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, (addr[0] << 9) |
>> addr[1]);
>> 
>> Is that supposed to be 9 ?
>
> Looks like it.
> Check http://www.marvell.com/switching/assets/marvell_linkstreet_88E6060_datasheet.pdf

Hum, David is correct, there is a bug in the driver which needs to be
addressed first. MAC address bit 40 is addr[0] & 0x1, thus we must
shift byte 0 by 8 and mask it against 0xfe.

I'll respin this serie including a fix for both net and net-next.


Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE
From: Masahiro Yamada @ 2017-10-13 16:41 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn, Florian Fainelli,
	Rob Herring, Mark Rutland, linux-arm-kernel,
	Linux Kernel Mailing List, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Masami Hiramatsu, Jassi Brar
In-Reply-To: <1507854928-4357-2-git-send-email-hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>

2017-10-13 9:35 GMT+09:00 Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>:
> DT bindings for the AVE ethernet controller found on Socionext's
> UniPhier platforms.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> Signed-off-by: Jassi Brar <jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../bindings/net/socionext,uniphier-ave4.txt       | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
>
> diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> new file mode 100644
> index 0000000..25f4d92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> @@ -0,0 +1,53 @@
> +* Socionext AVE ethernet controller
> +
> +This describes the devicetree bindings for AVE ethernet controller
> +implemented on Socionext UniPhier SoCs.
> +
> +Required properties:
> + - compatible: Should be
> +       - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> +       - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> +       - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> +       - "socionext,uniphier-ld11-ave4" : for LD11 SoC

Nit.  LD11, LD20, in this order please.

> + - reg: Address where registers are mapped and size of region.
> + - interrupts: Should contain the MAC interrupt.
> + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> +       "rgmii", "rmii", or "mii" according to the PHY.
> + - pinctrl-names: List of assigned state names, see pinctrl
> +       binding documentation.
> + - pinctrl-0: List of phandles to configure the GPIO pin,


configure the GPIO pin?

git-grep found this phrase.


$ git grep "List of phandles to configure the GPIO"
Documentation/devicetree/bindings/net/microchip,enc28j60.txt:-
pinctrl-0: List of phandles to configure the GPIO pin used as
interrupt line,





> +       see pinctrl binding documentation. Choose this appropriately
> +       according to phy-mode.
> +       - <&pinctrl_ether_rgmii> if phy-mode is "rgmii".
> +       - <&pinctrl_ether_rmii> if phy-mode is "rmii".
> +       - <&pinctrl_ether_mii> if phy-mode is "mii".

pinctrl_ether_rgmii is just a label
you just happened to write in your DT file.

This information is totally unrelated to hardware specification.
It is not stable, either.



> + - phy-handle: Should point to the external phy device.
> +       See ethernet.txt file in the same directory.
> + - mdio subnode: Should be device tree subnode with the following required
> +       properties:
> +       - #address-cells: Must be <1>.
> +       - #size-cells: Must be <0>.
> +       - reg: phy ID number, usually a small integer.


Are you talking about "Required subnode" in the "Required properties"?




> +Optional properties:
> + - local-mac-address: See ethernet.txt in the same directory.
> +
> +Example:
> +
> +       ether: ethernet@65000000 {
> +               compatible = "socionext,uniphier-ld20-ave4";
> +               reg = <0x65000000 0x8500>;
> +               interrupts = <0 66 4>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pinctrl_ether_rgmii>;
> +               phy-mode = "rgmii";
> +               phy-handle = <&ethphy>;
> +               local-mac-address = [00 00 00 00 00 00];
> +               mdio {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       ethphy: ethphy@1 {
> +                               reg = <1>;
> +                       };
> +               };
> +       };
> --
> 2.7.4
>



-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


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