* Re: [PATCH V2 1/3] Documentation: devicetree: add multiple cpu port DSA binding
From: Florian Fainelli @ 2017-06-08 19:57 UTC (permalink / raw)
To: Andrew Lunn
Cc: Rob Herring, John Crispin, Vivien Didelot, David S . Miller,
Sean Wang, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170608193112.GF20216-g2DYL2Zd6BY@public.gmane.org>
On 06/08/2017 12:31 PM, Andrew Lunn wrote:
>> Right now we don't have any mechanism, and statically doing this from
>> Device Tree is too inflexible. I have been working on a parallel path
>> where we use the bridge (which is already accelerated when there is a
>> switch) in order to define groups of ports, the idea would be do to e.g:
>>
>> brctl addbr br-lan
>> brctl addbr br-lan eth0
>> brctl addbr br-lan lan1
>> ...
>> brctl addbr br-lan lan4
>>
>> brctl addbr br-wan
>> brctl addbr br-wan eth1
>> brctl addbr br-wan wan
>>
>> Assuming that lan1-lan4 are your LAN ports, and wan is your WAN port and
>> you have two CPU ports.
>
> Hi Florian
>
> I don't like this, on multiple levels.
>
> My wan port typically never has more than 40Mbps of traffic on it. So
> dedicating a whole 1Gbps ethernet to it makes no sense. I want to
> share eth1 bandwidth with the wan port, and some of the other
> ports. Meaning i would have to add eth1 to br-lan as well as
> br-wan. Does the bridge allow that? And what sort of hacks do you have
> to allow a port to be added to a bridge, but not used by the bridge?
This is AN example of how to configure a port grouping based on John's
example and use case, not THE only example ;) The idea is obviously that
you can define association between user-facing ports and CPU ports as
you see fit, the idea is to use bridge to do that association because
that's already what controls VLAN membership and forwarding.
>
> And what is the point of br-wan? It only has one real port in it. So
> i'm adding per-packet overhead which i don't need, just in order to
> signal to the hardware how it should statically route cpu traffic for
> a port.
No, it has two ports in it, adding eth1 is necessary do define the VLAN
membership and forwarding rules, when eth1 is added we resolve the CPU
port this corresponds to in the switch and we use that to define the
forwarding decision between ports.
The overhead per-packet is extremely limited because the first thing
br_handle_frame() will do is see that eth1 is a DSA master network
device and pass packets back up the stack for processing through dst->rcv().
>
> Now say i have one of the bigger marvell switches, with 11 ports, in
> an industrial application. I setup 3 or 4 bridges. I then need to add
> eth0 and eth1 to two different bridges. And say i use some ports> without a bridge. How do i configure them?
If you don't add your conduit interface to the bridge, then the default
CPU interface (which could be left to the driver to decide which one is
relevant) gets used and things work as expected. When you add a DSA
master network device to the bridge, only then do we use that
information to refine the forwarding decision and map to the appropriate
port vectors. Doing this becomes necessary if you create a second (or
more) bridge to isolate a group of ports.
>
> And how do i dump the current mapping?
You look at both (or more) bridges' members, what's wrong or even any
different from what happens today?
>
> For me, this is the wrong architecture. What CPU port is used should
> be a port property, not a bridge property. I think we should use
> devlink. Add commands to dump the current mapping, and set it.
In premise I agree, although just like we need a bridge today to
configure VLAN memberships, it did not seem to me like a big stretch to
use a bridge to configure which CPU port you want.
--
Florian
--
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
* Re: [PATCH net] openvswitch: warn about missing first netlink attribute
From: David Miller @ 2017-06-08 19:57 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev, pshelar, dev
In-Reply-To: <1496911065-6912-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 8 Jun 2017 10:37:45 +0200
> The first netlink attribute (value 0) must always be defined
> as none/unspec.
>
> Because we cannot change an existing UAPI, I add a comment to point the
> mistake and avoid to propagate it in a new ovs API in the future.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Ok, I agree, we don't want people cut-and-pasting this kind of thing
and hopefully this comment prevents that.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] af_unix: Add sockaddr length checks before accessing sa_family in bind and connect handlers
From: David Miller @ 2017-06-08 20:04 UTC (permalink / raw)
To: mjurczyk
Cc: xiyou.wangcong, hannes, viro, keescook, mszeredi, iboukris,
avagin, netdev, linux-kernel
In-Reply-To: <20170608091336.8274-1-mjurczyk@google.com>
From: Mateusz Jurczyk <mjurczyk@google.com>
Date: Thu, 8 Jun 2017 11:13:36 +0200
> Verify that the caller-provided sockaddr structure is large enough to
> contain the sa_family field, before accessing it in bind() and connect()
> handlers of the AF_UNIX socket. Since neither syscall enforces a minimum
> size of the corresponding memory region, very short sockaddrs (zero or
> one byte long) result in operating on uninitialized memory while
> referencing .sa_family.
>
> Signed-off-by: Mateusz Jurczyk <mjurczyk@google.com>
The sockaddr comes from a structure on the caller's kernel stack, even
if the user gives a smaller length, it is legal to access that memory.
^ permalink raw reply
* Re: [PATCH net 1/3] bonding: fix 802.3ad support for 5G and 50G speeds
From: David Miller @ 2017-06-08 20:05 UTC (permalink / raw)
To: nicolas.dichtel
Cc: netdev, j.vosburgh, vfalico, andy, thibaut.collet, linville
In-Reply-To: <1496913493-23293-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 8 Jun 2017 11:18:11 +0200
> From: Thibaut Collet <thibaut.collet@6wind.com>
>
> This patch adds [5|50] Gbps enum definition, and fixes
> aggregated bandwidth calculation based on above slave links.
>
> Fixes: c9a70d43461d ("net-next: ethtool: Added port speed macros.")
> Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied.
^ permalink raw reply
* Re: [PATCH net 2/3] bonding: fix 802.3ad support for 14G speed
From: David Miller @ 2017-06-08 20:06 UTC (permalink / raw)
To: nicolas.dichtel
Cc: netdev, j.vosburgh, vfalico, andy, thibaut.collet, linville
In-Reply-To: <1496913493-23293-2-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 8 Jun 2017 11:18:12 +0200
> This patch adds 14 Gbps enum definition, and fixes
> aggregated bandwidth calculation based on above slave links.
>
> Fixes: 0d7e2d2166f6 ("IB/ipoib: add get_link_ksettings in ethtool")
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied.
^ permalink raw reply
* Re: [PATCH net 3/3] ethtool.h: remind to update 802.3ad when adding new speeds
From: David Miller @ 2017-06-08 20:06 UTC (permalink / raw)
To: nicolas.dichtel
Cc: netdev, j.vosburgh, vfalico, andy, thibaut.collet, linville
In-Reply-To: <1496913493-23293-3-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 8 Jun 2017 11:18:13 +0200
> Each time a new speed is added, the bonding 802.3ad isn't updated. Add a
> comment to remind the developer to update this driver.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied.
^ permalink raw reply
* Re: [PATCH 0/5] net: mvpp2: fixes and cleanups
From: David Miller @ 2017-06-08 20:11 UTC (permalink / raw)
To: thomas.petazzoni
Cc: netdev, mw, stefanc, nadavh, hannah, yehuday, antoine.tenart,
gregory.clement, elfring, marc.zyngier
In-Reply-To: <1496935647-29670-1-git-send-email-thomas.petazzoni@free-electrons.com>
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Thu, 8 Jun 2017 17:27:22 +0200
> Here is a small set of fixes/improvements for the mvpp2 driver.
>
> The first two patches are fixes: they fix bogus usage of
> smp_processor_id() in a migration-enabled context. Indeed currently
> the driver outputs some fat warnings in CONFIG_DEBUG_PREEMPT-enabled
> kernels. Therefore, some fixes should be pushed to stable.
>
> The last three patches are cleanups and not needed for stable, but
> they stack on top of the fixes.
Please do not mix genuine bug fixes and cleanups.
You must submit them separately.
Send the fixes targetting 'net', and once those are accepted and
'net' is merged into 'net-next', you can submit the cleanups.
Thank you.
^ permalink raw reply
* [PATCH v2 net-next] Ipvlan should return an error when an address is already in use.
From: Krister Johansen @ 2017-06-08 20:12 UTC (permalink / raw)
To: David Miller; +Cc: maheshb, netdev
In-Reply-To: <20170101.222632.843875843588487678.davem@davemloft.net>
The ipvlan code already knows how to detect when a duplicate address is
about to be assigned to an ipvlan device. However, that failure is not
propogated outward and leads to a silent failure.
Introduce a validation step at ip address creation time and allow device
drivers to register to validate the incoming ip addresses. The ipvlan
code is the first consumer. If it detects an address in use, we can
return an error to the user before beginning to commit the new ifa in
the networking code.
This can be especially useful if it is necessary to provision many
ipvlans in containers. The provisioning software (or operator) can use
this to detect situations where an ip address is unexpectedly in use.
Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
---
drivers/net/ipvlan/ipvlan_main.c | 69 ++++++++++++++++++++++++++++++++++++++++
include/linux/inetdevice.h | 7 ++++
include/net/addrconf.h | 10 +++++-
net/ipv4/devinet.c | 33 +++++++++++++++++++
net/ipv6/addrconf.c | 17 +++++++++-
net/ipv6/addrconf_core.c | 19 +++++++++++
6 files changed, 153 insertions(+), 2 deletions(-)
Apologies for letting this one languish for so long. This iteration is
more code than I hoped it would be, but I believe it addresses the
concerns raised in the prior iteration of the review.
Changes v1 -> v2:
- Add a separate validator chain with ipvlan as the first consumer.
[Address Dave M.'s comment about needing all chain users to agree
about use of notifier_[to|from]_errno]
- Run validator chain only during address creation.
[Address Dave M.'s comment about prior version failing to handle
primary address promotions.]
- Run validator step before the atomic section in the ip address create
path.
[Address my own dissatisfaction with having to rollback and
potentially issue an immediate up and then down RTNETLINK event.]
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 618ed88..e4141d6 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -824,6 +824,33 @@ static int ipvlan_addr6_event(struct notifier_block *unused,
return NOTIFY_OK;
}
+static int ipvlan_addr6_validator_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
+ struct net_device *dev = (struct net_device *)i6vi->i6vi_dev->dev;
+ struct ipvl_dev *ipvlan = netdev_priv(dev);
+
+ /* FIXME IPv6 autoconf calls us from bh without RTNL */
+ if (in_softirq())
+ return NOTIFY_DONE;
+
+ if (!netif_is_ipvlan(dev))
+ return NOTIFY_DONE;
+
+ if (!ipvlan || !ipvlan->port)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case NETDEV_UP:
+ if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true))
+ return notifier_from_errno(-EADDRINUSE);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
{
if (ipvlan_addr_busy(ipvlan->port, ip4_addr, false)) {
@@ -871,10 +898,37 @@ static int ipvlan_addr4_event(struct notifier_block *unused,
return NOTIFY_OK;
}
+static int ipvlan_addr4_validator_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in_validator_info *ivi = (struct in_validator_info *)ptr;
+ struct net_device *dev = (struct net_device *)ivi->ivi_dev->dev;
+ struct ipvl_dev *ipvlan = netdev_priv(dev);
+
+ if (!netif_is_ipvlan(dev))
+ return NOTIFY_DONE;
+
+ if (!ipvlan || !ipvlan->port)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case NETDEV_UP:
+ if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false))
+ return notifier_from_errno(-EADDRINUSE);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
static struct notifier_block ipvlan_addr4_notifier_block __read_mostly = {
.notifier_call = ipvlan_addr4_event,
};
+static struct notifier_block ipvlan_addr4_vtor_notifier_block __read_mostly = {
+ .notifier_call = ipvlan_addr4_validator_event,
+};
+
static struct notifier_block ipvlan_notifier_block __read_mostly = {
.notifier_call = ipvlan_device_event,
};
@@ -883,6 +937,10 @@ static struct notifier_block ipvlan_addr6_notifier_block __read_mostly = {
.notifier_call = ipvlan_addr6_event,
};
+static struct notifier_block ipvlan_addr6_vtor_notifier_block __read_mostly = {
+ .notifier_call = ipvlan_addr6_validator_event,
+};
+
static void ipvlan_ns_exit(struct net *net)
{
struct ipvlan_netns *vnet = net_generic(net, ipvlan_netid);
@@ -907,7 +965,10 @@ static int __init ipvlan_init_module(void)
ipvlan_init_secret();
register_netdevice_notifier(&ipvlan_notifier_block);
register_inet6addr_notifier(&ipvlan_addr6_notifier_block);
+ register_inet6addr_validator_notifier(
+ &ipvlan_addr6_vtor_notifier_block);
register_inetaddr_notifier(&ipvlan_addr4_notifier_block);
+ register_inetaddr_validator_notifier(&ipvlan_addr4_vtor_notifier_block);
err = register_pernet_subsys(&ipvlan_net_ops);
if (err < 0)
@@ -922,7 +983,11 @@ static int __init ipvlan_init_module(void)
return 0;
error:
unregister_inetaddr_notifier(&ipvlan_addr4_notifier_block);
+ unregister_inetaddr_validator_notifier(
+ &ipvlan_addr4_vtor_notifier_block);
unregister_inet6addr_notifier(&ipvlan_addr6_notifier_block);
+ unregister_inet6addr_validator_notifier(
+ &ipvlan_addr6_vtor_notifier_block);
unregister_netdevice_notifier(&ipvlan_notifier_block);
return err;
}
@@ -933,7 +998,11 @@ static void __exit ipvlan_cleanup_module(void)
unregister_pernet_subsys(&ipvlan_net_ops);
unregister_netdevice_notifier(&ipvlan_notifier_block);
unregister_inetaddr_notifier(&ipvlan_addr4_notifier_block);
+ unregister_inetaddr_validator_notifier(
+ &ipvlan_addr4_vtor_notifier_block);
unregister_inet6addr_notifier(&ipvlan_addr6_notifier_block);
+ unregister_inet6addr_validator_notifier(
+ &ipvlan_addr6_vtor_notifier_block);
}
module_init(ipvlan_init_module);
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index a2e9d6e..e7c04c4 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -150,8 +150,15 @@ struct in_ifaddr {
unsigned long ifa_tstamp; /* updated timestamp */
};
+struct in_validator_info {
+ __be32 ivi_addr;
+ struct in_device *ivi_dev;
+};
+
int register_inetaddr_notifier(struct notifier_block *nb);
int unregister_inetaddr_notifier(struct notifier_block *nb);
+int register_inetaddr_validator_notifier(struct notifier_block *nb);
+int unregister_inetaddr_validator_notifier(struct notifier_block *nb);
void inet_netconf_notify_devconf(struct net *net, int event, int type,
int ifindex, struct ipv4_devconf *devconf);
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index b43a4ee..d0889cb 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -48,11 +48,15 @@ struct prefix_info {
struct in6_addr prefix;
};
-
#include <linux/netdevice.h>
#include <net/if_inet6.h>
#include <net/ipv6.h>
+struct in6_validator_info {
+ struct in6_addr i6vi_addr;
+ struct inet6_dev *i6vi_dev;
+};
+
#define IN6_ADDR_HSIZE_SHIFT 4
#define IN6_ADDR_HSIZE (1 << IN6_ADDR_HSIZE_SHIFT)
@@ -278,6 +282,10 @@ int register_inet6addr_notifier(struct notifier_block *nb);
int unregister_inet6addr_notifier(struct notifier_block *nb);
int inet6addr_notifier_call_chain(unsigned long val, void *v);
+int register_inet6addr_validator_notifier(struct notifier_block *nb);
+int unregister_inet6addr_validator_notifier(struct notifier_block *nb);
+int inet6addr_validator_notifier_call_chain(unsigned long val, void *v);
+
void inet6_netconf_notify_devconf(struct net *net, int event, int type,
int ifindex, struct ipv6_devconf *devconf);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index df14815..a7dd088 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -176,6 +176,7 @@ EXPORT_SYMBOL(__ip_dev_find);
static void rtmsg_ifa(int event, struct in_ifaddr *, struct nlmsghdr *, u32);
static BLOCKING_NOTIFIER_HEAD(inetaddr_chain);
+static BLOCKING_NOTIFIER_HEAD(inetaddr_validator_chain);
static void inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
int destroy);
#ifdef CONFIG_SYSCTL
@@ -441,6 +442,8 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
{
struct in_device *in_dev = ifa->ifa_dev;
struct in_ifaddr *ifa1, **ifap, **last_primary;
+ struct in_validator_info ivi;
+ int ret;
ASSERT_RTNL();
@@ -471,6 +474,23 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
}
}
+ /* Allow any devices that wish to register ifaddr validtors to weigh
+ * in now, before changes are committed. The rntl lock is serializing
+ * access here, so the state should not change between a validator call
+ * and a final notify on commit. This isn't invoked on promotion under
+ * the assumption that validators are checking the address itself, and
+ * not the flags.
+ */
+ ivi.ivi_addr = ifa->ifa_address;
+ ivi.ivi_dev = ifa->ifa_dev;
+ ret = blocking_notifier_call_chain(&inetaddr_validator_chain,
+ NETDEV_UP, &ivi);
+ ret = notifier_to_errno(ret);
+ if (ret) {
+ inet_free_ifa(ifa);
+ return ret;
+ }
+
if (!(ifa->ifa_flags & IFA_F_SECONDARY)) {
prandom_seed((__force u32) ifa->ifa_local);
ifap = last_primary;
@@ -1356,6 +1376,19 @@ int unregister_inetaddr_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_inetaddr_notifier);
+int register_inetaddr_validator_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&inetaddr_validator_chain, nb);
+}
+EXPORT_SYMBOL(register_inetaddr_validator_notifier);
+
+int unregister_inetaddr_validator_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&inetaddr_validator_chain,
+ nb);
+}
+EXPORT_SYMBOL(unregister_inetaddr_validator_notifier);
+
/* Rename ifa_labels for a device name change. Make some effort to preserve
* existing alias numbering and to create unique labels if possible.
*/
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 25443fd..0aa36b0 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -963,6 +963,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
struct rt6_info *rt;
+ struct in6_validator_info i6vi;
unsigned int hash;
int err = 0;
int addr_type = ipv6_addr_type(addr);
@@ -974,6 +975,9 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
return ERR_PTR(-EADDRNOTAVAIL);
rcu_read_lock_bh();
+
+ in6_dev_hold(idev);
+
if (idev->dead) {
err = -ENODEV; /*XXX*/
goto out2;
@@ -984,6 +988,17 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
goto out2;
}
+ i6vi.i6vi_addr = *addr;
+ i6vi.i6vi_dev = idev;
+ rcu_read_unlock_bh();
+
+ err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
+
+ rcu_read_lock_bh();
+ err = notifier_to_errno(err);
+ if (err)
+ goto out2;
+
spin_lock(&addrconf_hash_lock);
/* Ignore adding duplicate addresses on an interface */
@@ -1034,7 +1049,6 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifa->rt = rt;
ifa->idev = idev;
- in6_dev_hold(idev);
/* For caller */
in6_ifa_hold(ifa);
@@ -1062,6 +1076,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
inet6addr_notifier_call_chain(NETDEV_UP, ifa);
else {
kfree(ifa);
+ in6_dev_put(idev);
ifa = ERR_PTR(err);
}
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index bfa941f..9e3488d 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -88,6 +88,7 @@ int __ipv6_addr_type(const struct in6_addr *addr)
EXPORT_SYMBOL(__ipv6_addr_type);
static ATOMIC_NOTIFIER_HEAD(inet6addr_chain);
+static ATOMIC_NOTIFIER_HEAD(inet6addr_validator_chain);
int register_inet6addr_notifier(struct notifier_block *nb)
{
@@ -107,6 +108,24 @@ int inet6addr_notifier_call_chain(unsigned long val, void *v)
}
EXPORT_SYMBOL(inet6addr_notifier_call_chain);
+int register_inet6addr_validator_notifier(struct notifier_block *nb)
+{
+ return atomic_notifier_chain_register(&inet6addr_validator_chain, nb);
+}
+EXPORT_SYMBOL(register_inet6addr_validator_notifier);
+
+int unregister_inet6addr_validator_notifier(struct notifier_block *nb)
+{
+ return atomic_notifier_chain_unregister(&inet6addr_validator_chain, nb);
+}
+EXPORT_SYMBOL(unregister_inet6addr_validator_notifier);
+
+int inet6addr_validator_notifier_call_chain(unsigned long val, void *v)
+{
+ return atomic_notifier_call_chain(&inet6addr_validator_chain, val, v);
+}
+EXPORT_SYMBOL(inet6addr_validator_notifier_call_chain);
+
static int eafnosupport_ipv6_dst_lookup(struct net *net, struct sock *u1,
struct dst_entry **u2,
struct flowi6 *u3)
--
2.7.4
^ permalink raw reply related
* [PATCH net] Fix an intermittent pr_emerg warning about lo becoming free.
From: Krister Johansen @ 2017-06-08 20:12 UTC (permalink / raw)
To: davem, eric.dumazet; +Cc: netdev
It looks like this:
Message from syslogd@flamingo at Apr 26 00:45:00 ...
kernel:unregister_netdevice: waiting for lo to become free. Usage count = 4
They seem to coincide with net namespace teardown.
The message is emitted by netdev_wait_allrefs().
Forced a kdump in netdev_run_todo, but found that the refcount on the lo
device was already 0 at the time we got to the panic.
Used bcc to check the blocking in netdev_run_todo. The only places
where we're off cpu there are in the rcu_barrier() and msleep() calls.
That behavior is expected. The msleep time coincides with the amount of
time we spend waiting for the refcount to reach zero; the rcu_barrier()
wait times are not excessive.
After looking through the list of callbacks that the netdevice notifiers
invoke in this path, it appears that the dst_dev_event is the most
interesting. The dst_ifdown path places a hold on the loopback_dev as
part of releasing the dev associated with the original dst cache entry.
Most of our notifier callbacks are straight-forward, but this one a)
looks complex, and b) places a hold on the network interface in
question.
I constructed a new bcc script that watches various events in the
liftime of a dst cache entry. Note that dst_ifdown will take a hold on
the loopback device until the invalidated dst entry gets freed.
[ __dst_free] on DST: ffff883ccabb7900 IF tap1008300eth0 invoked at 1282115677036183
__dst_free
rcu_nocb_kthread
kthread
ret_from_fork
---
[ dst_gc_task] completed in 4: start: 1282115783057516 end 1282115783062462
[ dst_gc_task] completed in 5: start: 1282116447054101 end 1282116447059392
[ dst_gc_task] completed in 5: start: 1282118111030391 end 1282118111036041
[ dst_gc_task] completed in 6: start: 1282121247074938 end 1282121247081141
[ dst_gc_task] completed in 5: start: 1282126815053252 end 1282126815058751
[ dst_gc_task] completed in 6: start: 1282135007055673 end 1282135007061877
[ dst_gc_task] completed in 6: start: 1282145759021745 end 1282145759027830
[ dst_release] on DST: ffff883ccabb7900 IF tap1008300eth0 invoked at 1282147838674257
dst_release
ip_queue_xmit
tcp_transmit_skb
tcp_write_xmit
__tcp_push_pending_frames
tcp_send_fin
tcp_close
inet_release
sock_release
sock_close
__fput
____fput
task_work_run
do_exit
do_group_exit
SyS_exit_group
do_syscall_64
return_from_SYSCALL_64
[ dst_ifdown] on DST: ffff883ccabb7900 IF tap1008300eth0 invoked at 1282148067377680
dst_ifdown
notifier_call_chain
raw_notifier_call_chain
call_netdevice_notifiers_info
netdev_run_todo
kretprobe_trampoline
default_device_exit_batch
ops_exit_list.isra.4
cleanup_net
process_one_work
worker_thread
kthread
ret_from_fork
---
[ dst_ifdown] on DST: ffff883ccabb7900 IF lo invoked at 1282148067411269
dst_ifdown
notifier_call_chain
raw_notifier_call_chain
call_netdevice_notifiers_info
netdev_run_todo
kretprobe_trampoline
default_device_exit_batch
ops_exit_list.isra.4
cleanup_net
process_one_work
worker_thread
kthread
ret_from_fork
---
<...>
[ dst_destroy] on DST: ffff883ccabb7900 IF lo invoked at 1282160607074022
dst_destroy
kretprobe_trampoline
worker_thread
kthread
ret_from_fork
---
[ dst_gc_task] completed in 30: start: 1282160607047161 end 1282160607077572
The way this works is that if there's still a reference on the dst entry
at the time we try to free it, it gets placed in the gc list by
__dst_free and the dst_destroy() call is invoked by the gc task once the
refcount is 0. If the gc task processes a 10th or less of its entries
on a single pass, it inreases the amount of time it waits between gc
intervals.
Looking at the gc_task intervals, they started at 663ms when we invoked
__dst_free(). After that, they increased to 1663, 3136, 5567, 8191,
10751, and 14848. The release that set the refcnt to 0 on our dst entry
occurred after the gc_task was enqueued for 14 second interval so we had
to wait longer than the warning time in wait_allrefs in order for the
dst entry to get free'd and the hold on 'lo' to be released.
A simple solution to this problem is to have dst_dev_event() reset the
gc timer, which causes us to process this list shortly after the
gc_mutex is relased when dst_dev_event() completes.
Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
---
net/core/dst.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/core/dst.c b/net/core/dst.c
index 6192f11..13ba4a0 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -469,6 +469,20 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event,
spin_lock_bh(&dst_garbage.lock);
dst = dst_garbage.list;
dst_garbage.list = NULL;
+ /* The code in dst_ifdown places a hold on the loopback device.
+ * If the gc entry processing is set to expire after a lengthy
+ * interval, this hold can cause netdev_wait_allrefs() to hang
+ * out and wait for a long time -- until the the loopback
+ * interface is released. If we're really unlucky, it'll emit
+ * pr_emerg messages to console too. Reset the interval here,
+ * so dst cleanups occur in a more timely fashion.
+ */
+ if (dst_garbage.timer_inc > DST_GC_INC) {
+ dst_garbage.timer_inc = DST_GC_INC;
+ dst_garbage.timer_expires = DST_GC_MIN;
+ mod_delayed_work(system_wq, &dst_gc_work,
+ dst_garbage.timer_expires);
+ }
spin_unlock_bh(&dst_garbage.lock);
if (last)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] ila_xlat: add missing hash secret initialization
From: Tom Herbert @ 2017-06-08 20:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, stephen hemminger,
Linux Kernel Network Developers, LKML
In-Reply-To: <20170608075448.2076975-1-arnd@arndb.de>
On Thu, Jun 8, 2017 at 12:54 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> While discussing the possible merits of clang warning about unused initialized
> functions, I found one function that was clearly meant to be called but
> never actually is.
>
> __ila_hash_secret_init() initializes the hash value for the ila locator,
> apparently this is intended to prevent hash collision attacks, but this ends
> up being a read-only zero constant since there is no caller. I could find
> no indication of why it was never called, the earliest patch submission
> for the module already was like this. If my interpretation is right, we
> certainly want to backport the patch to stable kernels as well.
>
> I considered adding it to the ila_xlat_init callback, but for best effect
> the random data is read as late as possible, just before it is first used.
> The underlying net_get_random_once() is already highly optimized to avoid
> overhead when called frequently.
>
> Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")
> Cc: stable@vger.kernel.org
> Link: https://www.spinics.net/lists/kernel/msg2527243.html
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> net/ipv6/ila/ila_xlat.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
> index 2fd5ca151dcf..77f7f8c7d93d 100644
> --- a/net/ipv6/ila/ila_xlat.c
> +++ b/net/ipv6/ila/ila_xlat.c
> @@ -62,6 +62,7 @@ static inline u32 ila_locator_hash(struct ila_locator loc)
> {
> u32 *v = (u32 *)loc.v32;
>
> + __ila_hash_secret_init();
> return jhash_2words(v[0], v[1], hashrnd);
> }
>
> --
> 2.9.0
>
Thanks Arnd!
Acked-by: Tom Herbert <tom@herbertland.com>
^ permalink raw reply
* Re: [PATCH net] bpf, tests: fix endianness selection
From: David Miller @ 2017-06-08 20:18 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev
In-Reply-To: <086cffcb4e5cf48bc21be6961c16df53db9155cb.1496941264.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 8 Jun 2017 19:06:25 +0200
> I noticed that test_l4lb was failing in selftests:
...
> Tracking down the issue actually revealed that endianness selection
> in bpf_endian.h is broken when compiled with clang with bpf target.
> test_pkt_access.c, test_l4lb.c is compiled with __BYTE_ORDER as
> __BIG_ENDIAN, test_xdp.c as __LITTLE_ENDIAN! test_l4lb noticeably
> fails, because the test accounts bytes via bpf_ntohs(ip6h->payload_len)
> and bpf_ntohs(iph->tot_len), and compares them against a defined
> value and given a wrong endianness, the test outcome is different,
> of course.
>
> Turns out that there are actually two bugs: i) when we do __BYTE_ORDER
> comparison with __LITTLE_ENDIAN/__BIG_ENDIAN, then depending on the
> include order we see different outcomes. Reason is that __BYTE_ORDER
> is undefined due to missing endian.h include. Before we include the
> asm/byteorder.h (e.g. through linux/in.h), then __BYTE_ORDER equals
> __LITTLE_ENDIAN since both are undefined, after the include which
> correctly pulls in linux/byteorder/little_endian.h, __LITTLE_ENDIAN
> is defined, but given __BYTE_ORDER is still undefined, we match on
> __BYTE_ORDER equals to __BIG_ENDIAN since __BIG_ENDIAN is also
> undefined at that point, sigh. ii) But even that would be wrong,
> since when compiling the test cases with clang, one can select between
> bpfeb and bpfel targets for cross compilation. Hence, we can also not
> rely on what the system's endian.h provides, but we need to look at
> the compiler's defined endianness. The compiler defines __BYTE_ORDER__,
> and we can match __ORDER_LITTLE_ENDIAN__ and __ORDER_BIG_ENDIAN__,
> which also reflects targets bpf (native), bpfel, bpfeb correctly,
> thus really only rely on that. After patch:
>
> # ./test_progs
> test_pkt_access:PASS:ipv4 74 nsec
> test_pkt_access:PASS:ipv6 42 nsec
> test_xdp:PASS:ipv4 2340 nsec
> test_xdp:PASS:ipv6 1461 nsec
> test_l4lb:PASS:ipv4 400 nsec
> test_l4lb:PASS:ipv6 530 nsec
> test_tcp_estats:PASS: 0 nsec
> Summary: 7 PASSED, 0 FAILED
>
> Fixes: 43bcf707ccdc ("bpf: fix _htons occurences in test_progs")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5] bpf: rewrite value tracking in verifier
From: David Miller @ 2017-06-08 20:18 UTC (permalink / raw)
To: ecree; +Cc: alexei.starovoitov, ast, daniel, netdev, iovisor-dev,
linux-kernel
In-Reply-To: <92db9689-af6a-e172-ba57-195e588f9cc0@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Wed, 7 Jun 2017 15:55:57 +0100
> This series simplifies alignment tracking, generalises bounds tracking and
> fixes some bounds-tracking bugs in the BPF verifier. Pointer arithmetic on
> packet pointers, stack pointers, map value pointers and context pointers has
> been unified, and bounds on these pointers are only checked when the pointer
> is dereferenced.
> Operations on pointers which destroy all relation to the original pointer
> (such as multiplies and shifts) are disallowed if !env->allow_ptr_leaks,
> otherwise they convert the pointer to an unknown scalar and feed it to the
> normal scalar arithmetic handling.
> Pointer types have been unified with the corresponding adjusted-pointer types
> where those existed (e.g. PTR_TO_MAP_VALUE[_ADJ] or FRAME_PTR vs
> PTR_TO_STACK); similarly, CONST_IMM and UNKNOWN_VALUE have been unified into
> SCALAR_VALUE.
> Pointer types (except CONST_PTR_TO_MAP, PTR_TO_MAP_VALUE_OR_NULL and
> PTR_TO_PACKET_END, which do not allow arithmetic) have a 'fixed offset' and
> a 'variable offset'; the former is used when e.g. adding an immediate or a
> known-constant register, as long as it does not overflow. Otherwise the
> latter is used, and any operation creating a new variable offset creates a
> new 'id' (and, for PTR_TO_PACKET, clears the 'range').
> SCALAR_VALUEs use the 'variable offset' fields to track the range of possible
> values; the 'fixed offset' should never be set on a scalar.
>
> Patch 2/5 is rather on the big side, but since it changes the contents and
> semantics of a fairly central data structure, I'm not really sure how to go
> about splitting it up further without producing broken intermediate states.
>
> With the changes in patch 5/5, all tools/testing/selftests/bpf/test_verifier
> tests pass.
Edward, I haven't had a chance to review this yet, but I wanted to thank you
for working on this.
I will find some time to test your work on sparc too.
Thanks again!
^ permalink raw reply
* Re: [PATCH net] ipv4: igmp: fix a use after free
From: Eric Dumazet @ 2017-06-08 20:33 UTC (permalink / raw)
To: Xin Long; +Cc: Andrey Konovalov, David S. Miller, netdev
In-Reply-To: <CADvbK_fWOFWZxKc8gbzeDeU1ALq5GHxKvoGsPt+GsdAP0a601w@mail.gmail.com>
On Fri, 2017-06-09 at 02:22 +0800, Xin Long wrote:
> On Thu, Jun 8, 2017 at 9:43 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Andrey reported a use-after-free in add_grec(), courtesy of syzkaller.
> >
> > Problem here is that igmp_stop_timer() uses a del_timer(), so we can not
> > guarantee that another cpu is not servicing the timer.
> >
> > Therefore, if igmp_group_dropped() call from ip_mc_dec_group() is
> > immediately followed by ip_mc_clear_src(), ip_mc_clear_src() might free
> > memory that could be used by the other cpu servicing the timer.
> >
> > To fix this issue, we should defer the memory freeing
> > (ip_mc_clear_src()) to the point all references to (struct
> > ip_mc_list)->refcnt have been released.
> > This happens in ip_ma_put()
> >
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in add_grec+0x101e/0x1090 net/ipv4/igmp.c:473
> > Read of size 8 at addr ffff88003053c1a0 by task swapper/0/0
> >
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.12.0-rc3+ #370
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > Call Trace:
> > <IRQ>
> > __dump_stack lib/dump_stack.c:16 [inline]
> > dump_stack+0x292/0x395 lib/dump_stack.c:52
> > print_address_description+0x73/0x280 mm/kasan/report.c:252
> > kasan_report_error mm/kasan/report.c:351 [inline]
> > kasan_report+0x22b/0x340 mm/kasan/report.c:408
> > __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:429
> > add_grec+0x101e/0x1090 net/ipv4/igmp.c:473
> > igmpv3_send_cr net/ipv4/igmp.c:663 [inline]
> > igmp_ifc_timer_expire+0x46d/0xa80 net/ipv4/igmp.c:768
> the call trace is igmp_ifc_timer_expire -> igmpv3_send_cr -> add_grec
> and the timer should be in_dev->mr_ifc_timer.
> but igmp_stop_timer you mentioned is used to stop im->timer
>
> It's possible that ip_mc_clear_src is done in ip_ma_put()
> while igmp_ifc_timer_expire is still using ip_mc_list under
> rcu_read_lock(). no ?
You might be right. I looked at the freeing side
> kfree+0xe8/0x2b0 mm/slub.c:3882
> ip_mc_clear_src+0x69/0x1c0 net/ipv4/igmp.c:2078
> ip_mc_dec_group+0x19a/0x470 net/ipv4/igmp.c:1618
> ip_mc_drop_socket+0x145/0x230 net/ipv4/igmp.c:2609
> inet_release+0x4e/0x1c0 net/ipv4/af_inet.c:411
> sock_release+0x8d/0x1e0 net/socket.c:597
> sock_close+0x16/0x20 net/socket.c:1072
Then I tried to catch a problem happening on another cpu, and found one.
I mentioned (in https://lkml.org/lkml/2017/5/31/619 ) that we might need
to defer freeing after rcu grace period but for some reason decided it
was not needed.
What about :
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 44fd86de2823dd17de16276a8ec01b190e69b8b4..80932880af861046849d7dbac5f5aa0a1117f166 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -171,12 +171,20 @@ static void ip_mc_clear_src(struct ip_mc_list *pmc);
static int ip_mc_add_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
int sfcount, __be32 *psfsrc, int delta);
+
+static void ip_mc_list_reclaim(struct rcu_head *head)
+{
+ struct ip_mc_list *im = container_of(head, struct ip_mc_list, rcu);
+
+ ip_mc_clear_src(im);
+ in_dev_put(im->interface);
+ kfree(im);
+}
+
static void ip_ma_put(struct ip_mc_list *im)
{
- if (atomic_dec_and_test(&im->refcnt)) {
- in_dev_put(im->interface);
- kfree_rcu(im, rcu);
- }
+ if (atomic_dec_and_test(&im->refcnt))
+ call_rcu(&im->rcu, ip_mc_list_reclaim);
}
#define for_each_pmc_rcu(in_dev, pmc) \
@@ -1615,7 +1623,6 @@ void ip_mc_dec_group(struct in_device *in_dev, __be32 addr)
*ip = i->next_rcu;
in_dev->mc_count--;
igmp_group_dropped(i);
- ip_mc_clear_src(i);
if (!in_dev->dead)
ip_rt_multicast_event(in_dev);
^ permalink raw reply related
* [PATCHv5] wlcore: add wl1285 compatible
From: Sebastian Reichel @ 2017-06-08 20:50 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, netdev, linux-kernel, Sebastian Reichel
Motorola Droid 4 uses a WL 1285C. With differences between
chips not being public let's add explicit binding for wl1285
instead of relying on wl1283 being very similar.
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Kalle Valo <kvalo@codeaurora.org>
Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
Changes since PATCHv4:
- Dropped droid4.dts change, patch can go in normally now
Changes since PATCHv3:
- add netdev@vger.kernel.org to cc
- add Acked-By from Tony & Kalle
Changes since PATCHv2:
- merge patch for DTS and driver
- add Acked-By from Rob
Changes since PATCHv1:
- patches did not exist in patchv1
---
Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt | 1 +
drivers/net/wireless/ti/wlcore/sdio.c | 1 +
drivers/net/wireless/ti/wlcore/spi.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt b/Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt
index 2a3d90de18ee..7b2cbb14113e 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt
@@ -10,6 +10,7 @@ Required properties:
* "ti,wl1273"
* "ti,wl1281"
* "ti,wl1283"
+ * "ti,wl1285"
* "ti,wl1801"
* "ti,wl1805"
* "ti,wl1807"
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
index 287023ef4a78..2fb38717346f 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -237,6 +237,7 @@ static const struct of_device_id wlcore_sdio_of_match_table[] = {
{ .compatible = "ti,wl1273", .data = &wl127x_data },
{ .compatible = "ti,wl1281", .data = &wl128x_data },
{ .compatible = "ti,wl1283", .data = &wl128x_data },
+ { .compatible = "ti,wl1285", .data = &wl128x_data },
{ .compatible = "ti,wl1801", .data = &wl18xx_data },
{ .compatible = "ti,wl1805", .data = &wl18xx_data },
{ .compatible = "ti,wl1807", .data = &wl18xx_data },
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index f949ad2bd898..1f5d9ebb0925 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -433,6 +433,7 @@ static const struct of_device_id wlcore_spi_of_match_table[] = {
{ .compatible = "ti,wl1273", .data = &wl127x_data},
{ .compatible = "ti,wl1281", .data = &wl128x_data},
{ .compatible = "ti,wl1283", .data = &wl128x_data},
+ { .compatible = "ti,wl1285", .data = &wl128x_data},
{ .compatible = "ti,wl1801", .data = &wl18xx_data},
{ .compatible = "ti,wl1805", .data = &wl18xx_data},
{ .compatible = "ti,wl1807", .data = &wl18xx_data},
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] Fix an intermittent pr_emerg warning about lo becoming free.
From: Eric Dumazet @ 2017-06-08 21:06 UTC (permalink / raw)
To: Krister Johansen; +Cc: davem, netdev, Wei Wang
In-Reply-To: <20170608201238.GC2553@templeofstupid.com>
On Thu, 2017-06-08 at 13:12 -0700, Krister Johansen wrote:
...
> Looking at the gc_task intervals, they started at 663ms when we invoked
> __dst_free(). After that, they increased to 1663, 3136, 5567, 8191,
> 10751, and 14848. The release that set the refcnt to 0 on our dst entry
> occurred after the gc_task was enqueued for 14 second interval so we had
> to wait longer than the warning time in wait_allrefs in order for the
> dst entry to get free'd and the hold on 'lo' to be released.
>
> A simple solution to this problem is to have dst_dev_event() reset the
> gc timer, which causes us to process this list shortly after the
> gc_mutex is relased when dst_dev_event() completes.
>
> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
> ---
> net/core/dst.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/net/core/dst.c b/net/core/dst.c
> index 6192f11..13ba4a0 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -469,6 +469,20 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event,
> spin_lock_bh(&dst_garbage.lock);
> dst = dst_garbage.list;
> dst_garbage.list = NULL;
> + /* The code in dst_ifdown places a hold on the loopback device.
> + * If the gc entry processing is set to expire after a lengthy
> + * interval, this hold can cause netdev_wait_allrefs() to hang
> + * out and wait for a long time -- until the the loopback
> + * interface is released. If we're really unlucky, it'll emit
> + * pr_emerg messages to console too. Reset the interval here,
> + * so dst cleanups occur in a more timely fashion.
> + */
> + if (dst_garbage.timer_inc > DST_GC_INC) {
> + dst_garbage.timer_inc = DST_GC_INC;
> + dst_garbage.timer_expires = DST_GC_MIN;
> + mod_delayed_work(system_wq, &dst_gc_work,
> + dst_garbage.timer_expires);
> + }
> spin_unlock_bh(&dst_garbage.lock);
>
> if (last)
Looks very nice to me !
Acked-by: Eric Dumazet <edumazet@google.com>
Wei Wan is actually working on a patch series removing all this
dst_garbage list stuff.
^ permalink raw reply
* Re: [RFC PATCH net-next 3/5] bpf/verifier: feed pointer-to-unknown-scalar casts into scalar ALU path
From: Alexei Starovoitov via iovisor-dev @ 2017-06-08 21:17 UTC (permalink / raw)
To: Edward Cree
Cc: Alexei Starovoitov, netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
LKML, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <05371ef3-f10a-21b0-def0-1cbdfe458171-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
On Thu, Jun 08, 2017 at 08:07:53PM +0100, Edward Cree wrote:
> On 08/06/17 19:41, Alexei Starovoitov wrote:
> > On Thu, Jun 08, 2017 at 06:12:39PM +0100, Edward Cree wrote:
> >> On 08/06/17 17:50, Alexei Starovoitov wrote:
> >>> On Thu, Jun 08, 2017 at 04:25:39PM +0100, Edward Cree wrote:
> >>>> On 08/06/17 03:35, Alexei Starovoitov wrote:
> >>>>> such large back and forth move doesn't help reviewing.
> >>>>> may be just merge it into previous patch?
> >>>>> Or keep that function in the right place in patch 2 already?
> >>>> I think 'diff' got a bit confused, and maybe with different options I could
> >>>> have got it to produce something more readable. But I think I will just
> >>>> merge this into patch 2; it's only separate because it started out as an
> >>>> experiment.
> >>> after sleeping on it I'm not sure we should be allowing such pointer
> >>> arithmetic. In normal C code people do fancy tricks with lower 3 bits
> >>> of the pointer, but in bpf code I cannot see such use case.
> >>> What kind of realistic code will be doing ptr & 0x40 ?
> >> Well, I didn't support it because I saw a use case. I supported it because
> >> it seemed easy to do and the code came out reasonably elegant-looking.
> >> Since this is guarded by env->allow_ptr_leaks, I can't see any reason _not_
> >> to let people try fancy tricks with the low bits of pointers.
> >> I agree ptr & 0x40 is a crazy thing with no imaginable use case, but...
> >> "Unix was not designed to stop its users from doing stupid things, as that
> >> would also stop them from doing clever things." ;-)
> > well, I agree with the philosophy :) but I also see few reasons not to allow it:
> > 1. it immediately becomes uapi and if later we find out that it's preventing us
> > to do something we actually really need we'll be stuck looking for workaround
> What could it prevent us from doing, though? It's basically equivalent to giving
> BPF an opcode that casts a pointer to a u64, which of course is only allowed if
> allow_ptr_leaks is true. And since we don't feed any knowledge about the pointer
> into the verifier, it's just like any other way of filling a register with
> arbitrary, unknown bits.
> I can fully appreciate why you're being cautious, what with uapi and all. But I
> don't think there's any actual problem here. Open to being convinced, though.
The leaking is not a concern. It's if we started accepting a certain
class of programs we need to keep accepting them in the future.
Another reason is 'ptr & mask' could have been simply a bug and rejecting it
suppose to help users find issues sooner...
but I don't have a strong opinion here.
> > 2. it's the same pruning concern. probably doesn't fully apply here, but
> > the reason we don't track 'if (reg == 1) ...'
> Don't we though?
> http://elixir.free-electrons.com/linux/v4.12-rc4/source/kernel/bpf/verifier.c#L2127
> > is if we mark that
> > register as known const_imm in the true branch, it will screw up
> > pruning quite badly. It's trivial to track and may seem useful,
> > but hurts instead.
> (Thinking out loud...)
>
> What would be really nice is a way to propagate limits backwards as well as
> forwards, so that the verifier can say "when I tested this branch, I used
> this part of the state, I read four bytes past this pointer". Then when it
> wants to prune, it can say "well, the state this time isn't as strong, but
> it still satisfies everything I actually used".
> But that sounds like it would be very hard indeed to do.
that's more or less what i'm trying to do. liveness info per basic block
will trim the state.
> Maybe with the basic-block DAG stuff David's been talking about, we could
> find all the paths that reach a block, and take the union of their states,
> and then run through the block feeding it that combined state. But that
> could reject code that relies on correlation of the state (i.e. if r1 != 0
> then r2 is valid ptr I can access, etc) so would still need the 'walk with
> each individual state' as a fallback. Though at least you'd have all the
> states at once so you could find out which ones were subsumed, instead of
> hoping you get to them in the right order.
I think it's important to optimize verification speed for good programs.
If bad program takes slightly longer, not a big deal. Right now we have
global lock which needs to go away, but that's a minor fix.
In that sense I see that combining the state can help find bad programs
sooner, but I don't see it's helping good programs.
Also we already have programs like:
if (...) {
var1 = ptr
var2 = size
} else {
var1 = different ptr
var2 = different size
}
call_helper(...var1, var2)
So the state needs to be considered together. Cannot just mix and match.
Initially I was thinking to build Use/Def chains for all operands
of loads, stores and calls and follow them from Use spot to all Defs
recursively to determine validity, but above use case breaks that.
^ permalink raw reply
* Re: [RFC PATCH net-next 2/5] bpf/verifier: rework value tracking
From: Alexei Starovoitov via iovisor-dev @ 2017-06-08 21:20 UTC (permalink / raw)
To: Edward Cree
Cc: Alexei Starovoitov, netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
LKML, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <9b7aaa39-aacf-6f41-6adf-fc9317c447aa-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
On Thu, Jun 08, 2017 at 08:38:29PM +0100, Edward Cree wrote:
> On 08/06/17 17:45, Alexei Starovoitov wrote:
> > On Thu, Jun 08, 2017 at 03:53:36PM +0100, Edward Cree wrote:
> >>>>
> >>>> - } else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
> >>>> + } else if (reg->type == PTR_TO_STACK) {
> >>>> + /* stack accesses must be at a fixed offset, so that we can
> >>>> + * determine what type of data were returned.
> >>>> + */
> >>>> + if (reg->align.mask) {
> >>>> + char tn_buf[48];
> >>>> +
> >>>> + tn_strn(tn_buf, sizeof(tn_buf), reg->align);
> >>>> + verbose("variable stack access align=%s off=%d size=%d",
> >>>> + tn_buf, off, size);
> >>>> + return -EACCES;
> >>> hmm. why this restriction?
> >>> I thought one of key points of the diff that ptr+var tracking logic
> >>> will now apply not only to map_value, but to stack_ptr as well?
> >> As the comment above it says, we need to determine what was returned:
> >> was it STACK_MISC or STACK_SPILL, and if the latter, what kind of pointer
> >> was spilled there? See check_stack_read(), which I should probably
> >> mention in the comment.
> > this piece of code is not only spill/fill, but normal ldx/stx stack access.
> > Consider the frequent pattern that many folks tried to do:
> > bpf_prog()
> > {
> > char buf[64];
> > int len;
> >
> > bpf_probe_read(&len, sizeof(len), kernel_ptr_to_filename_len);
> > bpf_probe_read(buf, sizeof(buf), kernel_ptr_to_filename);
> > buf[len & (sizeof(buf) - 1)] = 0;
> > ...
> >
> > currently above is not supported, but when 'buf' is a pointer to map value
> > it works fine. Allocating extra bpf map just to do such workaround
> > isn't nice and since this patch generalized map_value_adj with ptr_to_stack
> > we can support above code too.
> > We can check that all bytes of stack for this variable access were
> > initialized already.
> > In the example above it will happen by bpf_probe_read (in the verifier code):
> > for (i = 0; i < meta.access_size; i++) {
> > err = check_mem_access(env, meta.regno, i, BPF_B, BPF_WRITE, -1);
> > so at the time of
> > buf[len & ..] = 0
> > we can check that 'stx' is within the range of inited stack and allow it.
> Yes, we could check every byte of the stack within the range [buf, buf+63]
> is a STACK_MISC and if so allow it. But since this is not supported by the
> existing code (so it's not a regression), I'd prefer to leave that for a
> future patch - this one is quite big enough already ;-)
of course! just exploring.
> >>>> + if (!err && size < BPF_REG_SIZE && value_regno >= 0 && t == BPF_READ &&
> >>>> + state->regs[value_regno].type == SCALAR_VALUE) {
> >>>> + /* b/h/w load zero-extends, mark upper bits as known 0 */
> >>>> + state->regs[value_regno].align.value &= (1ULL << (size * 8)) - 1;
> >>>> + state->regs[value_regno].align.mask &= (1ULL << (size * 8)) - 1;
> >>> probably another helper from tnum.h is needed.
> >> I could rewrite as
> >> reg->align = tn_and(reg->align, tn_const((1ULL << (size * 8)) - 1))
> > yep. that's perfect.
> In the end I settled on adding a helper
> struct tnum tnum_cast(struct tnum a, u8 size);
> since I have a bunch of other places that cast things to 32 bits.
sounds good to me
> > I see. May be print verifier state in such warn_ons and make error
> > more human readable?
> Good idea, I'll do that.
> >>>> + case PTR_TO_MAP_VALUE_OR_NULL:
> >>> does this new state comparison logic helps? Do you have any numbers before/after in the number of insns it had to process for the tests in selftests ?
> >> I don't have the numbers, no (I'll try to collect them). This rewrite was
> > Thanks. The main concern is that right now some complex programs
> > that cilium is using are close to the verifier complexity limit and these
> > big changes to amount of info recognized by the verifier can cause pruning
> > to be ineffective, so we need to test on big programs.
> > I think Daniel will be happy to test your next rev of the patches.
> > I'll test them as well.
> > At least 'insn_processed' from C code in tools/testing/selftests/bpf/
> > is a good estimate of how these changes affect pruning.
> It looks like the only place this gets recorded is as "processed %d insns"
> in the log_buf. Is there a convenient way to get at this, or am I going
> to have to make bpf_verify_program grovel through the log sscanf()ing for
> a matching line?
typically we just run the tests with hacked log_level and grep.
similar stuff Dave did in test_align.c
^ permalink raw reply
* Aw: Re: [PATCH 2/2(net.git)] stmmac: fix for hw timestamp of GMAC3 unit
From: Mario Molitor @ 2017-06-08 21:20 UTC (permalink / raw)
To: Andy Shevchenko, David Miller
Cc: Giuseppe CAVALLARO, Alexandre TORGUE, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75VfOhjLB96wf33CCFiEhxQ5ve3DX+GKiwUX-jB8FQCUB=w@mail.gmail.com>
Hello David and Andy,
thanks for review response. I will fix the patches with your responses.
Thanks,
Mario
^ permalink raw reply
* Re: Repeatable inet6_dump_fib crash in stock 4.12.0-rc4+
From: Ben Greear @ 2017-06-08 21:27 UTC (permalink / raw)
To: Eric Dumazet, David Ahern; +Cc: netdev
In-Reply-To: <1496809166.736.25.camel@edumazet-glaptop3.roam.corp.google.com>
On 06/06/2017 09:19 PM, Eric Dumazet wrote:
> On Tue, 2017-06-06 at 18:34 -0600, David Ahern wrote:
>> On 6/6/17 6:27 PM, Eric Dumazet wrote:
>>> Good catch, but it looks like similar fix is needed a few lines before.
>>>
>>> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
>>> index deea901746c8570c5e801e40592c91e3b62812e0..b214443dc8346cef3690df7f27cc48a864028865 100644
>>> --- a/net/ipv6/ip6_fib.c
>>> +++ b/net/ipv6/ip6_fib.c
>>> @@ -372,12 +372,13 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
>>>
>>> read_lock_bh(&table->tb6_lock);
>>> res = fib6_walk(net, w);
>>> - read_unlock_bh(&table->tb6_lock);
>>> if (res > 0) {
>>> cb->args[4] = 1;
>>> cb->args[5] = w->root->fn_sernum;
>>> }
>>> + read_unlock_bh(&table->tb6_lock);
>>
>> indeed. tunnel vision on Ben's problem
>
> BTW, bug was already Ben's problem when Patrick tried to fix it
> in commit 2bec5a369ee79 ("ipv6: fib: fix crash when changing large fib
> while dumping it") seven years ago ;)
As far as I can tell, the patch did not help, or at least we still reproduce the
crash easily.
ct524-ffb0 login: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
IP: fib6_walk_continue+0x76/0x180 [ipv6]
PGD 3ec59a067
P4D 3ec59a067
PUD 3eb939067
PMD 0
Oops: 0000 [#1] PREEMPT SMP
Modules linked in: nf_conntrack_netlink nf_conntrack nfnetlink nf_defrag_ipv4 libcrc32c bridge stp llc veth bnep fuse macvlan pktgen cfg80211 ipmi_ssif iTCO_wdt
iTCO_vendor_support coretemp intel_rapl x86_pkg_temp_thermal intel_powerclamp kvm_intel kvm irqbypass joydev i2c_i801 ie31200_edac intel_pch_thermal shpchp
hci_uart ipmi_si btbcm btqca ipmi_devintf btintel ipmi_msghandler pinctrl_sunrisepoint bluetooth intel_lpss_acpi acpi_als video pinctrl_intel intel_lpss
kfifo_buf tpm_tis tpm_tis_core industrialio acpi_power_meter tpm acpi_pad sch_fq_codel nfsd auth_rpcgss nfs_acl lockd grace sunrpc ast drm_kms_helper ttm drm
igb hwmon ptp pps_core dca i2c_algo_bit i2c_hid i2c_core ipv6 crc_ccitt [last unloaded: nfnetlink]
CPU: 3 PID: 2185 Comm: ip Not tainted 4.12.0-rc4+ #32
Hardware name: Supermicro Super Server/X11SSM-F, BIOS 1.0b 12/29/2015
task: ffff8803e87fd940 task.stack: ffffc90009ae8000
RIP: 0010:fib6_walk_continue+0x76/0x180 [ipv6]
RSP: 0018:ffffc90009aebbc0 EFLAGS: 00010287
RAX: ffff880460df8ca0 RBX: ffff8803f20a2c60 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffc90009aebc00 RDI: ffffffff81eee280
RBP: ffffc90009aebbc8 R08: 0000000000000008 R09: ffff8803e87b47cd
R10: ffffc90009aebb70 R11: 0000000000000000 R12: 0000000000000001
R13: 0000000000000001 R14: ffff8803f20a2c60 R15: ffff8803ec601f80
FS: 00007f43520ee700(0000) GS:ffff8804778c0000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000018 CR3: 00000003ebb46000 CR4: 00000000003406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
inet6_dump_fib+0x1ab/0x2a0 [ipv6]
netlink_dump+0x11d/0x290
netlink_recvmsg+0x260/0x3f0
sock_recvmsg+0x38/0x40
___sys_recvmsg+0xe9/0x230
? alloc_pages_vma+0x9d/0x260
? page_add_new_anon_rmap+0x88/0xc0
? lru_cache_add_active_or_unevictable+0x31/0xb0
? __handle_mm_fault+0xce3/0xf70
__sys_recvmsg+0x3d/0x70
? __sys_recvmsg+0x3d/0x70
SyS_recvmsg+0xd/0x20
do_syscall_64+0x56/0xc0
entry_SYSCALL64_slow_path+0x25/0x25
RIP: 0033:0x7f4351a23050
RSP: 002b:00007ffdb1bfafb8 EFLAGS: 00000246 ORIG_RAX: 000000000000002f
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4351a23050
RDX: 0000000000000000 RSI: 00007ffdb1bfb020 RDI: 0000000000000004
RBP: 00007ffdb1bff044 R08: 0000000000003fe4 R09: 0000000000000000
R10: 00007ffdb1bfb060 R11: 0000000000000246 R12: 000000000064f360
R13: 00007ffdb1bff0b0 R14: 0000000000003fe4 R15: 0000000000000000
Code: f6 40 2a 04 74 11 8b 53 30 85 d2 0f 84 02 01 00 00 83 ea 01 89 53 30 c7 43 28 04 00 00 00 48 39 43 10 74 33 48 8b 10 48 89 53 18 <48> 39 42 18 0f 84 a3 00
00 00 48 39 42 08 0f 84 ae 00 00 00 48
RIP: fib6_walk_continue+0x76/0x180 [ipv6] RSP: ffffc90009aebbc0
CR2: 0000000000000018
---[ end trace 06ac9dee8b14db6b ]---
Kernel panic - not syncing: Fatal exception in interrupt
Kernel Offset: disabled
(gdb) l *(fib6_walk_continue+0x76)
0x188c6 is in fib6_walk_continue (/home/greearb/git/linux-2.6/net/ipv6/ip6_fib.c:1593).
1588 if (fn == w->root)
1589 return 0;
1590 pn = fn->parent;
1591 w->node = pn;
1592 #ifdef CONFIG_IPV6_SUBTREES
1593 if (FIB6_SUBTREE(pn) == fn) {
1594 WARN_ON(!(fn->fn_flags & RTN_ROOT));
1595 w->state = FWS_L;
1596 continue;
1597 }
(gdb) l *(inet6_dump_fib+0x1ab)
0x1939b is in inet6_dump_fib (/home/greearb/git/linux-2.6/net/ipv6/ip6_fib.c:392).
387 w->skip = w->count;
388 } else
389 w->skip = 0;
390
391 res = fib6_walk_continue(w);
392 read_unlock_bh(&table->tb6_lock);
393 if (res <= 0) {
394 fib6_walker_unlink(net, w);
395 cb->args[4] = 0;
396 }
(gdb)
[greearb@ben-dt3 linux-2.6]$ git diff
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index d4bf2c6..4e32a16 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -372,12 +372,13 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
read_lock_bh(&table->tb6_lock);
res = fib6_walk(net, w);
- read_unlock_bh(&table->tb6_lock);
if (res > 0) {
cb->args[4] = 1;
cb->args[5] = w->root->fn_sernum;
}
+ read_unlock_bh(&table->tb6_lock);
} else {
+ read_lock_bh(&table->tb6_lock);
if (cb->args[5] != w->root->fn_sernum) {
/* Begin at the root if the tree changed */
cb->args[5] = w->root->fn_sernum;
@@ -387,7 +388,6 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
} else
w->skip = 0;
- read_lock_bh(&table->tb6_lock);
res = fib6_walk_continue(w);
read_unlock_bh(&table->tb6_lock);
if (res <= 0) {
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply related
* [PATCH 1/2(net.git)] stmmac: fix ptp header for GMAC3 hw timestamp
From: Mario Molitor @ 2017-06-08 21:31 UTC (permalink / raw)
To: Giuseppe CAVALLARO, Alexandre TORGUE; +Cc: netdev, linux-kernelvger.kernel.org
>From ce9c334037fce37ccd715124cda57d1fd6d8cfe8 Mon Sep 17 00:00:00 2001
From: Mario Molitor <mario_molitor@web.de>
Date: Thu, 8 Jun 2017 22:41:02 +0200
Subject: [PATCH 1/2] stmmac: fix ptp header for GMAC3 hw timestamp
According the CYCLON V documention only the bit 16 of snaptypesel should
set.
(more information see Table 17-20 (cv_5v4.pdf) :
Timestamp Snapshot Dependency on Register Bits)
Fixes: d2042052a0aa ("stmmac: update the PTP header file")
Signed-off-by: Mario Molitor <mario_molitor@web.de>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 15 ++++++++++++---
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h | 3 ++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 12236da..d54e5d7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -546,7 +546,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
/* PTP v1, UDP, any kind of event packet */
config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
/* take time stamp for all event messages */
- snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+ if (priv->plat->has_gmac4)
+ snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+ else
+ snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
@@ -578,7 +581,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
- snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+ if (priv->plat->has_gmac4)
+ snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+ else
+ snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
@@ -612,7 +618,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
- snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+ if (priv->plat->has_gmac4)
+ snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+ else
+ snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;
ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
index 48fb72f..f4b31d6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
@@ -59,7 +59,8 @@
/* Enable Snapshot for Messages Relevant to Master */
#define PTP_TCR_TSMSTRENA BIT(15)
/* Select PTP packets for Taking Snapshots */
-#define PTP_TCR_SNAPTYPSEL_1 GENMASK(17, 16)
+#define PTP_TCR_SNAPTYPSEL_1 BIT(16)
+#define PTP_GMAC4_TCR_SNAPTYPSEL_1 GENMASK(17, 16)
/* Enable MAC address for PTP Frame Filtering */
#define PTP_TCR_TSENMACADDR BIT(18)
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2(net.git)] stmmac: fix for hw timestamp of GMAC3 unit
From: Mario Molitor @ 2017-06-08 21:35 UTC (permalink / raw)
To: Giuseppe CAVALLARO, Alexandre TORGUE; +Cc: netdev, linux-kernelvger.kernel.org
>From d5c520880a5f6b470cb150b9aae67341089b9395 Mon Sep 17 00:00:00 2001
From: Mario Molitor <mario_molitor@web.de>
Date: Thu, 8 Jun 2017 23:03:09 +0200
Subject: [PATCH 2/2] stmmac: fix for hw timestamp of GMAC3 unit
1.) Bugfix of function stmmac_get_tx_hwtstamp.
Corrected the tx timestamp available check (same as 4.8 and older)
Change printout from info syslevel to debug.
2.) Bugfix of function stmmac_get_rx_hwtstamp.
Corrected the rx timestamp available check (same as 4.8 and older)
Change printout from info syslevel to debug.
Fixes: ba1ffd74df74 ("stmmac: fix PTP support for GMAC4")
Signed-off-by: Mario Molitor <mario_molitor@web.de>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 11 +++++++----
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++-----
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index aa64764..e0ef02f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -214,13 +214,13 @@ static int dwmac4_wrback_get_tx_timestamp_status(struct dma_desc *p)
{
/* Context type from W/B descriptor must be zero */
if (le32_to_cpu(p->des3) & TDES3_CONTEXT_TYPE)
- return -EINVAL;
+ return 0;
/* Tx Timestamp Status is 1 so des0 and des1'll have valid values */
if (le32_to_cpu(p->des3) & TDES3_TIMESTAMP_STATUS)
- return 0;
+ return 1;
- return 1;
+ return 0;
}
static inline u64 dwmac4_get_timestamp(void *desc, u32 ats)
@@ -282,7 +282,10 @@ static int dwmac4_wrback_get_rx_timestamp_status(void *desc, u32 ats)
}
}
exit:
- return ret;
+ if (likely(ret == 0))
+ return 1;
+
+ return 0;
}
static void dwmac4_rd_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d54e5d7..d16d11b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -434,14 +434,14 @@ static void stmmac_get_tx_hwtstamp(struct stmmac_priv *priv,
return;
/* check tx tstamp status */
- if (!priv->hw->desc->get_tx_timestamp_status(p)) {
+ if (priv->hw->desc->get_tx_timestamp_status(p)) {
/* get the valid tstamp */
ns = priv->hw->desc->get_timestamp(p, priv->adv_ts);
memset(&shhwtstamp, 0, sizeof(struct skb_shared_hwtstamps));
shhwtstamp.hwtstamp = ns_to_ktime(ns);
- netdev_info(priv->dev, "get valid TX hw timestamp %llu\n", ns);
+ netdev_dbg(priv->dev, "get valid TX hw timestamp %llu\n", ns);
/* pass tstamp to stack */
skb_tstamp_tx(skb, &shhwtstamp);
}
@@ -468,19 +468,19 @@ static void stmmac_get_rx_hwtstamp(struct stmmac_priv *priv, struct dma_desc *p,
return;
/* Check if timestamp is available */
- if (!priv->hw->desc->get_rx_timestamp_status(p, priv->adv_ts)) {
+ if (priv->hw->desc->get_rx_timestamp_status(p, priv->adv_ts)) {
/* For GMAC4, the valid timestamp is from CTX next desc. */
if (priv->plat->has_gmac4)
ns = priv->hw->desc->get_timestamp(np, priv->adv_ts);
else
ns = priv->hw->desc->get_timestamp(p, priv->adv_ts);
- netdev_info(priv->dev, "get valid RX hw timestamp %llu\n", ns);
+ netdev_dbg(priv->dev, "get valid RX hw timestamp %llu\n", ns);
shhwtstamp = skb_hwtstamps(skb);
memset(shhwtstamp, 0, sizeof(struct skb_shared_hwtstamps));
shhwtstamp->hwtstamp = ns_to_ktime(ns);
} else {
- netdev_err(priv->dev, "cannot get RX hw timestamp\n");
+ netdev_dbg(priv->dev, "cannot get RX hw timestamp\n");
}
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] Fix an intermittent pr_emerg warning about lo becoming free.
From: Wei Wang @ 2017-06-08 21:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: Krister Johansen, David S . Miller,
Linux Kernel Network Developers
In-Reply-To: <1496955962.736.88.camel@edumazet-glaptop3.roam.corp.google.com>
> Wei Wan is actually working on a patch series removing all this
> dst_garbage list stuff.
Yes. I am working on removing the dst garbage collector completely.
dst_dev_event() will be removed from the list of callbacks that the
netdevice notifiers invoke in my patch series.
On Thu, Jun 8, 2017 at 2:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2017-06-08 at 13:12 -0700, Krister Johansen wrote:
>
> ...
>
>> Looking at the gc_task intervals, they started at 663ms when we invoked
>> __dst_free(). After that, they increased to 1663, 3136, 5567, 8191,
>> 10751, and 14848. The release that set the refcnt to 0 on our dst entry
>> occurred after the gc_task was enqueued for 14 second interval so we had
>> to wait longer than the warning time in wait_allrefs in order for the
>> dst entry to get free'd and the hold on 'lo' to be released.
>>
>> A simple solution to this problem is to have dst_dev_event() reset the
>> gc timer, which causes us to process this list shortly after the
>> gc_mutex is relased when dst_dev_event() completes.
>>
>> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
>> ---
>> net/core/dst.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/net/core/dst.c b/net/core/dst.c
>> index 6192f11..13ba4a0 100644
>> --- a/net/core/dst.c
>> +++ b/net/core/dst.c
>> @@ -469,6 +469,20 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event,
>> spin_lock_bh(&dst_garbage.lock);
>> dst = dst_garbage.list;
>> dst_garbage.list = NULL;
>> + /* The code in dst_ifdown places a hold on the loopback device.
>> + * If the gc entry processing is set to expire after a lengthy
>> + * interval, this hold can cause netdev_wait_allrefs() to hang
>> + * out and wait for a long time -- until the the loopback
>> + * interface is released. If we're really unlucky, it'll emit
>> + * pr_emerg messages to console too. Reset the interval here,
>> + * so dst cleanups occur in a more timely fashion.
>> + */
>> + if (dst_garbage.timer_inc > DST_GC_INC) {
>> + dst_garbage.timer_inc = DST_GC_INC;
>> + dst_garbage.timer_expires = DST_GC_MIN;
>> + mod_delayed_work(system_wq, &dst_gc_work,
>> + dst_garbage.timer_expires);
>> + }
>> spin_unlock_bh(&dst_garbage.lock);
>>
>> if (last)
>
> Looks very nice to me !
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
> Wei Wan is actually working on a patch series removing all this
> dst_garbage list stuff.
>
>
>
^ permalink raw reply
* [PATCH net-next 1/8] net: ena: fix rare uncompleted admin command false alarm
From: netanel @ 2017-06-08 21:46 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1496958410-4220-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
The current flow to detect admin completion is:
while (command_not_completed) {
if (timeout)
error
check_for_completion()
sleep()
}
So in case the sleep took more than the timeout
(in case the thread/workqueue was not scheduled due to higher priority
task or prolonged VMexit), the driver can detect a stall even if
the completion is present.
The fix changes the order of this function to first check for
completion and only after that check if the timeout expired.
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index 08d11ce..e1c2fab 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -508,15 +508,20 @@ static int ena_com_comp_status_to_errno(u8 comp_status)
static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_ctx,
struct ena_com_admin_queue *admin_queue)
{
- unsigned long flags;
- u32 start_time;
+ unsigned long flags, timeout;
int ret;
- start_time = ((u32)jiffies_to_usecs(jiffies));
+ timeout = jiffies + ADMIN_CMD_TIMEOUT_US;
+
+ while (1) {
+ spin_lock_irqsave(&admin_queue->q_lock, flags);
+ ena_com_handle_admin_completion(admin_queue);
+ spin_unlock_irqrestore(&admin_queue->q_lock, flags);
- while (comp_ctx->status == ENA_CMD_SUBMITTED) {
- if ((((u32)jiffies_to_usecs(jiffies)) - start_time) >
- ADMIN_CMD_TIMEOUT_US) {
+ if (comp_ctx->status != ENA_CMD_SUBMITTED)
+ break;
+
+ if (time_is_before_jiffies(timeout)) {
pr_err("Wait for completion (polling) timeout\n");
/* ENA didn't have any completion */
spin_lock_irqsave(&admin_queue->q_lock, flags);
@@ -528,10 +533,6 @@ static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_c
goto err;
}
- spin_lock_irqsave(&admin_queue->q_lock, flags);
- ena_com_handle_admin_completion(admin_queue);
- spin_unlock_irqrestore(&admin_queue->q_lock, flags);
-
msleep(100);
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/8] Bug fixes in ena ethernet driver
From: netanel @ 2017-06-08 21:46 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
From: Netanel Belgazal <netanel@amazon.com>
This patchset contains fixes for the bugs that were discovered so far.
Netanel Belgazal (8):
net: ena: fix rare uncompleted admin command false alarm
net: ena: fix bug that might cause hang after consecutive open/close
interface.
net: ena: add missing return when ena_com_get_io_handlers() fails
net: ena: fix race condition between submit and completion admin
command
net: ena: add missing unmap bars on device removal
net: ena: fix theoretical Rx hang on low memory systems
net: ena: disable admin msix while working in polling mode
net: ena: bug fix in lost tx packets detection mechanism
drivers/net/ethernet/amazon/ena/ena_com.c | 35 +++--
drivers/net/ethernet/amazon/ena/ena_ethtool.c | 2 +-
drivers/net/ethernet/amazon/ena/ena_netdev.c | 179 +++++++++++++++++++-------
drivers/net/ethernet/amazon/ena/ena_netdev.h | 16 ++-
4 files changed, 168 insertions(+), 64 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next 2/8] net: ena: fix bug that might cause hang after consecutive open/close interface.
From: netanel @ 2017-06-08 21:46 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1496958410-4220-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
Fixing a bug that the driver does not unmask the IO interrupts
in ndo_open():
occasionally, the MSI-X interrupt (for one or more IO queues)
can be masked when ndo_close() was called.
If that is followed by ndo open(),
then the MSI-X will be still masked so no interrupt
will be received by the driver.
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 41 ++++++++++++++++++----------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 7c1214d..0e3c60c7 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -1078,6 +1078,26 @@ inline void ena_adjust_intr_moderation(struct ena_ring *rx_ring,
rx_ring->per_napi_bytes = 0;
}
+static inline void ena_unmask_interrupt(struct ena_ring *tx_ring,
+ struct ena_ring *rx_ring)
+{
+ struct ena_eth_io_intr_reg intr_reg;
+
+ /* Update intr register: rx intr delay,
+ * tx intr delay and interrupt unmask
+ */
+ ena_com_update_intr_reg(&intr_reg,
+ rx_ring->smoothed_interval,
+ tx_ring->smoothed_interval,
+ true);
+
+ /* It is a shared MSI-X.
+ * Tx and Rx CQ have pointer to it.
+ * So we use one of them to reach the intr reg
+ */
+ ena_com_unmask_intr(rx_ring->ena_com_io_cq, &intr_reg);
+}
+
static inline void ena_update_ring_numa_node(struct ena_ring *tx_ring,
struct ena_ring *rx_ring)
{
@@ -1108,7 +1128,6 @@ static int ena_io_poll(struct napi_struct *napi, int budget)
{
struct ena_napi *ena_napi = container_of(napi, struct ena_napi, napi);
struct ena_ring *tx_ring, *rx_ring;
- struct ena_eth_io_intr_reg intr_reg;
u32 tx_work_done;
u32 rx_work_done;
@@ -1149,22 +1168,9 @@ static int ena_io_poll(struct napi_struct *napi, int budget)
if (ena_com_get_adaptive_moderation_enabled(rx_ring->ena_dev))
ena_adjust_intr_moderation(rx_ring, tx_ring);
- /* Update intr register: rx intr delay,
- * tx intr delay and interrupt unmask
- */
- ena_com_update_intr_reg(&intr_reg,
- rx_ring->smoothed_interval,
- tx_ring->smoothed_interval,
- true);
-
- /* It is a shared MSI-X.
- * Tx and Rx CQ have pointer to it.
- * So we use one of them to reach the intr reg
- */
- ena_com_unmask_intr(rx_ring->ena_com_io_cq, &intr_reg);
+ ena_unmask_interrupt(tx_ring, rx_ring);
}
-
ena_update_ring_numa_node(tx_ring, rx_ring);
ret = rx_work_done;
@@ -1485,6 +1491,11 @@ static int ena_up_complete(struct ena_adapter *adapter)
ena_napi_enable_all(adapter);
+ /* Enable completion queues interrupt */
+ for (i = 0; i < adapter->num_queues; i++)
+ ena_unmask_interrupt(&adapter->tx_ring[i],
+ &adapter->rx_ring[i]);
+
/* schedule napi in case we had pending packets
* from the last time we disable napi
*/
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox