Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 net-next 2/2] net/ipv6: support more tunnel interfaces for EUI64 link-local generation
From: Felix Jia @ 2017-01-19 22:46 UTC (permalink / raw)
  To: network dev; +Cc: Felix Jia, Hermin Anggawijaya

Signed-off-by: Felix Jia <felix.jia@alliedtelesis.co.nz>
---
 net/ipv6/addrconf.c | 5 +++++
 net/ipv6/ip6_gre.c  | 3 +++
 net/ipv6/ip6_vti.c  | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index e35259dd17ba..4c47656b9f09 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2146,12 +2146,14 @@ static int ipv6_generate_eui64(u8 *eui, struct net_device *dev)
 	case ARPHRD_SIT:
 		return addrconf_ifid_sit(eui, dev);
 	case ARPHRD_IPGRE:
+	case ARPHRD_TUNNEL:
 		return addrconf_ifid_gre(eui, dev);
 	case ARPHRD_6LOWPAN:
 		return addrconf_ifid_eui64(eui, dev);
 	case ARPHRD_IEEE1394:
 		return addrconf_ifid_ieee1394(eui, dev);
 	case ARPHRD_TUNNEL6:
+	case ARPHRD_IP6GRE:
 		return addrconf_ifid_ip6tnl(eui, dev);
 	}
 	return -1;
@@ -3195,6 +3197,9 @@ static void addrconf_dev_config(struct net_device *dev)
 	    (dev->type != ARPHRD_IEEE1394) &&
 	    (dev->type != ARPHRD_TUNNEL6) &&
 	    (dev->type != ARPHRD_6LOWPAN) &&
+	    (dev->type != ARPHRD_IP6GRE) &&
+	    (dev->type != ARPHRD_IPGRE) &&
+	    (dev->type != ARPHRD_TUNNEL) &&
 	    (dev->type != ARPHRD_NONE)) {
 		/* Alas, we support only Ethernet autoconfiguration. */
 		return;
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 75b6108234dd..7f22e1510082 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -998,6 +998,9 @@ static void ip6gre_tunnel_setup(struct net_device *dev)
 	dev->flags |= IFF_NOARP;
 	dev->addr_len = sizeof(struct in6_addr);
 	netif_keep_dst(dev);
+	/* This perm addr will be used as interface identifier by IPv6 */
+	dev->addr_assign_type = NET_ADDR_RANDOM;
+	eth_random_addr(dev->perm_addr);
 }
 
 static int ip6gre_tunnel_init_common(struct net_device *dev)
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index d82042c8d8fd..c795fee372c4 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -49,6 +49,7 @@
 #include <net/xfrm.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
+#include <linux/etherdevice.h>
 
 #define IP6_VTI_HASH_SIZE_SHIFT  5
 #define IP6_VTI_HASH_SIZE (1 << IP6_VTI_HASH_SIZE_SHIFT)
@@ -842,6 +843,9 @@ static void vti6_dev_setup(struct net_device *dev)
 	dev->flags |= IFF_NOARP;
 	dev->addr_len = sizeof(struct in6_addr);
 	netif_keep_dst(dev);
+	/* This perm addr will be used as interface identifier by IPv6 */
+	dev->addr_assign_type = NET_ADDR_RANDOM;
+	eth_random_addr(dev->perm_addr);
 }
 
 /**
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 net-next 1/2] net/ipv6: allow sysctl to change link-local address generation mode
From: Felix Jia @ 2017-01-19 22:46 UTC (permalink / raw)
  To: network dev; +Cc: Felix Jia, Hermin Anggawijaya

The address generation mode for IPv6 link-local can only be configured
by netlink messages. This patch adds the ability to change the address
generation mode via sysctl.

v1 -> v2
Removed the rtnl lock and switch to use RCU lock to iterate through
the netdev list.

v2 -> v3
Removed the addrgenmode variable from the idev structure and use the 
systcl storage for the flag.

Simplifed the logic for sysctl handling by removing the supported 
for all operation.

Added support for more types of tunnel interfaces for link-local 
address generation.

Based the patches from net-next.


Signed-off-by: Felix Jia <felix.jia@alliedtelesis.co.nz>
---
 include/linux/ipv6.h      |   3 +-
 include/net/if_inet6.h    |   5 +--
 include/uapi/linux/ipv6.h |   5 ++-
 net/ipv6/addrconf.c       | 104 +++++++++++++++++++++++++++++++++++++---------
 4 files changed, 91 insertions(+), 26 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 671d014e6429..74d0e0913211 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -69,6 +69,7 @@ struct ipv6_devconf {
 	__s32		seg6_require_hmac;
 #endif
 	__u32		enhanced_dad;
+	__u32		addr_gen_mode;
 
 	struct ctl_table_header *sysctl_header;
 };
@@ -99,7 +100,7 @@ static inline struct ipv6hdr *ipipv6_hdr(const struct sk_buff *skb)
 	return (struct ipv6hdr *)skb_transport_header(skb);
 }
 
-/* 
+/*
    This structure contains results of exthdrs parsing
    as offsets from skb->nh.
  */
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 0fa4c324b713..48e2a34a6ed3 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -1,9 +1,9 @@
 /*
  *	inet6 interface/address list definitions
- *	Linux INET6 implementation 
+ *	Linux INET6 implementation
  *
  *	Authors:
- *	Pedro Roque		<roque@di.fc.ul.pt>	
+ *	Pedro Roque		<roque@di.fc.ul.pt>
  *
  *
  *	This program is free software; you can redistribute it and/or
@@ -205,7 +205,6 @@ struct inet6_dev {
 	__s32			rs_interval;	/* in jiffies */
 	__u8			rs_probes;
 
-	__u8			addr_gen_mode;
 	unsigned long		tstamp; /* ipv6InterfaceTable update timestamp */
 	struct rcu_head		rcu;
 };
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index eaf65dc82e22..7207b9275c28 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -33,7 +33,7 @@ struct ip6_mtuinfo {
 struct in6_ifreq {
 	struct in6_addr	ifr6_addr;
 	__u32		ifr6_prefixlen;
-	int		ifr6_ifindex; 
+	int		ifr6_ifindex;
 };
 
 #define IPV6_SRCRT_STRICT	0x01	/* Deprecated; will be removed */
@@ -60,7 +60,7 @@ struct ipv6_rt_hdr {
 struct ipv6_opt_hdr {
 	__u8 		nexthdr;
 	__u8 		hdrlen;
-	/* 
+	/*
 	 * TLV encoded option data follows.
 	 */
 } __attribute__((packed));	/* required for some archs */
@@ -182,6 +182,7 @@ enum {
 	DEVCONF_SEG6_ENABLED,
 	DEVCONF_SEG6_REQUIRE_HMAC,
 	DEVCONF_ENHANCED_DAD,
+	DEVCONF_ADDR_GEN_MODE,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ac9bd5620f81..e35259dd17ba 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -243,6 +243,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.seg6_require_hmac	= 0,
 #endif
 	.enhanced_dad           = 1,
+	.addr_gen_mode		= IN6_ADDR_GEN_MODE_EUI64,
 };
 
 static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
@@ -294,6 +295,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.seg6_require_hmac	= 0,
 #endif
 	.enhanced_dad           = 1,
+	.addr_gen_mode		= IN6_ADDR_GEN_MODE_EUI64,
 };
 
 /* Check if a valid qdisc is available */
@@ -386,9 +388,9 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 	memcpy(&ndev->cnf, dev_net(dev)->ipv6.devconf_dflt, sizeof(ndev->cnf));
 
 	if (ndev->cnf.stable_secret.initialized)
-		ndev->addr_gen_mode = IN6_ADDR_GEN_MODE_STABLE_PRIVACY;
+		ndev->cnf.addr_gen_mode = IN6_ADDR_GEN_MODE_STABLE_PRIVACY;
 	else
-		ndev->addr_gen_mode = IN6_ADDR_GEN_MODE_EUI64;
+		ndev->cnf.addr_gen_mode = ipv6_devconf_dflt.addr_gen_mode;
 
 	ndev->cnf.mtu6 = dev->mtu;
 	ndev->nd_parms = neigh_parms_alloc(dev, &nd_tbl);
@@ -2387,8 +2389,8 @@ static void manage_tempaddrs(struct inet6_dev *idev,
 
 static bool is_addr_mode_generate_stable(struct inet6_dev *idev)
 {
-	return idev->addr_gen_mode == IN6_ADDR_GEN_MODE_STABLE_PRIVACY ||
-	       idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
+	return idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_STABLE_PRIVACY ||
+	       idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
 }
 
 int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
@@ -3152,7 +3154,7 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
 
 	ipv6_addr_set(&addr, htonl(0xFE800000), 0, 0, 0);
 
-	switch (idev->addr_gen_mode) {
+	switch (idev->cnf.addr_gen_mode) {
 	case IN6_ADDR_GEN_MODE_RANDOM:
 		ipv6_gen_mode_random_init(idev);
 		/* fallthrough */
@@ -3204,8 +3206,8 @@ static void addrconf_dev_config(struct net_device *dev)
 
 	/* this device type has no EUI support */
 	if (dev->type == ARPHRD_NONE &&
-	    idev->addr_gen_mode == IN6_ADDR_GEN_MODE_EUI64)
-		idev->addr_gen_mode = IN6_ADDR_GEN_MODE_RANDOM;
+	    idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_EUI64)
+		idev->cnf.addr_gen_mode = IN6_ADDR_GEN_MODE_RANDOM;
 
 	addrconf_addr_gen(idev, false);
 }
@@ -4982,6 +4984,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_SEG6_REQUIRE_HMAC] = cnf->seg6_require_hmac;
 #endif
 	array[DEVCONF_ENHANCED_DAD] = cnf->enhanced_dad;
+	array[DEVCONF_ADDR_GEN_MODE] = cnf->addr_gen_mode;
 }
 
 static inline size_t inet6_ifla6_size(void)
@@ -5093,7 +5096,7 @@ static int inet6_fill_ifla6_attrs(struct sk_buff *skb, struct inet6_dev *idev,
 	if (!nla)
 		goto nla_put_failure;
 
-	if (nla_put_u8(skb, IFLA_INET6_ADDR_GEN_MODE, idev->addr_gen_mode))
+	if (nla_put_u8(skb, IFLA_INET6_ADDR_GEN_MODE, idev->cnf.addr_gen_mode))
 		goto nla_put_failure;
 
 	read_lock_bh(&idev->lock);
@@ -5211,6 +5214,26 @@ static int inet6_validate_link_af(const struct net_device *dev,
 	return nla_parse_nested(tb, IFLA_INET6_MAX, nla, inet6_af_policy);
 }
 
+static int check_addr_gen_mode(int mode)
+{
+	if (mode != IN6_ADDR_GEN_MODE_EUI64 &&
+	    mode != IN6_ADDR_GEN_MODE_NONE &&
+	    mode != IN6_ADDR_GEN_MODE_STABLE_PRIVACY &&
+	    mode != IN6_ADDR_GEN_MODE_RANDOM)
+		return -EINVAL;
+	return 1;
+}
+
+static int check_stable_privacy(struct inet6_dev *idev, struct net *net,
+				int mode)
+{
+	if (mode == IN6_ADDR_GEN_MODE_STABLE_PRIVACY &&
+	    !idev->cnf.stable_secret.initialized &&
+	    !net->ipv6.devconf_dflt->stable_secret.initialized)
+		return -EINVAL;
+	return 1;
+}
+
 static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla)
 {
 	int err = -EINVAL;
@@ -5232,18 +5255,11 @@ static int inet6_set_link_af(struct net_device *dev, const struct nlattr *nla)
 	if (tb[IFLA_INET6_ADDR_GEN_MODE]) {
 		u8 mode = nla_get_u8(tb[IFLA_INET6_ADDR_GEN_MODE]);
 
-		if (mode != IN6_ADDR_GEN_MODE_EUI64 &&
-		    mode != IN6_ADDR_GEN_MODE_NONE &&
-		    mode != IN6_ADDR_GEN_MODE_STABLE_PRIVACY &&
-		    mode != IN6_ADDR_GEN_MODE_RANDOM)
-			return -EINVAL;
-
-		if (mode == IN6_ADDR_GEN_MODE_STABLE_PRIVACY &&
-		    !idev->cnf.stable_secret.initialized &&
-		    !dev_net(dev)->ipv6.devconf_dflt->stable_secret.initialized)
+		if (check_addr_gen_mode(mode) < 0 ||
+		    check_stable_privacy(idev, dev_net(dev), mode) < 0)
 			return -EINVAL;
 
-		idev->addr_gen_mode = mode;
+		idev->cnf.addr_gen_mode = mode;
 		err = 0;
 	}
 
@@ -5652,6 +5668,47 @@ int addrconf_sysctl_proxy_ndp(struct ctl_table *ctl, int write,
 	return ret;
 }
 
+static int addrconf_sysctl_addr_gen_mode(struct ctl_table *ctl, int write,
+					 void __user *buffer, size_t *lenp,
+					 loff_t *ppos)
+{
+	int ret = 0;
+	int new_val;
+	struct inet6_dev *idev = (struct inet6_dev *)ctl->extra1;
+	struct net *net = (struct net *)ctl->extra2;
+
+	ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
+
+	if (write) {
+		new_val = *((int *)ctl->data);
+
+		if (check_addr_gen_mode(new_val) < 0)
+			return -EINVAL;
+
+		/* request for default */
+		if (&net->ipv6.devconf_dflt->addr_gen_mode == ctl->data) {
+			ipv6_devconf_dflt.addr_gen_mode = new_val;
+
+		/* request for individual net device */
+		} else {
+			if (!idev)
+				return ret;
+
+			if (check_stable_privacy(idev, net, new_val) < 0)
+				return -EINVAL;
+
+			if (idev->cnf.addr_gen_mode != new_val) {
+				idev->cnf.addr_gen_mode = new_val;
+				rtnl_lock();
+				addrconf_dev_config(idev->dev);
+				rtnl_unlock();
+			}
+		}
+	}
+
+	return ret;
+}
+
 static int addrconf_sysctl_stable_secret(struct ctl_table *ctl, int write,
 					 void __user *buffer, size_t *lenp,
 					 loff_t *ppos)
@@ -5702,14 +5759,14 @@ static int addrconf_sysctl_stable_secret(struct ctl_table *ctl, int write,
 			struct inet6_dev *idev = __in6_dev_get(dev);
 
 			if (idev) {
-				idev->addr_gen_mode =
+				idev->cnf.addr_gen_mode =
 					IN6_ADDR_GEN_MODE_STABLE_PRIVACY;
 			}
 		}
 	} else {
 		struct inet6_dev *idev = ctl->extra1;
 
-		idev->addr_gen_mode = IN6_ADDR_GEN_MODE_STABLE_PRIVACY;
+		idev->cnf.addr_gen_mode = IN6_ADDR_GEN_MODE_STABLE_PRIVACY;
 	}
 
 out:
@@ -6097,6 +6154,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.proc_handler   = proc_dointvec,
 	},
 	{
+		.procname		= "addr_gen_mode",
+		.data			= &ipv6_devconf.addr_gen_mode,
+		.maxlen			= sizeof(int),
+		.mode			= 0644,
+		.proc_handler	= addrconf_sysctl_addr_gen_mode,
+	},
+	{
 		/* sentinel */
 	}
 };
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v7 00/13] net: ethernet: aquantia: Add AQtion 2.5/5 GB NIC driver
From: Florian Fainelli @ 2017-01-19 22:50 UTC (permalink / raw)
  To: David VomLehn, Alexander Loktionov, netdev
  Cc: David S . Miller, Simon Edelhaus, Dmitrii Tarakanov, Pavel Belous,
	Dmitry Bezrukov
In-Reply-To: <4e7c386f-e0bd-7647-ee2d-0afdcc9e6a91@texas.net>

On 01/19/2017 02:45 PM, David VomLehn wrote:
> On 01/19/2017 02:41 PM, Florian Fainelli wrote:
>> On 01/19/2017 01:33 PM, Alexander Loktionov wrote:
>>> From: David VomLehn <vomlehn@texas.net>
>>>
>>> This series introduces the AQtion NIC driver for the aQuantia
>>> AQC107/AQC108 network devices.
>>> v5: o Removed extra newline at the end of the files.
>>> v6: o Removed unnecessary cast from void*.
>>>      o Reworked strings array for ethtool statistics.
>>>      o Added stringset == ETH_SS_STATS checking.
>>>      o AQ_OBJ_HEADER replaced to aq_obj_header_s struct.
>>>      o AQ_OBJ_SET/TST/CLR macroses replaced to inline functions.
>>>      o Driver sources placed in to atlantic directory.
>>>      o Fixed compilation warnings (Make W=1)
>>>      o Added firmware version checking.
>>>      o Code cleaning.
>>> v7  o Removed unnecessary cast from memory allocation function
>>> (aq_ring.c).
>> In general, you may want for a couple of days before resubmitting new
>> versions that only fix minor stuff like what happened here between v6
>> and v7.
> Yeah, the number of comments has dropped enough that I had hopes this
> last one might be it. It makes me, I know, a foolish dreamer, but such
> is the lot of a software developer...

The driver looks in an okay shape, from a cursory glance at the files,
there is quite a bit of complexity in reading the functional code,
primarily because of the complicated helper functions and abstraction
(not the HAL though)...
-- 
Florian

^ permalink raw reply

* RE: [PATCHv2 7/7] IPVTAP: IP-VLAN based tap driver
From: Grandhi, Sainath @ 2017-01-19 23:33 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, mahesh@bandewar.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170118.162842.2116473511473193362.davem@davemloft.net>



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, January 18, 2017 1:29 PM
> To: Grandhi, Sainath <sainath.grandhi@intel.com>
> Cc: netdev@vger.kernel.org; mahesh@bandewar.net; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCHv2 7/7] IPVTAP: IP-VLAN based tap driver
> 
> From: Sainath Grandhi <sainath.grandhi@intel.com>
> Date: Tue, 17 Jan 2017 16:03:06 -0800
> 
> > +static void ipvtap_count_tx_dropped(struct tap_dev *tap) {
> > +	struct ipvl_dev *vlan = (struct ipvl_dev *)container_of(tap, struct
> > +ipvtap_dev, tap);
>  ...
> > +static void ipvtap_count_rx_dropped(struct tap_dev *tap) {
> > +	struct ipvl_dev *vlan = (struct ipvl_dev *)container_of(tap, struct
> > +ipvtap_dev, tap);
>  ...
> > +static void ipvtap_update_features(struct tap_dev *tap,
> > +				   netdev_features_t features)
> > +{
> > +	struct ipvl_dev *vlan = (struct ipvl_dev *)container_of(tap, struct
> > +ipvtap_dev, tap);
> 
> More unnecessary casts, please remove.

Will take care of it in next revision. Thanks.

^ permalink raw reply

* RE: [PATCHv2 3/7] TAP: Tap character device creation/destroy API
From: Grandhi, Sainath @ 2017-01-19 23:33 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, mahesh@bandewar.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170118.162647.1034141344569503382.davem@davemloft.net>



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, January 18, 2017 1:27 PM
> To: Grandhi, Sainath <sainath.grandhi@intel.com>
> Cc: netdev@vger.kernel.org; mahesh@bandewar.net; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCHv2 3/7] TAP: Tap character device creation/destroy API
> 
> From: Sainath Grandhi <sainath.grandhi@intel.com>
> Date: Tue, 17 Jan 2017 16:03:02 -0800
> 
> > +int tap_create_cdev(struct cdev *tap_cdev,
> > +		    dev_t *tap_major, const char *device_name) {
> > +	int err;
> > +
> > +	err = alloc_chrdev_region(tap_major, 0, TAP_NUM_DEVS,
> device_name);
> > +
> > +	if (err)
> > +		goto out1;
> 
> No need to have an empty line between the alloc_chrdev_region() call and
> the "if (err)" test.
> 
> > diff --git a/include/linux/if_tap.h b/include/linux/if_tap.h index
> > d887aaa..a2dfd90 100644
> > --- a/include/linux/if_tap.h
> > +++ b/include/linux/if_tap.h
> > @@ -10,7 +10,7 @@ struct file;
> >  struct socket;
> >  static inline struct socket *tap_get_socket(struct file *f)  {
> > -        return ERR_PTR(-EINVAL);
> > +	return ERR_PTR(-EINVAL);
> >  }
> 
> Instead of having to fix up the indentation here, get it right in the first place in
> patch #2.

Will work on these two comments in next version.

^ permalink raw reply

* Re: [PATCH v3 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Mark Greer @ 2017-01-19 23:35 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	lauro.venancio-430g2QfJUUCGglJvpFV4uA,
	aloisio.almeida-430g2QfJUUCGglJvpFV4uA,
	sameo-VuQAYsv1563Yd54FQh9/CA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	justin-R+k406RtEhcAvxtiuMwx3w
In-Reply-To: <1482380314-16440-1-git-send-email-geoff-R+k406RtEhcAvxtiuMwx3w@public.gmane.org>

On Wed, Dec 21, 2016 at 11:18:32PM -0500, Geoff Lansberry wrote:
> The TRF7970A has configuration options to support hardware designs
> which use a 27.12MHz clock. This commit adds a device tree option
> 'clock-frequency' to support configuring the this chip for default
> 13.56MHz clock or the optional 27.12MHz clock.
> 
> Signed-off-by: Geoff Lansberry <geoff-R+k406RtEhcAvxtiuMwx3w@public.gmane.org>
> ---

Acked-by: Mark Greer <mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org>

^ permalink raw reply

* RE: [EXT] Re: [PATCH v5 0/2] Add support for the ethernet switch on the ESPRESSObin
From: Jon Pannell @ 2017-01-19 23:41 UTC (permalink / raw)
  To: Andrew Lunn, Gregory CLEMENT, Bob Bernstein, Christine Qiu
  Cc: Vivien Didelot, Florian Fainelli, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, David S. Miller, Jason Cooper,
	Sebastian Hesselbarth, Thomas Petazzoni,
	linux-arm-kernel@lists.infradead.org, Nadav Haklai, Wilson Ding,
	Kostya Porotchkin, Joe Zhou
In-Reply-To: <20170119220607.GA25907@lunn.ch>

+ Bob + Christine

Jon Pannell


-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch] 
Sent: Thursday, January 19, 2017 2:06 PM
To: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>; Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; David S. Miller <davem@davemloft.net>; Jason Cooper <jason@lakedaemon.net>; Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>; Thomas Petazzoni <thomas.petazzoni@free-electrons.com>; linux-arm-kernel@lists.infradead.org; Nadav Haklai <nadavh@marvell.com>; Wilson Ding <dingwei@marvell.com>; Kostya Porotchkin <kostap@marvell.com>; Joe Zhou <shjzhou@marvell.com>; Jon Pannell <jpannell@marvell.com>
Subject: [EXT] Re: [PATCH v5 0/2] Add support for the ethernet switch on the ESPRESSObin

External Email

----------------------------------------------------------------------
> While comparing the datasheet and the ops functions used, some 
> question came to me. They should not prevent applying this series, but 
> their answer would help me to have a better understanding of the dsa 
> subsystem.
> 
> - Are the temperature related operation still useful with dsa2 ?

No. I'm in the process of moving the code into the Marvell PHY driver, since the sensor is in the embedded PHYs.

What ID does the embedded PHY use? The 6390 has a blank ID, where as older device have a real ID.

> - Why the setup is done differently between the 6390 and the 6352
>   families when the have exactly the same register?

EDSA on 6390 works differently to 6352, meaning it breaks. So we need to run the 6390 with DSA tagging, not EDSA. Maybe this is the source of the differences?

It should also be noted that the 6390 support is not yet complete. I have a few more patches in my tree to post.

>   - On the Port Controller 2, the bit PORT_CONTROL_2_MAP_DA is set for
>     6352 and not for 6390 whereas the same bit exists in 6360 and the
>     description for this bit is the same for both datasheet.

Humm, it does look like it is missing mv88e6xxx_6390_family(chip).
 
> 
>   - Register PORT_ATU_CONTROL and PORT_PRI_OVERRIDE are reset on 6352
>     and not on 6390. While here again the registers description are
>     the same.

And the same here. I've mostly been working on where the 6390 is different. Where it is the same i've mostly ignored it so far :-)

There is also an ongoing effort to remove all these big if statements with a list of families.

     Andrew

^ permalink raw reply

* [PATCH] inet: don't use sk_v6_rcv_saddr directly
From: Josef Bacik @ 2017-01-19 22:47 UTC (permalink / raw)
  To: davem, tom, netdev, kafai

When comparing two sockets we need to use inet6_rcv_saddr so we get a NULL
sk_v6_rcv_saddr if the socket isn't AF_INET6, otherwise our comparison function
can be wrong.

Fixes: 637bc8b ("inet: reset tb->fastreuseport when adding a reuseport sk")
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 net/ipv4/inet_connection_sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 096a085..a336c42 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -99,7 +99,7 @@ int inet_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
 #if IS_ENABLED(CONFIG_IPV6)
 	if (sk->sk_family == AF_INET6)
 		return ipv6_rcv_saddr_equal(&sk->sk_v6_rcv_saddr,
-					    &sk2->sk_v6_rcv_saddr,
+					    inet6_rcv_saddr(sk2),
 					    sk->sk_rcv_saddr,
 					    sk2->sk_rcv_saddr,
 					    ipv6_only_sock(sk),
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCH v5 1/2] net: dsa: mv88e6xxx: Don't forbid MDIO I/Os for PHY addr >= num_of_ports
From: Andrew Lunn @ 2017-01-20  0:06 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: Thomas Petazzoni, Florian Fainelli, Jason Cooper, Joe Zhou,
	netdev, linux-kernel, Jon Pannell, Nadav Haklai,
	Kostya Porotchkin, Gregory CLEMENT, Wilson Ding, David S. Miller,
	linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <871svylmvb.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

On Thu, Jan 19, 2017 at 05:13:12PM -0500, Vivien Didelot wrote:
> Hi Gregory,
> 
> Gregory CLEMENT <gregory.clement@free-electrons.com> writes:
> 
> > From: Romain Perier <romain.perier@free-electrons.com>
> >
> > Some Marvell ethernet switches have internal ethernet transceivers with
> > hardcoded phy addresses. These addresses can be greater than the number
> > of ports or its value might be different than the associated port number.
> > This is for example the case for MV88E6341 that has 6 ports and internal
> > Port 1 to Port4 PHYs mapped at SMI addresses from 0x11 to 0x14.
> 
> Isn't there an hardware table used to map the PHY addresses on such chip?

The 6390 has something like this. But if we can avoid it, lets keep it
KISS.

	Andrew

^ permalink raw reply

* Re: [PATCH v5 2/2] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341
From: Andrew Lunn @ 2017-01-20  0:12 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: Thomas Petazzoni, Florian Fainelli, Jason Cooper, Joe Zhou,
	netdev, linux-kernel, Jon Pannell, Nadav Haklai,
	Kostya Porotchkin, Gregory CLEMENT, Wilson Ding, David S. Miller,
	linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <87y3y6k7pg.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

On Thu, Jan 19, 2017 at 05:26:03PM -0500, Vivien Didelot wrote:
> Gregory CLEMENT <gregory.clement@free-electrons.com> writes:
> 
> > +static bool mv88e6xxx_6341_family(struct mv88e6xxx_chip *chip)
> > +{
> > +	return chip->info->family == MV88E6XXX_FAMILY_6341;
> > +}
> 
> I don't want to see these erronous family checks anymore, but I cannot
> blame you for adding it since not all the code is moved to ops yet ;)
> 
> >  	MV88E6XXX_FAMILY_6165,	/* 6123 6161 6165 */
> >  	MV88E6XXX_FAMILY_6185,	/* 6108 6121 6122 6131 6152 6155 6182 6185 */
> >  	MV88E6XXX_FAMILY_6320,	/* 6320 6321 */
> > +	MV88E6XXX_FAMILY_6341,	/* 6141 6341 */
> 
> Maybe I missed it, Andrew, can you confirm that 6341 is a proper Marvell
> family of switch chips?

My understand is that it is. Marvell have not added it to DSDT. There
is a new SDK called UMSD, also GPLv2 and BSD. They call this family
Topaz, and the 6390 is Peridot.

       Andrew

^ permalink raw reply

* [PATCH v2 0/2] net: dsa: Move temperature sensor code into PHY.
From: Andrew Lunn @ 2017-01-20  0:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Andrew Lunn

Marvell Ethernet switches contain a temperature sensor. There appears
to be one sensor, which is shared by each of the internal PHYs. Each
PHY has independent registers to read this sensor, and to set a limit
for when an alarm should be raised.

Some Marvell discrete PHY also have the same sensor and registers.
Moving the HWMON code from DSA into the PHY makes the sensor available
in discrete PHYs, and removes the layering violation, the switch
driver poking around in PHY registers.

While moving the code into the PHY driver, it has been re-written to
use the new HWMON APIs.

v2:

Better Cover note explaining one sensor, but multiple independent
registers

Simply error checking.


Andrew Lunn (2):
  phy: marvell: Add support for temperature sensor
  net: dsa: Remove hwmon support

 drivers/net/dsa/mv88e6xxx/chip.c      | 154 -------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  16 --
 drivers/net/phy/marvell.c             | 423 +++++++++++++++++++++++++++++++++-
 include/net/dsa.h                     |   8 -
 net/dsa/Kconfig                       |  11 -
 net/dsa/Makefile                      |   1 -
 net/dsa/dsa.c                         |   4 -
 net/dsa/dsa_priv.h                    |   9 -
 net/dsa/hwmon.c                       | 147 ------------
 9 files changed, 420 insertions(+), 353 deletions(-)
 delete mode 100644 net/dsa/hwmon.c

-- 
2.11.0

^ permalink raw reply

* [PATCH v2 2/2] net: dsa: Remove hwmon support
From: Andrew Lunn @ 2017-01-20  0:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1484872670-32585-1-git-send-email-andrew@lunn.ch>

Only the Marvell mv88e6xxx DSA driver made use of the HWMON support in
DSA. The temperature sensor registers are actually in the embedded
PHYs, and the PHY driver now supports it. So remove all HWMON support
from DSA and drivers.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 154 ----------------------------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  16 ----
 include/net/dsa.h                     |   8 --
 net/dsa/Kconfig                       |  11 ---
 net/dsa/Makefile                      |   1 -
 net/dsa/dsa.c                         |   4 -
 net/dsa/dsa_priv.h                    |   9 --
 net/dsa/hwmon.c                       | 147 --------------------------------
 8 files changed, 350 deletions(-)
 delete mode 100644 net/dsa/hwmon.c

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 987b2dbbd35a..c7e08e13bb54 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2964,154 +2964,6 @@ static void mv88e6xxx_mdio_unregister(struct mv88e6xxx_chip *chip)
 		of_node_put(chip->mdio_np);
 }
 
-#ifdef CONFIG_NET_DSA_HWMON
-
-static int mv88e61xx_get_temp(struct dsa_switch *ds, int *temp)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	u16 val;
-	int ret;
-
-	*temp = 0;
-
-	mutex_lock(&chip->reg_lock);
-
-	ret = mv88e6xxx_phy_write(chip, 0x0, 0x16, 0x6);
-	if (ret < 0)
-		goto error;
-
-	/* Enable temperature sensor */
-	ret = mv88e6xxx_phy_read(chip, 0x0, 0x1a, &val);
-	if (ret < 0)
-		goto error;
-
-	ret = mv88e6xxx_phy_write(chip, 0x0, 0x1a, val | (1 << 5));
-	if (ret < 0)
-		goto error;
-
-	/* Wait for temperature to stabilize */
-	usleep_range(10000, 12000);
-
-	ret = mv88e6xxx_phy_read(chip, 0x0, 0x1a, &val);
-	if (ret < 0)
-		goto error;
-
-	/* Disable temperature sensor */
-	ret = mv88e6xxx_phy_write(chip, 0x0, 0x1a, val & ~(1 << 5));
-	if (ret < 0)
-		goto error;
-
-	*temp = ((val & 0x1f) - 5) * 5;
-
-error:
-	mv88e6xxx_phy_write(chip, 0x0, 0x16, 0x0);
-	mutex_unlock(&chip->reg_lock);
-	return ret;
-}
-
-static int mv88e63xx_get_temp(struct dsa_switch *ds, int *temp)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
-	u16 val;
-	int ret;
-
-	*temp = 0;
-
-	mutex_lock(&chip->reg_lock);
-	ret = mv88e6xxx_phy_page_read(chip, phy, 6, 27, &val);
-	mutex_unlock(&chip->reg_lock);
-	if (ret < 0)
-		return ret;
-
-	*temp = (val & 0xff) - 25;
-
-	return 0;
-}
-
-static int mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-
-	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_TEMP))
-		return -EOPNOTSUPP;
-
-	if (mv88e6xxx_6320_family(chip) || mv88e6xxx_6352_family(chip))
-		return mv88e63xx_get_temp(ds, temp);
-
-	return mv88e61xx_get_temp(ds, temp);
-}
-
-static int mv88e6xxx_get_temp_limit(struct dsa_switch *ds, int *temp)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
-	u16 val;
-	int ret;
-
-	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_TEMP_LIMIT))
-		return -EOPNOTSUPP;
-
-	*temp = 0;
-
-	mutex_lock(&chip->reg_lock);
-	ret = mv88e6xxx_phy_page_read(chip, phy, 6, 26, &val);
-	mutex_unlock(&chip->reg_lock);
-	if (ret < 0)
-		return ret;
-
-	*temp = (((val >> 8) & 0x1f) * 5) - 25;
-
-	return 0;
-}
-
-static int mv88e6xxx_set_temp_limit(struct dsa_switch *ds, int temp)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
-	u16 val;
-	int err;
-
-	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_TEMP_LIMIT))
-		return -EOPNOTSUPP;
-
-	mutex_lock(&chip->reg_lock);
-	err = mv88e6xxx_phy_page_read(chip, phy, 6, 26, &val);
-	if (err)
-		goto unlock;
-	temp = clamp_val(DIV_ROUND_CLOSEST(temp, 5) + 5, 0, 0x1f);
-	err = mv88e6xxx_phy_page_write(chip, phy, 6, 26,
-				       (val & 0xe0ff) | (temp << 8));
-unlock:
-	mutex_unlock(&chip->reg_lock);
-
-	return err;
-}
-
-static int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int phy = mv88e6xxx_6320_family(chip) ? 3 : 0;
-	u16 val;
-	int ret;
-
-	if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_TEMP_LIMIT))
-		return -EOPNOTSUPP;
-
-	*alarm = false;
-
-	mutex_lock(&chip->reg_lock);
-	ret = mv88e6xxx_phy_page_read(chip, phy, 6, 26, &val);
-	mutex_unlock(&chip->reg_lock);
-	if (ret < 0)
-		return ret;
-
-	*alarm = !!(val & 0x40);
-
-	return 0;
-}
-#endif /* CONFIG_NET_DSA_HWMON */
-
 static int mv88e6xxx_get_eeprom_len(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
@@ -4386,12 +4238,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.get_sset_count		= mv88e6xxx_get_sset_count,
 	.set_eee		= mv88e6xxx_set_eee,
 	.get_eee		= mv88e6xxx_get_eee,
-#ifdef CONFIG_NET_DSA_HWMON
-	.get_temp		= mv88e6xxx_get_temp,
-	.get_temp_limit		= mv88e6xxx_get_temp_limit,
-	.set_temp_limit		= mv88e6xxx_set_temp_limit,
-	.get_temp_alarm		= mv88e6xxx_get_temp_alarm,
-#endif
 	.get_eeprom_len		= mv88e6xxx_get_eeprom_len,
 	.get_eeprom		= mv88e6xxx_get_eeprom,
 	.set_eeprom		= mv88e6xxx_set_eeprom,
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 466cfdadb7bd..ce8b43b14e96 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -497,12 +497,6 @@ enum mv88e6xxx_cap {
 	 */
 	MV88E6XXX_CAP_STU,
 
-	/* Internal temperature sensor.
-	 * Available from any enabled port's PHY register 26, page 6.
-	 */
-	MV88E6XXX_CAP_TEMP,
-	MV88E6XXX_CAP_TEMP_LIMIT,
-
 	/* VLAN Table Unit.
 	 * The VTU is used to program 802.1Q VLANs. See GLOBAL_VTU_OP.
 	 */
@@ -533,8 +527,6 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAG_G2_POT		BIT_ULL(MV88E6XXX_CAP_G2_POT)
 
 #define MV88E6XXX_FLAG_STU		BIT_ULL(MV88E6XXX_CAP_STU)
-#define MV88E6XXX_FLAG_TEMP		BIT_ULL(MV88E6XXX_CAP_TEMP)
-#define MV88E6XXX_FLAG_TEMP_LIMIT	BIT_ULL(MV88E6XXX_CAP_TEMP_LIMIT)
 #define MV88E6XXX_FLAG_VTU		BIT_ULL(MV88E6XXX_CAP_VTU)
 
 /* Ingress Rate Limit unit */
@@ -586,7 +578,6 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
 	 MV88E6XXX_FLAG_STU |		\
-	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
@@ -605,8 +596,6 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_2X |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
-	 MV88E6XXX_FLAG_TEMP |		\
-	 MV88E6XXX_FLAG_TEMP_LIMIT |	\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
@@ -621,7 +610,6 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
 	 MV88E6XXX_FLAG_STU |		\
-	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
@@ -637,8 +625,6 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_POT |	\
 	 MV88E6XXX_FLAG_STU |		\
-	 MV88E6XXX_FLAG_TEMP |		\
-	 MV88E6XXX_FLAG_TEMP_LIMIT |	\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
@@ -651,8 +637,6 @@ struct mv88e6xxx_ops;
 	(MV88E6XXX_FLAG_EEE |		\
 	 MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_STU |		\
-	 MV88E6XXX_FLAG_TEMP |		\
-	 MV88E6XXX_FLAG_TEMP_LIMIT |	\
 	 MV88E6XXX_FLAG_VTU |		\
 	 MV88E6XXX_FLAGS_IRL |		\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
diff --git a/include/net/dsa.h b/include/net/dsa.h
index c72ed7af2a2a..9d6cd923c48c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -307,14 +307,6 @@ struct dsa_switch_ops {
 	int	(*get_eee)(struct dsa_switch *ds, int port,
 			   struct ethtool_eee *e);
 
-#ifdef CONFIG_NET_DSA_HWMON
-	/* Hardware monitoring */
-	int	(*get_temp)(struct dsa_switch *ds, int *temp);
-	int	(*get_temp_limit)(struct dsa_switch *ds, int *temp);
-	int	(*set_temp_limit)(struct dsa_switch *ds, int temp);
-	int	(*get_temp_alarm)(struct dsa_switch *ds, bool *alarm);
-#endif
-
 	/* EEPROM access */
 	int	(*get_eeprom_len)(struct dsa_switch *ds);
 	int	(*get_eeprom)(struct dsa_switch *ds,
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 39bb5b3a82f2..9649238eef40 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -15,17 +15,6 @@ config NET_DSA
 
 if NET_DSA
 
-config NET_DSA_HWMON
-	bool "Distributed Switch Architecture HWMON support"
-	default y
-	depends on HWMON && !(NET_DSA=y && HWMON=m)
-	---help---
-	  Say Y if you want to expose thermal sensor data on switches supported
-	  by the Distributed Switch Architecture.
-
-	  Some of those switches contain thermal sensors. This data is available
-	  via the hwmon sysfs interface and exposes the onboard sensors.
-
 # tagging formats
 config NET_DSA_TAG_BRCM
 	bool
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 560b6747c276..a3380ed0e0be 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -1,7 +1,6 @@
 # the core
 obj-$(CONFIG_NET_DSA) += dsa_core.o
 dsa_core-y += dsa.o slave.o dsa2.o
-dsa_core-$(CONFIG_NET_DSA_HWMON) += hwmon.o
 
 # tagging formats
 dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 91f96e1bd2ec..77cb78767f1d 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -316,8 +316,6 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 	if (ret)
 		return ret;
 
-	dsa_hwmon_register(ds);
-
 	return 0;
 }
 
@@ -376,8 +374,6 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 {
 	int port;
 
-	dsa_hwmon_unregister(ds);
-
 	/* Destroy network devices for physical switch ports. */
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
 		if (!(ds->enabled_port_mask & (1 << port)))
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7e3385ec73f4..63ae1484abae 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -56,15 +56,6 @@ const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
 int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds);
 void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds);
 
-/* hwmon.c */
-#ifdef CONFIG_NET_DSA_HWMON
-void dsa_hwmon_register(struct dsa_switch *ds);
-void dsa_hwmon_unregister(struct dsa_switch *ds);
-#else
-static inline void dsa_hwmon_register(struct dsa_switch *ds) { }
-static inline void dsa_hwmon_unregister(struct dsa_switch *ds) { }
-#endif
-
 /* slave.c */
 extern const struct dsa_device_ops notag_netdev_ops;
 void dsa_slave_mii_bus_init(struct dsa_switch *ds);
diff --git a/net/dsa/hwmon.c b/net/dsa/hwmon.c
deleted file mode 100644
index 08831a811278..000000000000
--- a/net/dsa/hwmon.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/*
- * net/dsa/hwmon.c - HWMON subsystem support
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <linux/ctype.h>
-#include <linux/hwmon.h>
-#include <net/dsa.h>
-
-#include "dsa_priv.h"
-
-static ssize_t temp1_input_show(struct device *dev,
-				struct device_attribute *attr, char *buf)
-{
-	struct dsa_switch *ds = dev_get_drvdata(dev);
-	int temp, ret;
-
-	ret = ds->ops->get_temp(ds, &temp);
-	if (ret < 0)
-		return ret;
-
-	return sprintf(buf, "%d\n", temp * 1000);
-}
-static DEVICE_ATTR_RO(temp1_input);
-
-static ssize_t temp1_max_show(struct device *dev,
-			      struct device_attribute *attr, char *buf)
-{
-	struct dsa_switch *ds = dev_get_drvdata(dev);
-	int temp, ret;
-
-	ret = ds->ops->get_temp_limit(ds, &temp);
-	if (ret < 0)
-		return ret;
-
-	return sprintf(buf, "%d\n", temp * 1000);
-}
-
-static ssize_t temp1_max_store(struct device *dev,
-			       struct device_attribute *attr, const char *buf,
-			       size_t count)
-{
-	struct dsa_switch *ds = dev_get_drvdata(dev);
-	int temp, ret;
-
-	ret = kstrtoint(buf, 0, &temp);
-	if (ret < 0)
-		return ret;
-
-	ret = ds->ops->set_temp_limit(ds, DIV_ROUND_CLOSEST(temp, 1000));
-	if (ret < 0)
-		return ret;
-
-	return count;
-}
-static DEVICE_ATTR_RW(temp1_max);
-
-static ssize_t temp1_max_alarm_show(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct dsa_switch *ds = dev_get_drvdata(dev);
-	bool alarm;
-	int ret;
-
-	ret = ds->ops->get_temp_alarm(ds, &alarm);
-	if (ret < 0)
-		return ret;
-
-	return sprintf(buf, "%d\n", alarm);
-}
-static DEVICE_ATTR_RO(temp1_max_alarm);
-
-static struct attribute *dsa_hwmon_attrs[] = {
-	&dev_attr_temp1_input.attr,	/* 0 */
-	&dev_attr_temp1_max.attr,	/* 1 */
-	&dev_attr_temp1_max_alarm.attr,	/* 2 */
-	NULL
-};
-
-static umode_t dsa_hwmon_attrs_visible(struct kobject *kobj,
-				       struct attribute *attr, int index)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct dsa_switch *ds = dev_get_drvdata(dev);
-	const struct dsa_switch_ops *ops = ds->ops;
-	umode_t mode = attr->mode;
-
-	if (index == 1) {
-		if (!ops->get_temp_limit)
-			mode = 0;
-		else if (!ops->set_temp_limit)
-			mode &= ~S_IWUSR;
-	} else if (index == 2 && !ops->get_temp_alarm) {
-		mode = 0;
-	}
-	return mode;
-}
-
-static const struct attribute_group dsa_hwmon_group = {
-	.attrs = dsa_hwmon_attrs,
-	.is_visible = dsa_hwmon_attrs_visible,
-};
-__ATTRIBUTE_GROUPS(dsa_hwmon);
-
-void dsa_hwmon_register(struct dsa_switch *ds)
-{
-	const char *netname = netdev_name(ds->dst->master_netdev);
-	char hname[IFNAMSIZ + 1];
-	int i, j;
-
-	/* If the switch provides temperature accessors, register with hardware
-	 * monitoring subsystem. Treat registration error as non-fatal.
-	 */
-	if (!ds->ops->get_temp)
-		return;
-
-	/* Create valid hwmon 'name' attribute */
-	for (i = j = 0; i < IFNAMSIZ && netname[i]; i++) {
-		if (isalnum(netname[i]))
-			hname[j++] = netname[i];
-	}
-	hname[j] = '\0';
-	scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d", hname,
-		  ds->index);
-	ds->hwmon_dev = hwmon_device_register_with_groups(NULL, ds->hwmon_name,
-							  ds, dsa_hwmon_groups);
-	if (IS_ERR(ds->hwmon_dev)) {
-		pr_warn("DSA: failed to register HWMON subsystem for switch %d\n",
-			ds->index);
-		ds->hwmon_dev = NULL;
-	} else {
-		pr_info("DSA: registered HWMON subsystem for switch %d\n",
-			ds->index);
-	}
-}
-
-void dsa_hwmon_unregister(struct dsa_switch *ds)
-{
-	if (ds->hwmon_dev) {
-		hwmon_device_unregister(ds->hwmon_dev);
-		ds->hwmon_dev = NULL;
-	}
-}
-- 
2.11.0

^ permalink raw reply related

* [PATCH v2 1/2] phy: marvell: Add support for temperature sensor
From: Andrew Lunn @ 2017-01-20  0:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1484872670-32585-1-git-send-email-andrew@lunn.ch>

Some Marvell PHYs have an inbuilt temperature sensor. Add hwmon
support for this sensor.

There are two different variants. The simpler, older chips have a 5
degree accuracy. The newer devices have 1 degree accuracy.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: Simplify using PTR_ERR_OR_ZERO()
---
 drivers/net/phy/marvell.c | 423 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 420 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0b78210c0fa7..64229976ace1 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -17,8 +17,10 @@
  */
 #include <linux/kernel.h>
 #include <linux/string.h>
+#include <linux/ctype.h>
 #include <linux/errno.h>
 #include <linux/unistd.h>
+#include <linux/hwmon.h>
 #include <linux/interrupt.h>
 #include <linux/init.h>
 #include <linux/delay.h>
@@ -90,6 +92,17 @@
 #define MII_88E1121_PHY_MSCR_TX_DELAY	BIT(4)
 #define MII_88E1121_PHY_MSCR_DELAY_MASK	(~(0x3 << 4))
 
+#define MII_88E1121_MISC_TEST				0x1a
+#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK	0x1f00
+#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT	8
+#define MII_88E1510_MISC_TEST_TEMP_IRQ_EN		BIT(7)
+#define MII_88E1510_MISC_TEST_TEMP_IRQ			BIT(6)
+#define MII_88E1121_MISC_TEST_TEMP_SENSOR_EN		BIT(5)
+#define MII_88E1121_MISC_TEST_TEMP_MASK			0x1f
+
+#define MII_88E1510_TEMP_SENSOR		0x1b
+#define MII_88E1510_TEMP_SENSOR_MASK	0xff
+
 #define MII_88E1318S_PHY_MSCR1_REG	16
 #define MII_88E1318S_PHY_MSCR1_PAD_ODD	BIT(6)
 
@@ -172,6 +185,8 @@ static struct marvell_hw_stat marvell_hw_stats[] = {
 
 struct marvell_priv {
 	u64 stats[ARRAY_SIZE(marvell_hw_stats)];
+	char *hwmon_name;
+	struct device *hwmon_dev;
 };
 
 static int marvell_ack_interrupt(struct phy_device *phydev)
@@ -1468,6 +1483,371 @@ static void marvell_get_stats(struct phy_device *phydev,
 		data[i] = marvell_get_stat(phydev, i);
 }
 
+#ifdef CONFIG_HWMON
+static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
+{
+	int ret;
+	int val;
+
+	*temp = 0;
+
+	mutex_lock(&phydev->lock);
+
+	ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+	if (ret < 0)
+		goto error;
+
+	/* Enable temperature sensor */
+	ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+	if (ret < 0)
+		goto error;
+
+	ret = phy_write(phydev, MII_88E1121_MISC_TEST,
+			ret | MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+	if (ret < 0)
+		goto error;
+
+	/* Wait for temperature to stabilize */
+	usleep_range(10000, 12000);
+
+	val = phy_read(phydev, MII_88E1121_MISC_TEST);
+	if (val < 0) {
+		ret = val;
+		goto error;
+	}
+
+	/* Disable temperature sensor */
+	ret = phy_write(phydev, MII_88E1121_MISC_TEST,
+			ret & ~MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+	if (ret < 0)
+		goto error;
+
+	*temp = ((val & MII_88E1121_MISC_TEST_TEMP_MASK) - 5) * 5000;
+
+error:
+	phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+static int m88e1121_hwmon_read(struct device *dev,
+			       enum hwmon_sensor_types type,
+			       u32 attr, int channel, long *temp)
+{
+	struct phy_device *phydev = dev_get_drvdata(dev);
+	int err;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		err = m88e1121_get_temp(phydev, temp);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return err;
+}
+
+static umode_t m88e1121_hwmon_is_visible(const void *data,
+					 enum hwmon_sensor_types type,
+					 u32 attr, int channel)
+{
+	if (type != hwmon_temp)
+		return 0;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		return 0444;
+	default:
+		return 0;
+	}
+}
+
+static u32 m88e1121_hwmon_chip_config[] = {
+	HWMON_C_REGISTER_TZ,
+	0
+};
+
+static const struct hwmon_channel_info m88e1121_hwmon_chip = {
+	.type = hwmon_chip,
+	.config = m88e1121_hwmon_chip_config,
+};
+
+static u32 m88e1121_hwmon_temp_config[] = {
+	HWMON_T_INPUT,
+	0
+};
+
+static const struct hwmon_channel_info m88e1121_hwmon_temp = {
+	.type = hwmon_temp,
+	.config = m88e1121_hwmon_temp_config,
+};
+
+static const struct hwmon_channel_info *m88e1121_hwmon_info[] = {
+	&m88e1121_hwmon_chip,
+	&m88e1121_hwmon_temp,
+	NULL
+};
+
+static const struct hwmon_ops m88e1121_hwmon_hwmon_ops = {
+	.is_visible = m88e1121_hwmon_is_visible,
+	.read = m88e1121_hwmon_read,
+};
+
+static const struct hwmon_chip_info m88e1121_hwmon_chip_info = {
+	.ops = &m88e1121_hwmon_hwmon_ops,
+	.info = m88e1121_hwmon_info,
+};
+
+static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
+{
+	int ret;
+
+	*temp = 0;
+
+	mutex_lock(&phydev->lock);
+
+	ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+	if (ret < 0)
+		goto error;
+
+	ret = phy_read(phydev, MII_88E1510_TEMP_SENSOR);
+	if (ret < 0)
+		goto error;
+
+	*temp = ((ret & MII_88E1510_TEMP_SENSOR_MASK) - 25) * 1000;
+
+error:
+	phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
+{
+	int ret;
+
+	*temp = 0;
+
+	mutex_lock(&phydev->lock);
+
+	ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+	if (ret < 0)
+		goto error;
+
+	ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+	if (ret < 0)
+		goto error;
+
+	*temp = (((ret & MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) >>
+		  MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT) * 5) - 25;
+	/* convert to mC */
+	*temp *= 1000;
+
+error:
+	phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
+{
+	int ret;
+
+	mutex_lock(&phydev->lock);
+
+	ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+	if (ret < 0)
+		goto error;
+
+	ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+	if (ret < 0)
+		goto error;
+
+	temp = temp / 1000;
+	temp = clamp_val(DIV_ROUND_CLOSEST(temp, 5) + 5, 0, 0x1f);
+	ret = phy_write(phydev, MII_88E1121_MISC_TEST,
+			(ret & ~MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) |
+			(temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT));
+
+error:
+	phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
+{
+	int ret;
+
+	*alarm = false;
+
+	mutex_lock(&phydev->lock);
+
+	ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+	if (ret < 0)
+		goto error;
+
+	ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+	if (ret < 0)
+		goto error;
+	*alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
+
+error:
+	phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+static int m88e1510_hwmon_read(struct device *dev,
+			       enum hwmon_sensor_types type,
+			       u32 attr, int channel, long *temp)
+{
+	struct phy_device *phydev = dev_get_drvdata(dev);
+	int err;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		err = m88e1510_get_temp(phydev, temp);
+		break;
+	case hwmon_temp_crit:
+		err = m88e1510_get_temp_critical(phydev, temp);
+		break;
+	case hwmon_temp_max_alarm:
+		err = m88e1510_get_temp_alarm(phydev, temp);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return err;
+}
+
+static int m88e1510_hwmon_write(struct device *dev,
+				enum hwmon_sensor_types type,
+				u32 attr, int channel, long temp)
+{
+	struct phy_device *phydev = dev_get_drvdata(dev);
+	int err;
+
+	switch (attr) {
+	case hwmon_temp_crit:
+		err = m88e1510_set_temp_critical(phydev, temp);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return err;
+}
+
+static umode_t m88e1510_hwmon_is_visible(const void *data,
+					 enum hwmon_sensor_types type,
+					 u32 attr, int channel)
+{
+	if (type != hwmon_temp)
+		return 0;
+
+	switch (attr) {
+	case hwmon_temp_input:
+	case hwmon_temp_max_alarm:
+		return 0444;
+	case hwmon_temp_crit:
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
+static u32 m88e1510_hwmon_temp_config[] = {
+	HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_MAX_ALARM,
+	0
+};
+
+static const struct hwmon_channel_info m88e1510_hwmon_temp = {
+	.type = hwmon_temp,
+	.config = m88e1510_hwmon_temp_config,
+};
+
+static const struct hwmon_channel_info *m88e1510_hwmon_info[] = {
+	&m88e1121_hwmon_chip,
+	&m88e1510_hwmon_temp,
+	NULL
+};
+
+static const struct hwmon_ops m88e1510_hwmon_hwmon_ops = {
+	.is_visible = m88e1510_hwmon_is_visible,
+	.read = m88e1510_hwmon_read,
+	.write = m88e1510_hwmon_write,
+};
+
+static const struct hwmon_chip_info m88e1510_hwmon_chip_info = {
+	.ops = &m88e1510_hwmon_hwmon_ops,
+	.info = m88e1510_hwmon_info,
+};
+
+static int marvell_hwmon_name(struct phy_device *phydev)
+{
+	struct marvell_priv *priv = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	const char *devname = dev_name(dev);
+	size_t len = strlen(devname);
+	int i, j;
+
+	priv->hwmon_name = devm_kzalloc(dev, len, GFP_KERNEL);
+	if (!priv->hwmon_name)
+		return -ENOMEM;
+
+	for (i = j = 0; i < len && devname[i]; i++) {
+		if (isalnum(devname[i]))
+			priv->hwmon_name[j++] = devname[i];
+	}
+
+	return 0;
+}
+
+static int marvell_hwmon_probe(struct phy_device *phydev,
+			       const struct hwmon_chip_info *chip)
+{
+	struct marvell_priv *priv = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	int err;
+
+	err = marvell_hwmon_name(phydev);
+	if (err)
+		return err;
+
+	priv->hwmon_dev = devm_hwmon_device_register_with_info(
+		dev, priv->hwmon_name, phydev, chip, NULL);
+
+	return PTR_ERR_OR_ZERO(priv->hwmon_dev);
+}
+
+static int m88e1121_hwmon_probe(struct phy_device *phydev)
+{
+	return marvell_hwmon_probe(phydev, &m88e1121_hwmon_chip_info);
+}
+
+static int m88e1510_hwmon_probe(struct phy_device *phydev)
+{
+	return marvell_hwmon_probe(phydev, &m88e1510_hwmon_chip_info);
+}
+#else
+static int m88e1121_hwmon_probe(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int m88e1510_hwmon_probe(struct phy_device *phydev)
+{
+	return 0;
+}
+#endif
+
 static int marvell_probe(struct phy_device *phydev)
 {
 	struct marvell_priv *priv;
@@ -1481,6 +1861,39 @@ static int marvell_probe(struct phy_device *phydev)
 	return 0;
 }
 
+static int m88e1121_probe(struct phy_device *phydev)
+{
+	int err;
+
+	err = marvell_probe(phydev);
+	if (err)
+		return err;
+
+	return m88e1121_hwmon_probe(phydev);
+}
+
+static int m88e1510_probe(struct phy_device *phydev)
+{
+	int err;
+
+	err = marvell_probe(phydev);
+	if (err)
+		return err;
+
+	return m88e1510_hwmon_probe(phydev);
+}
+
+static void marvell_remove(struct phy_device *phydev)
+{
+#ifdef CONFIG_HWMON
+
+	struct marvell_priv *priv = phydev->priv;
+
+	if (priv && priv->hwmon_dev)
+		hwmon_device_unregister(priv->hwmon_dev);
+#endif
+}
+
 static struct phy_driver marvell_drivers[] = {
 	{
 		.phy_id = MARVELL_PHY_ID_88E1101,
@@ -1558,9 +1971,10 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1121R,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1121R",
+		.probe = &m88e1121_probe,
+		.remove = &marvell_remove,
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
-		.probe = marvell_probe,
 		.config_init = &m88e1121_config_init,
 		.config_aneg = &m88e1121_config_aneg,
 		.read_status = &marvell_read_status,
@@ -1671,8 +2085,10 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1510",
 		.features = PHY_GBIT_FEATURES | SUPPORTED_FIBRE,
+		.probe = &m88e1510_probe,
+		.remove = &marvell_remove,
+		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
-		.probe = marvell_probe,
 		.config_init = &m88e1510_config_init,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
@@ -1689,9 +2105,10 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1540,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1540",
+		.probe = m88e1510_probe,
+		.remove = &marvell_remove,
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
-		.probe = marvell_probe,
 		.config_init = &marvell_config_init,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
-- 
2.11.0

^ permalink raw reply related

* [PATCH net] net: mpls: Fix multipath selection for LSR use case
From: David Ahern @ 2017-01-20  0:51 UTC (permalink / raw)
  To: netdev; +Cc: rshearma, roopa, David Ahern

MPLS multipath for LSR is broken -- always selecting the first nexthop
in the one label case. For example:

    $ ip netns exec ns1 ip -f mpls ro ls
    100
            nexthop as to 200 via inet 172.16.2.2  dev virt12
            nexthop as to 300 via inet 172.16.3.2  dev virt13
    101
            nexthop as to 201 via inet6 2000:2::2  dev virt12
            nexthop as to 301 via inet6 2000:3::2  dev virt13

In this example incoming packets have a single MPLS labels which means
BOS bit is set. The BOS bit is passed from mpls_forward down to
mpls_multipath_hash which never processes the hash loop because BOS is 1.

Removing the bos arg from mpls_multipath_hash uncovers a number of other
problems with the hash loop that processes the MPLS label stack -- from
incorrect assumptions on the skb (skb has already pulled the first mpls
label in mpls_forward yet loop assumes it is there) to incorrect
pskb_may_pull checks (label_index starts at 0 and pskb_may_pull checks
all use sizeof() * label_index).

This patch addresses all problems by moving the skb_pull in mpls_forward
after mpls_select_multipath. This allows mpls_multipath_hash to see the
skb with the entire label stack as it arrived.

>From there mpls_multipath_hash is modified to additively compute the
total mpls header length on each pass (on pass N mpls_hdr_len is
N * sizeof(mpls_shim_hdr)). When the label is found with the BOS set it
verifies the skb has sufficient header for ipv4 or ipv6, and find the
IPv4 and IPv6 header by using the last mpls_hdr pointer and adding 1 to
advance past it.

With these changes I have verified the code correctly sees the label,
BOS, IPv4 and IPv6 addresses in the network header and icmp/tcp/udp
traffic for ipv4 and ipv6 are distributed across the nexthops.

Fixes: 1c78efa8319ca ("mpls: flow-based multipath selection")
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 net/mpls/af_mpls.c | 48 +++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 4dc81963af8f..64d3bf269a26 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -119,18 +119,19 @@ void mpls_stats_inc_outucastpkts(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(mpls_stats_inc_outucastpkts);
 
-static u32 mpls_multipath_hash(struct mpls_route *rt,
-			       struct sk_buff *skb, bool bos)
+static u32 mpls_multipath_hash(struct mpls_route *rt, struct sk_buff *skb)
 {
 	struct mpls_entry_decoded dec;
+	unsigned int mpls_hdr_len = 0;
 	struct mpls_shim_hdr *hdr;
 	bool eli_seen = false;
 	int label_index;
 	u32 hash = 0;
 
-	for (label_index = 0; label_index < MAX_MP_SELECT_LABELS && !bos;
+	for (label_index = 0; label_index < MAX_MP_SELECT_LABELS;
 	     label_index++) {
-		if (!pskb_may_pull(skb, sizeof(*hdr) * label_index))
+		mpls_hdr_len += sizeof(*hdr);
+		if (!pskb_may_pull(skb, mpls_hdr_len))
 			break;
 
 		/* Read and decode the current label */
@@ -155,37 +156,38 @@ static u32 mpls_multipath_hash(struct mpls_route *rt,
 			eli_seen = true;
 		}
 
-		bos = dec.bos;
-		if (bos && pskb_may_pull(skb, sizeof(*hdr) * label_index +
-					 sizeof(struct iphdr))) {
+		if (!dec.bos)
+			continue;
+
+		/* found bottom label; does skb have room for a header? */
+		if (pskb_may_pull(skb, mpls_hdr_len + sizeof(struct iphdr))) {
 			const struct iphdr *v4hdr;
 
-			v4hdr = (const struct iphdr *)(mpls_hdr(skb) +
-						       label_index);
+			v4hdr = (const struct iphdr *)(hdr + 1);
 			if (v4hdr->version == 4) {
 				hash = jhash_3words(ntohl(v4hdr->saddr),
 						    ntohl(v4hdr->daddr),
 						    v4hdr->protocol, hash);
 			} else if (v4hdr->version == 6 &&
-				pskb_may_pull(skb, sizeof(*hdr) * label_index +
-					      sizeof(struct ipv6hdr))) {
+				   pskb_may_pull(skb, mpls_hdr_len +
+						 sizeof(struct ipv6hdr))) {
 				const struct ipv6hdr *v6hdr;
 
-				v6hdr = (const struct ipv6hdr *)(mpls_hdr(skb) +
-								label_index);
-
+				v6hdr = (const struct ipv6hdr *)(hdr + 1);
 				hash = __ipv6_addr_jhash(&v6hdr->saddr, hash);
 				hash = __ipv6_addr_jhash(&v6hdr->daddr, hash);
 				hash = jhash_1word(v6hdr->nexthdr, hash);
 			}
 		}
+
+		break;
 	}
 
 	return hash;
 }
 
 static struct mpls_nh *mpls_select_multipath(struct mpls_route *rt,
-					     struct sk_buff *skb, bool bos)
+					     struct sk_buff *skb)
 {
 	int alive = ACCESS_ONCE(rt->rt_nhn_alive);
 	u32 hash = 0;
@@ -201,7 +203,7 @@ static struct mpls_nh *mpls_select_multipath(struct mpls_route *rt,
 	if (alive <= 0)
 		return NULL;
 
-	hash = mpls_multipath_hash(rt, skb, bos);
+	hash = mpls_multipath_hash(rt, skb);
 	nh_index = hash % alive;
 	if (alive == rt->rt_nhn)
 		goto out;
@@ -308,22 +310,22 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
 	hdr = mpls_hdr(skb);
 	dec = mpls_entry_decode(hdr);
 
-	/* Pop the label */
-	skb_pull(skb, sizeof(*hdr));
-	skb_reset_network_header(skb);
-
-	skb_orphan(skb);
-
 	rt = mpls_route_input_rcu(net, dec.label);
 	if (!rt) {
 		MPLS_INC_STATS(mdev, rx_noroute);
 		goto drop;
 	}
 
-	nh = mpls_select_multipath(rt, skb, dec.bos);
+	nh = mpls_select_multipath(rt, skb);
 	if (!nh)
 		goto err;
 
+	/* Pop the label */
+	skb_pull(skb, sizeof(*hdr));
+	skb_reset_network_header(skb);
+
+	skb_orphan(skb);
+
 	if (skb_warn_if_lro(skb))
 		goto err;
 
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH v2 0/2] net: dsa: Move temperature sensor code into PHY.
From: Andrew Lunn @ 2017-01-20  0:43 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot
In-Reply-To: <1484872670-32585-1-git-send-email-andrew@lunn.ch>

Hi Dave

Sorry, i forget the net-next in the subject line.

	Andrew

^ permalink raw reply

* Re: [PATCH v3 net-next 1/2] net/ipv6: allow sysctl to change link-local address generation mode
From: Stephen Hemminger @ 2017-01-20  1:27 UTC (permalink / raw)
  To: Felix Jia; +Cc: network dev, Hermin Anggawijaya
In-Reply-To: <20170119224619.18656-1-felix.jia@alliedtelesis.co.nz>

On Fri, 20 Jan 2017 11:46:19 +1300
Felix Jia <felix.jia@alliedtelesis.co.nz> wrote:

> diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
> index 0fa4c324b713..48e2a34a6ed3 100644
> --- a/include/net/if_inet6.h
> +++ b/include/net/if_inet6.h
> @@ -1,9 +1,9 @@
>  /*
>   *	inet6 interface/address list definitions
> - *	Linux INET6 implementation 
> + *	Linux INET6 implementation
>   *
>   *	Authors:
> - *	Pedro Roque		<roque@di.fc.ul.pt>	
> + *	Pedro Roque		<roque@di.fc.ul.pt>

Please don't add unnecessary whitespace changes to  your patch.

^ permalink raw reply

* Setting skb->csum in shared skb with csum-complete
From: Tom Herbert @ 2017-01-20  1:29 UTC (permalink / raw)
  To: Herbert Xu, Linux Kernel Network Developers

Hi Herbert,

I am looking at "net: Fix skb csum races when peeking". The context is
that I'm seeing checksum faults after backporting mlx5 from driver.
The handling of the checksum faults in interesting in itself. One side
effect is that we end up recalculating and doing the dump twice when
GRO is enabled. Another is that we validate checksum complete in a
shared skbuf with a knowingly bad skb->csum value.

In the checksum fault case we recalculate the checksum and find it was
actually correct in the packet. If the skb is not shared then we set
the skb->csum to the correct value and also set skb->csum_complete_sw
and skb->csum_valid. But if skb is shared we don't do any of that.
This is kind of odd since __skb_checksum_complete_head then returns
zero for an skb with checksum_complete where the checksum complete
value is not correct. I'm not sure what the consequences of doing that
are (I don't think it's related to issues I'm seeing) but this doesn't
seem like a good thing. Maybe we should set skb->csum in this case
since we know it's already wrong and the fact that skbuf is shared
can't make it more wrong...

Tom

^ permalink raw reply

* Re: [PATCH v5 1/2] net: dsa: mv88e6xxx: Don't forbid MDIO I/Os for PHY addr >= num_of_ports
From: Vivien Didelot @ 2017-01-20  1:54 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Gregory CLEMENT, Florian Fainelli, netdev, linux-kernel,
	David S. Miller, Jason Cooper, Sebastian Hesselbarth,
	Thomas Petazzoni, linux-arm-kernel, Nadav Haklai, Wilson Ding,
	Kostya Porotchkin, Joe Zhou, Jon Pannell
In-Reply-To: <20170120000645.GA31483@lunn.ch>

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>> Isn't there an hardware table used to map the PHY addresses on such chip?
>
> The 6390 has something like this. But if we can avoid it, lets keep it
> KISS.

I definitely agree. I was just curious about this new chip.

Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH v5 2/2] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341
From: Vivien Didelot @ 2017-01-20  1:55 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Gregory CLEMENT, Florian Fainelli, netdev, linux-kernel,
	David S. Miller, Jason Cooper, Sebastian Hesselbarth,
	Thomas Petazzoni, linux-arm-kernel, Nadav Haklai, Wilson Ding,
	Kostya Porotchkin, Joe Zhou, Jon Pannell
In-Reply-To: <20170120001203.GB31483@lunn.ch>

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>> Maybe I missed it, Andrew, can you confirm that 6341 is a proper Marvell
>> family of switch chips?
>
> My understand is that it is. Marvell have not added it to DSDT. There
> is a new SDK called UMSD, also GPLv2 and BSD. They call this family
> Topaz, and the 6390 is Peridot.

At least we could rename it anytime. They don't really make sense nor or
reliable anyway...

Thanks,

        Vivien

^ permalink raw reply

* Re: Setting skb->csum in shared skb with csum-complete
From: Eric Dumazet @ 2017-01-20  2:00 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Herbert Xu, Linux Kernel Network Developers
In-Reply-To: <CALx6S34ZfFzPOmzfLoverXQ215AvasYejyJUv8wtWAVRumi-eg@mail.gmail.com>

On Thu, 2017-01-19 at 17:29 -0800, Tom Herbert wrote:
> Hi Herbert,
> 
> I am looking at "net: Fix skb csum races when peeking". The context is
> that I'm seeing checksum faults after backporting mlx5 from driver.
> The handling of the checksum faults in interesting in itself. One side
> effect is that we end up recalculating and doing the dump twice when
> GRO is enabled. Another is that we validate checksum complete in a
> shared skbuf with a knowingly bad skb->csum value.
> 
> In the checksum fault case we recalculate the checksum and find it was
> actually correct in the packet. If the skb is not shared then we set
> the skb->csum to the correct value and also set skb->csum_complete_sw
> and skb->csum_valid. But if skb is shared we don't do any of that.
> This is kind of odd since __skb_checksum_complete_head then returns
> zero for an skb with checksum_complete where the checksum complete
> value is not correct. I'm not sure what the consequences of doing that
> are (I don't think it's related to issues I'm seeing) but this doesn't
> seem like a good thing. Maybe we should set skb->csum in this case
> since we know it's already wrong and the fact that skbuf is shared
> can't make it more wrong...

Why GRO can not change skb fields ?

skb can not be shared at GRO layer, so really this should be done there
if you want to avoid a double checksumming later.

You added all these skb fields, you know better than us ;)

^ permalink raw reply

* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Timur Tabi @ 2017-01-20  2:38 UTC (permalink / raw)
  To: Zefir Kurtisi, netdev; +Cc: andrew, f.fainelli
In-Reply-To: <759d6323-ffac-ebe8-b197-4e1165be5673@neratec.com>

Zefir Kurtisi wrote:
> It always operates at 675MHz, which with two lines gives 1.25Gbps,
> which at 10/8 coding gives exactly 1Gbps net data rate. If the
> at803x's copper side autonegotiates to 1Gbps, the bits traversing
> over the SGMII match the copper side 1:1. In case the copper side
> autonegotiates to e.g. 100Mbps, each bit at the copper side on the
> SGMII bus is replicated and sent 10x times - or 100x times in case of
> 10Mbps. The MAC side of the ETH needs to be aware of how the SGMII
> data has to be interpreted, and this is why you have to set the bits
> you are referring to.

So does this mean that the SGMII link should not be autonegotiated? I 
currently have this code:

     if (phydev->autoneg == AUTONEG_ENABLE) {
         val &= ~(FORCE_AN_RX_CFG | FORCE_AN_TX_CFG);
         val |= AN_ENABLE;
         writel(val, phy->base + EMAC_SGMII_PHY_AUTONEG_CFG2);
     } else {
         ...

So if the external PHY is set to autonegotiate, then the SGMII block is 
set to also negotiate.  Now that I think about it, this seems wrong. 
And in fact, I'm not sure how it works.  It seems that the this only 
makes sense if the SGMII block is configured to act as the only PHY. 
This is an option that the hardware supports but my driver does not.  So 
perhaps I should remove this part, and just do the rest:


	u32 speed_cfg;

	switch (phydev->speed) {
	case SPEED_10:
		speed_cfg = SPDMODE_10;
		break;
	case SPEED_100:
		speed_cfg = SPDMODE_100;
		break;
	case SPEED_1000:
		speed_cfg = SPDMODE_1000;
		break;
	default:
		return -EINVAL;
	}

	if (phydev->duplex == DUPLEX_FULL)
		speed_cfg |= DUPLEX_MODE;

	val &= ~AN_ENABLE;
	writel(speed_cfg, phy->base + EMAC_SGMII_PHY_SPEED_CFG1);
	writel(val, phy->base + EMAC_SGMII_PHY_AUTONEG_CFG2);

Should I be doing this all the time?

> To track down who is causing the additional message, I would proceed
> with following technique that helped me dig down a similar problem:
> since you control the events in question and there is no risk of
> flooding the kernel log, in the top of phy.c::phy_print_status() add
> a dump_stack() call. In the debug log ensure that all of the traces
> end up in the same phydev->adjust_link() callback handler (in your
> case emac_adjust_link()).

That's a good idea, thanks.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply

* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Alexei Starovoitov @ 2017-01-20  2:39 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Tejun Heo, Michal Hocko, Peter Zijlstra, David Ahern,
	Andy Lutomirski, Daniel Mack, Mickaël Salaün, Kees Cook,
	Jann Horn, David S. Miller, Thomas Graf, Michael Kerrisk,
	Linux API, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Network Development
In-Reply-To: <CALCETrVrEnL4cvkdDu2LUhxmeOZ+SMEmF=yKKsm9OYoW2y1Kpg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, Jan 18, 2017 at 06:29:22PM -0800, Andy Lutomirski wrote:
> I think it could work by making a single socket cgroup controller that
> handles all cgroup things that are bound to a socket.  Using

Such 'socket cgroup controller' would limit usability of the feature
to sockets and force all other use cases like landlock to invent
their own wheel, which is undesirable. Everyone will be
inventing new 'foo cgroup controller', while all of them
are really bpf features. They are different bpf program
types that attach to different hooks and use cgroup for scoping.

> Having thought about this some more, I think that making it would
> alleviate a bunch of my concerns, as it would make the semantics if
> the capable() check were relaxed to ns_capable() be sane.  Here's what

here we're on the same page. For any meaningful discussion about
'bpf cgroup controller' to happen bpf itself needs to become
delegatable in cgroup sense. In other words BPF_PROG_TYPE_CGROUP*
program types need to become available for unprivileged users.
The only unprivileged prog type today is BPF_PROG_TYPE_SOCKET_FILTER.
To make it secure we severely limited its functionality.
All bpf advances since then (like new map types and verifier extensions)
were done for root only. If early on the priv vs unpriv bpf features
were 80/20. Now it's close to 95/5. No work has been done to
make socket filter type more powerful. It still has to use
slow-ish ld_abs skb access while tc/xdp have direct packet access.
Things like register value tracking is root only as well and so on
and so forth.
We cannot just flip the switch and allow type_cgroup* to unpriv
and I don't see any volunteers willing to do this work.
Until that happens there is no point coming up with designs
for 'cgroup bpf controller'... whatever that means.

> I currently should happen before bpf+cgroup is enabled in a release:
> 
> 1. Make it netns-aware.  This could be as simple as making it only
> work in the root netns because then real netns awareness can be added
> later without breaking anything.  The current situation is bad in that
> network namespaces are just ignored and it's plausible that people
> will start writing user code that depends on having network namespaces
> be ignored.

nothing in bpf today is netns-aware and frankly I don't see
how cgroup+bpf has anything to do with netns.
For regular sockets+bpf we don't check netns.
When tcpdump opens raw socket and attaches bpf there are no netns
checks, since socket itself gives a scope for the program to run.
Same thing applies to cgroup+bpf. cgroup gives a scope for the program.
But, say, we indeed add 'if !root ns' check to BPF_CGROUP_INET_*
hooks. Then if the hooks are used for security, the process
only needs to do setns() to escape security sandbox. Obviously
broken semantics.

> 2. Make it inherit properly.  Inner cgroups should not override outer
> hooks.  As in (1), this could be simplified by preventing the same
> hook from being configured in both an ancestor and a descendent
> cgroup.  Then inheritance could be added for real later on.

In general it sounds fine, but it seems the reasoning to add
such restriction now (instead of later), so that program chain can
be added without breaking abi, since if we don't restrict it now
there will be no way to add it without breaking abi?!
That is incorrect assumption. We can add chaining and can add
'do not override' logic without breaking existing semantics.
For example, we can add 'priority' field to
struct { /* anonymous struct used by BPF_PROG_ATTACH/DETACH commands */}
which would indicate relative position of multiple chained programs
applied to the same cgroup+hook pair. Multiple programs with
the same priority will be executed in the order they were added.
Programs with different priorities will execute in the priority order.
Such scheme will be more generic and flexible than earlier proposals.
Similarly we can add another flag that will say 'dissallow override
of bpf program in descendent cgroup'. It's all trivial to do,
since bpf syscall was designed for extensibility.

Also until bpf_type_cgroup* becomes unprivileged there is no reason
to add this 'priority/prog chaining' feature, since if it's
used for security the root can always override it no matter cgroup
hierarchy.

> 3. Give cgroup delegation support some serious thought.  In
> particular, if delegation would be straightforward but the current API
> wouldn't work well with delegation, then at least consider whether the
> API should change before it becomes stable so that two APIs don't need
> to be supported going forward.

please see example above. Since we went with bpf syscall (instead of
inextensible ioctl) we can add any new cgroup+bpf logic without
breaking current abi.
No matter how you twist it the cgroup+bpf is bpf specific feature.

^ permalink raw reply

* Re: [PATCH] rtlwifi: rtl8192x: Enabling and disabling hardware interrupts after enabling local irq flags
From: Larry Finger @ 2017-01-20  2:41 UTC (permalink / raw)
  To: Bharat Kumar Gogada, chaoming_li, linux-wireless, linux-kernel
  Cc: kvalo, netdev, rgummal, Bharat Kumar Gogada
In-Reply-To: <1484820854-16719-1-git-send-email-bharatku@xilinx.com>

On 01/19/2017 04:14 AM, Bharat Kumar Gogada wrote:
> -Realtek 8192CE chipset maintains local irq flags after enabling/disabling
> hardware interrupts.
> -Hardware interrupts are enabled before enabling the local irq
> flags(these flags are being checked in interrupt handler),
> leading to race condition on some RP, where the irq line between
> bridge and GIC goes high at ASSERT_INTx and goes low only
> at DEASSERT_INTx. In this kind of RP by the time ASSERT_INTx is seen
> irq_enable flag is still set to false, resulting in continuous
> interrupts seen by CPU as DEASSERT_INTx cannot be sent since
> flag is still false and making CPU stall.
> -Changing the sequence of setting these irq flags.
>
> Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com>
> ---

This patch should be enhanced with the smb_xx() calls as suggested by by Lino.

The subject should be changed. I would suggest something like "rtlwifi: 
rtl8192ce: Prevent race condition when enabling interrupts", as it explains the 
condition you are preventing.

The other PCI drivers also have the same problem. Do you want to prepare the 
patches, or should I do it?

Larry

^ permalink raw reply

* Re: [net PATCH v5 6/6] virtio_net: XDP support for adjust_head
From: Jason Wang @ 2017-01-20  3:26 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: John Fastabend, john.r.fastabend, netdev, alexei.starovoitov,
	daniel
In-Reply-To: <20170119231027-mutt-send-email-mst@kernel.org>



On 2017年01月20日 05:11, Michael S. Tsirkin wrote:
> On Thu, Jan 19, 2017 at 11:05:40AM +0800, Jason Wang wrote:
>>
>> On 2017年01月18日 23:15, Michael S. Tsirkin wrote:
>>> On Tue, Jan 17, 2017 at 02:22:59PM -0800, John Fastabend wrote:
>>>> Add support for XDP adjust head by allocating a 256B header region
>>>> that XDP programs can grow into. This is only enabled when a XDP
>>>> program is loaded.
>>>>
>>>> In order to ensure that we do not have to unwind queue headroom push
>>>> queue setup below bpf_prog_add. It reads better to do a prog ref
>>>> unwind vs another queue setup call.
>>>>
>>>> At the moment this code must do a full reset to ensure old buffers
>>>> without headroom on program add or with headroom on program removal
>>>> are not used incorrectly in the datapath. Ideally we would only
>>>> have to disable/enable the RX queues being updated but there is no
>>>> API to do this at the moment in virtio so use the big hammer. In
>>>> practice it is likely not that big of a problem as this will only
>>>> happen when XDP is enabled/disabled changing programs does not
>>>> require the reset. There is some risk that the driver may either
>>>> have an allocation failure or for some reason fail to correctly
>>>> negotiate with the underlying backend in this case the driver will
>>>> be left uninitialized. I have not seen this ever happen on my test
>>>> systems and for what its worth this same failure case can occur
>>>> from probe and other contexts in virtio framework.
>>>>
>>>> Signed-off-by: John Fastabend<john.r.fastabend@intel.com>
>>> I've been thinking about it - can't we drop
>>> old buffers without the head room which were posted before
>>> xdp attached?
>>>
>>> Avoiding the reset would be much nicer.
>>>
>>> Thoughts?
>>>
>> As been discussed before, device may use them in the same time so it's not
>> safe. Or do you mean detect them after xdp were set and drop the buffer
>> without head room, this looks sub-optimal.
>>
>> Thanks
> Yes, this is what I mean.  Why is this suboptimal? It's a single branch
> in code. Yes we might lose some packets but the big hammer of device
> reset will likely lose more.
>

Maybe I was wrong but I think driver should try their best to avoid 
dropping packets. (And look at mlx4, it did something similar to this 
patch).

Thanks

^ permalink raw reply

* Re: [PATCH v2 0/2] net: dsa: Move temperature sensor code into PHY.
From: Vivien Didelot @ 2017-01-20  3:26 UTC (permalink / raw)
  To: Andrew Lunn, David Miller; +Cc: netdev
In-Reply-To: <20170120004328.GA381@lunn.ch>

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> Sorry, i forget the net-next in the subject line.

Maybe use this opportunity to resend this thread with the good subject
prefix and the people involved in v1?

Thanks,

        Vivien

^ 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